Mwpw 142003: Mini Compare Chart card fixes #2093

Axelcureno · 2024-03-28T23:58:01Z

Fixes multiple Mini Compare Chart card alignment and spacing issues.

Resolves: MWPW-142003

Note: Alignment across cards will show broken in test URL because prices are not resolved due to CORS policy. To properly test alignment, please use this URL: https://main--cc--adobecom.hlx.page/drafts/axel/mini-compare-chart-edgecase?milolibs=MWPW-142003--milo--axelcureno

Test URLs:

aem-code-sync · 2024-03-28T23:58:25Z

Page	Scores	Audits	Google
/drafts/axel/mini-compare-chart?martech=off

codecov · 2024-03-29T00:01:17Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.45%. Comparing base (521defb) to head (f0588fb).

❗ Current head f0588fb differs from pull request most recent head a53853b. Consider uploading reports for the commit a53853b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##            stage    #2093      +/-   ##
==========================================
- Coverage   96.63%   96.45%   -0.18%     
==========================================
  Files         165      166       +1     
  Lines       43523    43466      -57     
==========================================
- Hits        42058    41926     -132     
- Misses       1465     1540      +75

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

narcis-radu

I am requesting changes since I have identified a couple of issues:

all browsers - you cannot tab anymore through clickable elements in the card
safari and firefox - in the 3rd card, there's a small overlapping between "Save 19% ($US7.99/mo) for the first year if you buy now" and "Secure transaction"
all browsers - mobile view is broken, the cards don't adjust width; the same issue is visible in the before link though.

Axelcureno · 2024-04-02T18:51:51Z

I am requesting changes since I have identified a couple of issues:

all browsers - you cannot tab anymore through clickable elements in the card

safari and firefox - in the 3rd card, there's a small overlapping between "Save 19% ($US7.99/mo) for the first year if you buy now" and "Secure transaction"

all browsers - mobile view is broken, the cards don't adjust width; the same issue is visible in the before link though.

@narcis-radu thanks for the review, I have addressed the mentioned issues. The Test URL from my fork will not reflect the actual changes (alignment inside cards) because prices fail in milo-forked URLs. To see the changes with prices displaying properly please see https://main--cc--adobecom.hlx.page/drafts/axel/mini-compare-chart-edgecase?milolibs=MWPW-142003--milo--axelcureno
For testing mobile, this card has mobile-specific styles and height calculations triggered upon page load. Please refresh the page when testing in mobile. We decided not to trigger mobile view when the page is resized to avoid performance impact (since most users will not go from desktop to mobile in the same window).

yesil · 2024-04-03T08:02:02Z

libs/blocks/merch-card/merch-offer-select.js

@@ -25,6 +25,7 @@ function createDynamicSlots(el, bodySlot) {
      const descriptionSlot = el.querySelector('p[slot="description"]');
      if (descriptionSlot) {
        descriptionSlot.innerHTML += description.innerHTML;
+        description.parentNode.removeChild(description);


description.remove(); should do it as well.

narcis-radu · 2024-04-03T12:29:00Z

@Axelcureno - I still cannot tab through items when I use your feature branch. The #1 item from my initial feedback is still problematic.

Axelcureno · 2024-04-03T14:27:45Z

@Axelcureno - I still cannot tab through items when I use your feature branch. The #1 item from my initial feedback is still problematic.

@narcis-radu I forgot to mention, this is fixed in a separate ticket: #2078

Roycethan

@Axelcureno
1)On mobile version the pricing text is "per" is broken in next line
https://main--cc--adobecom.hlx.page/creativecloud?milolibs=MWPW-142003--milo--axelcureno

Regarding CTA alignment its different in different page, which on'es correct?
https://main--cc--adobecom.hlx.page/creativecloud?milolibs=MWPW-142003--milo--axelcureno

https://main--cc--adobecom.hlx.page/products/premiere?milolibs=MWPW-142003--milo--axelcureno

3)'Best value' either should be in center align with product title or at top - plz review https://main--cc--adobecom.hlx.page/creativecloud?milolibs=MWPW-142003--milo--axelcureno

4)Strikethrough pricing font size is bit larger https://main--cc--adobecom.hlx.page/creativecloud?milolibs=MWPW-142003--milo--axelcureno

