MWPW-186621: Fix mnemonic icon/field inheritance by yesil · Pull Request #547 · adobecom/mas

yesil · 2026-01-26T15:29:26Z

Resolves

https://jira.corp.adobe.com/browse/MWPW-186621
https://jira.corp.adobe.com/browse/MWPW-187023
https://jira.corp.adobe.com/browse/MWPW-186621
Fix shared reference bug in Fragment.replaceFrom that caused source and preview stores to share the same fields array, leading to unintended mutations when restoring mnemonic fields to parent values
Deep clone fields array in replaceFrom to ensure source and preview fragment stores remain independent
Add unit test for merch-card-editor mnemonic inheritance flow (remove → override → restore)

QA Checklist: https://wiki.corp.adobe.com/display/adobedotcom/M@S+Engineering+QA+Use+Cases

Please do the steps below before submitting your PR for a code review or QA

C1. Cover code with Unit Tests
C2. Add a Nala test (double check with #fishbags if nala test is needed)
C3. Verify all Checks are green (unit tests, nala tests)
C4. PR description contains working Test Page link where the feature can be tested
C5: you are ready to do a demo from Test Page in PR (bonus: write a working demo script that you'll use on Thursday, you can eventually put in your PR)
C.6 read your Jira one more time to validate that you've addressed all AC's and nothing is missing

🧪 Nala E2E Tests

Nala tests run automatically when you open this PR.

To run Nala tests again:

Add the run nala label to this PR (in the right sidebar)
Tests will run automatically on the current commit
Any future commits will also trigger tests as long as the label remains

To stop automatic Nala tests:

Remove the run nala label

Note: Tests only run on commits if the run nala label is present. Add the label whenever you need tests to run on new changes.

Test URLs:

aem-code-sync · 2026-01-26T15:29:28Z

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
In case there are problems, just click the checkbox below to rerun the respective action.

Re-sync branch

Commits

41d58e3 ✅ (latest)
7c2e01a ✅
2fcf554 ✅
bc57f77 ✅
c62f073 ✅
063b320 ✅
b91f4e9 ✅
7eff8ba ✅
19e7b09 ✅
7789894 ✅
6417727 ✅
33d7baa ✅
60658c4 ✅
3b30c1c ✅
3c8cdce ✅
d8aa337 ✅
4a27b87 ✅
344ea86 ✅
a9b3e27 ✅
c89cc1d ✅
c06d616 ✅
dc10e97 ✅
05151e5 ✅
45f9302 ✅
3f12f30 ✅
a5f08c7 ✅
b718f3e ✅

…nd preview stores to share the same fields array, leading to unintended mutations when restoring mnemonic fields to parent values Deep clone fields array in replaceFrom to ensure source and preview fragment stores remain independent Simplify resetMnemonicsToParent now that the underlying bug is fixed Add unit test for merch-card-editor mnemonic inheritance flow (remove → override → restore)

yesil · 2026-01-28T15:10:41Z

cursor review

cursor · 2026-01-28T15:10:44Z

Skipping Bugbot: Bugbot is disabled for this repository

yesil · 2026-02-02T16:59:30Z

cursor review

cursor · 2026-02-02T16:59:33Z

Skipping Bugbot: Bugbot is disabled for this repository

yesil · 2026-02-02T17:00:30Z

cursor review

cursor · 2026-02-02T17:00:33Z

Skipping Bugbot: Bugbot is disabled for this repository

Axelcureno

Found 2 significant issues with this changes and proposed fixes for both.

Modifying content and saving on a variant breaks the preview of the card (and removes the content modifications): See my comment in https://github.com/adobecom/mas/pull/547/changes#r2756045226 with a suggested fix
Adding mnemonics to a variation marks all mnemonics as overridden: See my comment in https://github.com/adobecom/mas/pull/547/changes#r2756182965 with a suggested fix.

>

Axelcureno · 2026-02-02T18:52:49Z

studio/src/reactivity/source-fragment-store.js

+ * @param {Fragment} parentFragment
+ * @returns {object}
+ */
+function createPreviewDataWithParent(sourceFragment, parentFragment) {


Code duplication of #createPreviewData()

Axelcureno · 2026-02-02T20:34:12Z

studio/src/reactivity/source-fragment-store.js

You will need to add the same parent-merging logic in discardChanges() here. Without this, saving a variation causes inherited content to disappear from the preview and breaks the experience.

Suggested change

// For variations, restore preview with parent-merged values (same as discardChanges)

if (this.parentFragment) {

const previewData = this.#createPreviewData(this.value, this.parentFragment);

this.previewStore.refreshFrom(previewData);

} else {

this.previewStore.refreshFrom(structuredClone(value));

}

Axelcureno · 2026-02-02T20:47:27Z

studio/src/aem/fragment.js

-            return ownField.values;
+        const ownValues = ownField?.values || [];
+
+        // [] (empty array) = inherit from parent if variation


Although this solves a real problem, I'm really not a fan of this brittle distinction of [] vs ['']. Why not use an explicit inherited flag instead?
After all the goal is to make the code simpler and more obvious.

brittle distinction of [] vs [''].

This has been properly covered with tests now, I don't consider it brittle.
I don't have strong preference though, let's run by @npeltier, was this already decided as inherited ?

Axelcureno · 2026-02-02T21:31:31Z

studio/src/editors/merch-card-editor.js

                    <mas-multifield
                        id="mnemonics"
                        button-label="Add visual"
+                        data-field-state="${this.getFieldState('mnemonicIcon')}"


This causes all variation mnemonics to show as "overridden" when the user adds a mnemonic.
field-state.js must return 'same-as-parent' instead of '' (empty string) to prevent multifield fallback to this container state.

isn't adding a mnemonic considered as override?

yesil · 2026-02-03T13:41:12Z

cursor review

cursor · 2026-02-03T13:41:14Z

Skipping Bugbot: Bugbot is disabled for this repository

yesil · 2026-02-03T14:14:09Z

@Axelcureno the PR is not in review yet, I'm still dealing with some of the issues you mentioned. I'll move the JIRA to review once ready.

MWPW-186621: Fix mnemonic icon/field inheritance

b718f3e

yesil force-pushed the MWPW-186621 branch from 4da0563 to b718f3e Compare January 26, 2026 15:30

aem-code-sync bot temporarily deployed to MWPW-186621 January 26, 2026 15:30 Inactive

yesil changed the title ~~Mwpw 186621~~ MWPW-186621: Fix mnemonic icon/field inheritance Jan 26, 2026

aem-code-sync bot temporarily deployed to MWPW-186621 January 27, 2026 10:47 Inactive

Merge branch 'main' of github.com:adobecom/mas into MWPW-186621

3f12f30

aem-code-sync bot temporarily deployed to MWPW-186621 January 27, 2026 10:50 Inactive

fix lint

45f9302

aem-code-sync bot temporarily deployed to MWPW-186621 January 27, 2026 13:03 Inactive

fix format

05151e5

aem-code-sync bot temporarily deployed to MWPW-186621 January 27, 2026 13:05 Inactive

increase timeout

dc10e97

aem-code-sync bot temporarily deployed to MWPW-186621 January 27, 2026 13:09 Inactive

increase timeout

c06d616

aem-code-sync bot temporarily deployed to MWPW-186621 January 27, 2026 13:13 Inactive

remove useless tests

c89cc1d

aem-code-sync bot temporarily deployed to MWPW-186621 January 27, 2026 13:18 Inactive

fix wcs offer

a9b3e27

aem-code-sync bot temporarily deployed to MWPW-186621 January 27, 2026 13:33 Inactive

fix format

344ea86

aem-code-sync bot temporarily deployed to MWPW-186621 January 27, 2026 13:34 Inactive

remove check

4a27b87

aem-code-sync bot temporarily deployed to MWPW-186621 January 27, 2026 13:43 Inactive

Merge branch 'main' of github.com:adobecom/mas into MWPW-186621

d8aa337

aem-code-sync bot temporarily deployed to MWPW-186621 January 28, 2026 09:41 Inactive

Merge branch 'main' of github.com:adobecom/mas into MWPW-186621

3c8cdce

yesil added 2 commits January 30, 2026 15:07

Merge branch 'main' of github.com:adobecom/mas into MWPW-186621

3b30c1c

wip

60658c4

Axelcureno requested changes Feb 2, 2026

View reviewed changes

clean up mas-side-nav and fix inheritence

33d7baa

aem-code-sync bot temporarily deployed to MWPW-186621 February 4, 2026 07:38 Inactive

Merge branch 'main' of github.com:adobecom/mas into MWPW-186621

6417727

aem-code-sync bot temporarily deployed to MWPW-186621 February 4, 2026 07:39 Inactive

fix lint

7789894

aem-code-sync bot temporarily deployed to MWPW-186621 February 4, 2026 07:41 Inactive

self review

19e7b09

aem-code-sync bot temporarily deployed to MWPW-186621 February 4, 2026 08:04 Inactive

self review

7eff8ba

aem-code-sync bot temporarily deployed to MWPW-186621 February 4, 2026 08:20 Inactive

yesil added 4 commits February 4, 2026 09:36

self review

b91f4e9

self review

063b320

Merge branch 'main' of github.com:adobecom/mas into MWPW-186621

c62f073

fix cancel on mnemomic and included apps modals

bc57f77

aem-code-sync bot temporarily deployed to MWPW-186621 February 4, 2026 13:04 Inactive

Merge branch 'main' into MWPW-186621

2fcf554

aem-code-sync bot temporarily deployed to MWPW-186621 February 4, 2026 14:22 Inactive

yesil mentioned this pull request Feb 4, 2026

MWPW-186107: Default Language browsing - top nav #528

Open

6 tasks

yesil added 2 commits February 4, 2026 18:22

Merge branch 'main' of github.com:adobecom/mas into MWPW-186621

7c2e01a

Merge branch 'MWPW-186621' of github.com:adobecom/mas into MWPW-186621

41d58e3

+        // For variations, restore preview with parent-merged values (same as discardChanges)
+        if (this.parentFragment) {
+            const previewData = this.#createPreviewData(this.value, this.parentFragment);
+            this.previewStore.refreshFrom(previewData);
+        } else {
+            this.previewStore.refreshFrom(structuredClone(value));
+        }

Conversation

yesil commented Jan 26, 2026 • edited by 3ch023 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧪 Nala E2E Tests

To run Nala tests again:

To stop automatic Nala tests:

Test URLs:

Uh oh!

aem-code-sync bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yesil commented Jan 28, 2026

Uh oh!

cursor bot commented Jan 28, 2026

Uh oh!

yesil commented Feb 2, 2026

Uh oh!

cursor bot commented Feb 2, 2026

Uh oh!

yesil commented Feb 2, 2026

Uh oh!

cursor bot commented Feb 2, 2026

Uh oh!

Axelcureno left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Axelcureno Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Axelcureno Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Axelcureno Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

yesil Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Axelcureno Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yesil Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

yesil commented Feb 3, 2026

Uh oh!

cursor bot commented Feb 3, 2026

Uh oh!

yesil commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yesil commented Jan 26, 2026 •

edited by 3ch023

Loading

aem-code-sync bot commented Jan 26, 2026 •

edited

Loading

Axelcureno left a comment •

edited

Loading

yesil Feb 4, 2026 •

edited

Loading

Axelcureno Feb 2, 2026 •

edited

Loading