-
Notifications
You must be signed in to change notification settings - Fork 0
feat: make images and cta links translatable fields #944
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
Conversation
|
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. |
commit: |
…al-editor into translatable-cta-image
…al-editor into translatable-cta-image
…al-editor into translatable-cta-image
…al-editor into translatable-cta-image
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: 1
♻️ Duplicate comments (1)
packages/visual-editor/src/components/migrations/0048_translatable_cta_image.ts (1)
158-163: Hardcoded "en" in default alternateText will omit other locales.The default alternateText only sets the
"en"key. WhenmigrateImagewraps this logo object for each locale (e.g.,fr,de), every locale receives an image containingalternateText: { en: "Logo", hasLocalizedValue: "true" }. Non-English locales will lack their own alternateText keys, causing missing or incorrect alt text.🔎 Suggested fix to build alternateText for all locales
+ const localizedAltText: any = { hasLocalizedValue: "true" }; + locales.forEach((locale) => { + localizedAltText[locale] = "Logo"; + }); logo = { height: 100, width: 100, - alternateText: { en: "Logo", hasLocalizedValue: "true" }, + alternateText: localizedAltText, ...logo, };Based on learnings, a similar issue was flagged in migration 0045.
🧹 Nitpick comments (2)
packages/visual-editor/src/components/pageSections/EventSection/EventCard.tsx (1)
450-456: Refactor to improve type safety and clarity.The
showImagelogic has several concerns:
- Excessive use of
as anydefeats TypeScript's type checking, making the code more fragile.- The check
(resolvedImage as any)?.image?.urlappears suspicious – based on the types (AssetImageTypeandTranslatableAssetImage), images shouldn't have a nestedimageproperty. This may be dead code.- Complex nested conditions make the logic hard to read and maintain.
Consider refactoring to use proper type guards or a helper function that handles the different image shapes (non-localized vs. localized) with explicit typing.
💡 Example refactoring approach
// Helper to check if image has a URL function hasImageUrl(image: unknown, language: string): boolean { if (!image) return false; // Non-localized image if (typeof (image as any).url === 'string') return true; // Localized image if ((image as any).hasLocalizedValue && (image as any)[language]?.url) { return true; } return false; } const showImage = hasImageUrl(resolvedImage, i18nComponentsInstance.language || "en");Could you verify whether the
(resolvedImage as any)?.image?.urlcheck is actually needed? If it's handling a specific edge case, please add a comment explaining when this structure occurs.packages/visual-editor/src/vite-plugin/defaultLayoutData.ts (1)
3513-3516: Nested alternateText structure only includes "en" locale.The footer logo's alternateText is nested within the English image object and contains only the
"en"key. When additional locales are added, they will inherit this structure withalternateText.enbut lack their own alternateText keys. This mirrors the issue in the migration defaults.Consider flattening alternateText to sit at the same level as the image localization, or ensure it's populated for all target locales when the default layout is instantiated. This would align with a fix to the migration's default alternateText handling.
Also applies to: 4257-4260
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
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 19 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/[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 19 props - expanded.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/[mobile] version 48 props.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/**
📒 Files selected for processing (8)
packages/visual-editor/src/components/footer/ExpandedFooter.test.tsxpackages/visual-editor/src/components/header/ExpandedHeader.test.tsxpackages/visual-editor/src/components/migrations/0048_translatable_cta_image.tspackages/visual-editor/src/components/pageSections/EventSection/EventCard.tsxpackages/visual-editor/src/components/pageSections/EventSection/EventSection.test.tsxpackages/visual-editor/src/components/pageSections/TeamSection/TeamCard.tsxpackages/visual-editor/src/components/pageSections/TeamSection/TeamSection.test.tsxpackages/visual-editor/src/vite-plugin/defaultLayoutData.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/visual-editor/src/components/pageSections/TeamSection/TeamSection.test.tsx
- packages/visual-editor/src/components/footer/ExpandedFooter.test.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 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/pageSections/EventSection/EventCard.tsxpackages/visual-editor/src/components/header/ExpandedHeader.test.tsxpackages/visual-editor/src/components/migrations/0048_translatable_cta_image.ts
📚 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/0048_translatable_cta_image.ts
⏰ 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 (20.x)
- GitHub Check: call_unit_test / unit_tests (18.x)
- GitHub Check: create-dev-release
- GitHub Check: semgrep/ci
🔇 Additional comments (9)
packages/visual-editor/src/components/header/ExpandedHeader.test.tsx (1)
838-935: LGTM! Version 48 test fixture correctly implements localized data format.The test case properly exercises the new translatable image and CTA structures introduced by migration 0048:
- Image data (lines 878-883): Correctly includes localized
url,height,width, andalternateTextunder theenlocale withhasLocalizedValue: "true".- CTA data (lines 898-916): Includes required
data.show: trueandstyles: { variant: "primary" }, with localizedlabelandlinkfields.Previously flagged issues have been addressed.
packages/visual-editor/src/components/pageSections/EventSection/EventSection.test.tsx (1)
1169-1268: LGTM! Well-structured localized test case.The version 48 test properly exercises the new translatable image and CTA link features with localized data structures. The test includes both
hasLocalizedValuemarkers and language-specific values, which aligns with the PR's localization objectives.packages/visual-editor/src/components/pageSections/EventSection/EventCard.tsx (2)
87-87: LGTM! Good accessibility practice.Adding a default
alternateTextfor the placeholder image ensures all images have alt text, which is essential for accessibility.
440-448: LGTM! Consistent localization-aware resolution.The
resolvedImagecomputation correctly prioritizesparentDataand falls back to field resolution with language support, consistent with the localization pattern used elsewhere in the component.packages/visual-editor/src/components/pageSections/TeamSection/TeamCard.tsx (2)
48-48: LGTM! Good accessibility improvement.Adding a default
alternateTextto the placeholder headshot improves accessibility compliance and aligns with making images fully translatable fields.
454-461: ThehasLocalizedValuepresence check is correctly used as a feature flag for translatable images.The three conditions on lines 454-461 properly detect different image data structures: translatable (marked with
hasLocalizedValue), direct URLs, and nested image objects. ThehasLocalizedValueproperty is a type marker indicating locale-specific image variants exist; the actual URL resolution and validation happens downstream in the Image component, which performs null checks (image?.url, property existence checks) before rendering. This design correctly shows the image slot for configured translatable fields while preventing empty placeholders through downstream validation.packages/visual-editor/src/components/migrations/0048_translatable_cta_image.ts (2)
4-10: LGTM! Locale extraction with sensible fallback.The helper correctly extracts locales from the streamDocument and provides a safe default.
13-71: LGTM! Helper functions correctly handle localization.Both
migrateImageandmigrateCTAproperly normalize inputs, check for existing localization, and safely wrap values in per-locale structures.packages/visual-editor/src/vite-plugin/defaultLayoutData.ts (1)
41-46: LGTM! Consistent localization pattern applied throughout.All images and CTA links have been successfully converted to the translatable structure with
hasLocalizedValueflags and locale-specific values. The implementation is consistent across headers, sections, cards, and footers.Also applies to: 321-326, 404-409, 694-699, 816-821, 951-956, 1086-1091, 1814-1819, 1985-1990, 2154-2159, 2382-2387, 2546-2551, 2710-2715, 3003-3009, 3150-3156, 3297-3303, 3508-3517, 3927-3932, 4252-4261
packages/visual-editor/src/components/migrations/0048_translatable_cta_image.ts
Show resolved
Hide resolved
…al-editor into translatable-cta-image
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: 0
🧹 Nitpick comments (1)
packages/visual-editor/src/components/header/ExpandedHeader.test.tsx (1)
869-892: Verify the image width styling.The image data specifies
width: 100(line 881), but the styles specifywidth: 640(line 889). Other version tests typically use matching or proportional values. While this could be intentional to test specific rendering scenarios, please confirm this large style width is correct.🔎 Comparison with other tests
For reference, version 41 test uses consistent dimensions:
- Line 530:
width: 100(data)- Line 538:
width: 100(styles)If the large style width is unintentional, consider:
- styles: { aspectRatio: 1, width: 640 }, + styles: { aspectRatio: 1, width: 100 },
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/visual-editor/src/components/header/ExpandedHeader.test.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 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/header/ExpandedHeader.test.tsx
⏰ 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 (3)
packages/visual-editor/src/components/header/ExpandedHeader.test.tsx (3)
838-847: LGTM - Well-structured test case for version 48.The test structure correctly initializes the version 48 props with the expected data shape, including the root-level primaryHeader field configuration.
893-919: LGTM - Translatable CTA link correctly implemented.The PrimaryCTASlot now includes the translatable link structure with localized
linkfield (lines 907-910), along with the requiredshowandstylesfields that were previously flagged. This aligns with the PR objective to make CTA links translatable.
920-936: LGTM - Test case properly configured.The test correctly completes with empty secondary slots (appropriate for focused testing), static header positioning, and proper version number. The configuration effectively demonstrates the translatable image and CTA link features introduced in this PR.
...ing/screenshots/HeroSection/[desktop] version 47 props with non-localized image and CTAs.png
Outdated
Show resolved
Hide resolved
...isual-editor/src/components/testing/screenshots/ExpandedFooter/[mobile] version 48 props.png
Show resolved
Hide resolved
.../src/components/testing/screenshots/ExpandedFooter/[desktop] version 19 props - expanded.png
Show resolved
Hide resolved
...isual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 48 props.png
Show resolved
Hide resolved
...sting/screenshots/PromoSection/[tablet] version 48 props with translatable image and CTA.png
Show resolved
Hide resolved
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: 0
🧹 Nitpick comments (1)
packages/visual-editor/src/components/header/ExpandedHeader.test.tsx (1)
838-936: LGTM! Version 48 test correctly validates translatable image and CTA structures.The test fixture properly exercises the new localized data shapes introduced in this PR:
- Image with per-locale
url,height,width, andalternateTextunderconstantValue.en- CTA with localized
labelandlinkfieldsAll previously flagged issues (missing dimensions, alt text, and CTA
show/stylesfields) have been addressed.Optional: Consider adding a multi-locale test fixture
While the current test validates the localized data structure, adding a fixture with multiple locales (e.g.,
en,fr) would more thoroughly exercise locale resolution logic. For example:constantValue: { en: { url: "https://placehold.co/100", alternateText: "Logo", height: 100, width: 100, }, fr: { url: "https://placehold.co/100", alternateText: "Logo (FR)", height: 100, width: 100, }, hasLocalizedValue: "true", }This is optional and can be deferred if multi-locale behavior is already covered in other test suites.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (24)
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 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 48 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 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 48 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 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 48 props.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/[mobile] version 48 props.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/PromoSection/[desktop] version 48 props with translatable image and CTA.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/PromoSection/[mobile] version 48 props with translatable image and CTA.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/PromoSection/[tablet] version 48 props with translatable image and CTA.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (2)
packages/visual-editor/src/components/footer/FooterUtilityImagesSlot.tsxpackages/visual-editor/src/components/header/ExpandedHeader.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/visual-editor/src/components/footer/FooterUtilityImagesSlot.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 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/header/ExpandedHeader.test.tsx
⏰ 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: create-dev-release
- GitHub Check: semgrep/ci
…al-editor into translatable-cta-image
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: 0
🧹 Nitpick comments (1)
packages/visual-editor/src/components/header/ExpandedHeader.test.tsx (1)
909-916: Consider addingfrlocale to CTA label and link for consistency with image localization.The LogoSlot image provides both
enandfrlocales (lines 878-889), but the CTAlabelandlinkonly provide theenlocale. Adding a French locale would improve test coverage of the translatable CTA feature and align with the image's localization pattern.🔎 Proposed enhancement
entityField: { field: "", constantValue: { label: { en: "Call to Action", + fr: "Appel à l'action", hasLocalizedValue: "true", }, link: { en: "#", + fr: "#", hasLocalizedValue: "true", }, linkType: "URL", }, constantValueEnabled: true, },
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (29)
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/[mobile] version 48 props.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/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/[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/[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/**
📒 Files selected for processing (1)
packages/visual-editor/src/components/header/ExpandedHeader.test.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 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/header/ExpandedHeader.test.tsx
⏰ 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: create-dev-release
- GitHub Check: semgrep/ci
🔇 Additional comments (1)
packages/visual-editor/src/components/header/ExpandedHeader.test.tsx (1)
838-942: Version 48 test case looks good—previous issues resolved.The v48 test correctly exercises translatable images and CTA links as intended by the PR. All previously flagged concerns have been addressed:
- Image includes height, width, and alternateText for accessibility
- CTASlot includes
show: trueandstylesfor proper rendering
This makes Images and CTA Links translatable, with an "Apply to All Locales" button below both of them.
Images:
Screen.Recording.2025-12-16.at.12.00.02.PM.mov
CTA Links:
Screen.Recording.2025-12-16.at.12.01.53.PM.mov