fix: only load necessary translations #802
Conversation
commit: |
WalkthroughThis PR reorganizes localization into separate components and platform paths, updates the i18next scanner config to read/write platform resources under Sequence Diagram(s)sequenceDiagram
%% Styling: light-highlight for loader and i18n steps
participant User
participant Template as Template (directory/locator/main)
participant Transform as transformProps
participant Injector as injectTranslations
participant Loader as Translation Loader
participant FSys as File System
participant i18n as i18next Instance
participant Component as Rendered Component
User->>Template: Request page (with locale)
Template->>Transform: transformProps(data)
Transform->>Injector: injectTranslations(data)
Injector->>Loader: loadComponentTranslations(locale) / loadPlatformTranslations(locale)
Loader->>FSys: read locales/components|platform/{locale}/visual-editor.json
alt locale file exists
FSys-->>Loader: JSON resource
Loader->>i18n: addResourceBundle / addResources
else file missing
FSys-->>Loader: EN fallback or error
Loader->>i18n: addResourceBundle (fallback)
end
Injector-->>Transform: augmented props (with translations)
Transform-->>Template: transformed props
Template->>Component: render with i18n instance
Component-->>User: localized UI
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/visual-editor/src/utils/VisualEditorProvider.tsx (1)
44-52: Race condition: translations may not load before language change.
loadComponentTranslationsis async but not awaited (line 45). WhennormalizedTemplateProps?.translationsis undefined, the function will asynchronously fetch translations viagetTranslations(), buti18nComponentsInstance.changeLanguage()executes immediately on line 49 without waiting. This can cause the language to change before translations are available, resulting in missing or fallback text.Consider one of these solutions:
Option 1: Await inside useEffect
+ React.useEffect(() => { if (normalizedTemplateProps?.document?.locale) { - loadComponentTranslations( + loadComponentTranslations( normalizedTemplateProps.document.locale, normalizedTemplateProps?.translations - ); - i18nComponentsInstance.changeLanguage( - normalizedTemplateProps.document.locale - ); + ).then(() => { + i18nComponentsInstance.changeLanguage( + normalizedTemplateProps.document.locale + ); + }); } + }, [normalizedTemplateProps?.document?.locale]);Option 2: Make the provider async-aware with state
+ const [translationsLoaded, setTranslationsLoaded] = React.useState(false); + + React.useEffect(() => { if (normalizedTemplateProps?.document?.locale) { - loadComponentTranslations( + loadComponentTranslations( normalizedTemplateProps.document.locale, normalizedTemplateProps?.translations - ); - i18nComponentsInstance.changeLanguage( - normalizedTemplateProps.document.locale - ); + ).then(() => { + i18nComponentsInstance.changeLanguage( + normalizedTemplateProps.document.locale + ); + setTranslationsLoaded(true); + }); } + }, [normalizedTemplateProps?.document?.locale]);
🧹 Nitpick comments (5)
packages/visual-editor/src/utils/i18n/fallbacks.ts (1)
31-31: Good refactor: removed redundant double negation.The truthiness check is cleaner without
!!while preserving the same behavior.starter/src/templates/dev.tsx (1)
131-135: Consider stronger typing for transformProps.The use of
TemplateProps<any>loses type safety. While this works, consider constraining the type parameter if the document structure is known, or at minimum usingTemplateProps<unknown>to signal the uncertainty.Example improvement:
-export const transformProps: TransformProps<TemplateProps<any>> = async ( +export const transformProps: TransformProps<TemplateProps<unknown>> = async ( data ) => { return await injectTranslations(data); };packages/visual-editor/src/vite-plugin/templates/locator.tsx (1)
1-1: Autogenerated file - consider updating generator for type safety.Line 1 indicates this is autogenerated. If
TemplateProps<any>at lines 95-99 is a concern, the fix should be applied to the template generator rather than this file.packages/visual-editor/src/utils/i18n/platform.ts (1)
22-53: Consider documenting race condition potential.While the
hasResourceBundlecheck inloadPlatformTranslationsprevents duplicate loading, concurrent calls with the same locale before the first completes could still cause race conditions. The error handling correctly falls back to "en", but this behavior should be documented.Consider adding a loading state map to prevent concurrent loads:
const loadingLocales = new Set<string>(); const getTranslations = async ( locale: string, isRetry = false ): Promise<Record<string, string>> => { if (!locale) { return {}; } // Wait if already loading while (loadingLocales.has(locale)) { await new Promise(resolve => setTimeout(resolve, 50)); } loadingLocales.add(locale); try { const module = await import( `../../../locales/platform/${locale}/visual-editor.json` ); return module.default; } catch (e) { // existing error handling... } finally { loadingLocales.delete(locale); } };packages/visual-editor/src/utils/i18n/components.ts (1)
81-94: Allow refreshing an existing resource bundleOnce a bundle for a locale exists, this early return prevents us from merging any new or overridden keys that arrive later (e.g., when
transformPropsstarts supplying additional strings). Dropping the guard and usingaddResourceBundle(..., true, true)keeps the fast path but still lets us refresh data when needed.export const loadComponentTranslations = async ( locale: string, translations?: Record<string, string> ) => { - if (i18nComponentsInstance.hasResourceBundle(locale, NAMESPACE)) { - return; - } - - const translationsToInject = translations || (await getTranslations(locale)); - - if (translationsToInject && Object.keys(translationsToInject).length > 0) { - i18nComponentsInstance.addResourceBundle( - locale, - NAMESPACE, - translationsToInject - ); - } + const translationsToInject = + translations ?? (await getTranslations(locale)); + + if (!translationsToInject || Object.keys(translationsToInject).length === 0) { + return; + } + + i18nComponentsInstance.addResourceBundle( + locale, + NAMESPACE, + translationsToInject, + true, + i18nComponentsInstance.hasResourceBundle(locale, NAMESPACE) + ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
packages/visual-editor/i18next-scanner.config.cjs(1 hunks)packages/visual-editor/locales/components/cs/visual-editor.json(1 hunks)packages/visual-editor/locales/components/da/visual-editor.json(1 hunks)packages/visual-editor/locales/components/de/visual-editor.json(1 hunks)packages/visual-editor/locales/components/en-GB/visual-editor.json(1 hunks)packages/visual-editor/locales/components/en/visual-editor.json(1 hunks)packages/visual-editor/locales/components/es/visual-editor.json(1 hunks)packages/visual-editor/locales/components/et/visual-editor.json(1 hunks)packages/visual-editor/locales/components/fi/visual-editor.json(1 hunks)packages/visual-editor/locales/components/fr/visual-editor.json(1 hunks)packages/visual-editor/locales/components/hr/visual-editor.json(1 hunks)packages/visual-editor/locales/components/hu/visual-editor.json(1 hunks)packages/visual-editor/locales/components/it/visual-editor.json(1 hunks)packages/visual-editor/locales/components/ja/visual-editor.json(1 hunks)packages/visual-editor/locales/components/lt/visual-editor.json(1 hunks)packages/visual-editor/locales/components/lv/visual-editor.json(1 hunks)packages/visual-editor/locales/components/nb/visual-editor.json(1 hunks)packages/visual-editor/locales/components/nl/visual-editor.json(1 hunks)packages/visual-editor/locales/components/pl/visual-editor.json(1 hunks)packages/visual-editor/locales/components/pt/visual-editor.json(1 hunks)packages/visual-editor/locales/components/ro/visual-editor.json(1 hunks)packages/visual-editor/locales/components/sk/visual-editor.json(1 hunks)packages/visual-editor/locales/components/sv/visual-editor.json(1 hunks)packages/visual-editor/locales/components/tr/visual-editor.json(1 hunks)packages/visual-editor/locales/components/zh-TW/visual-editor.json(1 hunks)packages/visual-editor/locales/components/zh/visual-editor.json(1 hunks)packages/visual-editor/scripts/checkEmptyTranslations.ts(1 hunks)packages/visual-editor/scripts/generateTranslations.ts(5 hunks)packages/visual-editor/scripts/sortTranslations.ts(2 hunks)packages/visual-editor/src/editor/Editor.tsx(2 hunks)packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx(3 hunks)packages/visual-editor/src/utils/VisualEditorProvider.tsx(2 hunks)packages/visual-editor/src/utils/i18n/components.ts(1 hunks)packages/visual-editor/src/utils/i18n/fallbacks.ts(1 hunks)packages/visual-editor/src/utils/i18n/index.ts(1 hunks)packages/visual-editor/src/utils/i18n/platform.ts(3 hunks)packages/visual-editor/src/utils/resolveComponentData.tsx(1 hunks)packages/visual-editor/src/utils/resolveYextEntityField.ts(1 hunks)packages/visual-editor/src/vite-plugin/plugin.ts(1 hunks)packages/visual-editor/src/vite-plugin/templates/directory.tsx(3 hunks)packages/visual-editor/src/vite-plugin/templates/locator.tsx(3 hunks)packages/visual-editor/src/vite-plugin/templates/main.tsx(3 hunks)starter/src/templates/dev.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx (2)
packages/visual-editor/src/utils/i18n/components.ts (2)
loadComponentTranslations(77-94)i18nComponentsInstance(6-6)packages/visual-editor/src/utils/i18n/platform.ts (2)
loadPlatformTranslations(58-73)i18nPlatformInstance(7-7)
packages/visual-editor/src/vite-plugin/templates/locator.tsx (4)
packages/visual-editor/src/vite-plugin/templates/directory.tsx (1)
transformProps(96-100)packages/visual-editor/src/vite-plugin/templates/main.tsx (1)
transformProps(97-101)starter/src/templates/dev.tsx (1)
transformProps(131-135)packages/visual-editor/src/utils/i18n/components.ts (1)
injectTranslations(22-34)
packages/visual-editor/scripts/generateTranslations.ts (1)
scripts/releaseUtils.ts (1)
isDryRun(17-17)
starter/src/templates/dev.tsx (4)
packages/visual-editor/src/vite-plugin/templates/directory.tsx (1)
transformProps(96-100)packages/visual-editor/src/vite-plugin/templates/locator.tsx (1)
transformProps(95-99)packages/visual-editor/src/vite-plugin/templates/main.tsx (1)
transformProps(97-101)packages/visual-editor/src/utils/i18n/components.ts (1)
injectTranslations(22-34)
packages/visual-editor/src/vite-plugin/templates/main.tsx (4)
packages/visual-editor/src/vite-plugin/templates/directory.tsx (1)
transformProps(96-100)packages/visual-editor/src/vite-plugin/templates/locator.tsx (1)
transformProps(95-99)starter/src/templates/dev.tsx (1)
transformProps(131-135)packages/visual-editor/src/utils/i18n/components.ts (1)
injectTranslations(22-34)
packages/visual-editor/src/vite-plugin/templates/directory.tsx (4)
packages/visual-editor/src/vite-plugin/templates/locator.tsx (1)
transformProps(95-99)packages/visual-editor/src/vite-plugin/templates/main.tsx (1)
transformProps(97-101)starter/src/templates/dev.tsx (1)
transformProps(131-135)packages/visual-editor/src/utils/i18n/components.ts (1)
injectTranslations(22-34)
packages/visual-editor/src/utils/VisualEditorProvider.tsx (1)
packages/visual-editor/src/utils/i18n/components.ts (1)
loadComponentTranslations(77-94)
packages/visual-editor/src/editor/Editor.tsx (1)
packages/visual-editor/src/utils/i18n/platform.ts (2)
loadPlatformTranslations(58-73)i18nPlatformInstance(7-7)
packages/visual-editor/src/utils/i18n/platform.ts (1)
packages/visual-editor/src/utils/i18n/fallbacks.ts (2)
applyI18nFallbacks(26-39)defaultI18nFallbacks(6-15)
🔇 Additional comments (30)
packages/visual-editor/locales/components/sk/visual-editor.json (1)
1-14: JSON structure and Slovak translations look good.The file is properly formatted and contains semantically appropriate translations for location/business hours status indicators and weekday names. The nested directory structure (
locales/components/sk/) aligns with the PR's goal of supporting granular, dynamic translation loading.Verify that this locale file is properly integrated into the new
loadComponentTranslations()runtime loading function so it's only bundled and loaded when the Slovak locale is needed.packages/visual-editor/locales/components/zh-TW/visual-editor.json (1)
1-14: Locale resource file looks good.The JSON structure is valid, all keys are present with non-empty translations, and the camelCase naming convention is consistent with typical i18n patterns. The translations cover status states and day-of-week labels as expected.
One minor observation:
"closed"and"closesAt"both map to the same translation"關閉". This is likely intentional, but if they should convey slightly different meanings in the UI, consider whether separate translations are needed.packages/visual-editor/locales/components/da/visual-editor.json (1)
1-14: JSON structure is valid and properly integrated into the new locale hierarchy.The file is well-formed, contains 12 UI translation keys for Danish, and correctly uses the new
locales/components/{locale}/visual-editor.jsonpath structure as part of the runtime translation loading system. Danish characters are properly encoded.packages/visual-editor/locales/components/nl/visual-editor.json (1)
1-14: JSON structure and formatting are valid.The translation file is properly formatted JSON with no syntax errors. However, I cannot verify the accuracy of Dutch translations or completeness of the key set without examining reference locale files or domain expertise.
Consider running the following to verify consistency with other locale files:
- Ensure all keys in this file match those in companion locale files (e.g.,
en/visual-editor.json)- Have Dutch translations reviewed by a native speaker or translation expert to confirm accuracy
- Verify that the key order is intentional or apply consistent alphabetical sorting across all locale files
packages/visual-editor/src/utils/resolveComponentData.tsx (1)
4-7: LGTM! Type-only imports correctly applied.Converting these imports to type-only is a good TypeScript practice that reduces the runtime bundle size. All four types (
RichText,TranslatableRichText,TranslatableString,YextEntityField) are used exclusively as type annotations throughout the file, making this change appropriate and aligned with the PR's goal of loading only necessary code.packages/visual-editor/src/utils/resolveYextEntityField.ts (1)
1-3: LGTM! Consistent type-only imports.The conversion to type-only imports is correct and follows the same pattern as the changes in
resolveComponentData.tsx. All three types (StreamDocument,YextEntityField,YextStructEntityField) are used exclusively for type annotations, making this refactor safe and beneficial for reducing the runtime bundle size.packages/visual-editor/locales/components/hu/visual-editor.json (1)
1-14: All required keys are present and consistent across locale files.The Hungarian locale file contains all required keys matching the English baseline (13 keys: location status and weekdays). The file is properly structured and integrated into the new two-tier locale hierarchy supporting granular component translation loading.
To complete validation, verify:
- Hungarian translations are accurate for the intended use context (location/hours display).
- The runtime
loadComponentTranslations()function properly loads and references this file.packages/visual-editor/locales/components/ro/visual-editor.json (1)
1-14: ✓ Translation key consistency verified.All 13 translation keys in the Romanian locale (closed, closesAt, friday, monday, open24Hours, openNow, saturday, sunday, temporarilyClosed, thursday, tuesday, wednesday) match exactly across all 25 locale files in the components namespace. The JSON structure is valid and properly formatted. No issues detected.
packages/visual-editor/locales/components/hr/visual-editor.json (1)
1-14: Translation content is accurate and properly integrated into the dynamic loading system.The Croatian locale file is correctly positioned at
packages/visual-editor/locales/components/hr/visual-editor.json, following the naming convention of all 24 other component translations. TheloadComponentTranslations()function dynamically imports component locales viaimport(\../../../locales/components/${locale}/visual-editor.json`)`, which correctly resolves to this file. The system uses runtime lazy loading—component translations are not processed by the i18next-scanner (which handles platform translations separately) but are loaded on-demand when the Croatian locale is selected, with automatic fallback to English if needed.packages/visual-editor/locales/components/nb/visual-editor.json (1)
1-14: Norwegian translations look reasonable.The translation keys and values are consistent with other locale files. The use of "Lukket" for closed status and "Åpent 24 timer" for 24-hour operation appears appropriate.
packages/visual-editor/locales/components/en/visual-editor.json (1)
1-14: English baseline translations are correct.All translations use appropriate grammatical forms for status indicators and are consistent across keys.
packages/visual-editor/src/vite-plugin/templates/directory.tsx (3)
12-12: LGTM: Required import added.The
TransformPropstype import is necessary for the newtransformPropsfunction.
27-27: LGTM: Translation injection utility imported.The
injectTranslationsimport enables runtime translation loading based on document locale.
96-100: LGTM: Translation injection follows established pattern.The
transformPropsimplementation correctly delegates toinjectTranslationsand matches the pattern used in other templates (locator.tsx, main.tsx). This enables dynamic loading of only the necessary translations for each locale, aligning with the PR objective.packages/visual-editor/locales/components/cs/visual-editor.json (1)
1-14: Czech translations look reasonable.The translations use appropriate grammatical forms. Note that "ZAVŘENO" (line 2) is in all caps, which may be intentional for emphasis on closed status.
packages/visual-editor/locales/components/et/visual-editor.json (1)
1-14: LGTM! Estonian translations are complete and properly localized.All required keys are translated and the structure is correct.
packages/visual-editor/locales/components/es/visual-editor.json (1)
1-14: LGTM! Spanish translations are complete and properly localized.All required keys are translated correctly.
packages/visual-editor/src/utils/i18n/index.ts (1)
1-1: LGTM! Export addition supports runtime translation injection.Adding
injectTranslationsto the public API aligns with the PR objective to load translations dynamically.packages/visual-editor/locales/components/pt/visual-editor.json (1)
1-14: LGTM! Portuguese translations are complete and properly localized.All required keys including weekdays are fully translated.
packages/visual-editor/locales/components/lv/visual-editor.json (1)
1-14: LGTM! Latvian translations are complete and properly localized.All required keys are fully translated.
packages/visual-editor/locales/components/en-GB/visual-editor.json (1)
1-14: LGTM!The UK English translations are correct and consistent with standard localization practices.
packages/visual-editor/src/vite-plugin/templates/main.tsx (1)
97-101: LGTM!The
transformPropsimplementation correctly awaits the asyncinjectTranslationscall and follows the same pattern used in other template files (directory.tsx, locator.tsx, dev.tsx). This ensures translations are loaded before template rendering.packages/visual-editor/locales/components/lt/visual-editor.json (1)
1-14: Structure looks good; verify translation accuracy.The JSON structure and formatting are correct. However, translation accuracy should be verified by a Lithuanian speaker to ensure the translations are natural and appropriate for the UI context.
packages/visual-editor/i18next-scanner.config.cjs (1)
55-56: LGTM! Path restructuring aligns with the two-tier locale hierarchy.The updated resource paths correctly scope translations under
locales/platform/, enabling separate handling of platform vs. component translations.packages/visual-editor/src/vite-plugin/plugin.ts (1)
154-158: LGTM! Cleanup now properly scoped to build mode.The conditional cleanup ensures generated files persist during dev mode while being cleaned up after builds. This aligns with the discussion in past review comments about
buildStartrunning for both server and build modes.packages/visual-editor/scripts/checkEmptyTranslations.ts (2)
15-50: LGTM! Improved resilience with two-tier structure support.The updated logic correctly handles the nested locale structure (
locales/<subfolder>/<locale>/visual-editor.json) and gracefully warns about missing files instead of failing. The two-tier traversal aligns with the PR's restructuring goals.
52-60: Good addition of explicit exit code handling.The post-processing check ensures the script exits with the correct code based on validation results, improving CI/CD integration.
packages/visual-editor/src/utils/i18n/platform.ts (2)
7-7: LGTM! Instance now exported for direct usage.Exporting
i18nPlatformInstanceenables components likeLayoutHeaderto directly control language changes after loading translations.
9-17: Good shift to runtime translation loading.Starting with empty resources and loading on-demand reduces bundle size and enables locale-specific loading, directly addressing the PR objective.
packages/visual-editor/scripts/sortTranslations.ts (1)
39-71: Add error handling around file I/O operations.The original review comment correctly identified missing error handling, but its structural assumptions do not match the actual directory layout. The locale structure is strictly
locales/{components,platform}/{language}/visual-editor.jsonwith no mixed-content directories or nested JSON files at intermediate levels. The algorithm handles this structure correctly.However, error handling is genuinely missing from file operations. Add try-catch blocks around
JSON.parse,fs.readFile, andfs.writeFile:if (file.isFile() && file.name.endsWith(".json")) { const filePath = path.join(fullPath, file.name); - const data = JSON.parse(await fs.readFile(filePath, "utf8")); - const sorted = sortObject(data); - await fs.writeFile( - filePath, - JSON.stringify(sorted, null, 2) + "\n" - ); + try { + const data = JSON.parse(await fs.readFile(filePath, "utf8")); + const sorted = sortObject(data); + await fs.writeFile( + filePath, + JSON.stringify(sorted, null, 2) + "\n" + ); + } catch (error) { + console.error(`Error processing ${filePath}:`, error); + throw error; + } }Likely an incorrect or invalid review comment.
packages/visual-editor/locales/components/fi/visual-editor.json
Outdated
Show resolved
Hide resolved
packages/visual-editor/locales/components/zh/visual-editor.json
Outdated
Show resolved
Hide resolved
packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/visual-editor/src/vite-plugin/plugin.ts (1)
151-158: Update the comment to reflect the new cleanup behavior.The comment at lines 95-96 states "Created files will be marked for deletion on buildEnd," but with the new conditional logic, files are only cleaned up on
buildEndduring build mode. In dev mode, cleanup is deferred to the SIGINT/SIGTERM handlers (lines 136-144).Apply this diff to update the comment:
/** * generateFiles generates the template files and .temlpate-manifest.json file * * Does not overwrite files that already exists * - * Created files will be marked for deletion on buildEnd + * Created files will be marked for deletion on buildEnd (build mode) or process termination (dev mode) */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/visual-editor/src/utils/resolveYextEntityField.ts(1 hunks)packages/visual-editor/src/vite-plugin/plugin.ts(1 hunks)packages/visual-editor/src/vite-plugin/templates/directory.tsx(3 hunks)packages/visual-editor/src/vite-plugin/templates/locator.tsx(3 hunks)packages/visual-editor/src/vite-plugin/templates/main.tsx(3 hunks)starter/src/templates/dev.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- starter/src/templates/dev.tsx
- packages/visual-editor/src/vite-plugin/templates/main.tsx
- packages/visual-editor/src/utils/resolveYextEntityField.ts
- packages/visual-editor/src/vite-plugin/templates/locator.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/visual-editor/src/vite-plugin/templates/directory.tsx (4)
packages/visual-editor/src/vite-plugin/templates/main.tsx (1)
transformProps(97-101)starter/src/templates/dev.tsx (1)
transformProps(133-137)packages/visual-editor/src/vite-plugin/templates/locator.tsx (1)
transformProps(95-99)packages/visual-editor/src/utils/i18n/components.ts (1)
injectTranslations(22-34)
🔇 Additional comments (1)
packages/visual-editor/src/vite-plugin/templates/directory.tsx (1)
96-100: LGTM! Translation injection is correctly implemented.The
transformPropsimplementation correctly delegates toinjectTranslationsand is consistent with the pattern used in other template files (main.tsx, locator.tsx). This aligns with the PR objective to load only necessary translations based on the document locale.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/visual-editor/scripts/generateTranslations.ts (4)
64-84: Update TSDoc to document thetypeparameter.The ENOENT handling from the previous review has been correctly implemented. However, the function documentation doesn't describe the new
typeparameter.Apply this diff to complete the documentation:
/** - * Reads all directories under localesDir and returns them as target languages. + * Reads all directories under localesDir/{type} and returns them as target languages. + * @param type - The translation type to get languages for ("components" or "platform") + * @returns Array of language directory names, or empty array if type directory doesn't exist */ async function getTargetLanguages( type: "components" | "platform" ): Promise<string[]> {
194-198: Update TSDoc to document thetypeparameter.The function documentation doesn't describe the new
typeparameter.Apply this diff:
/** * Loads the base English translation file, compares it to each target language file, * translates missing or empty keys using Google Translate, and optionally writes them back. + * @param type - The translation type to process ("components" or "platform") */ async function translateFile(type: "components" | "platform"): Promise<void> {
256-256: Make error logging consistent with other log messages.All other log messages in this function use the
[${type}/${lng}]format, but this error log uses only[${lng}].Apply this diff:
} catch (e) { failCount++; - console.error(`[${lng}] ❌ Failed to translate key "${key}":`, e); + console.error(`[${type}/${lng}] ❌ Failed to translate key "${key}":`, e); }
273-274: Await translation promises to ensure proper completion and error handling.The script invokes both
translateFilecalls without awaiting them. While Node.js keeps the process alive for pending async operations, this pattern has drawbacks: if either translation fails, the error is logged but the process may exit with code 0, causing CI/CD pipelines to incorrectly report success.Apply this diff to run both in parallel with proper exit code handling:
-// Kick off the translation process -translateFile("components").catch(console.error); -translateFile("platform").catch(console.error); +(async () => { + try { + await Promise.all([ + translateFile("components"), + translateFile("platform") + ]); + } catch (error) { + console.error(error); + process.exit(1); + } +})();Or, if you prefer sequential execution:
-// Kick off the translation process -translateFile("components").catch(console.error); -translateFile("platform").catch(console.error); +(async () => { + try { + await translateFile("components"); + await translateFile("platform"); + } catch (error) { + console.error(error); + process.exit(1); + } +})();packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx (1)
224-239: Consider adding error handling for translation loading.The race conditions from previous reviews have been correctly addressed by making the handlers async and awaiting the translation loaders. However, for improved developer experience in local dev mode, consider adding error handling:
<Button onClick={async () => { const locale = prompt("Enter components locale:") || "en"; - await loadComponentTranslations(locale); - i18nComponentsInstance.changeLanguage(locale); + try { + await loadComponentTranslations(locale); + i18nComponentsInstance.changeLanguage(locale); + } catch (error) { + console.error('Failed to load component translations:', error); + alert(`Failed to load translations for locale: ${locale}`); + } }} variant="outline" className="ve-ml-4" > Set Components Locale </Button> <Button onClick={async () => { const locale = prompt("Enter platform locale:") || "en"; - await loadPlatformTranslations(locale); - i18nPlatformInstance.changeLanguage(locale); + try { + await loadPlatformTranslations(locale); + i18nPlatformInstance.changeLanguage(locale); + } catch (error) { + console.error('Failed to load platform translations:', error); + alert(`Failed to load translations for locale: ${locale}`); + } }} variant="outline" className="ve-ml-4" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/visual-editor/scripts/generateTranslations.ts(5 hunks)packages/visual-editor/src/editor/Editor.tsx(2 hunks)packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/visual-editor/src/editor/Editor.tsx (1)
packages/visual-editor/src/utils/i18n/platform.ts (2)
loadPlatformTranslations(58-73)i18nPlatformInstance(7-7)
packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx (2)
packages/visual-editor/src/utils/i18n/components.ts (2)
loadComponentTranslations(77-94)i18nComponentsInstance(6-6)packages/visual-editor/src/utils/i18n/platform.ts (2)
loadPlatformTranslations(58-73)i18nPlatformInstance(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: call_unit_test / unit_tests (18.x)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: semgrep/ci
- GitHub Check: create-dev-release
🔇 Additional comments (2)
packages/visual-editor/src/editor/Editor.tsx (1)
19-22: LGTM! Imports correctly support the new translation loading flow.The addition of
loadPlatformTranslationsenables the pre-loading of platform translations before language changes.packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx (1)
16-19: LGTM! Imports correctly support the dev override buttons.The translation loading utilities are properly imported and enable the locale-switching functionality in
LocalDevOverrideButtons.Also applies to: 24-24
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (1)
packages/visual-editor/locales/components/zh/visual-editor.json (1)
4-5: Complete the weekday translations.The weekday names remain in English instead of Chinese, creating an inconsistent localization experience. Other locale files in this PR have properly translated weekdays.
🔎 Proposed fix
- "friday": "Friday", - "monday": "Monday", + "friday": "星期五", + "monday": "星期一",- "saturday": "Saturday", - "sunday": "Sunday", + "saturday": "星期六", + "sunday": "星期日",- "thursday": "Thursday", - "tuesday": "Tuesday", - "wednesday": "Wednesday", + "thursday": "星期四", + "tuesday": "星期二", + "wednesday": "星期三",Also applies to: 8-9, 11-13
🧹 Nitpick comments (4)
packages/visual-editor/locales/components/da/visual-editor.json (1)
1-24: Verify translation accuracy with a native Danish speaker.While the JSON structure is valid and all keys are translated, consider having these translations reviewed by a native Danish speaker to ensure accuracy, natural phrasing, and appropriate capitalization conventions for Danish.
packages/visual-editor/locales/components/es/visual-editor.json (1)
23-23: Consider more idiomatic navigation terminology.While "Obtener instrucciones" is correct, "Cómo llegar" or "Obtener indicaciones" are more commonly used in Spanish navigation/mapping contexts.
🔎 Suggested alternatives
- "getDirections": "Obtener instrucciones" + "getDirections": "Cómo llegar"Or:
- "getDirections": "Obtener instrucciones" + "getDirections": "Obtener indicaciones"packages/visual-editor/scripts/copyPlatformTranslations.ts (2)
1-1: Update comment to match actual filename.The comment references
copyTranslations.tsbut the file is namedcopyPlatformTranslations.ts.🔎 Proposed fix
-// scripts/copyTranslations.ts +// scripts/copyPlatformTranslations.ts
108-120: Consider key ordering after updates.The script adds new keys to the component translations, but they may not be in alphabetical order. While there's an
i18n:sortscript available, users might benefit from a reminder to run it after this script completes.Suggested improvement
Add a note to the final console output reminding users to run the sort script:
} - console.log("\nDone!"); + console.log("\nDone!"); + console.log("Tip: Run 'pnpm run i18n:sort' to alphabetize the updated translations."); };
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (53)
.gitattributespackages/visual-editor/locales/components/cs/visual-editor.jsonpackages/visual-editor/locales/components/da/visual-editor.jsonpackages/visual-editor/locales/components/de/visual-editor.jsonpackages/visual-editor/locales/components/en-GB/visual-editor.jsonpackages/visual-editor/locales/components/en/visual-editor.jsonpackages/visual-editor/locales/components/es/visual-editor.jsonpackages/visual-editor/locales/components/et/visual-editor.jsonpackages/visual-editor/locales/components/fi/visual-editor.jsonpackages/visual-editor/locales/components/fr/visual-editor.jsonpackages/visual-editor/locales/components/hr/visual-editor.jsonpackages/visual-editor/locales/components/hu/visual-editor.jsonpackages/visual-editor/locales/components/it/visual-editor.jsonpackages/visual-editor/locales/components/ja/visual-editor.jsonpackages/visual-editor/locales/components/lt/visual-editor.jsonpackages/visual-editor/locales/components/lv/visual-editor.jsonpackages/visual-editor/locales/components/nb/visual-editor.jsonpackages/visual-editor/locales/components/nl/visual-editor.jsonpackages/visual-editor/locales/components/pl/visual-editor.jsonpackages/visual-editor/locales/components/pt/visual-editor.jsonpackages/visual-editor/locales/components/ro/visual-editor.jsonpackages/visual-editor/locales/components/sk/visual-editor.jsonpackages/visual-editor/locales/components/sv/visual-editor.jsonpackages/visual-editor/locales/components/tr/visual-editor.jsonpackages/visual-editor/locales/components/zh-TW/visual-editor.jsonpackages/visual-editor/locales/components/zh/visual-editor.jsonpackages/visual-editor/locales/platform/cs/visual-editor.jsonpackages/visual-editor/locales/platform/da/visual-editor.jsonpackages/visual-editor/locales/platform/de/visual-editor.jsonpackages/visual-editor/locales/platform/en-GB/visual-editor.jsonpackages/visual-editor/locales/platform/en/visual-editor.jsonpackages/visual-editor/locales/platform/es/visual-editor.jsonpackages/visual-editor/locales/platform/et/visual-editor.jsonpackages/visual-editor/locales/platform/fi/visual-editor.jsonpackages/visual-editor/locales/platform/fr/visual-editor.jsonpackages/visual-editor/locales/platform/hr/visual-editor.jsonpackages/visual-editor/locales/platform/hu/visual-editor.jsonpackages/visual-editor/locales/platform/it/visual-editor.jsonpackages/visual-editor/locales/platform/ja/visual-editor.jsonpackages/visual-editor/locales/platform/lt/visual-editor.jsonpackages/visual-editor/locales/platform/lv/visual-editor.jsonpackages/visual-editor/locales/platform/nb/visual-editor.jsonpackages/visual-editor/locales/platform/nl/visual-editor.jsonpackages/visual-editor/locales/platform/pl/visual-editor.jsonpackages/visual-editor/locales/platform/pt/visual-editor.jsonpackages/visual-editor/locales/platform/ro/visual-editor.jsonpackages/visual-editor/locales/platform/sk/visual-editor.jsonpackages/visual-editor/locales/platform/sv/visual-editor.jsonpackages/visual-editor/locales/platform/tr/visual-editor.jsonpackages/visual-editor/locales/platform/zh-TW/visual-editor.jsonpackages/visual-editor/locales/platform/zh/visual-editor.jsonpackages/visual-editor/package.jsonpackages/visual-editor/scripts/copyPlatformTranslations.ts
💤 Files with no reviewable changes (1)
- packages/visual-editor/locales/platform/en/visual-editor.json
✅ Files skipped from review due to trivial changes (1)
- packages/visual-editor/locales/components/hr/visual-editor.json
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/visual-editor/locales/components/en/visual-editor.json
- packages/visual-editor/locales/components/et/visual-editor.json
- packages/visual-editor/locales/components/cs/visual-editor.json
- packages/visual-editor/locales/components/sv/visual-editor.json
- packages/visual-editor/locales/components/fr/visual-editor.json
- packages/visual-editor/locales/components/zh-TW/visual-editor.json
- packages/visual-editor/locales/components/nb/visual-editor.json
- packages/visual-editor/locales/components/de/visual-editor.json
- packages/visual-editor/locales/components/hu/visual-editor.json
- packages/visual-editor/locales/components/sk/visual-editor.json
- packages/visual-editor/locales/components/pl/visual-editor.json
- packages/visual-editor/locales/components/en-GB/visual-editor.json
- packages/visual-editor/locales/components/ja/visual-editor.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-23T16:36:42.670Z
Learnt from: benlife5
Repo: yext/visual-editor PR: 958
File: packages/visual-editor/locales/ro/visual-editor.json:394-394
Timestamp: 2025-12-23T16:36:42.670Z
Learning: In Romanian locale files (ro), "Top" is an acceptable translation for directional terms like "top_direction" rather than the native Romanian "Sus". The maintainer benlife5 confirmed this is the preferred translation in packages/visual-editor/locales/ro/visual-editor.json.
Applied to files:
packages/visual-editor/locales/components/ro/visual-editor.json
🧬 Code graph analysis (1)
packages/visual-editor/scripts/copyPlatformTranslations.ts (1)
packages/visual-editor/scripts/generateFontRegistry.js (2)
dir(216-216)data(42-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: semgrep/ci
🔇 Additional comments (10)
packages/visual-editor/locales/components/it/visual-editor.json (1)
2-4: Most translations are correct.The majority of the Italian translations are accurate and idiomatic. The weekdays (Venerdì, Sabato, Domenica, Giovedì, Martedì, Mercoledì) have correct accents, and the UI strings (Chiuso, Chiude a, Temporaneamente chiuso, Ricerca, Filtro, Distanza, etc.) are properly translated.
However, please address the previously flagged issues at lines 5-7 before merging.
Also applies to: 8-24
packages/visual-editor/locales/components/pt/visual-editor.json (1)
1-24: Portuguese translations are complete and accurate; they match the English locale with all 22 keys present.The Portuguese locale file contains all required translation keys and matches the English reference locale exactly. The JSON structure is valid and the translations are correct.
packages/visual-editor/locales/components/nl/visual-editor.json (1)
1-24: No action needed. All component locale files (packages/visual-editor/locales/components/*/visual-editor.json) across all 25 language versions contain the same 22 keys with consistent structure, including the reviewed Dutch (nl) file.packages/visual-editor/locales/components/ro/visual-editor.json (1)
1-24: No issues to address. These translations are copied from the established platform/ro source, which already contains both"openNow": "Deschide acum"and"filter": "Filtra". The verb forms used are consistent with the existing platform translations and have already been reviewed. This PR is a mechanical copy operation between locale organizational tiers.Likely an incorrect or invalid review comment.
packages/visual-editor/package.json (1)
58-59: LGTM!The new i18n scripts are properly formatted and follow the established naming convention. The
i18n:copy-platform-translationsscript aligns with the PR's objective to manage translations more efficiently..gitattributes (1)
4-5: LGTM!The updated linguist-generated markers correctly reflect the new platform/components locale structure. Marking English locale files as non-generated ensures they're properly tracked as source files.
packages/visual-editor/locales/components/lv/visual-editor.json (1)
1-24: LGTM!The Latvian locale file is properly formatted with valid JSON structure and follows the established key naming conventions used in other locale files.
packages/visual-editor/scripts/copyPlatformTranslations.ts (3)
20-47: LGTM!The helper functions are well-structured with appropriate error handling. The
loadJsonfunction gracefully handles missing files and parse errors, whilesaveJsonensures directories exist before writing and includes a trailing newline for clean git diffs.
87-127: LGTM!The main processing logic correctly copies missing translation keys from platform to component locale files. The conditional check ensures only missing keys are added, and files are only written when updates are made.
129-134: LGTM!The top-level error handling properly catches and reports unexpected errors with appropriate exit codes.
packages/visual-editor/locales/components/es/visual-editor.json
Outdated
Show resolved
Hide resolved
packages/visual-editor/locales/components/es/visual-editor.json
Outdated
Show resolved
Hide resolved
packages/visual-editor/locales/components/fi/visual-editor.json
Outdated
Show resolved
Hide resolved
packages/visual-editor/locales/components/fi/visual-editor.json
Outdated
Show resolved
Hide resolved
packages/visual-editor/locales/components/nl/visual-editor.json
Outdated
Show resolved
Hide resolved
packages/visual-editor/locales/components/tr/visual-editor.json
Outdated
Show resolved
Hide resolved
![semgrep-code-yext[bot]](https://avatars.githubusercontent.com/u/931250?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/visual-editor/scripts/dedupeTranslations.ts (2)
55-56: Verify JSON.stringify comparison reliability for object values.Using
JSON.stringify(value)to compare values can produce false conflicts if the same object has keys in different orders. While this may be acceptable for primitive values (strings, numbers, booleans, null), objects with reordered keys would be flagged as conflicts even though they're semantically identical.💡 Consider a deep-equality comparison for objects
If translation values can contain nested objects, consider using a deep-equality comparison instead of string comparison, or normalize object key ordering before comparison:
- const stringValue = JSON.stringify(value); + const stringValue = typeof value === 'object' && value !== null + ? JSON.stringify(value, Object.keys(value).sort()) + : JSON.stringify(value);Alternatively, use a library like
lodash.isEqualfor semantic equality checks.
99-106: Object reconstruction assumes no arrays in JSON structure.The rebuild logic at lines 99-106 assumes all intermediate values are objects. If translation JSON contains arrays, they would be overwritten with empty objects on line 103. While translation files typically use nested objects rather than arrays, consider adding a guard if arrays are possible.
💡 Add array handling if needed
const parts = keyPath.split("."); let obj = cleaned; for (let i = 0; i < parts.length - 1; i++) { const part = parts[i]; - if (!obj[part]) obj[part] = {}; + if (!obj[part] || typeof obj[part] !== 'object') obj[part] = {}; obj = obj[part]; } obj[parts[parts.length - 1]] = value;
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
packages/visual-editor/locales/components/es/visual-editor.jsonpackages/visual-editor/locales/components/fi/visual-editor.jsonpackages/visual-editor/locales/components/nl/visual-editor.jsonpackages/visual-editor/locales/components/tr/visual-editor.jsonpackages/visual-editor/locales/platform/en/visual-editor.jsonpackages/visual-editor/locales/platform/es/visual-editor.jsonpackages/visual-editor/locales/platform/fi/visual-editor.jsonpackages/visual-editor/locales/platform/nl/visual-editor.jsonpackages/visual-editor/locales/platform/tr/visual-editor.jsonpackages/visual-editor/package.jsonpackages/visual-editor/scripts/copyPlatformTranslations.tspackages/visual-editor/scripts/dedupeTranslations.ts
✅ Files skipped from review due to trivial changes (1)
- packages/visual-editor/locales/platform/nl/visual-editor.json
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/visual-editor/locales/components/es/visual-editor.json
- packages/visual-editor/locales/components/tr/visual-editor.json
- packages/visual-editor/locales/components/nl/visual-editor.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: semgrep/ci
🔇 Additional comments (7)
packages/visual-editor/locales/platform/fi/visual-editor.json (1)
484-484: LGTM! Improved Finnish translation.The change from "Suodattaa" (infinitive) to "Suodata" (imperative) is appropriate for UI context. The imperative form is more idiomatic for action labels and buttons in Finnish.
packages/visual-editor/locales/components/fi/visual-editor.json (1)
1-24: LGTM! All previous issues have been addressed.The Finnish translations look correct and consistent. All issues flagged in previous reviews have been successfully resolved:
- Proper time indicator "kello" for closesAt
- Consistent lowercase weekday names
- Correct imperative verb form "Suodata" for the filter action
packages/visual-editor/locales/platform/tr/visual-editor.json (1)
616-616: LGTM - Minor localization consistency improvement.The capitalization change from title case to sentence case for "hassaslaştırın" improves consistency with modern UI conventions for action labels. While this change appears incidental to the PR's main objective of optimizing translation loading, it's a reasonable localization refinement.
packages/visual-editor/locales/platform/es/visual-editor.json (1)
555-555: LGTM! Translation quality improvement.The change from "Abrir 24 horas" to "Abierto 24 horas" is grammatically correct. "Abierto" (open/opened) is the appropriate adjective form for indicating business hours status, rather than the infinitive "Abrir" (to open).
packages/visual-editor/scripts/dedupeTranslations.ts (1)
143-148: LGTM: Error handling is appropriate for a CLI script.The top-level error handler with
process.exit(1)is suitable for a build/maintenance script.packages/visual-editor/package.json (2)
58-60: LGTM: New i18n scripts align with the translation tooling additions.The new npm scripts correctly reference the corresponding TypeScript files added in this PR and follow the existing naming convention.
119-119: The AI summary was incorrect.oboeand@types/oboeare only indevDependencies, not in bothdependenciesanddevDependencies. There is no duplication to address.Likely an incorrect or invalid review comment.
|
Warning: Component files have been updated but no migrations have been added. See https://github.com/yext/visual-editor/blob/main/packages/visual-editor/src/components/migrations/README.md for more information. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (7)
packages/visual-editor/locales/components/it/visual-editor.json (1)
18-20: Italian translation errors still present.The previously flagged issues remain unaddressed:
- Line 18: "Lunedi" is missing the accent and should be "Lunedì"
- Lines 19-20: "Apri" is the imperative (command) form, but should be "Aperto" (adjective for status)
These corrections are necessary for grammatically correct Italian.
🔎 Apply these corrections
- "monday": "Lunedi", - "open24Hours": "Apri 24 ore", - "openNow": "Apri ora", + "monday": "Lunedì", + "open24Hours": "Aperto 24 ore", + "openNow": "Aperto ora",packages/visual-editor/locales/components/nl/visual-editor.json (1)
39-39: Zero-width space character still present in translation.The translation still contains an invisible zero-width space character between "een" and "locatie" which was flagged in a previous review. This can cause text processing, search, or display issues.
🔎 Proposed fix
- "useOurLocatorToFindALocationNearYou": "Gebruik onze locator om een locatie bij u in de buurt te vinden", + "useOurLocatorToFindALocationNearYou": "Gebruik onze locator om een locatie bij u in de buurt te vinden",packages/visual-editor/locales/components/de/visual-editor.json (1)
4-4: Incorrect preposition for time expression.The preposition "bei" is used for locations, but closing times require "um" in German.
packages/visual-editor/locales/components/es/visual-editor.json (1)
39-39: Correct the inconsistent formality in the imperative verb.The translation still mixes formal and informal forms: "Use" (formal imperative for usted) with "ti" (informal pronoun). Since the rest of the file uses informal tú forms ("Refina tu" on Line 21, "Busca" on Line 25), this should use the informal imperative "Usa" for consistency.
🔎 Proposed fix
- "useOurLocatorToFindALocationNearYou": "Use nuestro localizador para encontrar una ubicación cerca de ti", + "useOurLocatorToFindALocationNearYou": "Usa nuestro localizador para encontrar una ubicación cerca de ti",packages/visual-editor/locales/components/fr/visual-editor.json (1)
19-20: Use adjective form for status indicators (previously flagged, still unresolved).These lines still use infinitive verb forms ("Ouvrir") which read as commands rather than status descriptions. For UI status indicators, the adjective form "Ouvert" should be used.
🔎 Apply this fix to use proper status adjectives
- "open24Hours": "Ouvrir 24 heures", - "openNow": "Ouvrir maintenant", + "open24Hours": "Ouvert 24h/24", + "openNow": "Ouvert actuellement",packages/visual-editor/locales/components/en/visual-editor.json (1)
1-42: Verify completeness of component translations.The past review comment mentioned missing translations like
openMenufrom ExpandedHeader. WhilesearchThisAreais now present (line 25), other component-level keys may still be missing from this component locale file.Run the following script to identify component translations that may be missing from this English component locale:
#!/bin/bash # Description: Find translation keys used in component files that might be missing from the English component locale # Search for translation key usage in component files (t('keyName') or i18n.t('keyName') patterns) echo "=== Searching for translation keys in components ===" rg -n --type tsx --type ts "\.t\(['\"]([^'\"]+)['\"]" packages/visual-editor/src/components/ -o -r '$1' | sort -u echo "" echo "=== Keys in English component locale ===" jq -r 'keys[]' packages/visual-editor/locales/components/en/visual-editor.json echo "" echo "=== Comparing keys ===" # This will help identify if there are obvious gapspackages/visual-editor/locales/components/tr/visual-editor.json (1)
19-20: Status strings still use imperative verbs instead of adjectives.Despite the past review comment indicating this was addressed in commit 7695f51, the current code still shows imperative verb forms:
- Line 19: "açın" (open as command)
- Line 20: "aç" (open as command)
These should be status indicators using the adjective form "açık" (open as state):
- "24 saat açık" (open 24 hours as a state)
- "Şu anda açık" (currently open as a state)
🔎 Suggested fix
- "open24Hours": "24 saat açın", - "openNow": "Şimdi aç", + "open24Hours": "24 saat açık", + "openNow": "Şu anda açık",
🧹 Nitpick comments (12)
packages/visual-editor/locales/platform/fi/visual-editor.json (1)
494-502: Consider consolidating duplicate social link translations.Two nearly identical objects (
footerSocialLinksandsocialLinks) contain the same social media translations with only one key difference:footerSocialLinksuses"twitter": "Seuraa meitä Twitterissä"whilesocialLinksuses"xLink": "Seuraa meitä X:ssä (Twitter)". This duplication could create maintenance challenges.If these objects serve different UI contexts, please document the distinction. Otherwise, consider:
- Consolidating into a single object to reduce duplication
- Establishing a consistent naming convention for Twitter/X across all locale objects
Example consolidation approach
If the distinction isn't necessary, you could maintain a single
socialLinksobject and reference it from both contexts, or clearly document why both are needed (e.g., footer uses legacy Twitter branding while other components use X branding).Also applies to: 659-667
packages/visual-editor/locales/platform/da/visual-editor.json (1)
501-509: Verify the need for separatefooterSocialLinksandsocialLinksblocks.The translations are accurate, but this block is nearly identical to
socialLinks(lines 666-674) with one key difference: this usesxLink. This duplication could lead to maintenance issues.Please confirm:
- Is having both blocks intentional for different UI contexts?
- Should the
xLinkinconsistency be unified, or does each serve a specific purpose?If both are needed, consider adding a comment explaining their distinct purposes to aid future maintainers.
packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx (2)
294-299: Race condition resolved correctly.The handler now properly awaits translation loading before changing the language, addressing the previous issue flagged in past reviews.
Consider adding error handling to provide feedback if translation loading fails, though this is lower priority given the dev-only context:
Optional: Add error handling
- onClick={async () => { - const locale = prompt("Enter components locale:") || "en"; - await loadComponentTranslations(locale); - i18nComponentsInstance.changeLanguage(locale); + onClick={async () => { + try { + const locale = prompt("Enter components locale:") || "en"; + await loadComponentTranslations(locale); + i18nComponentsInstance.changeLanguage(locale); + } catch (error) { + console.error("Failed to load component translations:", error); + alert("Failed to load translations for locale: " + locale); + } }}
305-310: Race condition resolved correctly.The handler now properly awaits translation loading before changing the language, matching the fix applied to the component locale button.
Consider adding error handling similar to the component locale button:
Optional: Add error handling
- onClick={async () => { - const locale = prompt("Enter platform locale:") || "en"; - await loadPlatformTranslations(locale); - i18nPlatformInstance.changeLanguage(locale); + onClick={async () => { + try { + const locale = prompt("Enter platform locale:") || "en"; + await loadPlatformTranslations(locale); + i18nPlatformInstance.changeLanguage(locale); + } catch (error) { + console.error("Failed to load platform translations:", error); + alert("Failed to load translations for locale: " + locale); + } }}packages/visual-editor/src/components/Locator.tsx (3)
1224-1236: Use i18next's built-in pluralization instead of manual key selection.i18next automatically handles pluralization when you pass a
countparameter. The manual ternary selection of_onevs_otherkeys is unnecessary and non-idiomatic.🔎 Refactor to idiomatic i18next pluralization
<div> {t( - resultCount === 1 - ? "locationWithinDistanceOf_one" - : "locationsWithinDistanceOf_other", - resultCount === 1 - ? '{{count}} location within {{distance}} miles of "{{name}}"' - : '{{count}} locations within {{distance}} miles of "{{name}}"', + "locationWithinDistanceOf", + '{{count}} location within {{distance}} miles of "{{name}}"', { count: resultCount, distance: selectedDistanceMiles, name: filterDisplayName, } )} </div>When i18next receives the
countparameter, it automatically appends_one,_other, etc. to the base key based on the count value. The translation files should contain bothlocationWithinDistanceOf_oneandlocationWithinDistanceOf_otherkeys.
1241-1250: Use i18next's built-in pluralization instead of manual key selection.Same issue as above—manual selection defeats i18next's automatic pluralization feature.
🔎 Refactor to idiomatic i18next pluralization
<div> {t( - resultCount === 1 ? "locationNear_one" : "locationsNear_other", - resultCount === 1 - ? '{{count}} location near "{{name}}"' - : '{{count}} locations near "{{name}}"', + "locationNear", + '{{count}} location near "{{name}}"', { count: resultCount, name: filterDisplayName, } )} </div>
1257-1265: Use i18next's built-in pluralization instead of manual key selection.Third occurrence of manual plural handling. i18next handles this automatically.
🔎 Refactor to idiomatic i18next pluralization
<div> {t( - resultCount === 1 - ? "locationWithCount_one" - : "locationWithCount_other", - resultCount === 1 ? "{{count}} location" : "{{count}} locations", + "locationWithCount", + "{{count}} location", { count: resultCount, } )} </div>packages/visual-editor/locales/platform/zh/visual-editor.json (1)
494-502: The inconsistency betweenfooterSocialLinks.twitterandsocialLinks.xLinkappears intentional.Both objects exist across all locale files (en, zh, zh-TW, etc.) with the same pattern:
footerSocialLinksuses the"twitter"key whilesocialLinksuses"xLink". This is a deliberate design choice, likely for backward compatibility or different UI contexts. Additionally, no code in the repository directly accesses these locale keys, so the structural inconsistency poses no runtime risk.If standardization is desired for maintainability, both objects should use
"xLink"consistently. Otherwise, this is acceptable as-is.packages/visual-editor/locales/platform/nb/visual-editor.json (2)
659-667: Consider consolidating duplicate social link translations.The
socialLinksblock (lines 659-667) contains translations that are nearly identical tofooterSocialLinks(lines 494-502), with only the Twitter/X key differing. This duplication could increase maintenance burden when translations need to be updated.Consider refactoring to:
- Use a single shared social links object with the Twitter/X distinction handled programmatically
- Or clearly document why both blocks are necessary if they serve different UI contexts
💡 Potential consolidation approach
If both blocks are truly necessary, at least add a comment explaining the distinction:
+ // Social links for footer component (uses legacy 'twitter' key) "footerSocialLinks": { "facebook": "Følg oss på Facebook", ... }, ... + // Social links for general use (uses modern 'xLink' for X/Twitter) "socialLinks": { "facebook": "Følg oss på Facebook", ... }Alternatively, consider a structure that eliminates duplication:
"socialLinks": { "shared": { "facebook": "Følg oss på Facebook", "instagram": "Følg oss på Instagram", "linkedIn": "Følg oss på LinkedIn", "pinterest": "Følg oss på Pinterest", "tiktok": "Følg oss på TikTok", "youtube": "Abonner på vår YouTube-kanal" }, "twitter": "Følg oss på Twitter", "xLink": "Følg oss på X (Twitter)" }
494-502: Document the intentional use ofxLinkkeys in different contexts.The
footerSocialLinksblock uses the keysocialLinksblock usesxLink(line 665). This distinction is intentional—the Footer component referencesfooterSocialLinks.twitter, while other components (SocialLinksSlot) referencesocialLinks.xLinkas part of the Twitter → X rebrand migration. This pattern is consistent across all 28+ language files.To reduce potential confusion for maintainers, add inline documentation (comments) explaining:
- Why
footerSocialLinksretains "twitter" (legacy/backwards compatibility context)- Why
socialLinksuses "xLink" (new X branding context)- Which keys are used by which components
packages/visual-editor/locales/components/cs/visual-editor.json (1)
1-42: Component translations are well-structured for runtime use.This new component-level Czech translation file is well-organized and contains an appropriate focused subset of translations for frontend components (locator, search, social links, etc.). The JSON structure is valid and the translations follow consistent Czech patterns.
Note: The duplication concern regarding
footerSocialLinksandsocialLinksalso appearing in the platform file has been flagged in the platform file review.Optional: Consider native speaker verification
While the translations appear well-formed and consistent, consider having a native Czech speaker verify the accuracy and natural phrasing of these translations, particularly for:
- Social link call-to-actions ("Sledujte nás na..." / "Přihlaste se k odběru...")
- Locator-specific phrases ("Pomocí našeho lokátoru najdete místo ve vašem okolí")
packages/visual-editor/src/components/footer/FooterSocialLinksSlot.tsx (1)
28-34: Consider extracting the socialLink type to a shared location.The
socialLinktype definition is similar to the one in Footer.tsx. If these components evolve together, extracting this type to a shared types file could improve maintainability and ensure consistency.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
packages/visual-editor/locales/components/cs/visual-editor.jsonpackages/visual-editor/locales/components/da/visual-editor.jsonpackages/visual-editor/locales/components/de/visual-editor.jsonpackages/visual-editor/locales/components/en-GB/visual-editor.jsonpackages/visual-editor/locales/components/en/visual-editor.jsonpackages/visual-editor/locales/components/es/visual-editor.jsonpackages/visual-editor/locales/components/et/visual-editor.jsonpackages/visual-editor/locales/components/fi/visual-editor.jsonpackages/visual-editor/locales/components/fr/visual-editor.jsonpackages/visual-editor/locales/components/hr/visual-editor.jsonpackages/visual-editor/locales/components/hu/visual-editor.jsonpackages/visual-editor/locales/components/it/visual-editor.jsonpackages/visual-editor/locales/components/ja/visual-editor.jsonpackages/visual-editor/locales/components/lt/visual-editor.jsonpackages/visual-editor/locales/components/lv/visual-editor.jsonpackages/visual-editor/locales/components/nb/visual-editor.jsonpackages/visual-editor/locales/components/nl/visual-editor.jsonpackages/visual-editor/locales/components/pl/visual-editor.jsonpackages/visual-editor/locales/components/pt/visual-editor.jsonpackages/visual-editor/locales/components/ro/visual-editor.jsonpackages/visual-editor/locales/components/sk/visual-editor.jsonpackages/visual-editor/locales/components/sv/visual-editor.jsonpackages/visual-editor/locales/components/tr/visual-editor.jsonpackages/visual-editor/locales/components/zh-TW/visual-editor.jsonpackages/visual-editor/locales/components/zh/visual-editor.jsonpackages/visual-editor/locales/platform/cs/visual-editor.jsonpackages/visual-editor/locales/platform/da/visual-editor.jsonpackages/visual-editor/locales/platform/de/visual-editor.jsonpackages/visual-editor/locales/platform/en-GB/visual-editor.jsonpackages/visual-editor/locales/platform/en/visual-editor.jsonpackages/visual-editor/locales/platform/es/visual-editor.jsonpackages/visual-editor/locales/platform/et/visual-editor.jsonpackages/visual-editor/locales/platform/fi/visual-editor.jsonpackages/visual-editor/locales/platform/fr/visual-editor.jsonpackages/visual-editor/locales/platform/hr/visual-editor.jsonpackages/visual-editor/locales/platform/hu/visual-editor.jsonpackages/visual-editor/locales/platform/it/visual-editor.jsonpackages/visual-editor/locales/platform/ja/visual-editor.jsonpackages/visual-editor/locales/platform/lt/visual-editor.jsonpackages/visual-editor/locales/platform/lv/visual-editor.jsonpackages/visual-editor/locales/platform/nb/visual-editor.jsonpackages/visual-editor/locales/platform/nl/visual-editor.jsonpackages/visual-editor/locales/platform/pl/visual-editor.jsonpackages/visual-editor/locales/platform/pt/visual-editor.jsonpackages/visual-editor/locales/platform/ro/visual-editor.jsonpackages/visual-editor/locales/platform/sk/visual-editor.jsonpackages/visual-editor/locales/platform/sv/visual-editor.jsonpackages/visual-editor/locales/platform/tr/visual-editor.jsonpackages/visual-editor/locales/platform/zh-TW/visual-editor.jsonpackages/visual-editor/locales/platform/zh/visual-editor.jsonpackages/visual-editor/package.jsonpackages/visual-editor/scripts/copyPlatformTranslations.tspackages/visual-editor/src/components/Locator.tsxpackages/visual-editor/src/components/footer/Footer.tsxpackages/visual-editor/src/components/footer/FooterSocialLinksSlot.tsxpackages/visual-editor/src/components/pageSections/NearbyLocations/NearbyLocationsCardsWrapper.tsxpackages/visual-editor/src/internal/puck/components/LayoutHeader.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/visual-editor/locales/components/zh-TW/visual-editor.json
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/visual-editor/locales/platform/tr/visual-editor.json
- packages/visual-editor/locales/components/ro/visual-editor.json
- packages/visual-editor/locales/components/pl/visual-editor.json
- packages/visual-editor/locales/components/pt/visual-editor.json
- packages/visual-editor/locales/components/lt/visual-editor.json
- packages/visual-editor/locales/components/sk/visual-editor.json
- packages/visual-editor/locales/components/hr/visual-editor.json
- packages/visual-editor/locales/components/da/visual-editor.json
- packages/visual-editor/locales/platform/nl/visual-editor.json
- packages/visual-editor/locales/components/en-GB/visual-editor.json
- packages/visual-editor/locales/components/ja/visual-editor.json
- packages/visual-editor/locales/components/zh/visual-editor.json
- packages/visual-editor/locales/components/sv/visual-editor.json
- packages/visual-editor/scripts/copyPlatformTranslations.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-23T16:36:42.670Z
Learnt from: benlife5
Repo: yext/visual-editor PR: 958
File: packages/visual-editor/locales/ro/visual-editor.json:394-394
Timestamp: 2025-12-23T16:36:42.670Z
Learning: In Romanian locale files (ro), "Top" is an acceptable translation for directional terms like "top_direction" rather than the native Romanian "Sus". The maintainer benlife5 confirmed this is the preferred translation in packages/visual-editor/locales/ro/visual-editor.json.
Applied to files:
packages/visual-editor/locales/platform/ro/visual-editor.jsonpackages/visual-editor/locales/platform/et/visual-editor.json
📚 Learning: 2025-12-15T20:59:25.571Z
Learnt from: benlife5
Repo: yext/visual-editor PR: 943
File: packages/visual-editor/src/components/contentBlocks/image/Image.tsx:175-191
Timestamp: 2025-12-15T20:59:25.571Z
Learning: In the ImageWrapper component at packages/visual-editor/src/components/contentBlocks/image/Image.tsx, when an Image is wrapped in a MaybeLink, the aria-label should be omitted because the image's alt text serves as the accessible name for the link. Adding an aria-label would override the image's alt text.
Applied to files:
packages/visual-editor/src/components/footer/Footer.tsxpackages/visual-editor/src/components/footer/FooterSocialLinksSlot.tsx
📚 Learning: 2025-10-29T22:00:03.843Z
Learnt from: mkouzel-yext
Repo: yext/visual-editor PR: 833
File: packages/visual-editor/src/components/Locator.tsx:1050-1057
Timestamp: 2025-10-29T22:00:03.843Z
Learning: In packages/visual-editor/src/components/Locator.tsx, the AppliedFilters component is intentionally rendered in two locations (inside the filter modal and outside it) as per the design requirements. This dual rendering should not be flagged as a duplicate issue.
Applied to files:
packages/visual-editor/src/components/Locator.tsx
🧬 Code graph analysis (1)
packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx (3)
packages/visual-editor/src/utils/i18n/components.ts (2)
loadComponentTranslations(77-94)i18nComponentsInstance(6-6)packages/visual-editor/src/utils/i18n/index.ts (3)
i18nComponentsInstance(1-1)i18nPlatformInstance(3-3)pt(6-6)packages/visual-editor/src/utils/i18n/platform.ts (3)
loadPlatformTranslations(58-73)i18nPlatformInstance(7-7)pt(100-141)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: semgrep/ci
🔇 Additional comments (53)
packages/visual-editor/locales/platform/ro/visual-editor.json (1)
494-502: LGTM! Romanian translations are accurate and consistent.The social media link translations are correctly formatted and use appropriate Romanian phrasing:
- "Urmărește-ne pe [Platform]" (Follow us on [Platform]) for most social media
- "Abonați-vă la canalul nostru YouTube" (Subscribe to our YouTube channel) for YouTube
The distinction between
footerSocialLinks.twitterandsocialLinks.xLinkappropriately handles the Twitter→X rebrand for different contexts.Also applies to: 659-667
packages/visual-editor/locales/components/it/visual-editor.json (1)
1-42: The remaining translations are accurate.The rest of the Italian translations appear grammatically correct and appropriately localized, including:
- Days of the week with proper accents (Venerdì, Martedì, Mercoledì, Giovedì, Sabato, Domenica)
- Social media links with correct phrasing ("Seguici su..." / "Iscriviti al nostro canale...")
- UI strings and search-related phrases
packages/visual-editor/src/components/pageSections/NearbyLocations/NearbyLocationsCardsWrapper.tsx (1)
91-91: LGTM! Capitalization improves consistency.The label change from "styles" to "Styles" aligns with the capitalization pattern used by all other field labels in this component (e.g., "Data", "Radius (Miles)", "Limit", "Background Color"), improving UI consistency.
packages/visual-editor/locales/platform/fi/visual-editor.json (1)
484-484: Verify the translation change is correct.The translation changed from "Suodattaa" (infinitive: "to filter") to "Suodata" (imperative: "filter"). For UI actions, the imperative form is typically more appropriate, so this change appears correct. However, since translation accuracy is critical for user experience, please verify with a Finnish native speaker or locale expert.
packages/visual-editor/package.json (2)
58-58: LGTM! Good formatting practice.The trailing comma addition is a minor formatting improvement that makes future additions cleaner and follows JSON best practices.
59-59: Script file exists and follows established patterns.The new
i18n:copy-platform-translationsscript is well-structured, with consistent naming convention and command structure matching other i18n scripts in the configuration.packages/visual-editor/locales/platform/da/visual-editor.json (1)
666-674: LGTM! Translations are accurate.The Danish translations are correct and follow a consistent pattern. The use of
xLinkwith "Følg os på X (Twitter)" appropriately handles Twitter's rebrand while maintaining user clarity.Note: This block duplicates
footerSocialLinks(lines 501-509) except for the twitter/xLink difference—see comment on that section regarding verification.packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx (2)
16-25: LGTM! Imports correctly updated for dynamic translation loading.The addition of
loadComponentTranslationsandloadPlatformTranslationsimports aligns with the PR objective to load translations on-demand rather than bundling all locales upfront.
348-354: LGTM! Correctly maintains uppercase convention for category labels.Applying
.toUpperCase()to the translated strings ensures consistency with the original uppercase format of "PAGE SECTIONS" and "OTHER" labels in the UI.packages/visual-editor/locales/platform/es/visual-editor.json (4)
493-501: LGTM! Well-structured social link translations.The Spanish translations for footer social links are grammatically correct and follow appropriate conventions. The use of "Síguenos en" (Follow us on) for most platforms and "Suscríbete a nuestro canal de YouTube" (Subscribe to our YouTube channel) for YouTube is contextually appropriate.
564-564: Good grammar correction.The change from "Abrir 24 horas" (infinitive form) to "Abierto 24 horas" (adjective form) is grammatically correct and more natural in Spanish for describing business hours.
658-666: LGTM! Social links properly reflect platform updates.The translations are correct and appropriately differentiate from
footerSocialLinksby usingxLink("Síguenos en X (Twitter)") instead of
759-759: Previously flagged issue has been resolved.The past review comment mentioned that the translation for
useOurLocatorToFindALocationNearYouwas empty. This has been addressed—the key now contains a proper Spanish translation.packages/visual-editor/locales/components/es/visual-editor.json (1)
1-38: LGTM! Translation structure and most strings are correct.The JSON structure is well-formed, and the translations are accurate and natural. The previous issue with "Abierto 24 horas" (Line 19) has been correctly addressed using the adjective form.
packages/visual-editor/locales/components/fr/visual-editor.json (1)
7-15: Social links translations are well-structured.The translations appropriately use "Suivez-nous sur..." (Follow us on...) for social platforms and "Abonnez-vous à notre chaîne YouTube" (Subscribe to our YouTube channel) for YouTube, which is more natural in French. The distinction between "twitter" and "xLink" keys properly reflects Twitter's rebranding to X.
Also applies to: 26-34
packages/visual-editor/locales/platform/fr/visual-editor.json (1)
493-501: LGTM! Social links additions maintain consistency.The new
footerSocialLinksandsocialLinksblocks use appropriate French translations that are consistent with the component-level locale file. The translations follow proper French conventions and maintain the appropriate distinction between Twitter-branded and X-branded links.Also applies to: 659-667
packages/visual-editor/locales/components/hu/visual-editor.json (2)
1-42: JSON structure looks good.The file structure is valid, well-organized with nested objects for social link translations, and follows the expected pattern for locale resources in this i18n refactoring effort.
7-15: This is an intentional pattern, not an inconsistency.The naming difference between
footerSocialLinks.twitterandsocialLinks.xLinkis consistent across all 26 locale files and reflects Twitter's rebrand to X. ThexLinktranslation clarifies the mapping (e.g., "Kövess minket az X-en (Twitter)"). Different components use different key names by design—this is not a bug and will not cause runtime issues if components are properly mapped to their respective keys.packages/visual-editor/locales/platform/zh-TW/visual-editor.json (1)
495-503: This inconsistency is intentional and by design.The
footerSocialLinksobject with the "twitter" key is used by the legacyFootercomponent (packages/visual-editor/src/components/footer/Footer.tsx), while thesocialLinksobject with the "xLink" key is used by the newerFooterSocialLinksSlotcomponent (packages/visual-editor/src/components/footer/FooterSocialLinksSlot.tsx). The migration file0042_expanded_footer_slots.tsindicates a deliberate transition to the expanded footer slots implementation. Both translations are needed to support both the legacy and modern footer components.packages/visual-editor/locales/components/fi/visual-editor.json (5)
1-6: Previous translation issues successfully resolved.The previously flagged issues for "closesAt" (Line 4: now correctly uses "Sulkeutuu kello" instead of the inappropriate "jhk" abbreviation) and "filter" (Line 6: now correctly uses the imperative "Suodata" instead of infinitive "Suodattaa") have been properly addressed. The translations in this section are now accurate and follow Finnish UI conventions.
7-15: LGTM! Social media translations are accurate.The footerSocialLinks translations correctly use "Seuraa meitä" (Follow us) with appropriate Finnish locative case endings, and YouTube appropriately uses "Tilaa" (Subscribe) instead of "Follow". All translations are natural and grammatically correct.
16-25: Weekday capitalization consistency achieved.The previously flagged capitalization inconsistency (Line 22: "Lauantai") has been corrected to "lauantai", ensuring all weekday names follow consistent lowercase formatting. All other translations in this section (directions, hours, search terms) are accurate and natural Finnish.
26-34: LGTM! socialLinks translations are consistent and up-to-date.The socialLinks structure appropriately mirrors footerSocialLinks with consistent translations. The "xLink" translation correctly acknowledges the Twitter to X platform transition with "Seuraa meitä X:ssä (Twitter)", maintaining clarity for Finnish users.
35-42: LGTM! Remaining translations are accurate.All translations in this section are grammatically correct and natural Finnish. The weekday names maintain consistent lowercase formatting, and UI labels like "temporarilyClosed" and "visitPage" are appropriately translated.
packages/visual-editor/locales/platform/hu/visual-editor.json (2)
494-502: LGTM! Footer social links translations added.The
footerSocialLinksgroup provides appropriate Hungarian translations for footer social media links. The use of "subscribe" for YouTube instead of "follow" is contextually correct.
494-502: No action needed. ThefooterSocialLinks.twitterandsocialLinks.xLinkkeys serve different components by design:
Footer.tsxusesfooterSocialLinks.twitterfor the legacy footer implementationFooterSocialLinksSlot.tsxusessocialLinks.xLinkwith updated X/Twitter brandingBoth groups are actively used in separate components, and the naming difference is intentional to track the platform's branding transition. This is not an inconsistency requiring standardization.
Likely an incorrect or invalid review comment.
packages/visual-editor/locales/components/nb/visual-editor.json (1)
1-42: All locale files maintain consistent structure.Verification confirms all 25 locale files contain the same 36 keys with no mismatches, missing keys, or extra keys. This ensures consistent UI behavior across all supported languages.
packages/visual-editor/locales/platform/de/visual-editor.json (1)
499-507: Clarify intent of key naming split: "twitter" vs "xLink" across social link objects.The German translations are accurate. However, both
footerSocialLinksandsocialLinksobjects exist across all 25 platform locales with a consistent pattern:footerSocialLinksuses the "twitter" key whilesocialLinksuses "xLink". This naming split appears intentional (not a one-off oversight), but the purpose should be clarified:
- Are these objects designed for different UI contexts where separate key names are appropriate?
- Or should the keys be unified for consistency?
packages/visual-editor/locales/platform/sk/visual-editor.json (2)
660-668: LGTM! Social links structure correctly reflects the X (Twitter) rebrand.The
socialLinksblock properly uses the "xLink" key and includes "X (Twitter)" in the translation text, which correctly reflects the platform's rebrand. The translations follow the consistent Slovak pattern "Sledujte nás na [platform]" used throughout the file.
495-503: The inconsistency between "twitter" and "xLink" keys is intentional and correct.The
footerSocialLinks.twitterkey is used by the legacyFootercomponent, whilesocialLinks.xLinkis used by the newerFooterSocialLinksSlotcomponent. Both components are active in the codebase with different data structures and translation requirements. The locale file correctly reflects this:Footer.tsxreferencesfooterSocialLinks.twitter(using theFaTwittericon), whileFooterSocialLinksSlot.tsxreferencessocialLinks.xLinkwith theFaXTwittericon. The Slovak translations are accurate for their respective components and require no changes.packages/visual-editor/locales/platform/cs/visual-editor.json (1)
501-509: Address systematic duplication of social media translation keys across all 26 languages.Both
footerSocialLinksandsocialLinksobjects are present in both platform and component files for all 26 language variants. This systematic duplication contradicts the PR's goal to "only load necessary translations for the active locale."Please clarify:
- Should these social link translations exist only in
platform/, only incomponents/, or in both?- If both locations are needed, does the loading mechanism prevent redundant bundling?
Approve once the separation strategy is clarified.
Also applies to: 666-674
packages/visual-editor/locales/platform/pl/visual-editor.json (2)
495-503: Translations look correct; duplication appears intentional for different contexts.The Polish translations for social links are accurate and properly formatted:
- "Śledź nas na [platform]" (Follow us on [platform]) for most platforms
- "Subskrybuj nasz kanał YouTube" (Subscribe to our YouTube channel) for YouTube
The duplication between
footerSocialLinksandsocialLinksappears intentional to support different UI contexts (footer-specific vs. general social links), which is a common i18n pattern that allows for context-specific customization.Also applies to: 660-668
495-503: No changes needed. The "twitter" and "xLink" keys serve different components:Footer.tsxusesfooterSocialLinks.twitterfor the legacy footer, whileFooterSocialLinksSlot.tsxusessocialLinks.xLinkfor the newer slot component. This is an intentional separation with properly localized translations for each context.Likely an incorrect or invalid review comment.
packages/visual-editor/locales/platform/lt/visual-editor.json (2)
494-502: LGTM! Social link translations added.The
footerSocialLinksblock provides Lithuanian translations for social platforms. All translations follow a consistent pattern.Note: This block uses the
socialLinksblock below usesxLink. This appears to be an intentional distinction based on the PR-wide pattern.
659-667: LGTM! Social links group added with X branding.The
socialLinksblock mirrorsfooterSocialLinksbut usesxLink(with explicit "X (Twitter)" branding) instead ofpackages/visual-editor/locales/components/et/visual-editor.json (1)
1-42: LGTM! Estonian component translations added.The Estonian component locale file mirrors the structure of the English component locale with proper Estonian translations. All keys are consistently translated.
Note: This file may have the same completeness concerns as the English component locale regarding potentially missing component-level translation keys.
packages/visual-editor/locales/platform/pt/visual-editor.json (1)
494-502: LGTM! Portuguese social link translations added.Both
footerSocialLinksandsocialLinksblocks provide proper Portuguese translations. The translations follow natural Portuguese phrasing with "Siga-nos" (follow us) for most platforms and "Inscreva-se" (subscribe) for YouTube.The expected
xLinkkey distinction is properly maintained between the two blocks.Also applies to: 659-667
packages/visual-editor/locales/components/tr/visual-editor.json (1)
7-15: LGTM! Turkish social link translations added.The
footerSocialLinksandsocialLinksblocks provide proper Turkish translations following the established pattern. Translations use "Bizi [Platform]'ta/de takip edin" (Follow us on [Platform]) consistently.Also applies to: 26-34
packages/visual-editor/locales/platform/sv/visual-editor.json (1)
495-503: LGTM! Swedish social link translations added.Both
footerSocialLinksandsocialLinksblocks provide proper Swedish translations. The translations use "Följ oss på [Platform]" (Follow us on [Platform]) for most entries and "Prenumerera på vår YouTube-kanal" (Subscribe to our YouTube channel) for YouTube.The expected
xLinkkey distinction is maintained.Also applies to: 660-668
packages/visual-editor/locales/platform/it/visual-editor.json (1)
494-502: LGTM! Italian social link translations added.Both
footerSocialLinksandsocialLinksblocks provide proper Italian translations. The translations use "Seguici su [Platform]" (Follow us on [Platform]) for most entries and "Iscriviti al nostro canale YouTube" (Subscribe to our YouTube channel) for YouTube.The expected
xLinkkey distinction is maintained.Also applies to: 659-667
packages/visual-editor/locales/platform/et/visual-editor.json (1)
494-502: LGTM! Estonian platform social link translations added.Both
footerSocialLinksandsocialLinksblocks provide proper Estonian translations with correct case endings. The translations use "Jälgi meid [Platform]is" (Follow us on [Platform]) with appropriate Estonian inessive case endings (-is) and "Tellige meie YouTube'i kanal" (Subscribe to our YouTube channel) for YouTube.The expected
xLinkkey distinction is maintained.Also applies to: 659-667
packages/visual-editor/src/components/footer/FooterSocialLinksSlot.tsx (2)
42-92: LGTM! Translation keys are well-structured.The ariaLabel values are properly populated using translation keys that match the locale files. The consistent pattern (
socialLinks.{platform}) makes the translations easy to maintain across locales.
116-116: Improved accessibility pattern.Using the pre-computed
ariaLabelfrom the link object is cleaner than constructing it at render time and provides better screen reader support for social media links.packages/visual-editor/locales/components/lv/visual-editor.json (1)
1-42: LGTM! Well-structured locale file.The Latvian translations follow the established pattern with both
footerSocialLinksandsocialLinksgroups. The distinction betweenxLink(in socialLinks) appears intentional to support both legacy and updated Twitter/X branding.packages/visual-editor/src/components/footer/Footer.tsx (3)
27-33: Public type extended with ariaLabel field.The addition of
ariaLabel: stringto thesocialLinktype is a public API change. SincesocialLinkobjects are constructed internally and not passed via props, the impact is minimal, but consumers importing this type should be aware of the change.
97-147: Translation integration looks good.The translation keys follow the
footerSocialLinks.*pattern and match the locale files. All translations have appropriate fallback strings, and the hook is properly initialized.
223-223: Improved accessibility pattern.Delegating ariaLabel generation to the socialLink construction (where translations are loaded) is cleaner than handling it within the FooterSocialIcons component.
packages/visual-editor/locales/platform/hr/visual-editor.json (1)
494-502: LGTM! Consistent locale additions.The
footerSocialLinksandsocialLinkstranslation groups are properly structured and follow the established pattern across locales.Also applies to: 659-667
packages/visual-editor/locales/platform/en-GB/visual-editor.json (1)
493-501: LGTM! Properly structured locale additions.The translation groups are correctly added with appropriate British English localization.
Also applies to: 658-666
packages/visual-editor/locales/platform/lv/visual-editor.json (1)
494-502: LGTM! Consistent platform-level translations.The Latvian platform translations for social links are properly structured and align with the component-level translations.
Also applies to: 659-667
packages/visual-editor/locales/platform/en/visual-editor.json (3)
26-26: Minor capitalization improvements.The updates to
applyAll(sentence case) andlink(capitalization) improve consistency in the UI text styling.Also applies to: 517-517
493-501: LGTM! Translation groups properly added.The
footerSocialLinksandsocialLinksgroups are correctly structured and provide the translations used by Footer.tsx and FooterSocialLinksSlot.tsx.Also applies to: 649-657
532-537: Removal of English plural variants is safe and correct.No code explicitly references the removed
_few,_many, or_zerovariants. These variants are appropriately removed from the English locale since English pluralization only requires_oneand_otherforms. i18next's default behavior will fall back to_otherif any of these forms are requested, so there is no functional risk. Other language files (non-English) correctly retain their plural variants where needed.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (8)
packages/visual-editor/locales/components/nl/visual-editor.json (1)
43-43: Invisible character still present in translation.The zero-width space between "een" and "locatie" flagged in the previous review has not been addressed. This invisible character can cause display or text-processing issues.
🔎 Proposed fix
- "useOurLocatorToFindALocationNearYou": "Gebruik onze locator om een locatie bij u in de buurt te vinden", + "useOurLocatorToFindALocationNearYou": "Gebruik onze locator om een locatie bij u in de buurt te vinden",packages/visual-editor/locales/components/pl/visual-editor.json (1)
24-24: Use declarative form for status indicator, not imperative.The translation "Otwórz teraz" is in the imperative mood (command), but "openNow" is a status label indicating a location is currently open, not an action button. This should use the adjective/declarative form like other status indicators in this file ("Zamknięte" on line 3, "Otwarte 24 godziny" on line 23).
🔎 Suggested fix
- "openNow": "Otwórz teraz", + "openNow": "Otwarte teraz",Alternative:
"Obecnie otwarte"if you prefer emphasizing "currently."packages/visual-editor/locales/components/de/visual-editor.json (1)
4-4: Incorrect preposition for time expression (previously flagged).This issue was already identified in a previous review: "Schließt bei" should be "Schließt um" for closing times in German.
packages/visual-editor/locales/components/tr/visual-editor.json (1)
23-24: Status labels still use imperative verbs instead of adjectives.Despite being marked as addressed in a previous review, these lines still contain imperative verb forms ("açın" = "open [command]", "aç" = "open [command]") rather than the adjective form needed for status indicators.
Status labels should describe state, not give commands.
🔎 Proposed fix
- "open24Hours": "24 saat açın", - "openNow": "Şimdi aç", + "open24Hours": "24 saat açık", + "openNow": "Şu anda açık",packages/visual-editor/locales/components/lt/visual-editor.json (1)
23-24: Replace imperative verb forms with descriptive status indicators.Lines 23-24 use the imperative verb "Atidarykite" (command form "Open!"), which is grammatically incorrect for status labels. These should use descriptive forms instead:
- Line 23 "open24Hours": Use "Atvira visą parą" or "Atidaryta 24 valandas"
- Line 24 "openNow": Use "Atvira dabar" or "Atidaryta dabar"
Status indicators should describe the current state, not issue commands to the user.
🔎 Proposed fix
- "open24Hours": "Atidarykite 24 valandas", - "openNow": "Atidarykite dabar", + "open24Hours": "Atvira visą parą", + "openNow": "Atvira dabar",packages/visual-editor/locales/components/ja/visual-editor.json (3)
3-3: Incorrect tense for status indicator (previously flagged).This translation still uses "閉じた" (past tense "was closed"). For a status indicator, use "閉店" or "休業中" to indicate current state.
🔎 Proposed fix
- "closed": "閉じた", + "closed": "閉店",
24-24: Use status description, not imperative command (previously flagged).This translation still uses "今開いてください" which means "please open now" (a command). For a status indicator, use "営業中" to show the business is currently open.
🔎 Proposed fix
- "openNow": "今開いてください", + "openNow": "営業中",
40-40: Use present continuous tense for temporary status (previously flagged and reportedly fixed).This translation still uses "一時的に閉じた" (past tense). The previous review comment indicated this was addressed, but the file still contains the incorrect translation. For an active temporary closure status, use "一時休業中".
🔎 Proposed fix
- "temporarilyClosed": "一時的に閉じた", + "temporarilyClosed": "一時休業中",Please verify whether the previous fix was properly merged, as the past review indicated this was addressed in commits 8d394ba to 4fd1075.
🧹 Nitpick comments (6)
packages/visual-editor/locales/components/lt/visual-editor.json (1)
18-19: Use consistent prepositions for singular and plural forms.Lines 18 and 19 are singular/plural variants of the same phrase but use different prepositions:
- Line 18: "prie" (near/by)
- Line 19: "šalia" (near/beside)
For consistency, use the same preposition in both forms. Recommend using "šalia" in both as it's more commonly used in location contexts.
🔎 Proposed fix
- "locationsNear_one": "{{count}} vieta prie „{{name}}"", + "locationsNear_one": "{{count}} vieta šalia „{{name}}"", "locationsNear_other": "{{count}} vietos šalia „{{name}}"",packages/visual-editor/locales/components/ja/visual-editor.json (5)
7-15: Inconsistent spacing in social media translations.Lines 9 and 12 lack a space after the platform name, while other entries have consistent spacing (e.g., "Facebook で", "LinkedIn で"). Consider standardizing spacing for visual consistency.
🔎 Proposed fix
- "instagram": "Instagramでフォローしてください", + "instagram": "Instagram でフォローしてください",- "tiktok": "TikTokでフォローしてください", + "tiktok": "TikTok でフォローしてください",
20-20: Inconsistent spacing in variable interpolation.The
{{ distance}}has a space before the closing braces but not after the opening braces. Standardize to either{{distance}}or{{ distance }}.🔎 Proposed fix
- "locationsWithinDistanceOf_one": "「{{name}}」から {{ distance}} マイル以内の {{count}} 件の場所", + "locationsWithinDistanceOf_one": "「{{name}}」から {{distance}} マイル以内の {{count}} 件の場所",
23-23: Use half-width numbers for numeric values.The translation uses full-width numbers "２４" instead of the more standard half-width "24" for numeric values in Japanese text.
🔎 Proposed fix
- "open24Hours": "２４時間営業", + "open24Hours": "24時間営業",
29-29: Simplify verb form for button label.The translation "このエリアを検索します" uses a statement form ("will search"). For a button label, the more concise "このエリアを検索" is more natural and appropriate.
🔎 Proposed fix
- "searchThisArea": "このエリアを検索します", + "searchThisArea": "このエリアを検索",
30-38: Inconsistent spacing in social media translations.Similar to the footerSocialLinks section, lines 32 and 35 lack spacing after the platform name. Consider standardizing for consistency.
🔎 Proposed fix
- "instagram": "Instagramでフォローしてください", + "instagram": "Instagram でフォローしてください",- "tiktok": "TikTokでフォローしてください", + "tiktok": "TikTok でフォローしてください",
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
packages/visual-editor/locales/components/cs/visual-editor.jsonpackages/visual-editor/locales/components/da/visual-editor.jsonpackages/visual-editor/locales/components/de/visual-editor.jsonpackages/visual-editor/locales/components/en-GB/visual-editor.jsonpackages/visual-editor/locales/components/en/visual-editor.jsonpackages/visual-editor/locales/components/es/visual-editor.jsonpackages/visual-editor/locales/components/et/visual-editor.jsonpackages/visual-editor/locales/components/fi/visual-editor.jsonpackages/visual-editor/locales/components/fr/visual-editor.jsonpackages/visual-editor/locales/components/hr/visual-editor.jsonpackages/visual-editor/locales/components/hu/visual-editor.jsonpackages/visual-editor/locales/components/it/visual-editor.jsonpackages/visual-editor/locales/components/ja/visual-editor.jsonpackages/visual-editor/locales/components/lt/visual-editor.jsonpackages/visual-editor/locales/components/lv/visual-editor.jsonpackages/visual-editor/locales/components/nb/visual-editor.jsonpackages/visual-editor/locales/components/nl/visual-editor.jsonpackages/visual-editor/locales/components/pl/visual-editor.jsonpackages/visual-editor/locales/components/pt/visual-editor.jsonpackages/visual-editor/locales/components/ro/visual-editor.jsonpackages/visual-editor/locales/components/sk/visual-editor.jsonpackages/visual-editor/locales/components/sv/visual-editor.jsonpackages/visual-editor/locales/components/tr/visual-editor.jsonpackages/visual-editor/locales/components/zh-TW/visual-editor.jsonpackages/visual-editor/locales/components/zh/visual-editor.json
✅ Files skipped from review due to trivial changes (1)
- packages/visual-editor/locales/components/pt/visual-editor.json
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/visual-editor/locales/components/ro/visual-editor.json
- packages/visual-editor/locales/components/sk/visual-editor.json
- packages/visual-editor/locales/components/es/visual-editor.json
- packages/visual-editor/locales/components/hr/visual-editor.json
- packages/visual-editor/locales/components/zh/visual-editor.json
- packages/visual-editor/locales/components/da/visual-editor.json
- packages/visual-editor/locales/components/fi/visual-editor.json
- packages/visual-editor/locales/components/en/visual-editor.json
- packages/visual-editor/locales/components/lv/visual-editor.json
- packages/visual-editor/locales/components/cs/visual-editor.json
- packages/visual-editor/locales/components/fr/visual-editor.json
- packages/visual-editor/locales/components/sv/visual-editor.json
- packages/visual-editor/locales/components/nb/visual-editor.json
- packages/visual-editor/locales/components/et/visual-editor.json
- packages/visual-editor/locales/components/zh-TW/visual-editor.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-23T16:36:42.670Z
Learnt from: benlife5
Repo: yext/visual-editor PR: 958
File: packages/visual-editor/locales/ro/visual-editor.json:394-394
Timestamp: 2025-12-23T16:36:42.670Z
Learning: In Romanian locale files (ro), "Top" is an acceptable translation for directional terms like "top_direction" rather than the native Romanian "Sus". The maintainer benlife5 confirmed this is the preferred translation in packages/visual-editor/locales/ro/visual-editor.json.
Applied to files:
packages/visual-editor/locales/components/lt/visual-editor.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: call_unit_test / unit_tests (24.x)
- GitHub Check: call_unit_test / unit_tests (22.x)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: create-dev-release
- GitHub Check: semgrep/ci
🔇 Additional comments (3)
packages/visual-editor/locales/components/en-GB/visual-editor.json (1)
7-15: No action required—the social link key inconsistency is intentional.The difference between
footerSocialLinks.twitterandsocialLinks.xLinkis consistent across all locale files (en, en-GB, de, etc.), indicating this is a deliberate design pattern. The two groups serve distinct UI contexts:footerSocialLinksmaintains the "twitter" key whilesocialLinksuses the newer "xLink" key with X (Twitter) branding. This is not a maintenance concern or integration issue—the keys are accessed dynamically through i18next at runtime.packages/visual-editor/locales/components/hu/visual-editor.json (2)
18-18: Verify article placement in Hungarian.The structure
"{{count}} A hely közelében"places the definite article "A" after the count. Please verify with a native Hungarian speaker that this word order is grammatically correct for the singular form, as Hungarian article usage can be nuanced.
20-21: The use of miles ("mérföldön") is consistent across all locale files in the application, not a Hungarian-specific issue. Every locale—regardless of whether the country uses the metric system—standardizes on miles for distance measurements. This appears to be an intentional design decision by the application.Likely an incorrect or invalid review comment.
Before this change all translations were bundled and ultimately loaded on the frontend even though only a single locale is ever needed at a time.