MWPW-202435: Tech debt: avoid relocating mas-field CTA content outside the mas-field wrapper - #6586
MWPW-202435: Tech debt: avoid relocating mas-field CTA content outside the mas-field wrapper#6586joaquinrivero wants to merge 10 commits into
Conversation
…paint race mas-field now stamps its context (promo code, ids) onto the elements it renders, so it survives the unwrap by construction: drop copyMasFieldIdToParent / preserveInlineCommerceContext, keep only the merch.css load. Also hold an unwrapped CTA's action area hidden until the card's still-wrapped price resolves, closing the cold-load window where the CTA paints above the price. MWPW-202435.
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
|
|
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. |
yesil
left a comment
There was a problem hiding this comment.
@joaquinrivero AFAIK, with #6463, the MAS fields will become part of the merch autoblock.
Could you please check with @TsayAdobe whether this is still needed, or if you can rebase on stage once that PR is merged?
Axelcureno
left a comment
There was a problem hiding this comment.
Approving. This is the Milo half of MWPW-202435 — pairs with mas#1185, which adds the #stampContext price-span stamping this relies on. Just make sure that MAS bundle deploys before/with this, or price-field promo codes drop on unwrap.
One ask: the new holdCtaUntilPrice tests only hit the happy path — the 3s timeout fallback (the real safety net) and the .section walk boundary aren't covered. Worth adding.
yesil
left a comment
There was a problem hiding this comment.
I'll wait for @TsayAdobe's review.
There should not be any more mas-field logic in merch-card-autoblock
* origin/stage: (33 commits) INTL1207 - Add content-toggle block for A/B test campaign (#6451) Add a guard in decorateAutoBlock for merch links (#6567) [MWPW-204859] - Apply forced-reflow fix to brand-concierge floating (#6510) [MWPW-204698] Remove manifest overline (#6496) [Milo][Martech] Remove performance consent check from analytics event sending (#6491) MWPW-203941: Update authoring pattern for bc marquee (#6490) MWPW-204795 [MEP] Remove RainFocus registration-data (event-id) path (#6483) [MWPW-204301] [Lingo] MEP replace/remove on mep-lingo fragments + regional replacement resolution (#6474) MWPW-202286 Separate mas field from merch card autoblock (#6463) [MWPW-203054] [MWPW-203055] - Additional Manifest and Page Summary fields. (#6446) MWPW-200965: Port email-collection block to C2 (#6441) [MWPW-205693]: Adding rtl change for languageselector (#6581) [MWPW-204596] Validate the fedsbranch query parameter in C2 global-navigation (#6497) [MWPW-199609] Harden sanitizeHtml URL scheme allowlist (#6475) MWPW-204984: Revert eager IMS load (#6511) (#6549) MWPW-204984 [MWPW-204913]: C2 persistent cart (#6522) fix for mc autoblock MWPW-204875: eagerly load ims up to date with main ... # Conflicts: # libs/blocks/merch-card-autoblock/merch-card-autoblock.js # libs/blocks/merch/merch.js # test/blocks/merch-card-autoblock/merch-card-autoblock.test.js
feb9255 to
28cd2ab
Compare
|
@joaquinrivero I compared the DOMs of before and after I don not see any difference. what should we check in the DOM? |
|
Error merging 6586: MWPW-202435: Tech debt: avoid relocating mas-field CTA content outside the mas-field wrapper Pull Request has merge conflicts |
|
@yesil Since the mas PR got merge, we are seeing the context injected by mas now, instead of doing it from milo side, we expect to see the same context without copying it on milo |
* origin/stage: (21 commits) VULN-38270: Validate repo/owner and pin sharepoint.site to Adobe's Graph host (#6610) Brand Concierge - UI update based on chat history (#6624) Site redesign foundation (#6614) MWPW-205898 Update Field Names (#6606) MWPW-205871 Add Marketo form id param (#6593) MWPW-205706: bottom-align compare-pod CTAs when a promo makes a price taller (#6587) MWPW-205235 - Contribute Blog Author Block From da-bacom-blog to Milo (#6577) MWPW-203648 Conflicting hashes causing CTA not to display (#6500) MWPW-204427 [MEP][C2 Gnav] Scope gnav decorateBody hook to href localization (#6482) [Site Redesign] PDF-Space block RTL Support Sync (#6609) [MWPW-203651] Geo-IP placeholders: single sheet per repo (column-per-market + language tabs) (#6589) MWPW-203985 [Preview Indexer] Replace all site admin tokens with a single AEM EDS Service token (#6617) [NALA] Stabilize BC floating-button scroll assertions (webkit flake) (#6620) S2A v0.0.21 spacing and border-radius sync (#6439) [Lingo] MWPW-203985 Preview Indexer - Replace all site admin tokens with a single AEM EDS Service token (#6521) add logs and pass arp sesson token (#6580) [MWPW-203833] Debounce brand-concierge floating-input resize handler (#6555) MWPW-204858 M@S Preflight Price Unavailable Check (#6552) MWPW-204695: Fix video play/pause interaction (#6527) MWPW-204936: Adds Product Icon option to details text (#6518) ... # Conflicts: # libs/blocks/merch/merch.js
yesil
left a comment
There was a problem hiding this comment.
@joaquinrivero isn't this PR supposed to keep the CTA as child of mas-field ?
Re-nest mas-field to wrap the authored em/strong instead of unwrapping it, so promo/id context survives by structure. Needs mas-field display:contents (MWPW-206522) to stay boxless.
|
@yesil Reworked to do exactly that. The CTA now stays inside mas-field, thanks. |
Drop the hand-rolled timer/clearTimeout; race checkReady against the shared FIELD_TIMEOUT like checkFieldReady does.
Axelcureno
left a comment
There was a problem hiding this comment.
Two small things on the re-nest commit, both non-blocking. Approval stands.
| if (outer === masField) { | ||
| masField.replaceChildren(...content.childNodes); | ||
| } else { | ||
| masField.replaceWith(...content.childNodes); |
There was a problem hiding this comment.
Leaves the emptied <span data-role="mas-field-content"> in mas-field, unlike the replaceChildren path above. MAS #ensureContentElement matches :scope > span[data-role=...] and reuses it, so a re-render writes into that span ahead of the decorated <em><a>. Drop it after the append?
| if (!price?.checkReady) return; | ||
| container.style.visibility = 'hidden'; | ||
| const reveal = () => { container.style.visibility = ''; }; | ||
| withTimeout(price.checkReady().catch(() => false)).then(reveal); |
There was a problem hiding this comment.
Nice catch on the double reveal. This also moves the hold from 3s to FIELD_TIMEOUT (5s), so a slow WCS hides the CTA 2s longer. Intended?
Roycethan
left a comment
There was a problem hiding this comment.
@joaquinrivero Regression observed:
"Free trial" shifts right by 16px (past one phantom span+gap), and "Buy now" shifts by 32px (past two)
https://main--da-cc--adobecom.aem.live/products/premiere/plans?maslibs=MWPW-206522&milolibs=MWPW-202435
abbc786 to
84f85c2
Compare
|
Follow-ups from review:
Gnav fix stacked as #6660; retargets to |


Resolves: MWPW-202435
Keep the
mas-fieldwrapper around inline CTAs instead of unwrapping it, so commerce context (promo code, ids) survives by structure.Needs mas
display: contentsdeployed first, else the kept wrapper computes as a box and breaks layout.Companion PR (mas): adobecom/mas#1210
Test URLs: