[MWPW-158015] [Mini Compare Merch Card] Redesign mini compare card for testing #3147

rohitsahu · 2024-11-06T10:35:59Z

Redesign mini-compare chart. Added checkmarks for icons. Backward compatibility using "checkmark" class. Horizontal line added.
For mobile, implemented cheveron up and down functionality for checkmark copy list

Testing notes:

Please ensure backward compatibility is maintained.
This ticket also fixes a related issue: https://jira.corp.adobe.com/browse/MWPW-161399

Resolves: MWPW-158015

Test URLs:

…patibility using "checkmark" class. Horizontal line added.

aem-code-sync · 2024-11-06T10:37:18Z

	Page	Scores	Audits	Google
📱	/drafts/rosahu/rosahu/min-compare-redesign?martech=off
🖥️	/drafts/rosahu/rosahu/min-compare-redesign?martech=off

rahulgupta999 · 2024-11-07T01:30:08Z

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

@@ -2,6 +2,7 @@ import { decorateButtons, decorateBlockHrs } from '../../utils/decorate.js';
 import { getConfig, createTag, loadStyle } from '../../utils/utils.js';
 import { getMetadata } from '../section-metadata/section-metadata.js';
 import { processTrackingLabels } from '../../martech/attributes.js';
+import { chevronDownSVG, chevronUpSVG } from './img/chevron.js';


this can be on-demand for performance reasons

review comment

rohitsahu · 2024-11-07T14:44:03Z

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

