feat: (AIME-194) Spanish localization for Brand Concierge widget UI - #2791
feat: (AIME-194) Spanish localization for Brand Concierge widget UI#2791eliwangj wants to merge 8 commits into
Conversation
Add a Spanish (es) locale to the Brand Concierge widget, resolved from the URL path via getPathDetails().lang. English remains the untouched default. - brand-concierge-config.js: add BC_UI_EN (English chrome strings), BC_LOCALES.es overlay (ui/text/arrays), and a pure resolveBrandConciergeConfig(lang). - brand-concierge.js: resolve locale into activeConfig before mount; read chrome strings from activeConfig.ui; forward localized text/arrays and strip ui from the bootstrap options. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
|
@ajaydwarkani could you help verify the Spanish UI and confirm it works? for some reason it kept saying i'm unauthorized when i was doing local testing manually earlier today. thanks! |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
me and @ajaydwarkani did some debugging and seems like neither of us could solve the login/auth issue, which is blocking local testing. @ajaydwarkani already reached out to team for fixes. |
Code ReviewOverviewClean, well-scoped change: adds a Spanish ( Correctness
Minor / non-blocking
Already tracked
Overall: solid, low-risk, well-documented implementation. No new blocking issues found. |
BC hardcodes English "Send" in the send button's hover tooltip (a bc-tooltip element referenced via aria-describedby, not covered by the config text map). Patch it to the localized send label for non-default locales via the existing mount observer; English is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up review (latest commit)Reviewed the latest commit (
No new issues found. The previously flagged shallow-merge note on |
…#2798) On /es/ pages, configure the alloyBC instance with the Spanish concierge datastream (3098f7cc-…, same IMS org) instead of the default, so conversations reach the Spanish concierge/manifest. The locale datastream is resolved from BC_LOCALES via resolveBrandConciergeConfig(); other locales keep the default. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| const { stickySession = false, ...stylingConfigurations } = brandConciergeConfig; | ||
| // `ui` holds ExL chrome strings (rendered by our own code) — strip it so only BC's | ||
| // own styling/text/arrays are forwarded to the third-party web client. | ||
| const { stickySession = false, ui: _ui, ...stylingConfigurations } = activeConfig; |
There was a problem hiding this comment.
The new datastreamId field added to resolveBrandConciergeConfig's return value (config.js, the overlay.datastreamId spread) isn't stripped here alongside stickySession/ui. On /es/ pages, activeConfig.datastreamId ('3098f7cc-...') will fall into ...stylingConfigurations and get forwarded to the third-party BC web client via getBootstrapOptions() → window.adobe.concierge.bootstrap() — the same call this comment says should only carry "BC's own styling/text/arrays".
datastreamId is only meaningful to the Alloy configureWebSdk() call (line ~867), not to the BC web client's bootstrap options, so this looks like an unintentional leak of an ExL/Edge-only field into the third-party payload. Consider destructuring it out here too, e.g.:
| const { stickySession = false, ui: _ui, ...stylingConfigurations } = activeConfig; | |
| const { stickySession = false, ui: _ui, datastreamId: _datastreamId, ...stylingConfigurations } = activeConfig; |
There was a problem hiding this comment.
Addressed in the JSON refactor (824b932): datastreamId no longer lives on activeConfig — it moved to a separate BC_DATASTREAMS lookup (getBrandConciergeDatastreamId) resolved only for configureWebSdk, so it can't reach getBootstrapOptions/the BC client payload. Safe to resolve.
| * client), so they live here alongside the client-facing `text`/`arrays` above and are | ||
| * threaded through as `config.ui`. 'BETA' is intentionally left untranslated (brand term). | ||
| */ | ||
| const BC_UI_EN = { |
There was a problem hiding this comment.
Instead of having this in the code, can we move this under a language directory with it's own locale JSON file.
There was a problem hiding this comment.
Instead of having this in the code, can we move this under a language directory with it's own locale JSON file.
(or) we can keep it under brandConciergeConfig directly and considered an object name as "ui". Bocz, we already have objects like text, array and ideally this was considered as en.
There was a problem hiding this comment.
@noormd08 , as per I18n best practices, we should be maintaining different files for different locales as it's easy for maintainability and makes adding a new language easy.
Here is my suggestion for folder structure
- localization
- en.json
- es.json
and in future if we extend new locale for example French or Chinese, we can simply add new file with respective translation
- localization
- fr.json
- zh-hant.json
Thoughts?
There was a problem hiding this comment.
Done — adopted the file-per-locale structure. Strings now live in scripts/brand-concierge/localization/en.json and es.json, loaded + deep-merged at init by loadBrandConciergeConfig(lang); adding a new language is just dropping in <lang>.json. I co-located the folder under the BC module since these strings are BC-specific — happy to lift it to a shared top-level localization/ if you'd prefer. The deep-merge also gives per-field English fallback, so a partial future locale won't drop keys (closes the earlier shallow-merge note). Thanks for the steer!
| * client), so they live here alongside the client-facing `text`/`arrays` above and are | ||
| * threaded through as `config.ui`. 'BETA' is intentionally left untranslated (brand term). | ||
| */ | ||
| const BC_UI_EN = { |
There was a problem hiding this comment.
Instead of having this in the code, can we move this under a language directory with it's own locale JSON file.
(or) we can keep it under brandConciergeConfig directly and considered an object name as "ui". Bocz, we already have objects like text, array and ideally this was considered as en.
Per reviewer feedback on #2791, extract localized strings into scripts/brand-concierge/localization/{en,es}.json (one file per locale) and load + deep-merge them at init via loadBrandConciergeConfig(lang), replacing the in-code BC_UI_EN / BC_LOCALES overlays. Deep-merge gives per-field English fallback (closes the earlier shallow-merge note on ui.disclaimer). Move the per-locale Edge datastream to a separate BC_DATASTREAMS lookup (getBrandConciergeDatastreamId), keeping routing config out of activeConfig so it can't leak into the styling payload forwarded to the BC web client (addresses the bot's getBootstrapOptions leak finding). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| */ | ||
| export async function loadBrandConciergeConfig(lang) { | ||
| const key = (lang || 'en').toLowerCase(); | ||
| const en = await fetchLocaleSheet('en'); |
There was a problem hiding this comment.
loadBrandConciergeConfig only has a try/catch around fetching the locale sheet (fetchLocaleSheet(key), line 142) — the English base fetch on line 138 is unguarded. Previously all of this UI copy was hardcoded in the JS bundle, so BC always had a config to render. Now, if en.json 404s or the fetch otherwise fails (transient network blip, CDN hiccup, path typo after a deploy), loadBrandConciergeConfig rejects, and since activeConfig = await loadBrandConciergeConfig(activeLang); in brand-concierge.js (initBrandConcierge) sits before the try/catch block that wraps the rest of init, the whole widget silently fails to mount — on every locale, not just es — with only a console.warn from the top-level loadBrandConcierge().catch(...) in delayed.js.
This repo already has an established pattern for exactly this case — fetchLanguagePlaceholders in scripts/scripts.js wraps its lang-specific fetch and its English fallback fetch in try/catch, defaulting to {} rather than letting the whole feature disappear. Worth mirroring that here, e.g. wrap the en fetch in a try/catch too and fall back to an inline minimal ui/text/arrays default (or at least skip mounting gracefully) so a single failed network request for a static JSON file can't take down the entire Brand Concierge widget.
There was a problem hiding this comment.
Fixed in bea7190. The English base fetch is now wrapped too — loadBrandConciergeConfig returns null when en.json can't load, and initBrandConcierge skips mounting on null instead of throwing (so a failed base sheet no longer takes the widget down on every locale). Also stopped caching failed fetches so a transient blip can retry. Mirrors the fetchLanguagePlaceholders fallback pattern. Thanks!
Code Review — latest commit (824b932, per-locale JSON refactor)Reviewed the refactor that replaces the in-code Confirmed fixed from prior review threads:
New finding (posted inline on No other blocking issues found — XSS surface is clean ( |
Wrap the English base sheet fetch (not just the locale fetch) so a failed en.json returns null and BC skips mounting gracefully instead of throwing an unhandled rejection that breaks the widget on every locale. Also stop caching failed locale fetches so a transient blip can retry. Mirrors the fetchLanguagePlaceholders fallback pattern in scripts.js. Addresses the review comment on brand-concierge-config.js. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| export async function loadBrandConciergeConfig(lang) { | ||
| const key = (lang || 'en').toLowerCase(); | ||
| let en; | ||
| try { | ||
| en = await fetchLocaleSheet('en'); | ||
| } catch (err) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn('[BC] English localization sheet failed to load; skipping mount', err?.message || err); | ||
| return null; | ||
| } | ||
| let locale = en; | ||
| if (key !== 'en') { | ||
| try { | ||
| locale = await fetchLocaleSheet(key); | ||
| } catch { | ||
| locale = en; | ||
| } | ||
| } |
There was a problem hiding this comment.
loadBrandConciergeConfig always attempts to fetch ${LOCALES_BASE_PATH}/${key}.json for any non-en path locale (line 155-160), but only en.json and es.json actually exist in this PR. Experience League already serves many other locales (de, fr, it, ja, ko, pt-br, zh-hans, zh-hant, etc., per the languagesMap in scripts.js), so on every one of those locales, every single page load will issue a fetch that is guaranteed to 404 (and since fetch failures aren't cached in localeSheetCache, this repeats indefinitely — there's no way for it to ever be short-circuited).
On top of that, the two fetches are sequential, not parallel:
en = await fetchLocaleSheet('en'); // line 148
...
locale = await fetchLocaleSheet(key); // line 157For every non-en/es locale this doubles the network latency on the critical path of Brand Concierge's init (activeConfig = await loadBrandConciergeConfig(activeLang) blocks widget mount), since the doomed-to-404 locale fetch is awaited only after the English fetch resolves.
Two independent, low-effort fixes:
- Kick off
fetchLocaleSheet('en')andfetchLocaleSheet(key)concurrently (e.g.Promise.all/Promise.allSettled) instead of sequentially. - Gate the locale fetch on a known-supported-locales list (mirroring the
BC_DATASTREAMSkeys pattern) so locales without a translation file skip the network call entirely instead of hitting the CDN with a guaranteed 404 on every pageview.
| "feedback.thumbsDown.aria": "Thumbs down" | ||
| }, | ||
| "arrays": { | ||
| "welcome.examples": [ |
There was a problem hiding this comment.
@eliwangj , arrays are not supported by localization automation tools, so can you only add support for key/value pair. It's okay to have a nested objects but should only be key/value pair.
| { "text": "Set up an Adobe Analytics report suite" }, | ||
| { "text": "Explain Adobe Target A/B testing" } | ||
| ], | ||
| "feedback.positive.options": [ |
| "Friendly and conversational tone", | ||
| "Other" | ||
| ], | ||
| "feedback.negative.options": ["Not helpful or relevant", "Confusing or unclear", "Too formal or robotic", "Other"] |
| @@ -0,0 +1,59 @@ | |||
| { | |||
| "language": "en-US", | |||
There was a problem hiding this comment.
@eliwangj , We should avoid adding non-localizable data in language file as this may cause application to crash if it gets translated.
…ream (#2831) Swaps the Spanish datastream override to 152f88b1-… so /es/ routes to the new concierge/sandbox. Same IMS org, so only the datastream id changes — orgId, edge domain and web client url are unchanged. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
3 participants
Jira ID: AIME-194
Test URLs:
Before: https://main--exlm--adobe-experience-league.aem.live
After: https://exlm-bc-spanish-ui--exlm--adobe-experience-league.aem.live/es/browse?martech=off
After: https://exlm-bc-spanish-ui--exlm--adobe-experience-league.aem.live/en/browse?martech=off
Summary
Adds a Spanish (
es) locale to the Brand Concierge widget UI. English and all other locales are unchanged.getPathDetails().lang), matching repo convention.scripts/brand-concierge/localization/en.json,es.json);loadBrandConciergeConfig(lang)fetches and deep-merges the locale over the English base at init (per-field English fallback)./es/conversations route to the Spanish concierge datastream via a separategetBrandConciergeDatastreamId()lookup, kept out of the payload forwarded to the BC web client. The ES datastream ID was subsequently repointed at a different concierge/sandbox (152f88b1-ef07-4afe-8a23-6c0e21c6f017) by stacked PR feat: (AIME-194) point /es/ Brand Concierge at the new sandbox datastream #2831, so that is the value on this branch today — same IMS org, no other config changed.Files:
scripts/brand-concierge/localization/{en,es}.json,scripts/brand-concierge/brand-concierge-config.js,scripts/brand-concierge/brand-concierge.jsScreenshots
Screenshots captured by Claude Code during local verification.
Spanish — trigger button
Spanish — drawer
English — unchanged (regression check)
Known caveats
AI Review Notes
placeholders.json— do not flag as "should use placeholders.json".getPathDetails().lang(path-based), matching the existing convention.🤖 Generated with Claude Code