chore: address comments from coderabbit

[asanehisa]

Addresses some comments in PR #928

[github-actions]

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.

[coderabbitai]

Walkthrough

Multiple files updated: Slovak locale file with template placeholder change, ExpandedHeader component enhanced with optional chaining for safer property access and corrected width-compatibility logic, and three migration files adjusting parentData field references to use nested event/faq paths and reorganizing eventName configuration for CTA slots.

Possibly related PRs

• fix: header event names #892: Updates CTA event-name configuration for header slots, with changes to where eventName is declared for primary and secondary CTA components.

Suggested reviewers

• benlife5
• mkilpatrick
• briantstephan

Pre-merge checks and finishing touches

❌ Failed checks (2 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'chore: address comments from coderabbit' is vague and generic, using non-descriptive language that doesn't convey what specific changes were made.	Provide a more specific title that describes the actual changes, such as 'refactor: add optional chaining for slot safety and fix locale references' or reference the specific files/components modified.
Description check	❓ Inconclusive	The description 'Addresses some comments in PR #928' is too vague and doesn't convey any meaningful information about the actual changes or their purpose.	Expand the description to explain what specific comments were addressed and why the changes improve the codebase.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch address-comments

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9077fe7 and 3cf7c5c.

📒 Files selected for processing (5)

• packages/visual-editor/locales/sk/visual-editor.json (1 hunks)
• packages/visual-editor/src/components/header/ExpandedHeader.tsx (4 hunks)
• packages/visual-editor/src/components/migrations/0026_event_section_slots.ts (6 hunks)
• packages/visual-editor/src/components/migrations/0035_slotify_faq_section.ts (2 hunks)
• packages/visual-editor/src/components/migrations/0041_expanded_header_slots.ts (1 hunks)

🔇 Additional comments (8)

packages/visual-editor/locales/sk/visual-editor.json (1)

91-91: LGTM! Placeholder token standardization.

Standardizing the placeholder token from {{číslo}} to {{number}} ensures consistency across all locale files and improves maintainability of the i18n system.

packages/visual-editor/src/components/migrations/0041_expanded_header_slots.ts (1)

81-81: LGTM! Consistent eventName placement.

The eventName property is now correctly placed inside the props object, matching the pattern used for SecondaryCTASlot (line 104) and ensuring consistent slot configuration structure.

packages/visual-editor/src/components/migrations/0035_slotify_faq_section.ts (1)

55-55: LGTM! Consistent nested field path alignment.

The field references now correctly use props.data.faqs.field in both QuestionSlot and AnswerSlot parentData, aligning with the nested data structure and matching the pattern used in the FAQsWrapperSlot (line 120).

Also applies to: 77-77

packages/visual-editor/src/components/migrations/0026_event_section_slots.ts (1)

89-89: LGTM! Systematic nested field path migration.

All slot parentData field references now correctly use props.data.events.field instead of props.data.field, aligning with the nested data structure and matching the pattern used in CardsWrapperSlot (line 257). This creates consistent data flow through the slot hierarchy.

Also applies to: 119-119, 144-144, 171-171, 201-201, 219-219

packages/visual-editor/src/components/header/ExpandedHeader.tsx (4)

181-181: Corrects property path for visibility check.

The change from styles.show to data.show fixes the property path to correctly check the SecondaryHeaderSlot visibility, making it consistent with how PrimaryCTASlot and SecondaryCTASlot visibility are checked (lines 186-187, 193-194).

---

203-204: Fixes maxWidth comparison logic.

The change correctly compares the maxWidth property values instead of comparing the entire parentStyles object to a maxWidth value (which was a type mismatch). The optional chaining ?.maxWidth also prevents potential runtime errors when parentStyles is undefined.

---

214-215: Fixes maxWidth comparison logic for SecondaryHeaderSlot.

Applies the same fix as lines 203-204: correctly compares the maxWidth property values with optional chaining, instead of comparing the entire parentStyles object to a maxWidth value.

---

226-227: Adds defensive optional chaining.

The optional chaining on parentValues?.SecondaryHeaderSlot prevents potential runtime errors when parentValues is undefined, making the code more robust while checking if SecondaryHeaderSlot needs to be transferred to PrimaryHeaderSlot's parentValues.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}