Axelcureno · 2024-04-09T00:04:24Z

@Axelcureno 1)On mobile version the pricing text is "per" is broken in next line https://main--cc--adobecom.hlx.page/creativecloud?milolibs=MWPW-142003--milo--axelcureno

Regarding CTA alignment its different in different page, which on'es correct?
https://main--cc--adobecom.hlx.page/creativecloud?milolibs=MWPW-142003--milo--axelcureno

https://main--cc--adobecom.hlx.page/products/premiere?milolibs=MWPW-142003--milo--axelcureno 3)'Best value' either should be in center align with product title or at top - plz review https://main--cc--adobecom.hlx.page/creativecloud?milolibs=MWPW-142003--milo--axelcureno 4)Strikethrough pricing font size is bit larger https://main--cc--adobecom.hlx.page/creativecloud?milolibs=MWPW-142003--milo--axelcureno

I have addressed the mentioned comments @Roycethan, please check:

Roycethan · 2024-04-09T01:52:49Z

https://main--cc--adobecom.hlx.page/products/premiere?milolibs=MWPW-142003--milo--axelcureno

Thanks @Axelcureno 1,2,3 are fixed now

Roycethan · 2024-04-09T01:58:06Z

Also observed one weird issue with quantity selector here: https://main--dc--adobecom.hlx.page/acrobat/features?milolibs=MWPW-142003--milo--axelcureno
4) On inner edit box theres up/down for increment/decrement when i use that the quanity is not getting applied to the Buy now CTA Href. although it works when using dropdown.
will loop @VKniaz as well since he has worked on it before

fyi @afmicka

Roycethan

@Axelcureno Plz check 4 above

Axelcureno · 2024-04-09T02:04:34Z

Also observed one weird issue with quantity selector here: https://main--dc--adobecom.hlx.page/acrobat/features?milolibs=MWPW-142003--milo--axelcureno 4) On inner edit box theres up/down for increment/decrement when i use that the quanity is not getting applied to the Buy now CTA Href. although it works when using dropdown. will loop @VKniaz as well since he has worked on it before

fyi @afmicka

@Axelcureno Plz check 4 above

Thanks @Roycethan . Quantity selector issue is not related to this card but to the component itself. Will follow up offline to get browser details from your testing.

Regarding issue #4, strikethrough should be in normal font weight and smaller font size as per Consonant XD:

XD:

With PR changes:

Roycethan

Issue mentioned above related to this PR are fixed, for few created separate tickets since they are not related to this PR and exist in main. Few other issues noted while doing regressions are also fixed. Marking as verified since this urgent request and continue testing, and if theres anything comes up will create separate ticket.
@Axelcureno cc @afmicka

Fixes multiple Mini Compare Chart card alignment and spacing issues. Resolves: MWPW-142003 Note: Alignment across cards will show broken in test URL because prices are not resolved due to CORS policy. To properly test alignment, please use this URL: https://main--cc--adobecom.hlx.page/drafts/axel/mini-compare-chart-edgecase?milolibs=MWPW-142003--milo--axelcureno Test URLs: Before: https://main--milo--axelcureno.hlx.page/drafts/axel/mini-compare-chart?martech=off After: https://mwpw-142003--milo--axelcureno.hlx.page/drafts/axel/mini-compare-chart?martech=off Co-Authored-By: ilyas Stéphane Türkben <isturkben@gmail.com>

mokimo · 2024-04-19T09:33:46Z

This PR has changes requested from @narcis-radu - this cant be merged with changes requested and thus isn't ready for stage.

Axelcureno · 2024-04-19T15:35:47Z

This PR has changes requested from @narcis-radu - this cant be merged with changes requested and thus isn't ready for stage.

@narcis-radu as per our requirements, tab click is to navigate within inner clickable elements of a merch card and arrow keys to navigate between cards. You may check this functionality in the test URL.
This PR is getting escalated because it's blocking important CC and DC migrations.

cc @yesil @mokimo @npeltier

Axelcureno · 2024-04-19T23:25:51Z

As confirmed by PdMs, the requested changes by @narcis-radu are not in accord with our requirements, hence adding Ready for Stage label.

For any questions, please reach out to April Burness, Lucy Sogard or Nick Lam.
cc @mokimo @npeltier @yesil @3ch023

drashti1712 · 2024-04-22T08:51:22Z

Hi @Axelcureno
The aem-psi-check is failing for this PR. Could you please take a look at it?

The requested changes are not in accordance with our business requirements.

Axelcureno · 2024-04-22T17:04:28Z

Hi @Axelcureno The aem-psi-check is failing for this PR. Could you please take a look at it?

Thank you @drashti1712, I have retriggered and it is now passing all checks :)

* stage: MWPW-146999: kodiak CVE 25883 (adobecom#2183) MWPW-144549 CTA alignment for Text, Icon, and Media Blocks (adobecom#2098) MWPW-146494-keep gnav sticky when branch banner is displayed (adobecom#2175) MWPW-135821 introduce mep custom action & use it for card collection (adobecom#2152) MWPW-146129 [Original: adobecom#2123] App Launcher overlaps the menu in Unav in the devices (adobecom#2184) Revert "MWPW-146129 App Launcher overlaps the menu in Unav in the devices" (adobecom#2182) MWPW-146129 App Launcher overlaps the menu in Unav in the devices (adobecom#2123) MWPW-139842 [Revert] Fill button style (adobecom#2181) Mwpw 142003: Mini Compare Chart card fixes (adobecom#2093) MWPW-146756] Add support for RTL in aside notifications, horizontal cards"" (adobecom#2178) Revert "[MWPW-146756] Add support for RTL in aside notifications, horizontal cards" (adobecom#2177) [MWPW-146756] Add support for RTL in aside notifications, horizontal cards (adobecom#2167) Revert "MWPW-142590 - [Share] enhancement - ability to edit text above icons" (adobecom#2172)

Fixes multiple Mini Compare Chart card alignment and spacing issues. Resolves: MWPW-142003 Note: Alignment across cards will show broken in test URL because prices are not resolved due to CORS policy. To properly test alignment, please use this URL: https://main--cc--adobecom.hlx.page/drafts/axel/mini-compare-chart-edgecase?milolibs=MWPW-142003--milo--axelcureno Test URLs: Before: https://main--milo--axelcureno.hlx.page/drafts/axel/mini-compare-chart?martech=off After: https://mwpw-142003--milo--axelcureno.hlx.page/drafts/axel/mini-compare-chart?martech=off Co-authored-by: ilyas Stéphane Türkben <isturkben@gmail.com>

Axelcureno added commerce needs-verification PR requires E2E testing by a reviewer merch card labels Mar 28, 2024

Axelcureno self-assigned this Mar 28, 2024

Axelcureno requested a review from a team as a code owner March 28, 2024 23:58

narcis-radu self-requested a review April 1, 2024 13:11

narcis-radu previously requested changes Apr 1, 2024

View reviewed changes

Axelcureno force-pushed the MWPW-142003 branch from 74d2193 to e5b7353 Compare April 2, 2024 18:43

Axelcureno requested a review from narcis-radu April 2, 2024 18:51

yesil approved these changes Apr 3, 2024

View reviewed changes

Axelcureno requested review from npeltier, 3ch023 and Roycethan April 3, 2024 15:03

Axelcureno mentioned this pull request Apr 4, 2024

MWPW-143192: Merch card with 2 CTAs and secure transaction label #2015

Closed

Axelcureno requested review from vladen, VKniaz and honstar April 4, 2024 05:51

3ch023 approved these changes Apr 4, 2024

View reviewed changes

Roycethan requested changes Apr 8, 2024

View reviewed changes

Axelcureno requested a review from Roycethan April 9, 2024 00:04

Roycethan requested changes Apr 9, 2024

View reviewed changes

aem-code-sync bot temporarily deployed to MWPW-142003 April 18, 2024 19:47 Inactive

aem-code-sync bot temporarily deployed to MWPW-142003 April 18, 2024 19:52 Inactive

aem-code-sync bot temporarily deployed to MWPW-142003 April 18, 2024 20:05 Inactive

aem-code-sync bot temporarily deployed to MWPW-142003 April 18, 2024 20:36 Inactive

aem-code-sync bot temporarily deployed to MWPW-142003 April 18, 2024 20:53 Inactive

aem-code-sync bot temporarily deployed to MWPW-142003 April 18, 2024 21:24 Inactive

aem-code-sync bot deployed to MWPW-142003 April 18, 2024 21:38 View deployment

Roycethan self-requested a review April 18, 2024 21:43

Roycethan approved these changes Apr 18, 2024

View reviewed changes

Roycethan added verified PR has been E2E tested by a reviewer and removed needs-verification PR requires E2E testing by a reviewer labels Apr 18, 2024

Axelcureno added the Ready for Stage label Apr 18, 2024

Axelcureno force-pushed the MWPW-142003 branch from afea437 to 7914d52 Compare April 19, 2024 03:32

mokimo removed the Ready for Stage label Apr 19, 2024

Axelcureno added the Ready for Stage label Apr 19, 2024

Merge branch 'stage' into MWPW-142003

a53853b

Ruchika4 merged commit 443d511 into adobecom:stage Apr 22, 2024
10 checks passed

Ruchika4 mentioned this pull request Apr 22, 2024

[Release] Stage to Main #2179

Merged

github-actions bot mentioned this pull request Apr 30, 2024

[Release] Stage to Main mokimo/milo#57

Closed

mokimo mentioned this pull request May 7, 2024

[Release] Stage to Main mokimo/milo#59

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mwpw 142003: Mini Compare Chart card fixes #2093

Mwpw 142003: Mini Compare Chart card fixes #2093

Axelcureno commented Mar 28, 2024 •

edited

Loading

aem-code-sync bot commented Mar 28, 2024 •

edited

Loading

codecov bot commented Mar 29, 2024 •

edited

Loading

narcis-radu left a comment

Axelcureno commented Apr 2, 2024

yesil Apr 3, 2024

narcis-radu commented Apr 3, 2024

Axelcureno commented Apr 3, 2024

Roycethan left a comment

Axelcureno commented Apr 9, 2024

Roycethan commented Apr 9, 2024

Roycethan commented Apr 9, 2024 •

edited

Loading

Roycethan left a comment •

edited

Loading

Axelcureno commented Apr 9, 2024

Roycethan left a comment

mokimo commented Apr 19, 2024 •

edited

Loading

Axelcureno commented Apr 19, 2024

Axelcureno commented Apr 19, 2024

drashti1712 commented Apr 22, 2024

Axelcureno commented Apr 22, 2024 •

edited

Loading

Mwpw 142003: Mini Compare Chart card fixes #2093

Mwpw 142003: Mini Compare Chart card fixes #2093

Conversation

Axelcureno commented Mar 28, 2024 • edited Loading

aem-code-sync bot commented Mar 28, 2024 • edited Loading

codecov bot commented Mar 29, 2024 • edited Loading

Codecov Report

narcis-radu left a comment

Choose a reason for hiding this comment

Axelcureno commented Apr 2, 2024

yesil Apr 3, 2024

Choose a reason for hiding this comment

narcis-radu commented Apr 3, 2024

Axelcureno commented Apr 3, 2024

Roycethan left a comment

Choose a reason for hiding this comment

Axelcureno commented Apr 9, 2024

Roycethan commented Apr 9, 2024

Roycethan commented Apr 9, 2024 • edited Loading

Roycethan left a comment • edited Loading

Choose a reason for hiding this comment

Axelcureno commented Apr 9, 2024

Roycethan left a comment

Choose a reason for hiding this comment

mokimo commented Apr 19, 2024 • edited Loading

Axelcureno commented Apr 19, 2024

Axelcureno commented Apr 19, 2024

drashti1712 commented Apr 22, 2024

Axelcureno commented Apr 22, 2024 • edited Loading

Axelcureno commented Mar 28, 2024 •

edited

Loading

aem-code-sync bot commented Mar 28, 2024 •

edited

Loading

codecov bot commented Mar 29, 2024 •

edited

Loading

Roycethan commented Apr 9, 2024 •

edited

Loading

Roycethan left a comment •

edited

Loading

mokimo commented Apr 19, 2024 •

edited

Loading

Axelcureno commented Apr 22, 2024 •

edited

Loading