From 66cb6d1a1f927af72fc39ca0f20312daf01aa744 Mon Sep 17 00:00:00 2001 From: Sean Choi Date: Mon, 13 Apr 2026 09:10:56 -0600 Subject: [PATCH 1/4] MWPW-192450: Fix - Lingo placeholders in Studio card preview --- studio/src/mas-fragment-editor.js | 28 ++++++++++++------------- studio/src/mas-repository.js | 8 +++---- studio/test/mas-fragment-editor.test.js | 6 ++---- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/studio/src/mas-fragment-editor.js b/studio/src/mas-fragment-editor.js index 7bf2edbc3..1792d596d 100644 --- a/studio/src/mas-fragment-editor.js +++ b/studio/src/mas-fragment-editor.js @@ -701,6 +701,12 @@ export default class MasFragmentEditor extends LitElement { if (isVariationAfterContext && !skipVariation) { await this.#attachParentToCachedVariation(existingStore, fragmentPath); + const fragmentLocale = extractLocaleFromPath(fragmentPath); + if (fragmentLocale && fragmentLocale !== Store.localeOrRegion()) { + Store.search.set((prev) => ({ ...prev, region: fragmentLocale })); + await this.repository.loadPreviewPlaceholders(); + existingStore.resolvePreviewFragment(); + } } else { this.localeDefaultFragment = existingStore.parentFragment; } @@ -751,8 +757,6 @@ export default class MasFragmentEditor extends LitElement { // Initializes editor state for fragments that are not yet present in list store cache. async #initializeFromRepository(fragmentId) { try { - // Start loading placeholders early - const placeholdersPromise = this.repository.loadPreviewPlaceholders().catch(() => null); const fragmentData = await this.repository.aem.sites.cf.fragments.getById(fragmentId); const fragment = new Fragment(fragmentData); @@ -763,8 +767,14 @@ export default class MasFragmentEditor extends LitElement { const parentFragment = await this.#resolveParentForFetchedVariation(fragmentId, fragment, isVariationAfterContext); const isVariationForStore = isVariationAfterContext || !!parentFragment; - // Wait for placeholders before creating stores (needed for preview resolution) - await placeholdersPromise; + if (isVariationForStore) { + const fragmentLocale = extractLocaleFromPath(fragment.path); + if (fragmentLocale && fragmentLocale !== Store.localeOrRegion()) { + Store.search.set((prev) => ({ ...prev, region: fragmentLocale })); + } + } + + await this.repository.loadPreviewPlaceholders(); const fragmentStore = generateFragmentStore(fragment, parentFragment); // Only add to main list if not a variation (variations appear under parent's variations panel) @@ -775,16 +785,6 @@ export default class MasFragmentEditor extends LitElement { this.#activateEditorStore(fragmentStore); this.dispatchFragmentLoaded(); - // Handle locale-specific placeholder reload for variations - if (isVariationForStore) { - const fragmentLocale = extractLocaleFromPath(fragment.path); - if (fragmentLocale && fragmentLocale !== Store.localeOrRegion()) { - Store.search.set((prev) => ({ ...prev, region: fragmentLocale })); - await this.repository.loadPreviewPlaceholders(); - fragmentStore.resolvePreviewFragment(); - } - } - Store.editor.resetChanges(); this.updateTranslatedLocalesStore(isVariationForStore, fragment.path); // no need to await this.#markInitReady(); diff --git a/studio/src/mas-repository.js b/studio/src/mas-repository.js index 2e2e67ddc..b0e3da474 100644 --- a/studio/src/mas-repository.js +++ b/studio/src/mas-repository.js @@ -646,7 +646,7 @@ export class MasRepository extends LitElement { async loadPreviewPlaceholders() { if (!this.search.value.path) return; - const cacheKey = `${this.filters.value.locale}_${this.search.value.path}`; + const cacheKey = `${Store.localeOrRegion()}_${this.search.value.path}`; // Return cached result if available if (this.dictionaryCache.has(cacheKey)) { @@ -671,10 +671,10 @@ export class MasRepository extends LitElement { const result = await promise; // Verify cache key hasn't changed during fetch (prevents stale data) - const currentKey = `${this.filters.value.locale}_${this.search.value.path}`; + const currentKey = `${Store.localeOrRegion()}_${this.search.value.path}`; if (currentKey === cacheKey) { // If result is empty and locale isn't en_US, try fallback - if ((!result || Object.keys(result).length === 0) && this.filters.value.locale !== 'en_US') { + if ((!result || Object.keys(result).length === 0) && Store.localeOrRegion() !== 'en_US') { const fallbackContext = { preview: { url: 'https://odinpreview.corp.adobe.com/adobe/sites/cf/fragments', @@ -707,7 +707,7 @@ export class MasRepository extends LitElement { preview: { url: 'https://odinpreview.corp.adobe.com/adobe/sites/cf/fragments', }, - locale: this.filters.value.locale, + locale: Store.localeOrRegion(), surface: this.search.value.path, networkConfig: { mainTimeout: 15000, diff --git a/studio/test/mas-fragment-editor.test.js b/studio/test/mas-fragment-editor.test.js index 271c849d6..3dbac4c4f 100644 --- a/studio/test/mas-fragment-editor.test.js +++ b/studio/test/mas-fragment-editor.test.js @@ -4,7 +4,7 @@ import '../src/mas-fragment-editor.js'; import MasFragmentEditor from '../src/mas-fragment-editor.js'; import Store from '../src/store.js'; import { Fragment } from '../src/aem/fragment.js'; -import generateFragmentStore, { SourceFragmentStore } from '../src/reactivity/source-fragment-store.js'; +import generateFragmentStore from '../src/reactivity/source-fragment-store.js'; import { PAGE_NAMES, CARD_MODEL_PATH, ODIN_PREVIEW_ORIGIN } from '../src/constants.js'; import router from '../src/router.js'; import Events from '../src/events.js'; @@ -357,7 +357,6 @@ describe('MasFragmentEditor', () => { it('reloads locale placeholders for variations when active locale differs', async () => { const fragmentData = createFragmentData({ id: 'variation-id', locale: 'fr_FR', slug: 'variation' }); - const resolvePreviewSpy = sandbox.spy(SourceFragmentStore.prototype, 'resolvePreviewFragment'); Store.filters.value = { locale: 'tr_TR' }; mockRepo.aem.sites.cf.fragments.getById.resolves(fragmentData); @@ -367,8 +366,7 @@ describe('MasFragmentEditor', () => { await el.initFragment(); - expect(mockRepo.loadPreviewPlaceholders.callCount).to.equal(2); - expect(resolvePreviewSpy.calledOnce).to.be.true; + expect(mockRepo.loadPreviewPlaceholders.calledOnce).to.be.true; expect(Store.search.get().region).to.equal('fr_FR'); }); From 3db7e4224d204a2acca131739a96ee60d3e6c0fa Mon Sep 17 00:00:00 2001 From: Sean Choi Date: Mon, 13 Apr 2026 19:18:32 -0600 Subject: [PATCH 2/4] MWPW-192450: Applying Axel's comment --- studio/src/mas-repository.js | 12 +++--- studio/test/mas-fragment-editor.test.js | 52 +++++++++++++++++++++++++ studio/test/mas-repository.test.js | 38 ++++++++++++++++++ 3 files changed, 97 insertions(+), 5 deletions(-) diff --git a/studio/src/mas-repository.js b/studio/src/mas-repository.js index b0e3da474..3cfcfa9b8 100644 --- a/studio/src/mas-repository.js +++ b/studio/src/mas-repository.js @@ -646,7 +646,9 @@ export class MasRepository extends LitElement { async loadPreviewPlaceholders() { if (!this.search.value.path) return; - const cacheKey = `${Store.localeOrRegion()}_${this.search.value.path}`; + const locale = Store.localeOrRegion(); + const path = this.search.value.path; + const cacheKey = `${locale}_${path}`; // Return cached result if available if (this.dictionaryCache.has(cacheKey)) { @@ -666,7 +668,7 @@ export class MasRepository extends LitElement { this.#abortControllers.placeholders = new AbortController(); try { - const promise = this.fetchDictionary(this.#abortControllers.placeholders); + const promise = this.fetchDictionary(this.#abortControllers.placeholders, locale); this.inflightDictionaryRequest = { promise, cacheKey }; const result = await promise; @@ -674,7 +676,7 @@ export class MasRepository extends LitElement { const currentKey = `${Store.localeOrRegion()}_${this.search.value.path}`; if (currentKey === cacheKey) { // If result is empty and locale isn't en_US, try fallback - if ((!result || Object.keys(result).length === 0) && Store.localeOrRegion() !== 'en_US') { + if ((!result || Object.keys(result).length === 0) && locale !== 'en_US') { const fallbackContext = { preview: { url: 'https://odinpreview.corp.adobe.com/adobe/sites/cf/fragments', @@ -702,12 +704,12 @@ export class MasRepository extends LitElement { } } - async fetchDictionary(abortController) { + async fetchDictionary(abortController, locale = Store.localeOrRegion()) { const context = { preview: { url: 'https://odinpreview.corp.adobe.com/adobe/sites/cf/fragments', }, - locale: Store.localeOrRegion(), + locale, surface: this.search.value.path, networkConfig: { mainTimeout: 15000, diff --git a/studio/test/mas-fragment-editor.test.js b/studio/test/mas-fragment-editor.test.js index 3dbac4c4f..5d046ea37 100644 --- a/studio/test/mas-fragment-editor.test.js +++ b/studio/test/mas-fragment-editor.test.js @@ -239,6 +239,7 @@ describe('MasFragmentEditor', () => { search: structuredClone(Store.search.get()), filters: structuredClone(Store.filters.get()), translatedLocales: Store.fragmentEditor.translatedLocales.get(), + placeholdersPreview: Store.placeholders.preview.get(), }; Store.fragments.list.data.value = []; @@ -248,6 +249,7 @@ describe('MasFragmentEditor', () => { Store.search.value = {}; Store.filters.value = { locale: 'en_US' }; Store.fragmentEditor.translatedLocales.value = null; + Store.placeholders.preview.value = null; }); afterEach(() => { @@ -261,6 +263,7 @@ describe('MasFragmentEditor', () => { Store.search.value = originalStoreState.search; Store.filters.value = originalStoreState.filters; Store.fragmentEditor.translatedLocales.value = originalStoreState.translatedLocales; + Store.placeholders.preview.value = originalStoreState.placeholdersPreview; }); it('returns early when no fragment ID exists', async () => { @@ -364,10 +367,59 @@ describe('MasFragmentEditor', () => { sandbox.stub(el, 'resolveVariationParentFragment').resolves(null); Store.fragmentEditor.fragmentId.value = 'variation-id'; + mockRepo.loadPreviewPlaceholders.callsFake(async () => { + Store.placeholders.preview.set({ testDictionary: true }); + }); + await el.initFragment(); expect(mockRepo.loadPreviewPlaceholders.calledOnce).to.be.true; expect(Store.search.get().region).to.equal('fr_FR'); + expect(Store.placeholders.preview.get()).to.deep.equal({ testDictionary: true }); + }); + + it('reloads locale placeholders for cached variation when locale differs from Store.localeOrRegion()', async () => { + const fragmentData = createFragmentData({ id: 'cached-var-id', locale: 'fr_FR', slug: 'variation' }); + const parentData = createFragmentData({ id: 'parent-id', locale: 'en_US', slug: 'default' }); + const sourceStore = generateFragmentStore(new Fragment(fragmentData), new Fragment(parentData)); + sandbox.spy(sourceStore, 'resolvePreviewFragment'); + + mockRepo.loadPreviewPlaceholders.callsFake(async () => { + Store.placeholders.preview.set({ fromCachedVariation: true }); + }); + + el.editorContextStore.isVariation.returns(true); + sandbox.stub(el, 'resolveVariationParentFragment').resolves(new Fragment(parentData)); + + Store.filters.value = { locale: 'tr_TR' }; + Store.search.value = { path: 'sandbox' }; + Store.fragments.list.data.value = [sourceStore]; + Store.fragmentEditor.fragmentId.value = 'cached-var-id'; + + await el.initFragment(); + + expect(mockRepo.loadPreviewPlaceholders.calledOnce).to.be.true; + expect(Store.search.get().region).to.equal('fr_FR'); + expect(Store.placeholders.preview.get()).to.deep.equal({ fromCachedVariation: true }); + expect(sourceStore.resolvePreviewFragment.calledOnce).to.be.true; + }); + + it('does not reload preview placeholders when cached variation locale matches Store.localeOrRegion()', async () => { + const fragmentData = createFragmentData({ id: 'cached-var-id', locale: 'fr_FR', slug: 'variation' }); + const parentData = createFragmentData({ id: 'parent-id', locale: 'en_US', slug: 'default' }); + const sourceStore = generateFragmentStore(new Fragment(fragmentData), new Fragment(parentData)); + + el.editorContextStore.isVariation.returns(true); + sandbox.stub(el, 'resolveVariationParentFragment').resolves(new Fragment(parentData)); + + Store.filters.value = { locale: 'fr_FR' }; + Store.search.value = { path: 'sandbox' }; + Store.fragments.list.data.value = [sourceStore]; + Store.fragmentEditor.fragmentId.value = 'cached-var-id'; + + await el.initFragment(); + + expect(mockRepo.loadPreviewPlaceholders.called).to.be.false; }); it('uses pending parent from create variation event when context is not ready', async () => { diff --git a/studio/test/mas-repository.test.js b/studio/test/mas-repository.test.js index 70009bd45..ca020e10a 100644 --- a/studio/test/mas-repository.test.js +++ b/studio/test/mas-repository.test.js @@ -2260,4 +2260,42 @@ describe('MasRepository dictionary helpers', () => { expect(fragmentDeletedEmitStub.calledOnceWith(fragment)).to.be.true; }); }); + + describe('loadPreviewPlaceholders', () => { + let previousSearch; + let previousFilters; + let previousPreview; + + beforeEach(() => { + previousSearch = structuredClone(Store.search.get()); + previousFilters = structuredClone(Store.filters.get()); + previousPreview = Store.placeholders.preview.get(); + }); + + afterEach(() => { + Store.search.value = previousSearch; + Store.filters.value = previousFilters; + Store.placeholders.preview.value = previousPreview; + }); + + it('uses Store.localeOrRegion() for cache key and fetchDictionary locale', async () => { + const repository = createFullRepository(); + repository.dictionaryCache.clear(); + + Store.search.value = { path: 'sandbox' }; + Store.filters.value = { locale: 'en_US' }; + Store.search.set((prev) => ({ ...prev, region: 'fr_FR' })); + // Unconnected repo: StoreController does not receive Store updates unless we sync. + repository.search.value = Store.search.get(); + + const fetchStub = sandbox.stub(repository, 'fetchDictionary').resolves({ dictKey: 'dictVal' }); + + await repository.loadPreviewPlaceholders(); + + expect(fetchStub.calledOnce).to.be.true; + expect(fetchStub.firstCall.args[1]).to.equal('fr_FR'); + expect(repository.dictionaryCache.has('fr_FR_sandbox')).to.be.true; + expect(Store.placeholders.preview.get()).to.deep.equal({ dictKey: 'dictVal' }); + }); + }); }); From 93371e8de2eb0a9ccc90ac4267f4123d4310ac30 Mon Sep 17 00:00:00 2001 From: Sean Choi Date: Thu, 16 Apr 2026 00:37:43 -0600 Subject: [PATCH 3/4] MWPW-192450: Taking Ilyas's suggestion for performance. --- studio/src/editor-panel.js | 2 +- studio/src/mas-fragment-editor.js | 17 ++-- studio/src/mas-repository.js | 89 +++++++++++-------- .../src/reactivity/preview-fragment-store.js | 10 +-- studio/src/store.js | 8 +- ...card-collection-editor-variation.test.html | 2 +- .../test/editors/merch-card-editor.test.html | 4 +- studio/test/mas-fragment-editor.test.js | 23 ++--- studio/test/mas-repository.test.js | 6 +- .../reactivity/preview-fragment-store.test.js | 10 +-- 10 files changed, 100 insertions(+), 71 deletions(-) diff --git a/studio/src/editor-panel.js b/studio/src/editor-panel.js index 655e36fc6..48d610038 100644 --- a/studio/src/editor-panel.js +++ b/studio/src/editor-panel.js @@ -444,7 +444,7 @@ export default class EditorPanel extends LitElement { const fragmentLocale = extractLocaleFromPath(this.fragment?.path); if (fragmentLocale && fragmentLocale !== Store.filters.value.locale) { Store.filters.set((prev) => ({ ...prev, locale: fragmentLocale })); - await this.repository.loadPreviewPlaceholders(); + void this.repository.loadPreviewPlaceholders(); this.fragmentStore?.resolvePreviewFragment?.(); } } diff --git a/studio/src/mas-fragment-editor.js b/studio/src/mas-fragment-editor.js index e27a34ef7..a80538c74 100644 --- a/studio/src/mas-fragment-editor.js +++ b/studio/src/mas-fragment-editor.js @@ -698,6 +698,9 @@ export default class MasFragmentEditor extends LitElement { // Initializes editor state when the fragment already exists in the list store cache. async #initializeFromCachedStore(fragmentId, existingStore) { + if (this.repository.search.value.path) { + void this.repository.loadPreviewPlaceholders(Store.localeOrRegion()); + } const fragmentPath = existingStore.get().path; const fragmentLocale = extractLocaleFromPath(fragmentPath); @@ -717,14 +720,13 @@ export default class MasFragmentEditor extends LitElement { const localeChanged = fragmentLocale && fragmentLocale !== Store.localeOrRegion(); if (localeChanged) { Store.search.set((prev) => ({ ...prev, region: fragmentLocale })); - await this.repository.loadPreviewPlaceholders(); + void this.repository.loadPreviewPlaceholders(); existingStore.resolvePreviewFragment(); } await this.#attachParentToCachedVariation(existingStore, fragmentPath); - const fragmentLocale = extractLocaleFromPath(fragmentPath); if (fragmentLocale && fragmentLocale !== Store.localeOrRegion()) { Store.search.set((prev) => ({ ...prev, region: fragmentLocale })); - await this.repository.loadPreviewPlaceholders(); + void this.repository.loadPreviewPlaceholders(); existingStore.resolvePreviewFragment(); } } else { @@ -776,6 +778,9 @@ export default class MasFragmentEditor extends LitElement { // Initializes editor state for fragments that are not yet present in list store cache. async #initializeFromRepository(fragmentId) { try { + if (this.repository.search.value.path) { + void this.repository.loadPreviewPlaceholders(Store.localeOrRegion()); + } const fragmentData = await this.repository.aem.sites.cf.fragments.getById(fragmentId); const fragment = new Fragment(fragmentData); @@ -793,7 +798,9 @@ export default class MasFragmentEditor extends LitElement { } } - await this.repository.loadPreviewPlaceholders(); + if (this.repository.search.value.path) { + void this.repository.loadPreviewPlaceholders(); + } const fragmentStore = generateFragmentStore(fragment, parentFragment); // Only add to main list if not a variation (variations appear under parent's variations panel) @@ -811,7 +818,7 @@ export default class MasFragmentEditor extends LitElement { const localeChanged = fragmentLocale !== Store.localeOrRegion(); Store.search.set((prev) => ({ ...prev, region: fragmentLocale })); if (localeChanged) { - await this.repository.loadPreviewPlaceholders(); + void this.repository.loadPreviewPlaceholders(); fragmentStore.resolvePreviewFragment(); } } diff --git a/studio/src/mas-repository.js b/studio/src/mas-repository.js index c174c396c..f8755a33f 100644 --- a/studio/src/mas-repository.js +++ b/studio/src/mas-repository.js @@ -104,7 +104,7 @@ export class MasRepository extends LitElement { translations: null, }; this.dictionaryCache = new Map(); - this.inflightDictionaryRequest = null; + this.inflightDictionaryByKey = new Map(); this.saveFragment = this.saveFragment.bind(this); this.copyFragment = this.copyFragment.bind(this); this.publishFragment = this.publishFragment.bind(this); @@ -122,6 +122,8 @@ export class MasRepository extends LitElement { #abortControllers; #searchCursor = null; #addonPlaceholdersRequest = null; + #previewDictionaryAbortByKey = new Map(); + #previewDictionaryLoadingDepth = 0; /** * When personalization is off, exclude fragments that carry mas:pzn/… tags except mas:pzn/country/…. @@ -147,12 +149,14 @@ export class MasRepository extends LitElement { // Invalidate dictionary cache when filters or search path change Store.filters.subscribe(() => { this.dictionaryCache.clear(); + Store.placeholders.previewByLocale.set({}); if (this.page.value === PAGE_NAMES.CONTENT) { this.#searchCursor = null; } }); Store.search.subscribe(() => { this.dictionaryCache.clear(); + Store.placeholders.previewByLocale.set({}); this.#searchCursor = null; }); @@ -644,39 +648,47 @@ export class MasRepository extends LitElement { } } - async loadPreviewPlaceholders() { + /** + * Loads preview dictionary for `locale` (defaults to surface locale) into `Store.placeholders.previewByLocale`. + * Safe to call in parallel for different locales; duplicate cache keys share one in-flight request. + * @param {string} [locale] + */ + async loadPreviewPlaceholders(locale = Store.localeOrRegion()) { if (!this.search.value.path) return; - const locale = Store.localeOrRegion(); const path = this.search.value.path; const cacheKey = `${locale}_${path}`; - // Return cached result if available if (this.dictionaryCache.has(cacheKey)) { - Store.placeholders.preview.set(this.dictionaryCache.get(cacheKey)); + const cached = this.dictionaryCache.get(cacheKey); + Store.placeholders.previewByLocale.set((prev) => ({ ...prev, [locale]: cached })); return; } - // Return existing promise if same cache key is already in-flight - if (this.inflightDictionaryRequest?.cacheKey === cacheKey) { - return this.inflightDictionaryRequest.promise; + if (this.inflightDictionaryByKey.has(cacheKey)) { + return this.inflightDictionaryByKey.get(cacheKey); } - // Abort previous placeholder fetch if still running with different cache key - if (this.#abortControllers.placeholders) { - this.#abortControllers.placeholders.abort(); - } - this.#abortControllers.placeholders = new AbortController(); + const previousAbort = this.#previewDictionaryAbortByKey.get(cacheKey); + previousAbort?.abort(); + const abortController = new AbortController(); + this.#previewDictionaryAbortByKey.set(cacheKey, abortController); + + const promise = (async () => { + this.#previewDictionaryLoadingDepth += 1; + if (this.#previewDictionaryLoadingDepth === 1) { + Store.placeholders.list.loading.set(true); + } + try { + const result = await this.fetchDictionary(abortController, locale); + + if (this.search.value.path !== path) return; + + const mergeDict = (dict) => { + this.dictionaryCache.set(cacheKey, dict); + Store.placeholders.previewByLocale.set((prev) => ({ ...prev, [locale]: dict })); + }; - try { - const promise = this.fetchDictionary(this.#abortControllers.placeholders, locale); - this.inflightDictionaryRequest = { promise, cacheKey }; - const result = await promise; - - // Verify cache key hasn't changed during fetch (prevents stale data) - const currentKey = `${Store.localeOrRegion()}_${this.search.value.path}`; - if (currentKey === cacheKey) { - // If result is empty and locale isn't en_US, try fallback if ((!result || Object.keys(result).length === 0) && locale !== 'en_US') { const fallbackContext = { preview: { @@ -684,25 +696,30 @@ export class MasRepository extends LitElement { }, locale: 'en_US', surface: this.search.value.path, - signal: this.#abortControllers.placeholders?.signal, + signal: abortController.signal, }; const fallbackResult = await getDictionary(fallbackContext); - this.dictionaryCache.set(cacheKey, fallbackResult); - Store.placeholders.preview.set(fallbackResult); + if (this.search.value.path !== path) return; + mergeDict(fallbackResult); } else { - this.dictionaryCache.set(cacheKey, result); - Store.placeholders.preview.set(result); + mergeDict(result); + } + } catch (error) { + if (error.name === 'AbortError') return; + this.processError(error, 'Could not load preview placeholders.'); + } finally { + this.inflightDictionaryByKey.delete(cacheKey); + this.#previewDictionaryAbortByKey.delete(cacheKey); + this.#previewDictionaryLoadingDepth -= 1; + if (this.#previewDictionaryLoadingDepth === 0) { + Store.placeholders.list.loading.set(false); } } - } catch (error) { - if (error.name === 'AbortError') return; // Silent abort during navigation - this.processError(error, 'Could not load preview placeholders.'); - } finally { - this.inflightDictionaryRequest = null; - this.#abortControllers.placeholders = null; - Store.placeholders.list.loading.set(false); - } + })(); + + this.inflightDictionaryByKey.set(cacheKey, promise); + return promise; } async fetchDictionary(abortController, locale = Store.localeOrRegion()) { @@ -2073,7 +2090,7 @@ export class MasRepository extends LitElement { Store.placeholders.addons.loading.set(true); try { await this.loadPreviewPlaceholders(); - const dictionary = Store.placeholders.preview.get(); + const dictionary = Store.previewDictionary(); if (dictionary) { const addonFragments = Object.keys(dictionary) .filter((key) => /^addon-/.test(key)) diff --git a/studio/src/reactivity/preview-fragment-store.js b/studio/src/reactivity/preview-fragment-store.js index 112ad8cf2..a9bfe50fd 100644 --- a/studio/src/reactivity/preview-fragment-store.js +++ b/studio/src/reactivity/preview-fragment-store.js @@ -73,8 +73,8 @@ export class PreviewFragmentStore extends FragmentStore { super(fragmentInstance, validator); this.lazy = lazy; - this.placeholderUnsubscribe = Store.placeholders.preview.subscribe(() => { - if (!this.lazy && !this.resolved && Store.placeholders.preview.value) { + this.placeholderUnsubscribe = Store.placeholders.previewByLocale.subscribe(() => { + if (!this.lazy && !this.resolved && Store.previewDictionary()) { this.resolveFragment(true); } }); @@ -171,7 +171,7 @@ export class PreviewFragmentStore extends FragmentStore { return; } - if (this.isCollection || !Store.placeholders.preview.value) { + if (this.isCollection || !Store.previewDictionary()) { this.resolved = true; this.refreshAemFragment(true); this.notify(); @@ -215,7 +215,7 @@ export class PreviewFragmentStore extends FragmentStore { const context = { locale: Store.localeOrRegion(), surface: Store.surface(), - dictionary: Store.placeholders.preview.value, + dictionary: Store.previewDictionary(), }; const result = await previewStudioFragment(body, context); @@ -295,7 +295,7 @@ export class PreviewFragmentStore extends FragmentStore { */ dispose() { if (this.placeholderUnsubscribe) { - Store.placeholders.preview.unsubscribe(this.placeholderUnsubscribe); + Store.placeholders.previewByLocale.unsubscribe(this.placeholderUnsubscribe); this.placeholderUnsubscribe = null; } } diff --git a/studio/src/store.js b/studio/src/store.js index ea4690c61..30cfa99f7 100644 --- a/studio/src/store.js +++ b/studio/src/store.js @@ -71,7 +71,7 @@ const Store = { loading: new ReactiveStore(false), data: new ReactiveStore([{ value: 'disabled', itemText: 'disabled' }]), }, - preview: new ReactiveStore(null), + previewByLocale: new ReactiveStore({}), }, settings: new SettingsStore(), profile: new ReactiveStore({}), @@ -102,6 +102,10 @@ const Store = { localeOrRegion: function () { return Store.search.value.region || Store.filters.value.locale || 'en_US'; }, + previewDictionary: function () { + const locale = Store.localeOrRegion(); + return Store.placeholders.previewByLocale.value[locale]; + }, removeRegionOverride: function () { if (Store.search.value.region) { Store.search.set((prev) => ({ ...prev, region: null })); @@ -261,7 +265,7 @@ Store.page.subscribe((value) => { Store.sort.set({ sortBy: SORT_COLUMNS[value]?.[0], sortDirection: 'asc' }); }); -Store.placeholders.preview.subscribe(() => { +Store.placeholders.previewByLocale.subscribe(() => { if (Store.page.value === PAGE_NAMES.CONTENT) { for (const fragmentStore of Store.fragments.list.data.value) { fragmentStore.resolvePreviewFragment(); diff --git a/studio/test/editors/merch-card-collection-editor-variation.test.html b/studio/test/editors/merch-card-collection-editor-variation.test.html index e9a23a307..1b3ff4889 100644 --- a/studio/test/editors/merch-card-collection-editor-variation.test.html +++ b/studio/test/editors/merch-card-collection-editor-variation.test.html @@ -120,7 +120,7 @@ fragmentStore = generateFragmentStore(variationFragment, parentFragment); // Disable preview resolution — collection editor doesn't need it - Store.placeholders.preview.set(null); + Store.placeholders.previewByLocale.set({}); Store.fragments.list.data.set([fragmentStore]); const updateCalls = []; diff --git a/studio/test/editors/merch-card-editor.test.html b/studio/test/editors/merch-card-editor.test.html index 149524872..0057e51bd 100644 --- a/studio/test/editors/merch-card-editor.test.html +++ b/studio/test/editors/merch-card-editor.test.html @@ -219,7 +219,7 @@ Store.fragmentEditor.fragmentId.set(`test-variation-${testId}`); Store.fragments.inEdit.set(fragmentStore); Store.fragments.list.data.set([fragmentStore]); - Store.placeholders.preview.set({}); + Store.placeholders.previewByLocale.set({ [variationLocale]: {} }); Store.search.set((prev) => ({ ...prev, surface: 'web', path: 'nala', region: variationLocale })); Store.viewMode.set('editing'); Store.settings.rows.set(createDefaultSettingsRows()); @@ -810,7 +810,7 @@ Store.fragmentEditor.fragmentId.set(`test-settings-${testId}`); Store.fragments.inEdit.set(store); Store.fragments.list.data.set([store]); - Store.placeholders.preview.set({}); + Store.placeholders.previewByLocale.set({ en_AU: {} }); Store.search.set((prev) => ({ ...prev, surface: 'web', path: 'nala', region: 'en_AU' })); Store.viewMode.set('editing'); Store.settings.rows.set(settingsRows); diff --git a/studio/test/mas-fragment-editor.test.js b/studio/test/mas-fragment-editor.test.js index 3f51faca2..88763fdbe 100644 --- a/studio/test/mas-fragment-editor.test.js +++ b/studio/test/mas-fragment-editor.test.js @@ -215,6 +215,7 @@ describe('MasFragmentEditor', () => { mockRepo = { refreshFragment: sandbox.stub().resolves(), loadPreviewPlaceholders: sandbox.stub().resolves(), + search: { value: { path: 'sandbox' } }, aem: { sites: { cf: { @@ -240,7 +241,7 @@ describe('MasFragmentEditor', () => { search: structuredClone(Store.search.get()), filters: structuredClone(Store.filters.get()), translatedLocales: Store.fragmentEditor.translatedLocales.get(), - placeholdersPreview: Store.placeholders.preview.get(), + placeholdersPreview: Store.placeholders.previewByLocale.get(), }; Store.fragments.list.data.value = []; @@ -250,7 +251,7 @@ describe('MasFragmentEditor', () => { Store.search.value = {}; Store.filters.value = { locale: 'en_US' }; Store.fragmentEditor.translatedLocales.value = null; - Store.placeholders.preview.value = null; + Store.placeholders.previewByLocale.value = {}; }); afterEach(() => { @@ -264,7 +265,7 @@ describe('MasFragmentEditor', () => { Store.search.value = originalStoreState.search; Store.filters.value = originalStoreState.filters; Store.fragmentEditor.translatedLocales.value = originalStoreState.translatedLocales; - Store.placeholders.preview.value = originalStoreState.placeholdersPreview; + Store.placeholders.previewByLocale.value = originalStoreState.placeholdersPreview; }); it('returns early when no fragment ID exists', async () => { @@ -330,7 +331,7 @@ describe('MasFragmentEditor', () => { await el.initFragment(); - expect(mockRepo.loadPreviewPlaceholders.calledOnce).to.be.true; + expect(mockRepo.loadPreviewPlaceholders.callCount).to.equal(2); expect(el.editorContextStore.loadFragmentContext.calledOnceWith('new-id', fragmentData.path)).to.be.true; expect(Store.fragments.list.data.get()).to.have.lengthOf(1); expect(Store.fragments.list.data.get()[0].id).to.equal('new-id'); @@ -369,14 +370,14 @@ describe('MasFragmentEditor', () => { Store.fragmentEditor.fragmentId.value = 'variation-id'; mockRepo.loadPreviewPlaceholders.callsFake(async () => { - Store.placeholders.preview.set({ testDictionary: true }); + Store.placeholders.previewByLocale.set((prev) => ({ ...prev, fr_FR: { testDictionary: true } })); }); await el.initFragment(); - expect(mockRepo.loadPreviewPlaceholders.calledOnce).to.be.true; + expect(mockRepo.loadPreviewPlaceholders.callCount).to.equal(2); expect(Store.search.get().region).to.equal('fr_FR'); - expect(Store.placeholders.preview.get()).to.deep.equal({ testDictionary: true }); + expect(Store.previewDictionary()).to.deep.equal({ testDictionary: true }); }); it('reloads locale placeholders for cached variation when locale differs from Store.localeOrRegion()', async () => { @@ -386,7 +387,7 @@ describe('MasFragmentEditor', () => { sandbox.spy(sourceStore, 'resolvePreviewFragment'); mockRepo.loadPreviewPlaceholders.callsFake(async () => { - Store.placeholders.preview.set({ fromCachedVariation: true }); + Store.placeholders.previewByLocale.set((prev) => ({ ...prev, fr_FR: { fromCachedVariation: true } })); }); el.editorContextStore.isVariation.returns(true); @@ -399,9 +400,9 @@ describe('MasFragmentEditor', () => { await el.initFragment(); - expect(mockRepo.loadPreviewPlaceholders.calledOnce).to.be.true; + expect(mockRepo.loadPreviewPlaceholders.callCount).to.equal(2); expect(Store.search.get().region).to.equal('fr_FR'); - expect(Store.placeholders.preview.get()).to.deep.equal({ fromCachedVariation: true }); + expect(Store.previewDictionary()).to.deep.equal({ fromCachedVariation: true }); expect(sourceStore.resolvePreviewFragment.calledOnce).to.be.true; }); @@ -420,7 +421,7 @@ describe('MasFragmentEditor', () => { await el.initFragment(); - expect(mockRepo.loadPreviewPlaceholders.called).to.be.false; + expect(mockRepo.loadPreviewPlaceholders.callCount).to.equal(1); }); it('uses pending parent from create variation event when context is not ready', async () => { diff --git a/studio/test/mas-repository.test.js b/studio/test/mas-repository.test.js index a464167ee..0a613f6ab 100644 --- a/studio/test/mas-repository.test.js +++ b/studio/test/mas-repository.test.js @@ -2543,13 +2543,13 @@ describe('MasRepository dictionary helpers', () => { beforeEach(() => { previousSearch = structuredClone(Store.search.get()); previousFilters = structuredClone(Store.filters.get()); - previousPreview = Store.placeholders.preview.get(); + previousPreview = Store.placeholders.previewByLocale.get(); }); afterEach(() => { Store.search.value = previousSearch; Store.filters.value = previousFilters; - Store.placeholders.preview.value = previousPreview; + Store.placeholders.previewByLocale.value = previousPreview; }); it('uses Store.localeOrRegion() for cache key and fetchDictionary locale', async () => { @@ -2569,7 +2569,7 @@ describe('MasRepository dictionary helpers', () => { expect(fetchStub.calledOnce).to.be.true; expect(fetchStub.firstCall.args[1]).to.equal('fr_FR'); expect(repository.dictionaryCache.has('fr_FR_sandbox')).to.be.true; - expect(Store.placeholders.preview.get()).to.deep.equal({ dictKey: 'dictVal' }); + expect(Store.placeholders.previewByLocale.get().fr_FR).to.deep.equal({ dictKey: 'dictVal' }); }); }); }); diff --git a/studio/test/reactivity/preview-fragment-store.test.js b/studio/test/reactivity/preview-fragment-store.test.js index 383bf5f03..7b778e3f3 100644 --- a/studio/test/reactivity/preview-fragment-store.test.js +++ b/studio/test/reactivity/preview-fragment-store.test.js @@ -102,7 +102,7 @@ describe('mergeResolvedPreviewFields', () => { describe('PreviewFragmentStore', () => { let sandbox; let placeholderSubscribers; - let originalPlaceholdersPreview; + let originalPlaceholdersPreviewByLocale; let originalSurface; let originalLocaleOrRegion; @@ -122,10 +122,10 @@ describe('PreviewFragmentStore', () => { sandbox = sinon.createSandbox(); placeholderSubscribers = []; - originalPlaceholdersPreview = Store.placeholders.preview; + originalPlaceholdersPreviewByLocale = Store.placeholders.previewByLocale; - Store.placeholders.preview = { - value: { key: 'value' }, + Store.placeholders.previewByLocale = { + value: { en_US: { key: 'value' } }, subscribe: (fn) => { placeholderSubscribers.push(fn); return fn; @@ -144,7 +144,7 @@ describe('PreviewFragmentStore', () => { afterEach(() => { sandbox.restore(); - Store.placeholders.preview = originalPlaceholdersPreview; + Store.placeholders.previewByLocale = originalPlaceholdersPreviewByLocale; Store.surface = originalSurface; Store.localeOrRegion = originalLocaleOrRegion; }); From c0c5908a817bf214d4be573b1bce9d65288461d9 Mon Sep 17 00:00:00 2001 From: Sean Choi Date: Thu, 16 Apr 2026 00:53:42 -0600 Subject: [PATCH 4/4] MWPW-192450: handling empty dictionary case. --- studio/src/mas-repository.js | 2 +- studio/src/reactivity/preview-fragment-store.js | 4 ++-- studio/src/store.js | 5 +++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/studio/src/mas-repository.js b/studio/src/mas-repository.js index f8755a33f..01147a2dc 100644 --- a/studio/src/mas-repository.js +++ b/studio/src/mas-repository.js @@ -2091,7 +2091,7 @@ export class MasRepository extends LitElement { try { await this.loadPreviewPlaceholders(); const dictionary = Store.previewDictionary(); - if (dictionary) { + if (Store.previewDictionaryReady()) { const addonFragments = Object.keys(dictionary) .filter((key) => /^addon-/.test(key)) .map((key) => ({ value: key, itemText: key })); diff --git a/studio/src/reactivity/preview-fragment-store.js b/studio/src/reactivity/preview-fragment-store.js index a9bfe50fd..1a45eab65 100644 --- a/studio/src/reactivity/preview-fragment-store.js +++ b/studio/src/reactivity/preview-fragment-store.js @@ -74,7 +74,7 @@ export class PreviewFragmentStore extends FragmentStore { this.lazy = lazy; this.placeholderUnsubscribe = Store.placeholders.previewByLocale.subscribe(() => { - if (!this.lazy && !this.resolved && Store.previewDictionary()) { + if (!this.lazy && !this.resolved && Store.previewDictionaryReady()) { this.resolveFragment(true); } }); @@ -171,7 +171,7 @@ export class PreviewFragmentStore extends FragmentStore { return; } - if (this.isCollection || !Store.previewDictionary()) { + if (this.isCollection || !Store.previewDictionaryReady()) { this.resolved = true; this.refreshAemFragment(true); this.notify(); diff --git a/studio/src/store.js b/studio/src/store.js index 30cfa99f7..82488711c 100644 --- a/studio/src/store.js +++ b/studio/src/store.js @@ -106,6 +106,11 @@ const Store = { const locale = Store.localeOrRegion(); return Store.placeholders.previewByLocale.value[locale]; }, + /** True when the active locale has a loaded dictionary with at least one entry (empty `{}` is not ready). */ + previewDictionaryReady: function () { + const d = Store.previewDictionary(); + return d != null && Object.keys(d).length > 0; + }, removeRegionOverride: function () { if (Store.search.value.region) { Store.search.set((prev) => ({ ...prev, region: null }));