MWPW-174164: Collection header & UI fixes #4465
Conversation
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
|
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
eslint
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
libs/deps/mas/commerce.js
Outdated
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
libs/deps/mas/merch-card.js
Outdated
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
libs/deps/mas/merch-offer-select.js
Outdated
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
libs/deps/mas/merch-sidenav.js
Outdated
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
libs/deps/mas/merch-stock.js
Outdated
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
libs/features/mas/dist/mas.js
Outdated
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
eslint
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
libs/features/mas/src/utils.js
Outdated
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
|
This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR. |
There was a problem hiding this comment.
File ignored because of a matching ignore pattern. Use "--no-ignore" to override.
| .four-merch-cards.catalog { | ||
| grid-template-columns: repeat(3, var(--consonant-merch-card-catalog-width)); | ||
| @media screen and ${TABLET_UP} { | ||
| .collection-container.catalog { |
There was a problem hiding this comment.
Why are we deviating from *-merch-cards for the card grid. This is a feature we support in Milo, and with this change, it will work for plans and catalog card.
I know that these variants are afaik not used individually(1 exception for plans), but they could be.
What is the benefit of this change?
There was a problem hiding this comment.
@yesil Let's discuss in tomorrow's call.
|
@st-angelo-adobe is plans branch as target intentional? |
| @@ -4,25 +4,22 @@ import { postProcessAutoblock } from '../merch/autoblock.js'; | |||
| import '../../deps/mas/merch-card.js'; | |||
| import '../../deps/mas/merch-quantity-select.js'; | |||
|
|
|||
| const COLLECTION_AUTOBLOCK_TIMEOUT = 5000; | |||
| const DEPS_TIMEOUT = 5000; | |||
There was a problem hiding this comment.
5s might be a bit short with really slow connections.
| @@ -82,6 +82,10 @@ export const EVENT_MERCH_SEARCH_CHANGE = 'merch-search:change'; | |||
|
|
|||
| export const EVENT_MERCH_CARD_COLLECTION_SORT = 'merch-card-collection:sort'; | |||
|
|
|||
| export const EVENT_MERCH_CARD_COLLECTION_LITERALS_CHANGED = 'merch-card-collection:literals-changed'; | |||
There was a problem hiding this comment.
nit: merch-card-collection specific stuff could stay in merch-card-collection.js and be exported/imported from there.
NOTE: CAN'T be merged before adobecom/cc#754 is merged
Extracted the collection header functionality into a
merch-card-collection-headercomponent.How it works:
{variant}-containerclass became 2 classescollection-container {variant}, withcollection-containerbeing agridwhich displays the sidenav on the side, and the header and collection stacked - any variant specific style changes in the future can be done via the{variant}classgridwith a layout secified withgrid-template-areasandgrid-template-columns- the areas aresearch,filter,sort,result,customgrid-template-areasandgrid-template-columnscan be set with css variables for each screen size,mobile,tablet,desktop, overriding some defaults, for example:customis basically a custom area in the header, configured in the variant class as a static property,collectionOptions- this was introduced for maximum flexibility (for plans, it's used in the desktop size for the filter label - previously it was implemented in theresultarea - viable, but I went a step further)collectionOptions- seeplans.jsas an exampleResolves: MWPW-174164, MWPW-175520, MWPW-175406, MWPW-175330
Test URLs: