-
Notifications
You must be signed in to change notification settings - Fork 0
feat: added open in new tab and center aligned the secondary footer #967
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis PR adds internationalization and functionality for opening links in new tabs across the visual editor's footer and header components. It updates 23 locale JSON files with new translation keys for an external link ARIA label, link positioning, and a new tab toggle. Component changes include adding an Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (256)
📒 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). (4)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (5)
packages/visual-editor/locales/zh/visual-editor.json (1)
300-300: Consider using consistent terminology for browser tabs.The translation uses "选项卡" (option card/tab), while Line 33 uses "标签页" (tab page) for the same concept of browser tabs. In standard Chinese localization, "标签页" is more commonly used for browser tabs and would provide better consistency within this locale file.
🔎 Suggested fix for terminology consistency
- "openInNewTab": "在新选项卡中打开", + "openInNewTab": "在新标签页中打开",packages/visual-editor/locales/nb/visual-editor.json (1)
279-279: New field translation added successfully.The
linksPositionfield has been added to support the property rename fromlinksAlignment. Note that both keys now coexist in the file (line 278:linksAlignment, line 279:linksPosition), which appears intentional for backward compatibility during the migration phase.Optional: Consider using consistent terminology for "links" — line 278 uses "Koblinger" while line 279 uses "Linker". While both are valid Norwegian words, using the same term throughout might improve consistency. However, this may be following an established style guide.
packages/visual-editor/src/components/footer/CopyrightMessageSlot.tsx (1)
34-46: Consider simplifying the alignment logic.The switch statement with array initialization and push operations works correctly but could be more concise.
🔎 Proposed refactor using conditional logic
- let alignmentStyle = ["text-center"]; - switch (alignment) { - case "left": { - alignmentStyle.push("md:text-left"); - break; - } - case "right": { - alignmentStyle.push("md:text-right"); - break; - } - default: - break; - } + const alignmentStyle = [ + "text-center", + ...(alignment === "left" ? ["md:text-left"] : + alignment === "right" ? ["md:text-right"] : []) + ];Or using template literals directly:
- let alignmentStyle = ["text-center"]; - switch (alignment) { - case "left": { - alignmentStyle.push("md:text-left"); - break; - } - case "right": { - alignmentStyle.push("md:text-right"); - break; - } - default: - break; - } - return resolvedText ? ( - <Body variant="xs" className={alignmentStyle.join(" ")}> + <Body variant="xs" className={`text-center ${alignment === "left" ? "md:text-left" : alignment === "right" ? "md:text-right" : ""}`}> {resolvedText} </Body>packages/visual-editor/src/components/footer/FooterLinksSlot.tsx (1)
139-148: Add a default value for the openInNewTab field.The field definition for
openInNewTablacks a default value. WhiledefaultLink(imported from ExpandedFooter.tsx) setsopenInNewTab: false, explicitly specifying a default in the field definition ensures consistency and prevents undefined values if the field is added independently.🔎 Proposed fix
openInNewTab: YextField( msg("fields.openInNewTab", "Open in new tab"), { type: "radio", options: [ { label: msg("fields.options.yes", "Yes"), value: true }, { label: msg("fields.options.no", "No"), value: false }, ], + defaultValue: false, } ),packages/visual-editor/src/components/footer/ExpandedFooter.tsx (1)
60-60: Consider adding "center" alignment option for consistency.
ExpandedFooterStyles.primaryFooter.linksPositiononly supports"left" | "right", whileSecondaryFooterSlot.styles.linksPosition(Line 47 in SecondaryFooterSlot.tsx) supports"left" | "center" | "right". This inconsistency means users can center-align secondary footer links but not primary footer links, which may be unexpected.If center alignment is intentionally excluded from the primary footer due to design constraints, this is fine. Otherwise, consider adding center alignment support for consistency.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (70)
packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[desktop] default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[desktop] version 10 props - expanded.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[desktop] version 10 props - no data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[desktop] version 10 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[desktop] version 19 props - expanded.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[desktop] version 20 props - expanded.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[desktop] version 42 props - basic.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[desktop] version 42 props - expanded with all data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[desktop] version 42 props - no data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[desktop] version 48 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[desktop] version 50 - with external links, centered secondary links - Dark BG.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[desktop] version 50 - with external links, centered secondary links.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] version 10 props - expanded.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] version 10 props - no data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] version 10 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] version 19 props - expanded.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] version 20 props - expanded.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] version 42 props - basic.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] version 42 props - expanded with all data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] version 42 props - no data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] version 48 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] version 50 - with external links, centered secondary links - Dark BG.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] version 50 - with external links, centered secondary links.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[tablet] default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[tablet] version 10 props - expanded.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[tablet] version 10 props - no data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[tablet] version 10 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[tablet] version 19 props - expanded.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[tablet] version 20 props - expanded.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[tablet] version 42 props - basic.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[tablet] version 42 props - expanded with all data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[tablet] version 42 props - no data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[tablet] version 48 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[tablet] version 50 - with external links, centered secondary links - Dark BG.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedFooter/[tablet] version 50 - with external links, centered secondary links.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 10 props - secondary header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 10 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 11 props - secondary header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 15 props - fixed header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 15 props - sticky header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 20 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 41 props - no secondary header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 41 props - with secondary header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 48 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 50 props - with external links.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] default props (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 10 props (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 10 props - secondary header (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 11 props - secondary header (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 15 props - fixed header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 15 props - sticky header (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 20 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 50 props - with external links (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 50 props - with external links.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 10 props - secondary header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 11 props - secondary header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 15 props - fixed header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 15 props - sticky header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 20 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 41 props - no secondary header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 41 props - with secondary header.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 48 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 50 props - with external links.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Header/[desktop] default props with document data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Header/[desktop] version 2 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Header/[mobile] default props with document data (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Header/[mobile] version 2 props (after interactions).pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (40)
packages/visual-editor/locales/cs/visual-editor.jsonpackages/visual-editor/locales/da/visual-editor.jsonpackages/visual-editor/locales/de/visual-editor.jsonpackages/visual-editor/locales/en-GB/visual-editor.jsonpackages/visual-editor/locales/en/visual-editor.jsonpackages/visual-editor/locales/es/visual-editor.jsonpackages/visual-editor/locales/et/visual-editor.jsonpackages/visual-editor/locales/fi/visual-editor.jsonpackages/visual-editor/locales/fr/visual-editor.jsonpackages/visual-editor/locales/hr/visual-editor.jsonpackages/visual-editor/locales/hu/visual-editor.jsonpackages/visual-editor/locales/it/visual-editor.jsonpackages/visual-editor/locales/ja/visual-editor.jsonpackages/visual-editor/locales/lt/visual-editor.jsonpackages/visual-editor/locales/lv/visual-editor.jsonpackages/visual-editor/locales/nb/visual-editor.jsonpackages/visual-editor/locales/nl/visual-editor.jsonpackages/visual-editor/locales/pl/visual-editor.jsonpackages/visual-editor/locales/pt/visual-editor.jsonpackages/visual-editor/locales/ro/visual-editor.jsonpackages/visual-editor/locales/sk/visual-editor.jsonpackages/visual-editor/locales/sv/visual-editor.jsonpackages/visual-editor/locales/tr/visual-editor.jsonpackages/visual-editor/locales/zh-TW/visual-editor.jsonpackages/visual-editor/locales/zh/visual-editor.jsonpackages/visual-editor/src/components/atoms/cta.tsxpackages/visual-editor/src/components/footer/CopyrightMessageSlot.tsxpackages/visual-editor/src/components/footer/ExpandedFooter.test.tsxpackages/visual-editor/src/components/footer/ExpandedFooter.tsxpackages/visual-editor/src/components/footer/FooterExpandedLinkSectionSlot.tsxpackages/visual-editor/src/components/footer/FooterExpandedLinksWrapper.tsxpackages/visual-editor/src/components/footer/FooterLinksSlot.tsxpackages/visual-editor/src/components/footer/SecondaryFooterSlot.tsxpackages/visual-editor/src/components/header/ExpandedHeader.test.tsxpackages/visual-editor/src/components/header/HeaderLinks.tsxpackages/visual-editor/src/components/migrations/0050_update_link_align_prop.tspackages/visual-editor/src/components/migrations/migrationRegistry.tspackages/visual-editor/src/docs/ai/components.d.tspackages/visual-editor/src/docs/components.mdpackages/visual-editor/src/types/types.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-12T20:17:37.033Z
Learnt from: benlife5
Repo: yext/visual-editor PR: 946
File: packages/visual-editor/src/components/migrations/migrationRegistry.ts:47-48
Timestamp: 2025-12-12T20:17:37.033Z
Learning: In the yext/visual-editor repository, empty object placeholders (`{}`) in the migrationRegistry array are sometimes intentionally used to reserve migration slots for parallel PRs to prevent merge conflicts and maintain consistent migration indices.
Applied to files:
packages/visual-editor/src/components/migrations/0050_update_link_align_prop.tspackages/visual-editor/src/components/migrations/migrationRegistry.ts
📚 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/ro/visual-editor.jsonpackages/visual-editor/locales/it/visual-editor.jsonpackages/visual-editor/locales/fi/visual-editor.jsonpackages/visual-editor/locales/en/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/atoms/cta.tsx
🧬 Code graph analysis (2)
packages/visual-editor/src/components/migrations/migrationRegistry.ts (1)
packages/visual-editor/src/components/migrations/0050_update_link_align_prop.ts (1)
updateLinksAlignmentMigration(3-46)
packages/visual-editor/src/components/footer/ExpandedFooter.tsx (2)
packages/visual-editor/src/editor/YextField.tsx (1)
YextField(188-327)packages/visual-editor/src/editor/index.ts (1)
YextField(15-15)
⏰ 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 (69)
packages/visual-editor/locales/it/visual-editor.json (3)
32-34: Verify aria object structure is consistent across all locale files.The new
ariaobject with nestedopensInNewTabuses a different structure than existingariaLabel*keys (which are flat). Confirm this restructuring is intentional and applied uniformly across the 20+ locale files mentioned in the PR summary to avoid inconsistency.
278-279: Clarify the coexistence oflinksAlignmentandlinksPosition.Both
linksAlignment(line 278) and the newlinksPosition(line 279) are present in the locale file. The PR summary describes this as a "rename" operation. Confirm whetherlinksAlignmentshould be deprecated or if both keys are intentionally retained for backward compatibility during migration.
300-300: Translation quality is solid.The Italian translation "Apri in una nuova scheda" (Open in a new tab) is correct and well-placed within the
fieldsobject. It complements the newaria.opensInNewTablabel appropriately.packages/visual-editor/locales/sk/visual-editor.json (3)
32-34: LGTM! Verify Slovak translation accuracy.The aria label template structure is correct and follows the expected i18n pattern with the
{{label}}variable properly positioned. The implementation will improve accessibility by announcing that links open in a new tab.Please verify the Slovak translation with a native speaker to ensure "{{label}} (otvorí sa na novej karte)" accurately conveys "(opens in new tab)".
280-280: LGTM! Confirm both keys are intentionally preserved for migration.The new
linksPositionkey has been added whilelinksAlignment(line 279) remains in the file. This is likely intentional to support the migration strategy mentioned in the PR summary.Please confirm that retaining both
linksAlignmentandlinksPositionis the intended migration strategy, allowing components to gradually transition from the old to the new property name.
301-301: LGTM! Verify Slovak translation accuracy.The
openInNewTabfield label has been correctly added and aligns with the PR objective to enable links to open in a new tab.Please verify with a native Slovak speaker that "Otvoriť na novej karte" accurately translates to "Open in new tab".
packages/visual-editor/locales/et/visual-editor.json (3)
32-34: LGTM! Structure and placement are correct.The new
aria.opensInNewTabkey is properly placed at the top level and uses correct placeholder syntax for accessibility labels. The JSON structure is valid.Since I cannot verify Estonian language accuracy, please confirm the translation with a native speaker or through your localization review process. The translation should convey: "{{label}} (opens in a new tab)".
279-279: LGTM! Field label properly added.The
linksPositionfield label is correctly placed within thefieldsobject and follows the established pattern. The coexistence withlinksAlignment(Line 278) aligns with the migration strategy described in the PR summary.Please verify the Estonian translation "Linkide asukoht" accurately conveys "Links position" or "Links location" through your localization review process.
300-300: LGTM! Field label correctly added.The
openInNewTabfield label is properly placed within thefieldsobject with valid JSON structure, supporting the new functionality for opening links in new tabs.Please verify through your localization process that the Estonian translation "Ava uuel vahelehel" correctly means "Open in new tab".
packages/visual-editor/locales/zh/visual-editor.json (2)
32-34: LGTM!The new
ariaobject with theopensInNewTabtranslation is correctly structured and uses appropriate Chinese terminology for browser tabs.
279-279: LGTM!The
linksPositionfield translation is correct and appropriately placed alongside the existinglinksAlignmentfield.packages/visual-editor/locales/tr/visual-editor.json (3)
32-34: LGTM! Accessibility enhancement added correctly.The new
aria.opensInNewTabtranslation key correctly uses interpolation ({{label}}) to create dynamic accessible labels for links that open in new tabs, improving screen reader experience.
279-279: LGTM! New positioning key added.The
linksPositiontranslation has been added to support the expanded alignment options (left | center | right) mentioned in the PR objectives. Note thatlinksAlignment(line 278) still exists, which is expected during the migration period for backward compatibility.
300-300: LGTM! Form field label added.The
openInNewTabtranslation has been added to support the new boolean prop on TranslatableCTA components, allowing users to configure links to open in new tabs.packages/visual-editor/locales/es/visual-editor.json (3)
32-34: LGTM! Aria label for new tab accessibility.The Spanish translation "{{label}} (se abre en una nueva pestaña)" correctly implements the accessibility pattern for links opening in new tabs. The template variable interpolation follows the expected i18next format.
278-278: LGTM! New linksPosition translation added.The translation "Posición de los enlaces" is appropriate. Note that both
linksAlignment(line 277) andlinksPositionare present, which aligns with the migration strategy mentioned in the PR summary for renaming this property.
299-299: LGTM! Translation for openInNewTab option.The translation "Abrir en nueva pestaña" is correct and natural Spanish for the new tab functionality being added to CTA components.
packages/visual-editor/locales/ro/visual-editor.json (3)
32-34: LGTM! New accessibility label added correctly.The new
aria.opensInNewTabtranslation is well-structured and uses appropriate Romanian grammar. The reflexive form "se deschide" correctly describes the automatic action, and the interpolation placeholder is properly positioned.
279-279: LGTM! Field label properly positioned and translated.The
linksPositiontranslation is placed logically next to the relatedlinksAlignmentfield (line 278) and follows the consistent noun phrase pattern used for similar UI field labels throughout the file.
300-300: LGTM! Boolean field label follows established patterns.The
openInNewTabtranslation uses the imperative form "Deschide într-o filă nouă", which is consistent with other boolean action fields in this locale file (e.g.,includeHyperlinkat line 258). The translation accurately conveys the field's purpose.packages/visual-editor/locales/hu/visual-editor.json (3)
32-34: LGTM! Accessibility enhancement properly structured.The new
aria.opensInNewTabkey follows the correct i18n interpolation pattern and is positioned correctly in the JSON structure.Optionally verify the Hungarian translation accuracy with a native speaker or translation tool to ensure "új lapon nyílik meg" is the most natural phrasing.
279-279: LGTM! New translation key added for position configuration.The
linksPositionfield is correctly placed and structured. Note that bothlinksAlignment(line 278) andlinksPositioncoexist, which aligns with the migration strategy mentioned in the PR context.
300-300: LGTM! Translation key for new tab functionality added.The
openInNewTabfield is correctly structured and positioned alphabetically within the fields object.packages/visual-editor/locales/lt/visual-editor.json (3)
32-34: LGTM! Well-structured accessibility label.The
aria.opensInNewTabtranslation correctly uses interpolation syntax{{label}}for dynamic content, which will properly support the accessibility enhancement for links opening in new tabs.
279-279: LGTM! New translation key added correctly.The
linksPositionfield translation is properly placed in thefieldssection. Note thatlinksAlignment(line 278) still exists alongside this new key, which is expected during the migration period mentioned in the PR objectives.
300-300: LGTM! Translation key added correctly.The
openInNewTabfield translation is properly placed within thefieldssection and follows the consistent format of other field labels in the file.packages/visual-editor/locales/nb/visual-editor.json (2)
32-34: LGTM! Accessibility enhancement properly implemented.The new ARIA label template correctly uses the
{{label}}placeholder and provides screen reader users with information that the link opens in a new tab.
300-300: LGTM! Field translation is correct.The translation "Åpne i ny fane" accurately represents "Open in new tab" in Norwegian and will be used as the label for the new feature.
packages/visual-editor/src/components/footer/CopyrightMessageSlot.tsx (2)
18-18: LGTM! Center alignment option added.The addition of "center" to the alignment union type aligns with the PR objectives to support center-aligned secondary footer sections.
34-46: Verify mobile-first centering is intentional.The new alignment logic centers text on mobile screens for all alignment values, with left/right alignment only applying to
md:breakpoint and above. This changes the behavior for existing sites:
alignment="left": Previously left-aligned on all screens → Now centered on mobile, left on desktop+alignment="right": Previously right-aligned on all screens → Now centered on mobile, right on desktop+The AI summary states "default alignment behavior remains," but this mobile-first approach represents a behavioral change. Please confirm this is intentional and not a breaking change for existing component instances.
packages/visual-editor/locales/pt/visual-editor.json (2)
32-34: LGTM! Accessibility enhancement added correctly.The new ARIA label translation properly supports screen reader announcements for links that open in new tabs.
279-279: LGTM! New field translation added.The translation "Posição dos links" (Position of links) is clear and appropriate for the UI field label.
packages/visual-editor/locales/en-GB/visual-editor.json (3)
32-34: Excellent accessibility enhancement!The ARIA label follows WCAG best practices by informing assistive technology users that a link will open in a new tab. The interpolation pattern with
{{label}}allows for flexible, context-aware announcements.
299-299: LGTM!The field label is clear, concise, and consistent with the existing naming conventions in the locale file. It appropriately describes the boolean control for opening links in a new tab.
277-278: Verify the migration strategy for the linksAlignment → linksPosition transition.Both
linksAlignment(line 277) andlinksPosition(line 278) coexist in this locale file. Confirm that:
- The migration properly updates all existing component instances using
linksAlignment- A clear deprecation timeline exists for
linksAlignment- The old key will be removed from all locale files once migration is complete
packages/visual-editor/locales/nl/visual-editor.json (3)
32-34: LGTM! Good accessibility enhancement.The new ARIA translation key provides screen reader users with context when links open in new tabs, following accessibility best practices.
300-300: LGTM!The
openInNewTabfield translation is correctly added and aligns with the PR's objective to support opening links in new tabs.
279-279: BothlinksAlignmentandlinksPositionshould coexist in the locale files.The presence of both keys is intentional. Migration
0050_update_link_align_prop.tsexplicitly maps the legacylinksAlignmentproperty to the newlinksPositionproperty, using it as a fallback:linksPosition: primaryFooter.linksPosition ?? linksAlignment ?? "right". The newlinksPositionproperty is already active in footer components (ExpandedFooter.tsx,SecondaryFooterSlot.tsx), and both keys are consistently maintained across all 26 locale files as part of the migration strategy. No action needed.packages/visual-editor/locales/ja/visual-editor.json (3)
300-300: Verify Japanese translation for the UI label.The placement and structure are correct. Please verify that "新しいタブで開く" is the appropriate Japanese translation for "Open in new tab" as a UI field label.
278-279: BothlinksAlignmentandlinksPositionkeys should coexist during the migration period.The migration file (0050_update_link_align_prop.ts) transforms
linksAlignmenttolinksPositionin component data. Since legacy data still containslinksAlignmentand the migration reads this property to map it tolinksPosition, both locale keys are necessary. Once defaultLayoutData.ts and all test data are updated to uselinksPosition, thelinksAlignmentkey can be removed in a follow-up PR.
32-34: Translation is accurate and follows accessibility best practices.The translation "{{label}} (新しいタブで開きます)" correctly conveys "opens in new tab" in Japanese using the polite form appropriate for user-facing ARIA labels. The parenthetical format matches standard accessibility label patterns and the
{{label}}placeholder is used correctly for dynamic content. No changes needed.packages/visual-editor/locales/zh-TW/visual-editor.json (1)
279-279: Note: AI summary incorrectly describes the location of this key.The AI summary states this key is under
approvals.linksPosition, but the actual code shows it's located underfields.linksPosition(within the fields object starting at line 173).packages/visual-editor/locales/hr/visual-editor.json (1)
32-34: LGTM! Locale additions are correct.The new translation keys for ARIA label, links position, and open in new tab feature are properly added and follow the expected structure across the codebase.
Also applies to: 279-279, 300-300
packages/visual-editor/locales/en/visual-editor.json (1)
32-34: LGTM! English locale additions are correct.The base English translations are clear and consistent with the feature requirements. These serve as the reference for other locale translations.
Also applies to: 278-278, 299-299
packages/visual-editor/locales/fi/visual-editor.json (1)
32-34: LGTM! Finnish locale additions are correct.The new translation keys are properly added with appropriate Finnish translations that align with the feature requirements.
Also applies to: 279-279, 300-300
packages/visual-editor/locales/sv/visual-editor.json (1)
32-34: LGTM! Swedish locale additions are correct.The new translation keys are properly added with appropriate Swedish translations following the expected pattern.
Also applies to: 280-280, 301-301
packages/visual-editor/locales/pl/visual-editor.json (1)
32-34: LGTM! Polish locale additions are correct.The new translation keys are properly added with appropriate Polish translations that align with the feature implementation.
Also applies to: 280-280, 301-301
packages/visual-editor/locales/lv/visual-editor.json (1)
32-34: LGTM! Translation keys added correctly.The new translation keys for
aria.opensInNewTab,fields.linksPosition, andfields.openInNewTabare properly structured and support the new feature for opening links in new tabs with accessibility labels.Also applies to: 279-279, 300-300
packages/visual-editor/src/components/header/ExpandedHeader.test.tsx (1)
943-1125: LGTM! Comprehensive test coverage for the new feature.The new test case "version 50 props - with external links" effectively validates the
openInNewTabfunctionality:
- Tests the feature in both primary and secondary header slots
- Includes links with and without
openInNewTab: trueto verify both states- Properly includes mobile menu interactions for complete coverage
- Follows the established test pattern and structure
packages/visual-editor/locales/fr/visual-editor.json (1)
32-34: LGTM! French translations added correctly.The new translation keys are properly structured and consistent with the additions in other locale files. The French translations appropriately support the new
openInNewTabfunctionality and link positioning features.Also applies to: 278-279, 299-300
packages/visual-editor/locales/de/visual-editor.json (1)
32-34: LGTM! German translations added correctly.The new translation keys are properly structured and consistent with other locale files. The German translations "öffnet sich in einem neuen Tab" (opens in a new tab), "Linkposition" (link position), and "In neuem Tab öffnen" (open in new tab) appropriately support the new features.
Also applies to: 284-285, 305-306
packages/visual-editor/locales/cs/visual-editor.json (1)
32-34: LGTM! Czech translations added correctly.The new translation keys are properly structured and complete the localization coverage. The Czech translations "otevře se na nové kartě" (opens in a new tab), "Pozice odkazů" (link position), and "Otevřít na nové kartě" (open in new tab) are consistent with the additions across all locale files.
Also applies to: 286-287, 307-308
packages/visual-editor/src/types/types.ts (1)
13-13: LGTM! Clean type extension.The optional
openInNewTabfield properly extends theTranslatableCTAtype to support the new functionality for opening links in a new tab.packages/visual-editor/src/docs/components.md (1)
257-260: LGTM! Documentation correctly reflects the property rename and new center alignment option.The documentation updates properly capture:
- Property rename from
linksAlignmenttolinksPosition- Extension of allowed values to include
"center"This aligns with the migration logic and PR objectives.
Also applies to: 749-751
packages/visual-editor/locales/da/visual-editor.json (2)
32-34: LGTM! New translations added correctly.The additions properly support the new features:
aria.opensInNewTabfor accessibility announcementsfields.linksPositionfor the renamed propertyfields.openInNewTabfor the new fieldAlso applies to: 286-286, 307-307
285-286: BothlinksAlignmentandlinksPositionare consistently present together across all 24 locale files. This appears to be an intentional pattern, not an inconsistency requiring resolution. No cleanup or backward compatibility decision is needed at this time.packages/visual-editor/src/components/footer/ExpandedFooter.test.tsx (2)
33-38: LGTM! Test data properly exercises the newopenInNewTabfeature.The
testExternalFooterLinksconstant includes a mix of links with and withoutopenInNewTab: true, which will help verify the rendering of external link indicators.
1325-1676: LGTM! Comprehensive test coverage for version 50 changes.The new test cases properly verify:
- Usage of
linksPosition(renamed fromlinksAlignment)- New "center" alignment option for secondary footer
- External link rendering with
openInNewTabflag- Both light and dark background variants
The tests include accessibility checks via
axe()which will validate the ARIA labels for links opening in new tabs.packages/visual-editor/src/components/migrations/migrationRegistry.ts (1)
51-51: LGTM! Migration properly registered.The
updateLinksAlignmentMigrationis correctly imported and appended to the registry, which will ensure existing component instances are migrated fromlinksAlignmenttolinksPositionwith proper fallback defaults.Based on learnings, the migration index (50) is intentional and maintains the migration sequence.
Also applies to: 108-108
packages/visual-editor/src/docs/ai/components.d.ts (1)
551-551: LGTM! Type definitions correctly reflect the new features.The type definitions accurately capture both the renamed
linksPositionproperty with the new "center" option and the new optionalopenInNewTabboolean on TranslatableCTA.Also applies to: 898-898
packages/visual-editor/src/components/footer/FooterExpandedLinksWrapper.tsx (1)
77-77: Verify that defaultLink includes an openInNewTab default value.The implementation correctly passes
openInNewTabto CTA and defines the schema field. However, likeFooterExpandedLinkSectionSlot.tsx, verify that the importeddefaultLinkincludes a default value foropenInNewTab(compare withHeaderLinks.tsxwhich explicitly setsopenInNewTab: false).The verification script in the previous file review will also check this, as both components import the same
defaultLinkfromExpandedFooter.tsx.Also applies to: 137-149
packages/visual-editor/src/components/header/HeaderLinks.tsx (1)
36-36: LGTM! Complete and correct implementation.The implementation correctly:
- Sets an explicit default value (
openInNewTab: false) indefaultLink- Defines the schema field with appropriate Yes/No radio options
- Passes the prop to the CTA component
- Uses TypeScript's
satisfiesoperator for type safetyThis serves as the reference pattern for the footer components.
Also applies to: 58-67, 104-104
packages/visual-editor/src/components/migrations/0050_update_link_align_prop.ts (1)
3-46: Migration defaults are correct and implementation is sound.The migration properly handles the property rename from
linksAlignmenttolinksPositionwith defaults verified against component implementations:
- ExpandedFooter defaults to
"right"✓ (matches component default)- SecondaryFooterSlot defaults to
"left"✓ (matches component default)The implementation correctly uses guard clauses, preserves all properties via spread operators, and implements the proper fallback chain:
linksPosition ?? linksAlignment ?? default.packages/visual-editor/src/components/footer/FooterLinksSlot.tsx (1)
45-59: LGTM! Clean switch-based alignment logic.The switch statement properly handles all three alignment options (left, center, right) with appropriate Tailwind justify classes for the secondary variant.
packages/visual-editor/src/components/atoms/cta.tsx (3)
302-304: LGTM! Proper security attributes for new tab links.The implementation correctly applies
target="_blank"andrel="noopener noreferrer"when opening in a new tab, preventing potential security vulnerabilities (tabnabbing).
308-313: LGTM! Accessible external link indicator.The external link icon is properly hidden from screen readers with
aria-hidden="true"since the ARIA label already conveys the "opens in a new tab" information. The inline styling keeps the icon aligned with the text.
299-299: Verify that the className change doesn't break existing layouts.The Link's
classNamechanged to"inline leading-none". This introduces a CSS conflict: the parent Button applies flex centering classes (flex items-center justify-center), whileinlinedisplay on the Link prevents it from respecting flex layout. Additionally,leading-noneremoves line-height entirely, which could affect vertical alignment of the icon and text content.With 25+ CTA component usages across the codebase, this change warrants visual regression testing to ensure no unintended layout breakage. Consider testing:
- Vertical alignment of text and icon within each CTA variant
- Line-height/spacing in primary, secondary, and link button variants
- Behavior across different content lengths and icon combinations
packages/visual-editor/src/components/footer/ExpandedFooter.tsx (1)
135-154: LGTM! Consistent property rename from linksAlignment to linksPosition.The rename is applied consistently across:
- Type definition (Line 60)
- Field schema (Lines 135-154)
- Data destructuring (Line 195)
- Default props (Lines 414, 445)
The migration file mentioned in the AI summary (0050_update_link_align_prop.ts) should handle backward compatibility for existing component instances.
Also applies to: 195-195, 414-414, 445-445
packages/visual-editor/src/components/footer/SecondaryFooterSlot.tsx (1)
47-47: LGTM! Property rename and center alignment support implemented correctly.The changes consistently rename
linksAlignmenttolinksPositionand add center alignment support:
- Type definition extended to
"left" | "center" | "right"(Line 47)- Field schema includes all three options with proper i18n labels (Lines 84-106)
- Default value set to
"left"(Line 183)resolveDatacorrectly readslinksPositionand propagates it asalignmentto child slots (Line 233)The migration (0050_update_link_align_prop.ts) should handle backward compatibility.
Also applies to: 84-106, 183-183, 233-233
| "mobileContentAlignment": "Alinhamento de conteúdo móvel", | ||
| "mobileImagePosition": "Posição da imagem móvel", | ||
| "name": "Nome", | ||
| "openInNewTab": "Abrir em nova aba", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider standardizing terminology for "tab".
The translation uses "aba" for "tab", while Line 33 uses "guia". Both are valid Portuguese terms, but the inconsistency may cause confusion.
Consider standardizing on "aba" throughout, as it's more commonly used in UI contexts for browser tabs and is less ambiguous than "guia" (which can also mean "guide").
🔎 Suggested fix to standardize terminology
Update Line 33 to use "aba" for consistency:
"aria": {
- "opensInNewTab": "{{label}} (abre em uma nova guia)"
+ "opensInNewTab": "{{label}} (abre em uma nova aba)"
},Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/visual-editor/locales/pt/visual-editor.json around line 300 (and
notably line 33), the file mixes "aba" and "guia" to translate "tab";
standardize terminology by replacing instances of "guia" with "aba" (update the
value at line 33 to use "aba"), and scan the file for any other occurrences of
"guia" to replace so the translation is consistent across the locale.
| "aria": { | ||
| "opensInNewTab": "{{label}}(在新標籤頁中打開)" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use consistent Chinese terminology for "tab" throughout the locale file.
Lines 33 and 300 translate "tab" using different Chinese terms: "標籤頁" and "選項卡" respectively. While both are valid Chinese terms, maintaining consistency across the locale file provides a better user experience. Verify with a native Traditional Chinese speaker which term is preferred for your audience, then apply it consistently to both aria.opensInNewTab and fields.openInNewTab.
🤖 Prompt for AI Agents
In packages/visual-editor/locales/zh-TW/visual-editor.json around lines 32 to 34
(and also at line ~300), the Chinese translations use two different terms for
"tab" ("標籤頁" and "選項卡"); pick the preferred Traditional Chinese term after
confirming with a native speaker and update both aria.opensInNewTab and
fields.openInNewTab to use that same term consistently across the file (search
for other occurrences of "標籤頁" and "選項卡" and normalize them as well), then run
localization checks or unit tests to ensure no keys or formatting were
accidentally changed.
| "mobileContentAlignment": "移動內容對齊", | ||
| "mobileImagePosition": "移動圖像位置", | ||
| "name": "姓名", | ||
| "openInNewTab": "在新選項卡中打開", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent terminology and summary discrepancy.
Two issues identified:
-
Terminology inconsistency: This translation uses "選項卡" (tab), but line 33 uses "標籤頁" (tab). Both should use the same Chinese term for consistency.
-
AI summary discrepancy: The summary states this is
fields.name.openInNewTab, but the code shows it's actuallyfields.openInNewTab(a direct child of the fields object, not nested under a name property).
🔎 Suggested fix for terminology consistency
After determining which term to use consistently (標籤頁 or 選項卡), update this line accordingly. For example, if choosing "標籤頁":
- "openInNewTab": "在新選項卡中打開",
+ "openInNewTab": "在新標籤頁中打開",Also update line 33 to match if choosing "選項卡" instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "openInNewTab": "在新選項卡中打開", | |
| "openInNewTab": "在新標籤頁中打開", |
🤖 Prompt for AI Agents
In packages/visual-editor/locales/zh-TW/visual-editor.json around line 300, the
value for "openInNewTab" uses "選項卡" which is inconsistent with line 33's "標籤頁"
and the summary incorrectly references fields.name.openInNewTab while the actual
key is fields.openInNewTab; pick one term ("標籤頁" or "選項卡") and update this line
(and line 33) to use that same term, and correct the summary/documentation to
reference fields.openInNewTab (not fields.name.openInNewTab).
|
Not sure why there are over 300+ files changed again. Could this possibly be because of the migration? |
| "linkType": "Typ odkazu", | ||
| "linkedInLink": "LinkedIn Link", | ||
| "links": "Odkazy", | ||
| "linksAlignment": "Sladění odkazů", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is linksAlignment being used anywhere anymore? If not, can we remove from translations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did the spacing in the footer links change? less padding vertically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cta padding changed a bit
Tickets - OPAQF-41, OPAQF-53. Closed previous PR.
Ran test on older header by mistake, please ignore.
Screen.Recording.2026-01-02.at.1.58.52.PM.mov