Skip to content

MWPW-204598 AU ABM tax label display - #1190

Open
bozojovicic wants to merge 14 commits into
mainfrom
MWPW-204598
Open

MWPW-204598 AU ABM tax label display#1190
bozojovicic wants to merge 14 commits into
mainfrom
MWPW-204598

Conversation

@bozojovicic

@bozojovicic bozojovicic commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

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

Display annual prices (generated after ABM price) in a new line in MAS cards, in a way we did it for Plans cards.

Milo PR adobecom/milo#6604

Test pages :
https://mwpw204598--milo--adobecom.aem.page/au/drafts/bozo/cards?martech=off&maslibs=MWPW-204598
https://mwpw204598--milo--adobecom.aem.page/drafts/bozo/annual-cards?martech=off&maslibs=MWPW-204598

Please do the steps below before submitting your PR for a code review or QA

  • 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 (bonus: write a working demo script that you'll use on Thursday, you can eventually put in your PR)
  • C.6 read your Jira one more time to validate that you've addressed all AC's and nothing is missing

🧪 Nala E2E Tests

Nala tests run automatically when you open this PR.

To run Nala tests again:

  1. Add the run nala label to this PR (in the right sidebar)
  2. Tests will run automatically on the current commit
  3. Any future commits will also trigger tests as long as the label remains

To stop automatic Nala tests:

  • Remove the run nala label

Note: Tests only run on commits if the run nala label is present. Add the label whenever you need tests to run on new changes.

Test URLs:

@aem-code-sync

aem-code-sync Bot commented Aug 31, 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

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.90566% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.79%. Comparing base (2f2e131) to head (6fe2af9).

Files with missing lines Patch % Lines
...components/src/variants/mini-compare-chart-mweb.js 82.60% 4 Missing ⚠️
web-components/src/variants/mini-compare-chart.js 82.60% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1190      +/-   ##
==========================================
+ Coverage   90.71%   90.79%   +0.07%     
==========================================
  Files         318      318              
  Lines      101712   101746      +34     
==========================================
+ Hits        92271    92381     +110     
+ Misses       9441     9365      -76     
Files with missing lines Coverage Δ
web-components/src/variants/plans-v2.css.js 100.00% <100.00%> (ø)
web-components/src/variants/product.css.js 100.00% <100.00%> (ø)
web-components/src/variants/segment.css.js 100.00% <100.00%> (ø)
...components/src/variants/mini-compare-chart-mweb.js 96.59% <82.60%> (-1.53%) ⬇️
web-components/src/variants/mini-compare-chart.js 83.28% <82.60%> (+2.18%) ⬆️

... and 8 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 2f2e131...6fe2af9. 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.

@Roycethan Roycethan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@bozojovicic Feedbacks:
I see different issues on different pages - got time only to review these for now:

  1. No space after incl. GST:
    https://main--da-dc--adobecom.aem.live/au/acrobat/features?maslibs=MWPW-204598&milolibs=mwpw204598
Image
  1. No spacing before 'per license' - is 'per license' should be placed before 'incl GST' here ? plz check the placement as well
    https://main--da-cc--adobecom.aem.live/au/creativecloud?maslibs=MWPW-204598&milolibs=mwpw204598
Image
  1. Edu cards didn't get same treatment as others:
    https://main--edu--adobecom.aem.live/au/education/students/creativecloud/features?maslibs=MWPW-204598&milolibs=mwpw204598
Image

4)Does Express need Annual pricing ? Im not sure , but it doesn't exist currently:
https://main--da-express-milo--adobecom.aem.live/express/?country=au&tab=2

  1. Are they expecting 'per licence'/'incl GST' etc.. labels before the Annual price like in above cases or per expected results in the ticket ? If so that should be same for these cards also right ?
    https://main--da-cc--adobecom.aem.live/au/products/dreamweaver?maslibs=MWPW-204598&milolibs=mwpw204598
Image

https://main--da-dc--adobecom.aem.live/au/acrobat/pricing?maslibs=MWPW-204598&milolibs=mwpw204598

Image

this has issue on per license as well:
https://main--da-dc--adobecom.aem.live/au/acrobat/pricing/business?maslibs=MWPW-204598&milolibs=mwpw204598
Image

See (*) here:
https://main--da-dc--adobecom.aem.live/au/acrobat/pricing/students?maslibs=MWPW-204598&milolibs=mwpw204598

Image

Not sure if on plans cards also the per license/Incl GST should come before Annual pricing ?
https://main--cc--adobecom.aem.live/au/creativecloud/plans?maslibs=MWPW-204598&milolibs=mwpw204598

  1. CTA's here lost same line alignment
    https://main--da-cc--adobecom.aem.live/au/products/indesign/plans?maslibs=MWPW-204598&milolibs=mwpw204598
Image

@Roycethan Roycethan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

See above

@bozojovicic

Copy link
Copy Markdown
Contributor Author

@Roycethan

  1. Fixed
  2. Fixed
  3. This is Milo card. Lucy said that I should not waste time on Milo cards since they will be replaced soon.
  4. I also don't know if we need annual (after ABM) for Express cards but I implemented them. You don't see it because Ilyas recently added the change in global settings, if annual template is not enabled for some card variant in some surface then you will not see it. So you need to enable annual template for these express cards in global settings in the surface where these cards are created. It is not enough any more just to set mas-ff-annual-price. I'm not sure why he did that.
  5. For segment cards - NO. Lucy asked for this only in templates where these labels are displayed after the annual price in the same line. She wanted that annual price to be extracted and displayed in the next line, and everything else should be the same. Here these labels are displayed separatelly and that was the AC for segment cards from the beginning.
    This "per license" and "*" displayed after the annual price in the same line, this is authoring mistake. They added this text manually. I cannot do anything here, they need to fix this somehow.
    Regarding the last item in 5 - for the plans page - I did not change anything here. This was like this before and it should be like this.
  6. This should be fixed now, I just updated the Milo PR with fresh code from Stage branch.

@Roycethan
Roycethan self-requested a review September 4, 2026 20:54

@Roycethan Roycethan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@bozojovicic Fixes look good. Plz resolve the conflicts.

@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.

global.css.js:614 isn't flag-gated, so it changes legal-price spacing on every merch-card in every market. .price-tax-inclusivity loses its leading nbsp and only gets it back via .price-unit-type::after. With displayPerUnit false and displayTax true, is there anything left in front of the tax label?

Also the flag block is duplicated verbatim in both mini-compare variants, with no unit tests on either branch.

@bozojovicic

Copy link
Copy Markdown
Contributor Author

global.css.js:614 isn't flag-gated, so it changes legal-price spacing on every merch-card in every market.

@Axelcureno Right. I rollbacked that change and corrected the mis-aligned tax label in the code for product and segment only, since only in these 2 variants I had this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants