MWPW-156940 AU price display #3115

bozojovicic · 2024-10-30T13:53:51Z

This is the cleaner version of PR #3065
ABM price on Australian pages must be displayed together with annual price in brackets. Few examples from Figma :

Test URLs :
https://main--cc--adobecom.hlx.live/au/products/photoshop?milolibs=mwpw156940auabm2--milo--bozojovicic
https://main--cc--adobecom.hlx.live/au/products/illustrator?milolibs=mwpw156940auabm2--milo--bozojovicic
https://main--cc--adobecom.hlx.live/au/products/photoshop/plans?milolibs=mwpw156940auabm2--milo--bozojovicic
https://main--cc--adobecom.hlx.live/au/products/illustrator/plans?milolibs=mwpw156940auabm2--milo--bozojovicic

https://mwpw156940auabm2--milo--bozojovicic.aem.page/au/docs/library/kitchen-sink/merch-card

Resolves: MWPW-156940

Test URLs:

aem-code-sync · 2024-10-30T13:54:15Z

	Page	Scores	Audits	Google
📱	/au/docs/library/kitchen-sink/merch-card
🖥️	/au/docs/library/kitchen-sink/merch-card
📱	/drafts/bozo/price-cta?martech=off
🖥️	/drafts/bozo/price-cta?martech=off

codecov · 2024-10-30T13:57:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.35%. Comparing base (d809cd5) to head (b84d095).
Report is 9 commits behind head on stage.

Additional details and impacted files

@@           Coverage Diff           @@
##            stage    #3115   +/-   ##
=======================================
  Coverage   96.35%   96.35%           
=======================================
  Files         245      245           
  Lines       56316    56329   +13     
=======================================
+ Hits        54261    54274   +13     
  Misses       2055     2055

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

npeltier

looks much better to me @bozojovicic, just one move proposal comment left! can you create AU drafts so we can see it in action?

npeltier · 2024-10-30T16:03:13Z

libs/blocks/hero-marquee/hero-marquee.js

@@ -258,6 +258,10 @@ export default async function init(el) {
      firstCol.parentElement.classList.add(`row-${parsed.key}`, 'con-block');
      firstCol.remove();
      cols[1].classList.add('row-wrapper');
+      if (locale.prefix === '/au') {


can't this logic be in merch.js?

typically in merch's initService, you know you will need merch, and you could eventually check for AU & hero?

@npeltier I removed this whole change related with hero-marquee from this PR. The request is :

if we have strikethru price next to the ABM then the annual price needs to go to new line

if there is no strikethru price, then ABM and annual price need to be in the same line

this counts for hero-marquee component

But it is just too complicated change for something that is asked for AU only. And now I see that we have some marquee component, similar to hero-marquee ... So, let's go without this change. If they don't like it, we can discuss it again. It is not well defined anyway.

Therefore, there will be no au-merch.js file. Only au-merch.css

What I mean by "not well defined"? They just sent few example screenshots in figma file. They never wrote in Jira some specific requirements like : do changes A and B in component C on page D.

npeltier

looks good to me now, thanks @bozojovicic :)

yesil · 2024-10-31T08:17:02Z

libs/blocks/merch/au-merch.css

+  content: '\A';
+  white-space: pre;
+}
+merch-card [slot='heading-m'] [is="inline-price"] .price-annual-prefix,


merch-card styles do not belong here. They should go in global.css.js in mas web-components.

@npeltier @yesil we need some decision here. We had this piece of CSS in libs/features/mas/web-components/src/global.css.js before. Nicolas proposed to move all such changes to au-merch.css which will be loaded on AU pages only. Do you think that this part should be an exception and that I should keep it in global.css.js ? Or it is better like this?

@bozojovicic (cc @yesil ) the two informations are not necessary contradictory.
i say i don't like having AU css (and logic back then) in all of the code,
ilyas says that he does not like having card WC styling in milo styles.

@yesil do you think that styling is generic to all m@s consumers? i would rather keep it in milo for now

@npeltier part of the CSS rules like the following look generic to me for annual price formatting.
anything that is for annual price formatting should stay in global.css.js IMO.

merch-card [slot='heading-m'] [is="inline-price"] .price-annual-prefix + .price-annual

@yesil CSS rules that we have now in au-merch.css I initially had in

libs/blocks/table/table.css

libs/features/mas/web-components/src/global.css.js

libs/features/mas/web-components/src/variants/mini-compare-chart.css.js

and then we decided to move all of that to au-merch.css and load that file for AU pages only.
If I move this part to global.css.js, then I need to do the same for the part I copied from mini-compare-chart.css.js
And then only few rules will stay in au-merch.css and then this files loses its purpose.

For me, both approaches do the job. I personally vote to keep it all in au-merch.css. But I just need some decision which part goes where.
CC @npeltier

yesil

Discussed with @npeltier , I'm fine with keeping these styles in Milo au specific file.

afmicka · 2024-11-01T14:51:05Z

@bozojovicic Have added the following comments to the jira as well

Even though it has been agreed by my understanding that the manually added tax label placeholders will be fixed after the Nov 6th launch, there is one page where the display issue is more prominent and visible. needs verification if acceptable for the launch
https://main--cc--adobecom.hlx.live/au/creativecloud?milolibs=mwpw156940auabm2--milo--bozojovicic