+
+  if (isCheckmark) {
+    const firstRow = footerRows[0];
+    const bgStyle = firstRow.querySelector('div > div:first-child').innerHTML;


use "#E8E8E8" as default

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

rohitsahu · 2024-11-07T15:42:41Z

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

+  if (!footerRows) return;
+
+  const footerRowsSlot = createTag('div', { slot: 'footer-rows' });
+  const isCheckmark = merchCard.classList.contains('checkmark');


replace checkmark with a more business/functional term

Axelcureno · 2024-11-08T01:37:33Z

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

@@ -46,6 +46,9 @@ export class MiniCompareChart extends VariantLayout {
        'promo-text',
        'callout-content',
    ];
+    if (this.card.classList.contains('checkmark')) {


Looking for a class added in Milo inside the web component code might seem a bit odd. I would suggest to move this code to Milo or move the footer row code to web component class. Ideally they should be decoupled.

thoughts @yesil, @npeltier @3ch023 ?

I can go with adjusting the footer rows height in milo if others agree. Only drawback I see is height alignment logic spread across 2 places.
How about changing the footer rows slot name for new mini compare design. Then I just need to add the new slot name here @Axelcureno ?

On a second thought, I think it's okay to leave it as is. Anyways this card will only be used in Milo.

yesil

@rohitsahu could you please add unit tests for both the custom element and block.

yesil · 2024-11-08T18:13:10Z

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

@@ -68,6 +68,14 @@ export const CSS = `
    height: var(--consonant-merch-card-mini-compare-chart-icon-size);
  }

+  merch-card[variant="mini-compare-chart"] .footer-rows-title {
+    font-color: #2C2C2C;
+    font-weight: 700;


do we have merch variables for these values?

I could not find variables for these 2. For others using variables

I think it is: --merch-color-grey-80

rohitsahu · 2024-11-11T10:31:00Z

@rohitsahu could you please add unit tests for both the custom element and block.

Added @yesil

rohitsahu · 2024-11-27T17:04:38Z

Code scanning is complaining post merge and "npm run build" in mas component

github-actions · 2024-12-05T01:42:46Z

This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label.

mirafedas · 2024-12-11T12:46:42Z

@afmicka conflicts resolved, could you please verify?

afmicka · 2024-12-11T13:26:15Z

@mirafedas @rohitsahu
Cards with Secure transaction do not match the design
https://mwpw-158015--milo--rohitsahu.hlx.page/drafts/rosahu/rosahu/min-compare-redesign?martech=off

both desktop and mobile:

Extra white space at the bottom on desktop i assume is also not by the design:

cc: @Roycethan

afmicka · 2024-12-12T11:39:02Z

@mirafedas this change is affecting CTAs on consumers page
https://main--cc--adobecom.hlx.live/pl/creativecloud?milolibs=mwpw-158015--milo--rohitsahu

on mobile as well:

mirafedas · 2024-12-13T13:27:25Z

@afmicka fixed, could you please check again?

Roycethan · 2024-12-14T00:58:49Z

@mirafedas
Here are few issues/observations:

https://mwpw-158015--milo--rohitsahu.hlx.page/drafts/rosahu/rosahu/min-compare-redesign?martech=off

Tablet should not have chevron and dropdown per Figma - it should be same as desktop:

Expect:

On mobile: Secure transaction should be vertically aligned with other elements, its slightly off or outside:

Expect:

3)Not sure if this case will happen or not but if happens ( like one card with CTA long string and other card with short ) -
Then the secure transaction label is not horizontally aligned between cards: Mentioning here but if theres no good fix then we will keep as in - hopefully this won't happen

Yellow label is slightly above and not in algnment with product title

Expect:

5)Can you plz cross check the gap between the components comparing figma:
specially its very narrow here:
gap between mnemonics and product title
gap between description and pricing

mirafedas · 2024-12-18T12:19:18Z

@Roycethan thanks for verification,
1, 2, 4, and 5 corrected, as for the #4 - there is no good fix for this, because the secure transaction label should always be located close to the buttons.

Roycethan · 2024-12-20T06:29:23Z

MWPW-158015

@mirafedas Thanks for fixing issues reported.

I see a regression where the price is bit right shifted and not vertically aligned with other elements
https://mwpw-158015--milo--rohitsahu.hlx.page/drafts/rosahu/rosahu/min-compare-redesign?martech=off

Can you plz pull the latest from stage -
Since theres a PR Revert "MWPW-163754 Color difference is seen in the milo pages" #3365 in [Release] Stage to Main #3355 which reverted in stage which was causing the below issue which exist in your branch:
https://main--homepage--adobecom.hlx.live/homepage/index-loggedout?milolibs=mwpw-158015--milo--rohitsahu

mirafedas · 2024-12-20T14:16:37Z

@Roycethan thank you, fixed 👍

Roycethan · 2024-12-20T15:30:37Z

@Roycethan thank you, fixed 👍

Thanks @mirafedas Fixed

Rohit Sahu added 3 commits November 5, 2024 18:17

Redesign mini-compare chart. Added checkmarks for icons. Backward com…

6c6d3c9

…patibility using "checkmark" class. Horizontal line added.

implemented cheveron up and down functionality for checkmark copy list

4c51f63

added isMobile check

8a6c32e

rohitsahu requested a review from a team as a code owner November 6, 2024 10:35

Rohit Sahu added 4 commits November 6, 2024 16:10

Fix backward compatibility issues

d725219

minor

f0a65a6

minor css fixes as per figma

62e2ec2

refactored large "decorateFooterRows" method

37b70b4

rohitsahu requested review from yesil, npeltier, Axelcureno, rahulgupta999, 3ch023 and mirafedas November 6, 2024 11:53

rohitsahu added commerce merch card run-nala Run Nala Test Automation against PR labels Nov 6, 2024

rahulgupta999 reviewed Nov 7, 2024

View reviewed changes

remove duplicate css

8dfa0e9

review comment

rohitsahu requested a review from rahulgupta999 November 7, 2024 04:41

rohitsahu commented Nov 7, 2024

View reviewed changes

libs/blocks/merch-card/merch-card.js Show resolved Hide resolved

rohitsahu commented Nov 7, 2024

View reviewed changes

Axelcureno reviewed Nov 8, 2024

View reviewed changes

review comments

daadcbb

Axelcureno approved these changes Nov 8, 2024

View reviewed changes

yesil requested changes Nov 8, 2024

View reviewed changes

UT added

eb4ba26

github-actions bot added the Stale label Dec 5, 2024

3ch023 removed the Stale label Dec 11, 2024

mirafedas added 2 commits December 11, 2024 13:24

Merge branch 'stage' into MWPW-158015

dc5ee8e

resolved conflicts

11ee7bd

mirafedas added 2 commits December 11, 2024 15:08

fixed mini-compare-chart footer cta alignment

ef3f6cd

decreased footer rows height to match the design

4472303

fixed mini-compare footer alignment

5a56237

rahulgupta999 removed their request for review December 14, 2024 05:23

mirafedas added 6 commits December 16, 2024 12:54

fixed footer horizontal padding and mobile breakpoint

16dee1a

updated unit tests

f02ae16

corrected paddings, font sizes and other

882af05

corrected slots

6c7342b

corrected padding

4deca34

footer padding corrected

e679d79

mirafedas added 2 commits December 20, 2024 12:09

Merge branch 'stage' into MWPW-158015

385903e

corrected heading-m-price padding

4883637

Roycethan added verified PR has been E2E tested by a reviewer Ready for Stage and removed needs-verification PR requires E2E testing by a reviewer labels Dec 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MWPW-158015] [Mini Compare Merch Card] Redesign mini compare card for testing #3147

[MWPW-158015] [Mini Compare Merch Card] Redesign mini compare card for testing #3147

rohitsahu commented Nov 6, 2024 •

edited

Loading

aem-code-sync bot commented Nov 6, 2024 •

edited

Loading

rahulgupta999 Nov 7, 2024

rohitsahu Nov 7, 2024

rohitsahu Nov 7, 2024

Axelcureno Nov 8, 2024

rohitsahu Nov 8, 2024 •

edited

Loading

Axelcureno Nov 8, 2024

yesil left a comment

yesil Nov 8, 2024

rohitsahu Nov 11, 2024

yesil Nov 13, 2024

rohitsahu Nov 13, 2024

rohitsahu commented Nov 11, 2024

rohitsahu commented Nov 27, 2024 •

edited

Loading

github-actions bot commented Dec 5, 2024

mirafedas commented Dec 11, 2024

afmicka commented Dec 11, 2024 •

edited

Loading

afmicka commented Dec 12, 2024 •

edited

Loading

mirafedas commented Dec 13, 2024

Roycethan commented Dec 14, 2024 •

edited

Loading

mirafedas commented Dec 18, 2024

Roycethan commented Dec 20, 2024 •

edited

Loading

mirafedas commented Dec 20, 2024

Roycethan commented Dec 20, 2024

[MWPW-158015] [Mini Compare Merch Card] Redesign mini compare card for testing #3147

Are you sure you want to change the base?

[MWPW-158015] [Mini Compare Merch Card] Redesign mini compare card for testing #3147

Conversation

rohitsahu commented Nov 6, 2024 • edited Loading

aem-code-sync bot commented Nov 6, 2024 • edited Loading

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

rohitsahu Nov 8, 2024 • edited Loading

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

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

rohitsahu commented Nov 11, 2024

rohitsahu commented Nov 27, 2024 • edited Loading

github-actions bot commented Dec 5, 2024

mirafedas commented Dec 11, 2024

afmicka commented Dec 11, 2024 • edited Loading

afmicka commented Dec 12, 2024 • edited Loading

mirafedas commented Dec 13, 2024

Roycethan commented Dec 14, 2024 • edited Loading

mirafedas commented Dec 18, 2024

Roycethan commented Dec 20, 2024 • edited Loading

mirafedas commented Dec 20, 2024

Roycethan commented Dec 20, 2024

rohitsahu commented Nov 6, 2024 •

edited

Loading

aem-code-sync bot commented Nov 6, 2024 •

edited

Loading

rohitsahu Nov 8, 2024 •

edited

Loading

rohitsahu commented Nov 27, 2024 •

edited

Loading

afmicka commented Dec 11, 2024 •

edited

Loading

afmicka commented Dec 12, 2024 •

edited

Loading

Roycethan commented Dec 14, 2024 •

edited

Loading

Roycethan commented Dec 20, 2024 •

edited

Loading