MWPW-160015: M@S: support for mnemonic multifield #3075
Conversation
during card hydration.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #3075 +/- ##
=======================================
Coverage 96.44% 96.44%
=======================================
Files 246 246
Lines 55988 55989 +1
=======================================
+ Hits 53999 54001 +2
+ Misses 1989 1988 -1 ☔ View full report in Codecov by Sentry. |
| @@ -12,29 +12,33 @@ export async function hydrate(fragmentData, merchCard) { | |||
| if (!variant) return; | |||
| fragment.model = fragment.model; | |||
|
|
|||
| merchCard.variantLayout?.refs?.forEach((ref) => ref.remove()); | |||
| // remove all previous slotted content except the default slot | |||
| merchCard.querySelectorAll('[slot]').forEach((el) => { | |||
There was a problem hiding this comment.
this is handle use-case when card is refreshed?
how about we have a method inside merch-card to do it? We might need to also remove all the previous attributes
There was a problem hiding this comment.
since merch-card is intended to be modular, I think keeping this fragment related logic in hydrate.js ok.
Also, for Milo cards, it doesn't really make sense as we don't support refetching the content.
| const { aemFragmentMapping } = merchCard.variantLayout; | ||
|
|
||
| if (!aemFragmentMapping) return; | ||
|
|
||
| const appendFn = (el) => { | ||
| merchCard.variantLayout.refs.push(el); |
There was a problem hiding this comment.
actually, what was merchCard.variantLayout.refs? what was merchCard.variantLayout.refs.push(el); needed for?
There was a problem hiding this comment.
with merchCard.variantLayout.refs I initially tried to track elements that were added, but it seemed a bit unnecessary and clearing all named slots is easier and safer. (especially when you change card variant in MAS Studio)
| link: fragment.mnemonicLink[index] ?? '', | ||
| })); | ||
|
|
||
| fragmentData.computed = { mnemonics }; |
There was a problem hiding this comment.
I thought we are building merch-card out of fragmentData, but why setting computed? How is it used afterwards?
There was a problem hiding this comment.
As we design mnemonic icon, alt text and link as 3 multi fields, these values need to be pre-processed before mapping them to fields in Studio.
There was a problem hiding this comment.
imo we should have no trace of Studio related logic in Milo, why not doing it in Studio when the edit window is opened?
there is a risk of someone removing it in the future because it looks in milo like it's not used
There was a problem hiding this comment.
@3ch023 since MAS changes already have been merged, I raised https://jira.corp.adobe.com/browse/MWPW-160932 to track this.
I propose to address it post PR merge.
|
@yesil The authored target mnemonic link is not getting opened up from studio - 3rd AC - Can you plz check ? |
|
@Roycethan yes it is by design. It is to prevent Studio users from leaving after unintended link clicks. |
|
Skipped 3075: "MWPW-160015: M@S: support for mnemonic multifield" due to file "libs/deps/mas/mas.js" overlap. Merging will be attempted in the next batch |
during card hydration.
Resolves:
MWPW-160015
MWPW-160079
Test URLs:
MAS Studio link:
https://main--mas--adobecom.hlx.live/studio.html?milolibs=mwpw-160015-final--milo--yesil