-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add ProfessionalHeroSection #983
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. |
WalkthroughAdds a new ProfessionalHeroSection to the visual-editor package: exported types ( Sequence Diagram(s)sequenceDiagram
participant Editor as Visual Editor UI
participant Provider as VisualEditorProvider
participant Section as ProfessionalHeroSection
participant Resolver as Data Resolver
participant Analytics as AnalyticsScopeProvider
participant DOM as Rendered PageSection
Editor->>Provider: mount editor with section config
Provider->>Section: instantiate ProfessionalHeroSection with props
Section->>Resolver: resolve slots (Image, Phone, Email, Reviews, Address)
Resolver-->>Section: return resolved data (including isRightColumnVisible)
Section->>Analytics: wrap render with AnalyticsScopeProvider (scope from props)
Section->>DOM: render PageSection (image column, content column, conditional phone/email, ReviewStars)
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2026-01-12T21:31:40.196ZApplied to files:
🧬 Code graph analysis (2)packages/visual-editor/src/docs/ai/components.d.ts (2)
packages/visual-editor/src/components/categories/PageSectionCategory.tsx (2)
⏰ 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). (3)
🔇 Additional comments (6)
✏️ Tip: You can disable this entire section by setting 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: 2
🤖 Fix all issues with AI agents
In
@packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.test.tsx:
- Around line 36-88: ProfessionalHeroSection.defaultProps is typed optional,
causing TS18048; declare a non-null defaultProps alias at the top of the test
file (e.g., const defaultProps = ProfessionalHeroSection.defaultProps! or use
null-coalescing to an empty object) and then replace all uses of
ProfessionalHeroSection.defaultProps in the tests array with that alias (e.g.,
props: { ...defaultProps } and styles spread from defaultProps.styles) so the
compiler knows defaults are present.
In
@packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx:
- Around line 30-34: The exported ProfessionalHeroData interface (and its
backgroundImage field) is unused; either remove the interface/export or wire it
into the component: if you intend to use it, add ProfessionalHeroData to the
component props/field definitions and replace or supplement slots.ImageSlot
rendering by reading backgroundImage from props (or component fields) and
passing it to the Image renderer, referencing
ProfessionalHeroData.backgroundImage and the component that renders images;
otherwise delete the ProfessionalHeroData declaration and any imports/exports
referencing it to avoid dead code.
🧹 Nitpick comments (2)
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (1)
457-469: TODO: ComponentErrorBoundary is noted but not implemented.The component currently lacks error boundary protection. This could cause the entire page to crash if the hero section encounters a rendering error.
Would you like me to help implement the
ComponentErrorBoundarywrapper, or should I open an issue to track this task?packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.test.tsx (1)
125-131: Consider adding more specific assertions.The current assertions are minimal. Consider adding checks for:
- Presence of expected slot content (headings, address, phone, email)
- Absence of image when
showImage: false- Absence of review stars when
showAverageReview: falseThe
expect(container).toBeInTheDocument()on line 130 is redundant since the render always succeeds if we reach that point.♻️ Example enhanced assertions
// Basic assertions if (test.props.styles.showImage !== false) { expect(container.querySelector("img")).toBeInTheDocument(); + } else { + expect(container.querySelector("img")).not.toBeInTheDocument(); } - expect(container).toBeInTheDocument(); + // Verify the section renders + expect(container.querySelector('[aria-label="Professional Hero"]')).toBeInTheDocument(); });
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/visual-editor/src/components/categories/PageSectionCategory.tsxpackages/visual-editor/src/components/pageSections/ProfessionalHeroSection.test.tsxpackages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsxpackages/visual-editor/src/components/pageSections/index.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (1)
packages/visual-editor/src/types/images.ts (2)
AssetImageType(57-63)TranslatableAssetImage(71-71)
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.test.tsx (3)
packages/visual-editor/src/components/testing/componentTests.setup.ts (3)
ComponentTest(109-121)transformTests(92-107)axe(77-81)packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (1)
ProfessionalHeroSection(267-470)packages/visual-editor/src/components/categories/SlotsCategory.tsx (1)
SlotsCategoryComponents(190-263)
🪛 GitHub Actions: main
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.test.tsx
[error] 57-57: TS18048: 'ProfessionalHeroSection.defaultProps' is possibly 'undefined'.
⏰ 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). (1)
- GitHub Check: semgrep/ci
🔇 Additional comments (3)
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (1)
98-167: Well-structured component implementation.The
ProfessionalHerocomponent correctly handles:
- Conditional image rendering based on
showImage- Responsive layout positioning with
desktopImagePositionandmobileImagePosition- Review stars conditional on both
reviewCount > 0andshowAverageReview- Proper accessibility with
aria-labeland translation supportpackages/visual-editor/src/components/pageSections/index.ts (1)
80-85: Exports are consistent with existing patterns.The new exports follow the established convention used by other page sections in this file.
packages/visual-editor/src/components/categories/PageSectionCategory.tsx (1)
59-62: Proper integration into the page sections category system.The new component is correctly imported, added to the props interface, and registered in the components map, following the established patterns.
Also applies to: 76-76, 96-96
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.test.tsx
Show resolved
Hide resolved
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx
Outdated
Show resolved
Hide resolved
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx
Outdated
Show resolved
Hide resolved
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx
Show resolved
Hide resolved
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.test.tsx
Outdated
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: 2
🤖 Fix all issues with AI agents
In @packages/visual-editor/locales/de/visual-editor.json:
- Line 123: The translation value for the key professionalHeroSection is
inconsistently formatted (it currently uses German quotation marks and a
trailing period); update the value to match the other section labels (simple
noun phrase without quotes or terminal punctuation) — e.g., change
professionalHeroSection from "Abschnitt „Professionelle Helden“." to a form
consistent with bannerSection and heroSection such as "Professionelle Helden".
In
@packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx:
- Around line 175-271: Add a missing "data" field to the
professionalHeroSectionFields object to match the required
ProfessionalHeroSectionProps.data (use YextField with the appropriate
type/objectFields reflecting ProfessionalHeroData), and also add a corresponding
defaultProps.data entry in the ProfessionalHeroSection config so the component
has a default value; update professionalHeroSectionFields and the config's
defaultProps to consistently reflect the ProfessionalHeroData shape.
🧹 Nitpick comments (2)
packages/visual-editor/locales/ja/visual-editor.json (1)
122-122: LGTM!The Japanese translation keys are correctly placed and use appropriate katakana conventions for loanwords.
Minor observation: The translation "プロヒーロー部門" uses "部門" while the existing "heroSection" (line 107) uses "セクション". This inconsistency is minor and doesn't block the PR, but consider aligning to "プロヒーローセクション" for consistency if a follow-up localization pass is planned.
Also applies to: 620-620
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (1)
463-463: TODO: Add ComponentErrorBoundary.This comment indicates missing error boundary handling. Would you like me to help implement the error boundary or open an issue to track this task?
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
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/pageSections/ProfessionalHeroSection.test.tsxpackages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/visual-editor/locales/lt/visual-editor.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.test.tsx
🧰 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/ro/visual-editor.jsonpackages/visual-editor/locales/de/visual-editor.json
🧬 Code graph analysis (1)
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (3)
packages/visual-editor/src/components/pageSections/index.ts (4)
ProfessionalHeroData(83-83)ProfessionalHeroStyles(84-84)ProfessionalHeroSectionProps(82-82)ProfessionalHeroSection(81-81)packages/visual-editor/src/types/images.ts (2)
AssetImageType(57-63)TranslatableAssetImage(71-71)packages/visual-editor/src/components/contentBlocks/index.ts (1)
PhoneListProps(13-13)
🪛 GitHub Actions: main
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx
[error] 175-175: TypeScript error TS2741: Property 'data' is missing in type '{ styles: Field; slots: { type: "object"; objectFields: { ImageSlot: { type: "slot"; }; BusinessNameSlot: { type: "slot"; }; ProfessionalNameSlot: { ...; }; ... 4 more ...; EmailSlot: { ...; }; }; visible: false; }; analytics: Field<...>; liveVisibility: Field<...>; }' but required in type 'Fields'.
⏰ 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). (1)
- GitHub Check: semgrep/ci
🔇 Additional comments (26)
packages/visual-editor/locales/fi/visual-editor.json (1)
122-122: LGTM!The Finnish translations for the new Professional Hero Section are correct and consistent with existing hero-related translations in the file (e.g., "Sankari" for "Hero"). The keys are properly placed in their respective alphabetical positions within the JSON structure.
Also applies to: 620-620
packages/visual-editor/locales/fr/visual-editor.json (1)
121-121: LGTM!The French translations for the new Professional Hero Section are correctly added and follow the existing naming conventions in the locale file.
Also applies to: 620-620
packages/visual-editor/locales/ro/visual-editor.json (1)
122-122: LGTM!The Romanian translations for the new Professional Hero Section are correctly added and appropriately placed within the locale file structure.
Also applies to: 620-620
packages/visual-editor/locales/lv/visual-editor.json (1)
122-122: LGTM!The Latvian translations for the new Professional Hero Section are correctly added and follow the existing conventions in the locale file.
Also applies to: 620-620
packages/visual-editor/locales/zh-TW/visual-editor.json (1)
122-122: LGTM!The Traditional Chinese translations are correctly added. Minor note:
professionalHeroSectionuses "組" (group) whileheroSectionat line 107 uses "部分" (section), but both are acceptable translations in this context.Also applies to: 621-621
packages/visual-editor/locales/de/visual-editor.json (1)
625-625: LGTM!The root-level German translation "Professioneller Held" is correctly added and follows proper German grammar.
packages/visual-editor/locales/et/visual-editor.json (1)
122-122: LGTM!The new Estonian translation keys for the Professional Hero Section are correctly placed in alphabetical order and follow the existing translation patterns in this locale file.
Also applies to: 620-620
packages/visual-editor/locales/sk/visual-editor.json (1)
122-122: LGTM!The Slovak translation keys are properly placed and follow the established conventions in this locale file.
Also applies to: 621-621
packages/visual-editor/locales/pl/visual-editor.json (1)
122-122: LGTM!The Polish translation keys are correctly placed and the translations follow the established patterns in this locale file.
Also applies to: 621-621
packages/visual-editor/locales/nl/visual-editor.json (1)
122-122: LGTM!The Dutch translation keys are properly placed in alphabetical order and the translations are consistent with existing patterns (e.g., "Heldsectie" for hero section).
Also applies to: 620-620
packages/visual-editor/locales/nb/visual-editor.json (1)
122-122: LGTM!The Norwegian translations are consistent with existing patterns (
Heltseksjon→Profesjonell helteseksjon) and keys are correctly placed in alphabetical order.Also applies to: 620-620
packages/visual-editor/locales/it/visual-editor.json (1)
122-122: LGTM!Italian translations are correctly added and keys are properly placed in alphabetical order.
Also applies to: 620-620
packages/visual-editor/locales/sv/visual-editor.json (1)
122-122: LGTM!The Swedish translations follow the existing naming conventions (
Hjältesektion→Professionell hjältesektion) and are correctly placed.Also applies to: 621-621
packages/visual-editor/locales/cs/visual-editor.json (1)
124-124: LGTM!Czech translations are correctly added. The word order variation from the base
heroSectionkey reflects natural Czech grammar for the professional variant.Also applies to: 627-627
packages/visual-editor/locales/pt/visual-editor.json (1)
122-122: LGTM!Portuguese translations are correctly added and follow the existing section naming pattern.
Also applies to: 620-620
packages/visual-editor/locales/hr/visual-editor.json (1)
122-122: LGTM!The Croatian locale keys are correctly added in alphabetical order within their respective sections. The translations accurately convey "Professional Hero Section" and "Professional Hero" in Croatian.
Also applies to: 620-620
packages/visual-editor/locales/hu/visual-editor.json (1)
122-122: LGTM!The Hungarian locale keys are correctly added. The translations appropriately use "Professzionális hős szekció" for the section and the shorter "Profi hős" for the standalone hero label, both of which are natural Hungarian expressions.
Also applies to: 620-620
packages/visual-editor/locales/zh/visual-editor.json (1)
122-122: LGTM!The Simplified Chinese locale keys are correctly added. The translations "职业英雄组" (Professional Hero Section) and "职业英雄" (Professional Hero) are accurate and use appropriate Simplified Chinese characters.
Also applies to: 620-620
packages/visual-editor/locales/en-GB/visual-editor.json (1)
121-121: LGTM!The British English locale keys are correctly added in alphabetical order. The values appropriately match the US English locale as no UK-specific spelling variations are needed for these terms.
Also applies to: 619-619
packages/visual-editor/locales/en/visual-editor.json (1)
121-121: LGTM!The English locale keys are correctly added in alphabetical order. The key naming follows existing conventions (
components.professionalHeroSectionaligns with other section labels likeheroSection, and root-levelprofessionalHerofollows the pattern of existing hero-related keys).Also applies to: 619-619
packages/visual-editor/locales/da/visual-editor.json (1)
124-124: LGTM!The Danish translations for the new Professional Hero Section are correctly added and follow the existing naming conventions in the locale file.
Also applies to: 627-627
packages/visual-editor/locales/es/visual-editor.json (1)
121-121: LGTM!The Spanish translations for the new Professional Hero Section are correctly added and grammatically appropriate.
Also applies to: 619-619
packages/visual-editor/locales/tr/visual-editor.json (1)
122-122: LGTM!The Turkish translations for the new Professional Hero Section are correctly added and grammatically appropriate.
Also applies to: 620-620
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (3)
30-34:ProfessionalHeroData.backgroundImageis defined but never consumed.The
dataproperty containingbackgroundImageis declared in the interface but never used in theProfessionalHerocomponent. The image is rendered viaslots.ImageSlotinstead.Consider either:
- Removing the unused
datainterface if the slot-based approach is intentional- Implementing the
backgroundImagefield usage if entity-driven images are required
104-173: Well-structured component with good accessibility.The component implementation is clean with:
- Proper responsive layout handling for desktop/mobile image positions
- Conditional rendering of ReviewStars based on review data availability
- Accessible
aria-labelon the PageSection
1-28: Clean import structure.The imports are well-organized, pulling from appropriate packages (
@measured/puck,@yext/visual-editor,@yext/pages-components) and following the established patterns in the codebase.
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx
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/pageSections/ProfessionalHeroSection.tsx (1)
448-448: Address the TODO: Add ComponentErrorBoundary.The TODO comment indicates missing error boundary implementation. This should be tracked to ensure the component handles render errors gracefully.
Would you like me to open a new issue to track adding the
ComponentErrorBoundaryto this component?
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/visual-editor/locales/de/visual-editor.jsonpackages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsxpackages/visual-editor/src/components/pageSections/index.tspackages/visual-editor/src/docs/ai/components.d.tspackages/visual-editor/src/docs/components.md
✅ Files skipped from review due to trivial changes (1)
- packages/visual-editor/src/docs/components.md
🧰 Additional context used
🧬 Code graph analysis (2)
packages/visual-editor/src/docs/ai/components.d.ts (1)
packages/visual-editor/src/utils/themeConfigOptions.ts (1)
BackgroundStyle(36-43)
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (2)
packages/visual-editor/src/components/pageSections/index.ts (3)
ProfessionalHeroStyles(83-83)ProfessionalHeroSectionProps(82-82)ProfessionalHeroSection(81-81)packages/visual-editor/src/components/contentBlocks/index.ts (1)
PhoneListProps(13-13)
⏰ 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 (24.x)
- GitHub Check: call_unit_test / unit_tests (22.x)
- GitHub Check: semgrep/ci
🔇 Additional comments (12)
packages/visual-editor/src/docs/ai/components.d.ts (3)
44-44: LGTM!The
ProfessionalHeroSectionentry is correctly added toPageSectionCategoryProps, following the alphabetical ordering pattern of the existing entries.
405-430: LGTM!The
ProfessionalHeroSectionPropsinterface is well-structured and follows the established patterns in this file:
stylesproperty with proper JSDoc documentationslotsobject with all required slot definitions matching the implementationliveVisibilityas optional with proper documentationanalyticsmarked as@internal
801-827: LGTM!The
ProfessionalHeroStylesinterface is consistent with other style interfaces in the file (e.g.,HeroStyles). All properties have proper JSDoc documentation with@defaultValueannotations.packages/visual-editor/src/components/pageSections/index.ts (1)
80-84: LGTM!The export follows the established pattern in this barrel file. The component, props interface, and styles interface are correctly exported.
packages/visual-editor/locales/de/visual-editor.json (2)
123-123: LGTM!The translation key is correctly placed alphabetically within the
componentsobject and follows the naming convention of other section translations.
625-625: LGTM!The top-level translation key is correctly added and maintains alphabetical ordering.
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (6)
1-25: LGTM!Imports are well-organized and all appear to be utilized within the component.
27-57: LGTM!The
ProfessionalHeroStylesinterface is well-documented with appropriate JSDoc annotations and default value documentation.
59-87: LGTM!The props interface properly defines all required properties with appropriate documentation. The slot definitions provide clear naming for each configurable area of the component.
89-158: LGTM!The component implementation is clean and well-structured:
- Proper use of hooks for translations and document data
- Good defensive coding with
styles.showImage ?? truefallback- Accessibility consideration with
aria-labelon the PageSection- Conditional rendering of ReviewStars only when
reviewCount > 0- Consistent slot rendering pattern with
style={{ height: "auto" }}
160-256: LGTM!Field definitions follow the established patterns:
- Proper use of
YextFieldwrapper with translation keys- Consistent radio button options for boolean and position fields
- Slots and analytics correctly hidden from the UI (
visible: false)- Uses
ThemeOptions.VERTICAL_POSITIONfor consistency with other components
258-461: Well-structured ComponentConfig with comprehensive defaults.The component configuration is well-implemented:
- Default props provide sensible initial values for all style options
- Slot defaults use
satisfiesfor type safety- Render function correctly wraps the component with
AnalyticsScopeProviderandVisibilityWrapper- Analytics scope includes a hash for uniqueness
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx
Show resolved
Hide resolved
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx
Outdated
Show resolved
Hide resolved
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx
Outdated
Show resolved
Hide resolved
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx
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: 1
🤖 Fix all issues with AI agents
In @packages/visual-editor/locales/ro/visual-editor.json:
- Line 127: The translation for the key professionalHeroSection is inconsistent
with the existing heroSection pattern; update the value of
professionalHeroSection to use the singular, patterned form "Secțiunea Erou
Profesionist" so it matches the style used by heroSection (and maintain
capitalization/diacritics consistency across locale entries).
🧹 Nitpick comments (1)
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (1)
489-522: Consider adding explicit typing for the phone config parameter.The
anytype on line 503 reduces type safety. Consider defining a type for the phone number configuration.Suggested improvement
+type PhoneNumberConfig = { + number: { + field: string; + constantValue: string; + }; + label?: Record<string, string>; +}; - const hasPhones = phoneNumbersConfig.some((config: any) => { + const hasPhones = phoneNumbersConfig.some((config: PhoneNumberConfig) => {
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[desktop] default props with data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[desktop] default props with no data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[desktop] hide average review.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[desktop] image right desktop, image bottom mobile.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[desktop] no image.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[mobile] default props with data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[mobile] default props with no data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[mobile] hide average review.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[mobile] image right desktop, image bottom mobile.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[mobile] no image.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[tablet] default props with data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[tablet] default props with no data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[tablet] hide average review.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[tablet] image right desktop, image bottom mobile.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[tablet] no image.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (29)
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/pageSections/ProfessionalHeroSection.test.tsxpackages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsxpackages/visual-editor/src/docs/ai/components.d.tspackages/visual-editor/src/docs/components.md
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/visual-editor/locales/hr/visual-editor.json
- packages/visual-editor/locales/sk/visual-editor.json
- packages/visual-editor/locales/hu/visual-editor.json
- packages/visual-editor/locales/nl/visual-editor.json
- packages/visual-editor/locales/es/visual-editor.json
- packages/visual-editor/locales/pl/visual-editor.json
- packages/visual-editor/locales/nb/visual-editor.json
- packages/visual-editor/locales/en/visual-editor.json
- packages/visual-editor/locales/et/visual-editor.json
- packages/visual-editor/locales/zh-TW/visual-editor.json
- packages/visual-editor/locales/en-GB/visual-editor.json
- packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.test.tsx
- packages/visual-editor/locales/de/visual-editor.json
- packages/visual-editor/locales/it/visual-editor.json
- packages/visual-editor/locales/pt/visual-editor.json
🧰 Additional context used
🧠 Learnings (2)
📚 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.json
📚 Learning: 2025-12-23T16:30:55.497Z
Learnt from: benlife5
Repo: yext/visual-editor PR: 958
File: packages/visual-editor/src/components/pageSections/heroVariants/CompactHero.tsx:38-41
Timestamp: 2025-12-23T16:30:55.497Z
Learning: In CompactHero and CompactPromo components, duplicate IDs on image elements are intentional: conditional visibility classes (hidden/sm:block/sm:hidden) ensure only one image renders at a time based on responsive breakpoints and desktopImagePosition/mobileImagePosition settings.
Applied to files:
packages/visual-editor/src/docs/components.md
🧬 Code graph analysis (1)
packages/visual-editor/src/docs/ai/components.d.ts (1)
packages/visual-editor/src/utils/themeConfigOptions.ts (1)
BackgroundStyle(36-43)
⏰ 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). (3)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: call_unit_test / unit_tests (24.x)
- GitHub Check: semgrep/ci
🔇 Additional comments (21)
packages/visual-editor/locales/sv/visual-editor.json (1)
127-127: LGTM!The Swedish translations for the new ProfessionalHeroSection component are correctly added. Both keys follow the existing naming conventions, are alphabetically sorted within their respective sections, and the translations are consistent with similar existing entries (e.g., "heroSection" → "Hjältesektion").
Also applies to: 627-627
packages/visual-editor/locales/ro/visual-editor.json (1)
626-626: LGTM!The translation "Erou profesionist" correctly uses singular form and follows the established naming pattern.
packages/visual-editor/locales/lv/visual-editor.json (1)
127-127: LGTM!The new Latvian translation keys for
professionalHeroSectionandprofessionalHeroare correctly placed and follow the existing terminology patterns in the locale file.Also applies to: 626-626
packages/visual-editor/locales/zh/visual-editor.json (1)
127-127: LGTM!The Chinese translations for the new Professional Hero Section keys are correctly placed and use consistent terminology with existing hero-related translations in the locale file.
Also applies to: 626-626
packages/visual-editor/locales/ja/visual-editor.json (1)
127-127: LGTM with minor observation.The Japanese translations are correctly placed. Note that
professionalHeroSectionuses "部門" while the existingheroSectionuses "セクション" — both are valid translations for "section," but you may want to verify this stylistic difference is intentional for consistency with other locales.Also applies to: 626-626
packages/visual-editor/locales/tr/visual-editor.json (1)
127-127: LGTM!The Turkish translations are correctly placed and follow the existing terminology patterns consistently (e.g., "Kahraman Bölümü" matching the existing "heroSection" translation style).
Also applies to: 626-626
packages/visual-editor/locales/lt/visual-editor.json (1)
127-127: LGTM!The Lithuanian translations are correctly placed. The use of "herojų" (genitive plural) in
professionalHeroSectionvs "Herojaus" (genitive singular) in the existingheroSectionis a minor stylistic difference that appears intentional.Also applies to: 626-626
packages/visual-editor/locales/da/visual-editor.json (1)
129-129: LGTM!The Danish locale keys for the new ProfessionalHeroSection are correctly placed in alphabetical order and the translations appear appropriate.
Also applies to: 633-633
packages/visual-editor/locales/fr/visual-editor.json (1)
126-126: LGTM!The French locale keys are correctly placed and the translations follow proper French conventions.
Also applies to: 626-626
packages/visual-editor/locales/cs/visual-editor.json (1)
129-129: LGTM!The Czech locale keys are correctly placed in alphabetical order and the translations appear appropriate.
Also applies to: 633-633
packages/visual-editor/locales/fi/visual-editor.json (1)
127-127: LGTM!The Finnish locale keys are correctly placed in alphabetical order and the translations appear appropriate.
Also applies to: 626-626
packages/visual-editor/src/docs/components.md (1)
669-694: The documentation is correct as-is and does not require changes.The ProfessionalHeroSection's
slotsproperty is marked with/** @internal */in the component definition, indicating it is part of the internal API. The documentation correctly omits it. While HeroSection documents its slots, that's because HeroSection's slots lack the@internalmarker. The current pattern is consistent: public properties are documented, internal properties are not.packages/visual-editor/src/docs/ai/components.d.ts (3)
44-44: LGTM!The new
ProfessionalHeroSectionentry follows the established pattern inPageSectionCategoryProps.
405-436: LGTM!The interface is well-structured with appropriate JSDoc annotations. The slot-based design for data input is consistent with the component's architecture, and internal properties are properly marked.
807-833: LGTM!The style interface is well-defined with clear JSDoc default values and appropriate type constraints for position options.
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (6)
1-27: LGTM!Imports are well-organized and all appear to be utilized within the component.
29-96: LGTM!Interface definitions are well-documented and properly exported. They correctly match the public API declarations in
components.d.ts.
98-170: LGTM!The component implementation is well-structured:
- Proper accessibility with
aria-label- Responsive layout using Tailwind classes with
themeManagerCn- Correct conditional rendering logic for image and right column visibility
- Reviews display only when data exists (
reviewCount > 0)
172-269: LGTM!Field definitions are well-structured with proper internationalization using
msg(). The internal fields (slots, analytics) are correctly hidden from the editor UI.
276-488: LGTM!Default props are comprehensive with appropriate placeholder content for all slots. The use of
satisfiesensures type safety, and defaults align with the documented@defaultValueannotations.
523-540: LGTM!The render function properly composes error boundary, analytics scope, and visibility wrapper. This follows established patterns in the codebase and provides good resilience with
ComponentErrorBoundary.
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx
Outdated
Show resolved
Hide resolved
.../components/testing/screenshots/ProfessionalHeroSection/[tablet] default props with data.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 (3)
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (3)
137-141: Consider using<div>instead of nested<section>.The
<section>element here is semantically nested insidePageSection(which likely renders another<section>). Nested<section>elements without associated headings can be confusing for assistive technologies. Since this is a presentational grouping rather than a standalone section of content, a<div>would be more appropriate.Suggested change
- <section className="flex flex-col gap-2"> + <div className="flex flex-col gap-2"> <slots.BusinessNameSlot style={{ height: "auto" }} allow={[]} /> <slots.ProfessionalNameSlot style={{ height: "auto" }} allow={[]} /> <slots.ProfessionalTitleSlot style={{ height: "auto" }} allow={[]} /> - </section> + </div>
195-207: Inconsistent boolean option labels.The
showImagefield uses "True"/"False" labels, whileshowAverageReviewandliveVisibilityuse "Show"/"Hide" labels. Consider using consistent terminology across all boolean toggle fields for a better user experience.Suggested change for consistency
showImage: YextField(msg("fields.showImage", "Show Image"), { type: "radio", options: [ { - label: msg("fields.options.true", "True"), + label: msg("fields.options.show", "Show"), value: true, }, { - label: msg("fields.options.false", "False"), + label: msg("fields.options.hide", "Hide"), value: false, }, ], }),
505-512: Replaceanywith proper type for phone config items.The suggested type is incomplete. Since
phoneNumbersConfigisArray<PhoneProps["data"]>, the config parameter should be typed asPhoneProps["data"], which includes both thenumberandlabelfields. The suggested type only captures thenumberstructure and omitslabel.Suggested improvement
- const hasPhones = phoneNumbersConfig.some((config: any) => { + const hasPhones = phoneNumbersConfig.some((config: PhoneProps["data"]) => { const resolved = resolveComponentData( config.number, locale, streamDocument ); return !!resolved; });Or with an expanded inline type:
- const hasPhones = phoneNumbersConfig.some((config: any) => { + const hasPhones = phoneNumbersConfig.some((config: { number: YextEntityField<string>; label: TranslatableString }) => { const resolved = resolveComponentData( config.number, locale, streamDocument ); return !!resolved; });
PhonePropsis exported from@yext/visual-editorand should be imported if using the first approach.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[desktop] default props with data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[desktop] default props with no data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[desktop] hide average review.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[desktop] image right desktop, image bottom mobile.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[desktop] no image.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[mobile] default props with data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[mobile] default props with no data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[mobile] hide average review.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[mobile] image right desktop, image bottom mobile.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[mobile] no image.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[tablet] default props with data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[tablet] default props with no data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[tablet] hide average review.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[tablet] image right desktop, image bottom mobile.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/ProfessionalHeroSection/[tablet] no image.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (2)
packages/visual-editor/locales/ro/visual-editor.jsonpackages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/visual-editor/locales/ro/visual-editor.json
🧰 Additional context used
🧬 Code graph analysis (1)
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (3)
packages/visual-editor/src/components/pageSections/index.ts (3)
ProfessionalHeroStyles(83-83)ProfessionalHeroSectionProps(82-82)ProfessionalHeroSection(81-81)packages/visual-editor/src/components/contentBlocks/index.ts (1)
PhoneListProps(13-13)packages/visual-editor/src/internal/components/ComponentErrorBoundary.tsx (1)
ComponentErrorBoundary(14-97)
⏰ 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 (24.x)
- GitHub Check: call_unit_test / unit_tests (22.x)
- GitHub Check: semgrep/ci
🔇 Additional comments (4)
packages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsx (4)
29-59: LGTM!The interface is well-documented with JSDoc comments and appropriate type definitions for the styling options.
61-96: LGTM!Props interface is well-structured with appropriate internal markers and slot definitions.
525-541: LGTM!The render function properly composes the error boundary, analytics provider, and visibility wrapper following established patterns. The fallback for
analytics.scopeensures graceful handling when the scope is undefined.
278-489: LGTM!The
defaultPropsconfiguration is comprehensive with sensible defaults for all slots. The use ofsatisfiestype assertions ensures type safety while allowing the object literal syntax.
benlife5
left a 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.
nits, LGTM
This adds a new hero section component designed for professionals rather than locations.
Screen.Recording.2026-01-13.at.3.06.53.PM.mov