MWPW-135160: Mnemonics inside merch card #2024

Axelcureno · 2024-03-16T02:47:23Z

Adds ability to display inline mnemonic list block inside merch card.
Simplifies Mnemonic list code.

Test URLs:

aem-code-sync · 2024-03-16T02:47:46Z

Page	Scores	Audits	Google
/drafts/axel/mwpw-135160?martech=off

yesil · 2024-03-25T09:04:20Z

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

+  const mnemonicList = el.querySelector('.mnemonic-list');
+  if (mnemonicList) {
+    bodySlot.append(mnemonicList);
+    await decorateMnemonicList(bodySlot);


I wonder whether bodySlot already has some content here. If so, we should limit the scope of decoration to mnemonicList.

suggestion:

await decorateMnemonicList(mnemonicList); bodySlot.append(mnemonicList);

Updated. Mnemonics need to be inserted after paragraph content, so first decoration happens then insertion into body slot. LMK if this looks okay, otherwise I can make the foreach async and do all at once.

yesil · 2024-03-25T09:05:18Z

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

-  ];
+const parseContent = async (el, merchCard) => {
+  const allElements = el.querySelectorAll('h2, h3, h4, h5, p, ul, em');
+  const innerElements = Array.from(allElements).filter((element) => !element.closest('.mnemonic-list'));


this is getting complicated.
Let's first process mnemonic, then you would not need to change this part.

github-actions · 2024-03-27T12:48:12Z

This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried.

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

libs/blocks/marquee/marquee.js

@@ -3,7 +3,8 @@
 */

 import { decorateButtons, getBlockSize, decorateBlockBg } from '../../utils/decorate.js';
-import { createTag, getConfig, loadStyle } from '../../utils/utils.js';
+import { createTag } from '../../utils/utils.js';
+import { decorateMnemonicList } from '../mnemonic-list/mnemonic-list.js';


if mnemonic-list is not needed for all marquees on adobe.com, it should be imported dynamically.

Addressed. It is now dynamically imported.

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

libs/blocks/marquee/marquee.js

@@ -64,20 +65,6 @@ const decorateImage = (media) => {
  }
 };

-export async function loadMnemonicList(foreground) {


why remove ?

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

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

@@ -3,12 +3,15 @@ import { getConfig, createTag } from '../../utils/utils.js';
 import { getMetadata } from '../section-metadata/section-metadata.js';
 import { processTrackingLabels } from '../../martech/attributes.js';
 import { replaceKey } from '../../features/placeholders.js';
+import { decorateMnemonicList } from '../mnemonic-list/mnemonic-list.js';


decorateMnemonicList is not needed for majority of the cards, hence should be loaded dynamically and conditionally.

Fixed, now dynamically imported.

codecov · 2024-04-04T06:05:39Z

Codecov Report

Attention: Patch coverage is 56.25000% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 96.39%. Comparing base (fa5f12a) to head (8e5045f).
Report is 140 commits behind head on stage.

❗ Current head 8e5045f differs from pull request most recent head 678dcdc. Consider uploading reports for the commit 678dcdc to get more accurate results

Files	Patch %	Lines
libs/blocks/merch-card/merch-card.js	53.33%	14 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            stage    #2024      +/-   ##
==========================================
+ Coverage   96.36%   96.39%   +0.02%     
==========================================
  Files         166      166              
  Lines       42531    43486     +955     
==========================================
+ Hits        40986    41918     +932     
- Misses       1545     1568      +23

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

Axelcureno · 2024-04-04T15:15:00Z

@Roycethan this is ready for verification. Thanks.

github-actions · 2024-04-10T01:08:25Z

This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR.

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

ryanmparrish · 2024-04-18T18:19:09Z

libs/blocks/marquee/marquee.js

@@ -78,6 +78,7 @@ export async function loadMnemonicList(foreground) {
  }
 }

+


@Axelcureno can you fix this eslint issue. Looks like this file shouldn't even be in the change list?

Roycethan

@Axelcureno
https://main--cc--adobecom.hlx.page/drafts/axel/defects/mwpw-135160?milolibs=MWPW-135160--milo--axelcureno
1)In Ipad viewport the card left/right borders are touching the viewport:

Axelcureno · 2024-04-29T21:36:59Z

I didn't push any changes and I see it displaying properly in tablet. Probably got fixed in a different PR. Please check @Roycethan

Roycethan

@Axelcureno So the example where i can repro this is on 3 col width card as mentioned earlier:
https://main--cc--adobecom.hlx.page/drafts/axel/defects/mwpw-135160?milolibs=MWPW-135160--milo--axelcureno#

Axelcureno · 2024-04-30T01:34:33Z

@Axelcureno So the example where i can repro this is on 3 col width card as mentioned earlier: https://main--cc--adobecom.hlx.page/drafts/axel/defects/mwpw-135160?milolibs=MWPW-135160--milo--axelcureno#

@Roycethan thanks, I was able to reproduce this time. I have pushed the fix, please check once more.

Roycethan

Thanks @Axelcureno Fixed now

* MWPW-135160: Mnemonics inside merch card * Update merch-card.js --------- Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>