The price gets outside the table area on mobile view (iPhone 12 Pro, real iPhone 14 Pro)
https://main--cc--adobecom.hlx.live/au/products/illustrator/plans?milolibs=mwpw156940auabm2--milo--bozojovicic

The rest of regression looks ok to me. If GWP approves to go as is, we can add verify and Ready for stage labels.
cc @Roycethan (please have a second look if time permits)

bozojovicic · 2024-11-01T16:16:12Z

@afmicka @Roycethan I added couple of fixes.

The label "incl GST" is not displaced any more, now it naturally flows after the annual price. It almost looks ok now. ABM price also goes after the ST price, in the same line now. Of course content still needs to be fixed to have what is requested in figma.
Should also be fixed now. And will be even better once the content is fixed.

Roycethan · 2024-11-01T17:26:58Z

@bozojovicic
1)Prices should not break at line end, Annual price here should wrap in one line
https://main--cc--adobecom.hlx.live/au/products/special-offers?milolibs=mwpw156940auabm2--milo--bozojovicic#

Annual price is not rendered in this case - Looks like optical pricing is used... Are these optical pricing are not cleaned up or planned for migration ?
https://main--cc--adobecom.hlx.live/au/products/photoshop-lightroom?milolibs=mwpw156940auabm2--milo--bozojovicic
https://main--cc--adobecom.hlx.live/au/products/premiere/plans?milolibs=mwpw156940auabm2--milo--bozojovicic

cc @afmicka

MWPW-156940 AU price display

cf81949

Bozo Jovicic added 3 commits October 30, 2024 14:58

MWPW-156940 AU price display

b82167c

MWPW-156940 AU price display

1992843

Trigger Build

359b3a4

bozojovicic mentioned this pull request Oct 30, 2024

MWPW-156940 AU ABM price display #3065

Closed

bozojovicic marked this pull request as ready for review October 30, 2024 15:55

bozojovicic requested a review from a team as a code owner October 30, 2024 15:55

bozojovicic requested review from npeltier and 3ch023 October 30, 2024 16:00

npeltier reviewed Oct 30, 2024

View reviewed changes

Bozo Jovicic added 4 commits October 30, 2024 17:40

MWPW-156940 AU price display

403b429

MWPW-156940 AU price display

1b0f127

Trigger Build

c4f3734

Trigger Build

6eed077

npeltier approved these changes Oct 31, 2024

View reviewed changes

yesil requested changes Oct 31, 2024

View reviewed changes

yesil approved these changes Oct 31, 2024

View reviewed changes

Bozo Jovicic added 4 commits November 1, 2024 16:33

MWPW-156940 AU price display

13c1df2

Merge branch 'stage' into mwpw156940auabm2

efe1399

MWPW-156940 AU price display

25bf1ac

Trigger Build

b84d095

npeltier added the commerce label Nov 4, 2024

afmicka added verified PR has been E2E tested by a reviewer Ready for Stage labels Nov 4, 2024

3ch023 added the high priority Why is this a high priority? Blocker? Critical? Dependency? label Nov 4, 2024

milo-pr-merge bot merged commit 7dab64d into adobecom:stage Nov 4, 2024
30 checks passed

milo-pr-merge bot mentioned this pull request Nov 4, 2024

[Release] Stage to Main #3138

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MWPW-156940 AU price display #3115

MWPW-156940 AU price display #3115

bozojovicic commented Oct 30, 2024 •

edited

Loading

aem-code-sync bot commented Oct 30, 2024 •

edited

Loading

codecov bot commented Oct 30, 2024 •

edited

Loading

npeltier left a comment

npeltier Oct 30, 2024

npeltier Oct 30, 2024

bozojovicic Oct 30, 2024

bozojovicic Oct 30, 2024

npeltier left a comment

yesil Oct 31, 2024

bozojovicic Oct 31, 2024

npeltier Oct 31, 2024

yesil Oct 31, 2024

bozojovicic Oct 31, 2024

yesil left a comment

afmicka commented Nov 1, 2024 •

edited

Loading

bozojovicic commented Nov 1, 2024

Roycethan commented Nov 1, 2024 •

edited

Loading

MWPW-156940 AU price display #3115

MWPW-156940 AU price display #3115

Conversation

bozojovicic commented Oct 30, 2024 • edited Loading

aem-code-sync bot commented Oct 30, 2024 • edited Loading

codecov bot commented Oct 30, 2024 • edited Loading

Codecov Report

npeltier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

npeltier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yesil left a comment

Choose a reason for hiding this comment

afmicka commented Nov 1, 2024 • edited Loading

bozojovicic commented Nov 1, 2024

Roycethan commented Nov 1, 2024 • edited Loading

bozojovicic commented Oct 30, 2024 •

edited

Loading

aem-code-sync bot commented Oct 30, 2024 •

edited

Loading

codecov bot commented Oct 30, 2024 •

edited

Loading

afmicka commented Nov 1, 2024 •

edited

Loading

Roycethan commented Nov 1, 2024 •

edited

Loading