MWPW-192744 [Site Redesign] [Product: Acrobat Pro] | Product Marquee#5925
MWPW-192744 [Site Redesign] [Product: Acrobat Pro] | Product Marquee#5925suhjainadobe wants to merge 9 commits into
Conversation
suhjainadobe
changed the title
suhjainadobe
requested review from
narcis-radu,
overmyheadandbody,
rgclayton and
robert-bogos
| font-family: var(--heading-font-family); | ||
| font-size: var(--s2a-typography-font-size-super); | ||
| font-weight: var(--s2a-font-weight-adobe-clean-display-black); | ||
| letter-spacing: var(--s2a-typography-letter-spacing-super); | ||
| line-height: var(--s2a-typography-line-height-super); |
There was a problem hiding this comment.
Same comment as before. Even better in this scenario, you could add a title super variant to the block and the logic should pick it up and style it corrrectly.
There was a problem hiding this comment.
By adding a variant, do you mean that we should make it authorable or add it directly via JS?
robert-bogos
left a comment
robert-bogos
left a comment
There was a problem hiding this comment.
Looks good overall, just a few comments.
I wanted to check out the test URL but is not loading the product marquee. Might be something you'd want to look into.
| } | ||
|
|
||
| .pm-foreground { | ||
| flex: 1; |
There was a problem hiding this comment.
This is already declared in the base rules. I think it can be removed
| flex: 1; |
| background: var(--s2a-color-background-default); | ||
| display: flex; | ||
| flex-direction: column; | ||
| min-block-size: 494px; |
There was a problem hiding this comment.
how would this work on small screen sizes?
There was a problem hiding this comment.
@narcis-radu : For smaller devices as well, the height of the marquee remains the same. Only the width will change based on the smaller devices.
suhjainadobe
requested review from
narcis-radu,
overmyheadandbody and
robert-bogos
|
@narcis-radu : Added support for viewport greater than 1920px. Could you please review. |
| padding-inline-start: var(--s2a-spacing-2xs); | ||
| } | ||
|
|
||
| .pm-promo-chevron { |
There was a problem hiding this comment.
For RTL, I think we might want to rotate the chevron 180deg, but not a blocker.
| const promoArea = createTag('div', { class: 'pm-promo-area' }); | ||
| if (ctaLink) { |
There was a problem hiding this comment.
another nit: const promoArea = ... should be moved inside the if block, otherwise we could end up creating the promoArea div for no reason, when ctaLink is not present.
| const promoArea = createTag('div', { class: 'pm-promo-area' }); | |
| if (ctaLink) { | |
| if (ctaLink) { | |
| const promoArea = createTag('div', { class: 'pm-promo-area' }); |
There was a problem hiding this comment.
We are appending promoArea inside the outer div content. Thus, I had it outside the ctaLink check
|
I am not sure if design changed but shouldn't the chevron point down ? |
overmyheadandbody
left a comment
overmyheadandbody
left a comment
There was a problem hiding this comment.
I'd like to avoid having a non-standard grid implementation before we merge this.
Thanks for pointing this out. I referred to the first canvas in the following figma. Ill confirm with Erin which chevron to use. |
5 participants
Adding product-marquee-grid block to site-redesign
Resolves: MWPW-192744
Test URLs:
Before: https://main--upp--adobecom.aem.page/homepage/drafts/suhjain/redesign-demo
After: https://main--da-dc--adobecom.aem.page/dc-shared/fragments/tests/2026/q2/ace1205/fragments/ace1205-product?milolibs=pm-grid&mep=%2Fdc-shared%2Ffragments%2Ftests%2F2026%2Fq2%2Face1205%2Fdc1205-base-page-dc.json--all