Skip to content

MWPW-206315: {{plan-type-text}} system placeholder (plan-type-only legal price, card-scoped) - #1202

Merged
afmicka merged 20 commits into
mainfrom
MWPW-206315
Sep 7, 2026
Merged

MWPW-206315: {{plan-type-text}} system placeholder (plan-type-only legal price, card-scoped)#1202
afmicka merged 20 commits into
mainfrom
MWPW-206315

Conversation

@yesil

@yesil yesil commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Resolves https://jira.corp.adobe.com/browse/MWPW-206315
QA Checklist: https://wiki.corp.adobe.com/display/adobedotcom/M@S+Engineering+QA+Use+Cases

Introduces a {{plan-type-text}} system placeholder that renders the plan-type label only (via the inline-price legal template), sourcing the offer from the surrounding card/field.

How it works

  • io/www{{plan-type-text}} is a generic system placeholder: a SYSTEM_PLACEHOLDERS map merged into the dictionary in replace, so it resolves on any fragment to <span is="inline-price" data-template="legal" data-placeholder="plan-type-text">. System keys are spread last, so an Odin dictionary entry of the same key can't shadow them.
  • web-components — a common price-options provider on merch-card and mas-field (registered by both hosts) backfills the OSI from a new osi getter (promo → regular → fragment osi field), forces plan-type-only (all other display options false), and applies sentence-boundary casing in price/legal.js. mas-field also merges the fragment's priceLiterals (parity with merch-card) so locale plan-type labels render. The marker's CSS (inline display + visibility over the consumer's hide-until-resolved rule) self-injects from the feature module, so it works on mas-field-only pages too. No changes to inline-price.js / hydrate.js.

Notes

  • Casing follows sentence boundaries (. ! ? / start → uppercase; else lowercase). Case-less scripts (e.g. Hebrew) are natural no-ops.

  • Accepted limitation: during a total Odin dictionary outage, {{plan-type-text}} is left raw like any placeholder (system placeholders merge into the dictionary only when it resolves).

  • Coverage: io/www 100%; web-components suite green. NALA E2E recommended (marker on a real card and a mas-field).

  • C1. Cover code with Unit Tests

  • C2. Add a Nala test (double check with #fishbags if nala test is needed)

  • C3. Verify all Checks are green (unit tests, nala tests)

  • C4. PR description contains working Test Page link where the feature can be tested

  • C5: you are ready to do a demo from Test Page in PR

  • C.6 read your Jira one more time to validate that you've addressed all AC's and nothing is missing

Test URLs:

Screenshots

image

@yesil yesil added the run nala label Sep 2, 2026
@aem-code-sync

aem-code-sync Bot commented Sep 2, 2026

Copy link
Copy Markdown

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-sync branch
Commits

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

SNOW Change Request Transaction ID [IO]: 0000019c1ef37582-20bacbe0

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.79%. Comparing base (26e2f94) to head (9bfb9af).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1202      +/-   ##
==========================================
+ Coverage   90.76%   90.79%   +0.02%     
==========================================
  Files         318      319       +1     
  Lines      101712   101830     +118     
==========================================
+ Hits        92317    92454     +137     
+ Misses       9395     9376      -19     
Files with missing lines Coverage Δ
web-components/src/mas-field.js 98.94% <100.00%> (+0.02%) ⬆️
web-components/src/merch-card.js 89.56% <100.00%> (-0.20%) ⬇️
web-components/src/plan-type-text.js 100.00% <100.00%> (ø)
web-components/src/price/legal.js 100.00% <100.00%> (ø)

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26e2f94...9bfb9af. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Axelcureno Axelcureno left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

planTypeTextOptionsProvider sets displayPlanType = true, but ten other providers set it unconditionally for data-template="legal", which the marker is: mas-field.js:110 plus the nine variant layouts via merch-card.js:79. providers.price is a Set and both hosts register in connectedCallback, so whichever connects second runs last.

Repro'd both ways: card first breaks the mas-field's marker, field first the card's. Single-host pages always order right, so the tests pass.

price.js:54 applies placeholder.dataset after every provider, so data-display-plan-type="true" in the emitted markup fixes it.

Comment thread web-components/src/merch-card.js Outdated
Comment thread web-components/src/mas-field.js
Comment thread web-components/src/plan-type-text.js Outdated
Comment thread web-components/test/plan-type-text.test.js
Comment thread web-components/test/merch-card.osi.test.js
Comment thread web-components/src/global.css.js Outdated

@Andrei4226 Andrei4226 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bundles in dist/ are unminified, unlike on main (+55,110)
I think the fix is to run the production build (with minify on) and re-commit dist/.

@yesil
yesil requested a review from Axelcureno September 4, 2026 11:48
…ard prices, skips cancel-context), block-ancestor casing, test cleanup + rename
…text), block-ancestor casing, afterEach cleanup
@yesil

yesil commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @Axelcureno — great catches, all addressed:

  • displayPlanType ordering: moved it (plus the two suppressions) onto the marker as data-display-plan-type/per-unit/tax, so collectPriceOptions' placeholder.dataset merge wins regardless of which host connects last. The provider is now just OSI + casing. Added a regression test that runs the marker through collectPriceOptions with the mas-field provider registered last.
  • osi getter: extracted to a shared hostOsi() in plan-type-text.js — now scoped past <merch-addon> prices and skipping the cancel-context sentinel (both covered by tests).
  • precedingChar: ranges from the nearest block ancestor now, so <em>{{plan-type-text}}</em> mid-sentence cases as lower (regression test added).
  • tests: added afterEach cleanup, renamed to merch-card.osi.test.js, and dropped the stray blank line in global.css.js.

commits 5146ee5 and 8571d72.

@Axelcureno Axelcureno left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All six addressed. Recreated the pre-fix state locally and the ordering test fails, so it's a real guard. The three flags on the marker are exactly what legalTemplate reads, so nothing's missing.

# Conflicts:
#	web-components/dist/commerce.js
#	web-components/dist/mas.js
#	web-components/dist/merch-card-collection.js
@yesil

yesil commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

@Andrei4226 good catch — synced with main and re-ran the production (minified) build, re-committed dist/. It's now ~+28 lines vs main (just the feature); the earlier bloat was diff churn from rebuilds diverging from the base. Build and Test Web Components is green.

@Andrei4226 Andrei4226 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@afmicka
afmicka merged commit 78960eb into main Sep 7, 2026
20 checks passed
@afmicka
afmicka deleted the MWPW-206315 branch September 7, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants