Skip to content

MWPW-192450: Fix - Lingo placeholders in Studio card preview #760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
afmicka merged 11 commits into from
Apr 21, 2026
Merged

MWPW-192450: Fix - Lingo placeholders in Studio card preview #760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
66cb6d1
MWPW-192450: Fix - Lingo placeholders in Studio card preview
seanchoi-dev Apr 13, 2026
3db7e42
MWPW-192450: Applying Axel's comment
seanchoi-dev Apr 14, 2026
5d0cf6a
Merge branch 'main' into MWPW-192450
honstar Apr 14, 2026
cf636c8
Merge branch 'main' into MWPW-192450
mirafedas Apr 15, 2026
6da772f
Merge branch 'main' into MWPW-192450
seanchoi-dev Apr 16, 2026
93371e8
MWPW-192450: Taking Ilyas's suggestion for performance.
seanchoi-dev Apr 16, 2026
c0c5908
MWPW-192450: handling empty dictionary case.
seanchoi-dev Apr 16, 2026
f15ad35
Merge branch 'main' into MWPW-192450
seanchoi-dev Apr 16, 2026
d7abacf
Merge branch 'main' into MWPW-192450
afmicka Apr 17, 2026
b6d4999
Merge branch 'main' into MWPW-192450
3ch023 Apr 17, 2026
009e7f8
Merge branch 'main' into MWPW-192450
seanchoi-dev Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion studio/src/editor-panel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?.();
}
}
Expand Down
29 changes: 23 additions & 6 deletions studio/src/mas-fragment-editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -717,10 +720,15 @@ 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);
if (fragmentLocale && fragmentLocale !== Store.localeOrRegion()) {
Store.search.set((prev) => ({ ...prev, region: fragmentLocale }));
Copy link
Copy Markdown
Member

@Axelcureno Axelcureno Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New cached-store locale reload path has no test coverage

These 6 new lines add locale-specific placeholder loading to #initializeFromCachedStore — this is the path Codecov flagged (4 missing lines, 71.42% patch coverage on this file).

The existing test at mas-fragment-editor.test.js:358 only exercises #initializeFromRepository (non-cached). A test for this path would look like:

it('reloads locale placeholders for cached variations when locale differs', async () => {
    const fragmentData = createFragmentData({ id: 'cached-var', locale: 'fr_FR', slug: 'var' });
    const fragment = new Fragment(fragmentData);
    const sourceStore = generateFragmentStore(fragment);
    Store.fragments.list.data.set([sourceStore]);
    Store.filters.value = { locale: 'en_US' };

    el.editorContextStore.isVariation.returns(true);
    sandbox.stub(el, 'resolveVariationParentFragment').resolves(null);
    Store.fragmentEditor.fragmentId.value = 'cached-var';

    await el.initFragment();

    expect(mockRepo.loadPreviewPlaceholders.calledOnce).to.be.true;
    expect(Store.search.get().region).to.equal('fr_FR');
});

void this.repository.loadPreviewPlaceholders();
existingStore.resolvePreviewFragment();
}
} else {
this.localeDefaultFragment = existingStore.parentFragment;
}
Expand Down Expand Up @@ -770,8 +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 {
// Start loading placeholders early
const placeholdersPromise = this.repository.loadPreviewPlaceholders().catch(() => null);
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);

Expand All @@ -782,8 +791,16 @@ 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 }));
}
}

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)
Expand All @@ -801,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();
}
}
Expand Down
99 changes: 59 additions & 40 deletions studio/src/mas-repository.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export class MasRepository extends LitElement {
collections: 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);
Expand All @@ -124,6 +124,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/….
Expand All @@ -149,12 +151,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;
});

Expand Down Expand Up @@ -760,71 +764,86 @@ 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 cacheKey = `${this.filters.value.locale}_${this.search.value.path}`;
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);

try {
const promise = this.fetchDictionary(this.#abortControllers.placeholders);
this.inflightDictionaryRequest = { promise, cacheKey };
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}`;
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') {
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 }));
};

if ((!result || Object.keys(result).length === 0) && locale !== 'en_US') {
const fallbackContext = {
preview: {
url: 'https://odinpreview.corp.adobe.com/adobe/sites/cf/fragments',
},
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) {
async fetchDictionary(abortController, locale = Store.localeOrRegion()) {
const context = {
preview: {
url: 'https://odinpreview.corp.adobe.com/adobe/sites/cf/fragments',
},
locale: this.filters.value.locale,
locale,
surface: this.search.value.path,
networkConfig: {
mainTimeout: 15000,
Expand Down Expand Up @@ -2187,8 +2206,8 @@ export class MasRepository extends LitElement {
Store.placeholders.addons.loading.set(true);
try {
await this.loadPreviewPlaceholders();
const dictionary = Store.placeholders.preview.get();
if (dictionary) {
const dictionary = Store.previewDictionary();
if (Store.previewDictionaryReady()) {
const addonFragments = Object.keys(dictionary)
.filter((key) => /^addon-/.test(key))
.map((key) => ({ value: key, itemText: key }));
Expand Down
10 changes: 5 additions & 5 deletions studio/src/reactivity/preview-fragment-store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.previewDictionaryReady()) {
this.resolveFragment(true);
}
});
Expand Down Expand Up @@ -171,7 +171,7 @@ export class PreviewFragmentStore extends FragmentStore {
return;
}

if (this.isCollection || !Store.placeholders.preview.value) {
if (this.isCollection || !Store.previewDictionaryReady()) {
this.resolved = true;
this.refreshAemFragment(true);
this.notify();
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
}
}
Expand Down
13 changes: 11 additions & 2 deletions studio/src/store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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({}),
Expand Down Expand Up @@ -102,6 +102,15 @@ 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];
},
/** 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 }));
Expand Down Expand Up @@ -261,7 +270,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();
Expand Down
2 changes: 1 addition & 1 deletion studio/test/editors/merch-card-collection-editor-variation.test.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand Down
4 changes: 2 additions & 2 deletions studio/test/editors/merch-card-editor.test.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand Down
Loading
Loading