Axelcureno added run-nala Run Nala Test Automation against PR commerce needs-verification PR requires E2E testing by a reviewer merch card labels Mar 16, 2024

Axelcureno requested review from yesil, 3ch023, vladen and VKniaz March 16, 2024 02:47

Axelcureno self-assigned this Mar 16, 2024

Axelcureno requested review from a team, ryanmparrish, Sartxi and auniverseaway as code owners March 16, 2024 02:47

Axelcureno marked this pull request as draft March 16, 2024 03:07

Axelcureno marked this pull request as ready for review March 22, 2024 16:36

yesil reviewed Mar 25, 2024

View reviewed changes

Axelcureno force-pushed the MWPW-135160 branch from 8b5e5bd to 2818e84 Compare March 25, 2024 20:30

Axelcureno requested a review from yesil March 25, 2024 22:05

yesil requested changes Apr 3, 2024

View reviewed changes

Axelcureno requested a review from yesil April 4, 2024 05:49

yesil approved these changes Apr 4, 2024

View reviewed changes

Axelcureno requested a review from Roycethan April 4, 2024 15:14

npeltier requested changes Apr 15, 2024

View reviewed changes

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

Axelcureno requested a review from npeltier April 18, 2024 16:48

ryanmparrish reviewed Apr 18, 2024

View reviewed changes

Axelcureno requested a review from ryanmparrish April 18, 2024 20:19

MWPW-135160: Mnemonics inside merch card

46ab8fb

Axelcureno force-pushed the MWPW-135160 branch from 4c6c329 to 46ab8fb Compare April 22, 2024 21:54

Roycethan requested changes Apr 23, 2024

View reviewed changes

Axelcureno added 4 commits April 23, 2024 09:08

Merge branch 'stage' into MWPW-135160

b80ff8f

Merge branch 'stage' into MWPW-135160

b8ab664

Merge branch 'stage' into MWPW-135160

b19285b

Merge branch 'stage' into MWPW-135160

57b2c2e

Axelcureno requested a review from Roycethan April 29, 2024 21:37

Roycethan requested changes Apr 30, 2024

View reviewed changes

Update merch-card.js

2b5898c

Roycethan self-requested a review April 30, 2024 01:46

Roycethan approved these changes Apr 30, 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 30, 2024

Axelcureno added the Ready for Stage label Apr 30, 2024

Axelcureno and others added 2 commits April 30, 2024 08:15

Merge branch 'stage' into MWPW-135160

162a073

Merge branch 'stage' into MWPW-135160

678dcdc

Blainegunn merged commit 3b3149a into adobecom:stage Apr 30, 2024
10 checks passed

Blainegunn mentioned this pull request Apr 30, 2024

[Release] Stage to Main #2238

Merged

yesil added a commit to yesil/milo that referenced this pull request May 21, 2024

MWPW-135160: Mnemonics inside merch card (adobecom#2024)

88e20b2

* MWPW-135160: Mnemonics inside merch card * Update merch-card.js --------- Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MWPW-135160: Mnemonics inside merch card #2024

MWPW-135160: Mnemonics inside merch card #2024

Axelcureno commented Mar 16, 2024 •

edited

Loading

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

edited

Loading

yesil Mar 25, 2024

Axelcureno Mar 25, 2024

yesil Mar 25, 2024

Axelcureno Mar 25, 2024

github-actions bot commented Mar 27, 2024

yesil Apr 3, 2024

Axelcureno Apr 4, 2024

yesil Apr 3, 2024

Axelcureno Apr 4, 2024

yesil Apr 3, 2024

Axelcureno Apr 4, 2024

codecov bot commented Apr 4, 2024 •

edited

Loading

Axelcureno commented Apr 4, 2024

github-actions bot commented Apr 10, 2024

ryanmparrish Apr 18, 2024

Axelcureno Apr 18, 2024

Roycethan left a comment •

edited

Loading

Axelcureno commented Apr 29, 2024

Roycethan left a comment

Axelcureno commented Apr 30, 2024

Roycethan left a comment

		@@ -78,6 +78,7 @@ export async function loadMnemonicList(foreground) {
		}
		}

MWPW-135160: Mnemonics inside merch card #2024

MWPW-135160: Mnemonics inside merch card #2024

Conversation

Axelcureno commented Mar 16, 2024 • edited Loading

aem-code-sync bot commented Mar 16, 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

github-actions bot commented Mar 27, 2024

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

codecov bot commented Apr 4, 2024 • edited Loading

Codecov Report

Axelcureno commented Apr 4, 2024

github-actions bot commented Apr 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roycethan left a comment • edited Loading

Choose a reason for hiding this comment

Axelcureno commented Apr 29, 2024

Roycethan left a comment

Choose a reason for hiding this comment

Axelcureno commented Apr 30, 2024

Roycethan left a comment

Choose a reason for hiding this comment

Axelcureno commented Mar 16, 2024 •

edited

Loading

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

edited

Loading

codecov bot commented Apr 4, 2024 •

edited

Loading

Roycethan left a comment •

edited

Loading