hs1 demo: albany template + codex improvements to core code#1128
Open
bryanreed wants to merge 13 commits intocomponent-genfrom
Open
hs1 demo: albany template + codex improvements to core code#1128bryanreed wants to merge 13 commits intocomponent-genfrom
bryanreed wants to merge 13 commits intocomponent-genfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new HOURS constant-field module and registers it for fields of type Sequence Diagram(s)sequenceDiagram
participant User as rgba(34,139,230,0.5)
participant InternalEditor as rgba(60,179,113,0.5)
participant PuckStore as rgba(255,165,0,0.5)
participant Clipboard as rgba(128,0,128,0.5)
participant Resolver as rgba(220,20,60,0.5)
participant Document as rgba(70,130,180,0.5)
User->>InternalEditor: Trigger copyToClipboard
InternalEditor->>PuckStore: read itemSelector (s.appState?.ui?.itemSelector ?? null)
PuckStore-->>InternalEditor: itemSelector
InternalEditor->>Clipboard: write serialized item
Clipboard-->>User: copied
User->>InternalEditor: Trigger pasteFromClipboard
InternalEditor->>PuckStore: read itemSelector (s.appState?.ui?.itemSelector ?? null)
InternalEditor->>Clipboard: read serialized item
Clipboard-->>InternalEditor: item data
InternalEditor->>Resolver: resolveYextEntityField / resolveComponentData
Resolver->>Document: query stream document (if present)
Document-->>Resolver: document data
Resolver-->>InternalEditor: resolved value
InternalEditor->>PuckStore: dispatch replace with selector (itemSelector.zone/index)
PuckStore-->>InternalEditor: update applied
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/visual-editor/src/utils/resolveComponentData.tsx (1)
275-280:⚠️ Potential issue | 🟠 MajorDon’t key YextEntityField detection off generic
field/constantValuenames alone.With this guard, any literal object that happens to expose a root
fieldorconstantValueproperty is now treated as a binding and routed throughresolveYextEntityField(). That means inputs like{ field: "literal" }will resolve as KG lookups/undefinedinstead of staying literal data. Please use an explicit discriminator for YextEntityField payloads, or normalize saved field configs so they always carry one before broadening this check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/utils/resolveComponentData.tsx` around lines 275 - 280, The type guard isYextEntityField incorrectly treats any object with a top-level "field" or "constantValue" as a YextEntityField, causing plain literals like { field: "literal" } to be routed through resolveYextEntityField; update the guard to require a clear discriminator or a stricter shape before treating something as a YextEntityField (for example, check for a dedicated discriminator property or verify that "field" is an object with the expected keys/type), and then use that stricter guard in resolveComponentData where resolveYextEntityField is called so only genuine YextEntityField payloads are resolved via resolveYextEntityField.
🧹 Nitpick comments (2)
starter/src/templates/dev.tsx (1)
282-296: Consider keeping a Back to Editor action in preview mode.
openPreview()still does same-tab navigation on Line 273, so once this branch hides the toolbar there is no in-app way back to the editor. A single back action would keep the preview clean without forcing users to rely on browser chrome.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/templates/dev.tsx` around lines 282 - 296, The preview branch hides the toolbar and leaves no in-app way back to the editor; add a "Back to Editor" action when isPreviewMode is true that restores the editor view instead of relying on browser chrome. Update the component to render a Back button in the preview branch which calls a handler that either toggles the same preview state (e.g., call setIsPreviewMode(false) or a new closePreview function) or navigates programmatically back to the editor (matching how openPreview() works), ensuring the handler name (closePreview or setIsPreviewMode) is used consistently with openPreview and isPreviewMode.starter/src/registry/hs1-albany/components/Hs1AlbanyHeaderSection.tsx (1)
124-133: Consider using unique keys for array items.Using
${item.label}-${item.link}as keys could cause collisions if duplicate label/link combinations exist. While unlikely in practice, using the array index as a fallback or a unique ID would be more robust.💡 Optional: Add index to ensure unique keys
- {props.primaryLinks.map((item) => ( - <li key={`${item.label}-${item.link}`}> + {props.primaryLinks.map((item, index) => ( + <li key={`${item.label}-${item.link}-${index}`}>Apply similarly to
moreLinks(line 143) andpatientEducation.items(line 163).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany/components/Hs1AlbanyHeaderSection.tsx` around lines 124 - 133, The list item keys in the Hs1AlbanyHeaderSection component (inside props.primaryLinks.map) use `${item.label}-${item.link}` which can collide; update the key generation to ensure uniqueness by falling back to the array index or a unique id when label+link is not unique (apply the same fix for moreLinks mapping and patientEducation.items mapping), e.g., prefer a unique identifier on the item (id) and if absent append the map index to the key so every <li> key is guaranteed unique.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@starter/src/registry/hs1-albany/.captured-artifact/plan.md`:
- Around line 148-162: The checklist lists generated components and layout as
unfinished, which is misleading; either mark each completed item
(Hs1AlbanyHeaderSection, Hs1AlbanyHeroSection, Hs1AlbanyServicesSection,
Hs1AlbanyWelcomeSection, Hs1AlbanySignupFormSection,
Hs1AlbanyTestimonialsSection, Hs1AlbanyHoursSection, Hs1AlbanyLocationSection,
Hs1AlbanyContactFormSection, Hs1AlbanyFooterSection, Hs1AlbanyCopyrightSection,
registration in ve.config.tsx, and creation of defaultLayout.json) as done, or
replace the TODO checklist with a plain reference list of the added components
and artifacts so the file no longer implies work remains.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyContactFormSection.tsx`:
- Around line 90-123: The form in Hs1AlbanyContactFormSection.tsx is
non-functional and inaccessible: change the button from type="button" to
type="submit", add an onSubmit handler on the form (e.g., handleContactSubmit)
that prevents default, validates fields, and sends data (fetch/axios) or calls a
provided prop; also give each input and the textarea name and required
attributes and add visible <label> elements or aria-label attributes (or
associate labels via htmlFor/id) so the fields are accessible; alternatively, if
no submission is intended, convert the inputs/button into non-interactive text
or disable them and update copy to indicate the form is not functional. Ensure
you edit the form element, the three <input> fields, the <textarea>, and the
Submit <button> in Hs1AlbanyContactFormSection.tsx accordingly.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyFooterSection.tsx`:
- Around line 106-115: In Hs1AlbanyFooterSection, social link buttons rendered
from props.socialLinks currently only output SVG icons (Icon) inside Link,
making them inaccessible; update the Link to include an accessible name by
adding an aria-label={item.label} (or include visually-hidden text with
item.label) on the Link component so screen readers receive the label for each
Icon, keeping the existing key and cta props intact.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyHeroSection.tsx`:
- Around line 138-144: The code computes resolvedHeroImage via
resolveComponentData(props.heroImage, locale, streamDocument) but immediately
discards it and always uses capturedHeroImageUrl; replace that by using the
resolved asset's url (e.g., heroImageUrl = (resolvedHeroImage as
TranslatableAssetImage)?.url ?? capturedHeroImageUrl) and remove the void
resolvedHeroImage; ensure you reference resolvedHeroImage and fall back to
capturedHeroImageUrl so entity-selected images render correctly.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyHoursSection.tsx`:
- Around line 255-263: The desktop rendering in Hs1AlbanyHoursSection.tsx
currently splits item.hours on every " - ", which breaks multi-interval strings
from formatHoursForDay; update the conditional so if item.hours contains a comma
(", ") you render item.hours as-is, otherwise split only the first " - "
occurrence (e.g., find the firstIndexOf " - " and use substring before/after)
when constructing the two parts; reference the item.hours usage in the JSX and
adjust the split logic to use indexOf/substring (or a single split with a limit)
instead of splitting on all " - "s.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyLocationSection.tsx`:
- Around line 133-139: The distance badge in Hs1AlbanyLocationSection currently
renders a hardcoded "--", so either wire it to the real distance value or hide
the badge when no distance exists: locate the Hs1AlbanyLocationSection component
and replace the static "--" text with the actual distance variable (e.g.,
props.distance or computed formattedDistance) and ensure you format it
(round/trim) and append "mi"; alternatively, wrap the entire <div
className="mb-1 flex items-end gap-1"> block in a conditional render (e.g., only
render when distance != null/undefined) so the placeholder never appears if the
data is absent.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanySignupFormSection.tsx`:
- Around line 91-106: The form in Hs1AlbanySignupFormSection uses a plain <form>
with inputs that only have placeholders and a button set to type="button", so it
never submits and lacks accessible labels; change the button in this component
to type="submit" and add an onSubmit handler (e.g., handleSignupSubmit) on the
<form> to process/validate the name/email, or if this is purely demo chrome,
replace the <form> with a non-interactive container (or explicitly mark it as
mock) and keep the CTA as a non-submitting element; additionally, add proper
<label> elements or visible/aria-label attributes for the two inputs
(name/email) and wire them to state or form refs so the submit handler can read
values.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyTestimonialsSection.tsx`:
- Around line 104-133: The component currently only uses props.quotes[0]
(activeQuote) and always renders a single <p>, so additional testimonials and
per-quote typography are ignored; update Hs1AlbanyTestimonialsSection to iterate
over props.quotes (e.g., map) and for each quote call
resolveComponentData(quote.quote.text, locale, streamDocument) to compute
quoteText, render each testimonial block (including the quote mark, text, and
any per-quote typography props from the quote item) instead of the hard-coded
single <p>, and remove or repurpose the activeQuote variable if you keep a
single-view mode; ensure you reference props.quotes, resolveComponentData, and
any per-quote typography fields when implementing the mapping.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyWelcomeSection.tsx`:
- Around line 146-160: The subtitle and body render blocks in
Hs1AlbanyWelcomeSection are using fixed classes/styles so the passed-in
StyledTextProps (subtitle and body) fontSize, fontColor, fontWeight, and
textTransform are ignored; update the subtitle and body render logic to consume
those props: compute style entries (fontSize -> fontSize, fontColor -> color,
fontWeight -> fontWeight, textTransform -> textTransform, plus existing
fontFamily/lineHeight where applicable) and merge them into the inline style
object, and/or compute className fragments from those prop values instead of
hardcoded text sizes/colors; target the JSX elements that reference subtitle and
body in Hs1AlbanyWelcomeSection.tsx and replace the fixed className/style usage
so the StyledTextProps values are applied at render time.
---
Outside diff comments:
In `@packages/visual-editor/src/utils/resolveComponentData.tsx`:
- Around line 275-280: The type guard isYextEntityField incorrectly treats any
object with a top-level "field" or "constantValue" as a YextEntityField, causing
plain literals like { field: "literal" } to be routed through
resolveYextEntityField; update the guard to require a clear discriminator or a
stricter shape before treating something as a YextEntityField (for example,
check for a dedicated discriminator property or verify that "field" is an object
with the expected keys/type), and then use that stricter guard in
resolveComponentData where resolveYextEntityField is called so only genuine
YextEntityField payloads are resolved via resolveYextEntityField.
---
Nitpick comments:
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyHeaderSection.tsx`:
- Around line 124-133: The list item keys in the Hs1AlbanyHeaderSection
component (inside props.primaryLinks.map) use `${item.label}-${item.link}` which
can collide; update the key generation to ensure uniqueness by falling back to
the array index or a unique id when label+link is not unique (apply the same fix
for moreLinks mapping and patientEducation.items mapping), e.g., prefer a unique
identifier on the item (id) and if absent append the map index to the key so
every <li> key is guaranteed unique.
In `@starter/src/templates/dev.tsx`:
- Around line 282-296: The preview branch hides the toolbar and leaves no in-app
way back to the editor; add a "Back to Editor" action when isPreviewMode is true
that restores the editor view instead of relying on browser chrome. Update the
component to render a Back button in the preview branch which calls a handler
that either toggles the same preview state (e.g., call setIsPreviewMode(false)
or a new closePreview function) or navigates programmatically back to the editor
(matching how openPreview() works), ensuring the handler name (closePreview or
setIsPreviewMode) is used consistently with openPreview and isPreviewMode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5cd46e32-dd2d-417e-a1ee-53d35a263d57
⛔ Files ignored due to path filters (5)
starter/src/registry/hs1-albany/.captured-artifact/rendered-preview.diff.pngis excluded by!**/*.pngstarter/src/registry/hs1-albany/.captured-artifact/rendered-preview.pngis excluded by!**/*.pngstarter/src/registry/hs1-albany/.captured-artifact/screenshot.pngis excluded by!**/*.pngstarter/src/registry/hs1-albany/assets/hero-crop.pngis excluded by!**/*.pngstarter/src/registry/hs1-albany/assets/map-crop.pngis excluded by!**/*.png
📒 Files selected for processing (25)
packages/visual-editor/src/editor/YextEntityFieldSelector.tsxpackages/visual-editor/src/internal/components/InternalLayoutEditor.tsxpackages/visual-editor/src/internal/puck/constant-value-fields/Hours.tsxpackages/visual-editor/src/internal/puck/ui/UIButtonsToggle.tsxpackages/visual-editor/src/utils/resolveComponentData.test.tsxpackages/visual-editor/src/utils/resolveComponentData.tsxpackages/visual-editor/src/utils/resolveYextEntityField.tsstarter/src/registry/hs1-albany/.captured-artifact/combined.cssstarter/src/registry/hs1-albany/.captured-artifact/manifest.jsonstarter/src/registry/hs1-albany/.captured-artifact/page.htmlstarter/src/registry/hs1-albany/.captured-artifact/plan.mdstarter/src/registry/hs1-albany/components/Hs1AlbanyContactFormSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyCopyrightSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyFooterSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyHeaderSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyHeroSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyHoursSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyLocationSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyServicesSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanySignupFormSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyTestimonialsSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyWelcomeSection.tsxstarter/src/registry/hs1-albany/defaultLayout.jsonstarter/src/templates/dev.tsxstarter/src/ve.config.tsx
Comment on lines
+148
to
+162
| ## Implementation Checklist | ||
|
|
||
| - [ ] `Hs1AlbanyHeaderSection` -> `components/Hs1AlbanyHeaderSection.tsx` | ||
| - [ ] `Hs1AlbanyHeroSection` -> `components/Hs1AlbanyHeroSection.tsx` | ||
| - [ ] `Hs1AlbanyServicesSection` -> `components/Hs1AlbanyServicesSection.tsx` | ||
| - [ ] `Hs1AlbanyWelcomeSection` -> `components/Hs1AlbanyWelcomeSection.tsx` | ||
| - [ ] `Hs1AlbanySignupFormSection` -> `components/Hs1AlbanySignupFormSection.tsx` | ||
| - [ ] `Hs1AlbanyTestimonialsSection` -> `components/Hs1AlbanyTestimonialsSection.tsx` | ||
| - [ ] `Hs1AlbanyHoursSection` -> `components/Hs1AlbanyHoursSection.tsx` | ||
| - [ ] `Hs1AlbanyLocationSection` -> `components/Hs1AlbanyLocationSection.tsx` | ||
| - [ ] `Hs1AlbanyContactFormSection` -> `components/Hs1AlbanyContactFormSection.tsx` | ||
| - [ ] `Hs1AlbanyFooterSection` -> `components/Hs1AlbanyFooterSection.tsx` | ||
| - [ ] `Hs1AlbanyCopyrightSection` -> `components/Hs1AlbanyCopyrightSection.tsx` | ||
| - [ ] Register all generated components in `starter/src/ve.config.tsx` | ||
| - [ ] Generate `starter/src/registry/hs1-albany/defaultLayout.json` in final visual order |
Contributor
There was a problem hiding this comment.
This checklist will be stale as soon as this PR merges.
The PR already adds these components and the default layout, so committing this section with every box unchecked leaves misleading guidance in the repo. Please either mark the completed items or rewrite this as a reference list instead of a TODO list.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-albany/.captured-artifact/plan.md` around lines 148
- 162, The checklist lists generated components and layout as unfinished, which
is misleading; either mark each completed item (Hs1AlbanyHeaderSection,
Hs1AlbanyHeroSection, Hs1AlbanyServicesSection, Hs1AlbanyWelcomeSection,
Hs1AlbanySignupFormSection, Hs1AlbanyTestimonialsSection, Hs1AlbanyHoursSection,
Hs1AlbanyLocationSection, Hs1AlbanyContactFormSection, Hs1AlbanyFooterSection,
Hs1AlbanyCopyrightSection, registration in ve.config.tsx, and creation of
defaultLayout.json) as done, or replace the TODO checklist with a plain
reference list of the added components and artifacts so the file no longer
implies work remains.
Comment on lines
+90
to
+123
| <form className="grid gap-4 md:grid-cols-[minmax(0,1fr)_minmax(0,1fr)]"> | ||
| <div className="space-y-4"> | ||
| <input | ||
| className="h-[44px] w-full border border-white/80 bg-transparent px-3 text-[13px] text-white placeholder:text-white/80" | ||
| placeholder="Enter your name (Required)" | ||
| /> | ||
| <input | ||
| className="h-[44px] w-full border border-white/80 bg-transparent px-3 text-[13px] text-white placeholder:text-white/80" | ||
| placeholder="Enter email (Required)" | ||
| /> | ||
| <input | ||
| className="h-[44px] w-full border border-white/80 bg-transparent px-3 text-[13px] text-white placeholder:text-white/80" | ||
| placeholder="(XXX)XXX-XXXX (Required)" | ||
| /> | ||
| </div> | ||
| <textarea | ||
| className="min-h-[164px] w-full border border-white/80 bg-transparent px-3 py-3 text-[13px] text-white placeholder:text-white/80" | ||
| placeholder="Notes to the Doctor" | ||
| /> | ||
| </form> | ||
| <p | ||
| className="mb-0 mt-5 text-center text-[12px] text-white" | ||
| style={{ fontFamily: "Montserrat, Open Sans, sans-serif" }} | ||
| > | ||
| Please do not submit any Protected Health Information (PHI). | ||
| </p> | ||
| <div className="mt-5 text-center"> | ||
| <button | ||
| type="button" | ||
| className="inline-flex h-[44px] min-w-[220px] items-center justify-center border border-white/80 px-8 text-[13px] font-bold uppercase tracking-[0.08em] text-white" | ||
| style={{ fontFamily: "Nunito Sans, Open Sans, sans-serif" }} | ||
| > | ||
| Submit | ||
| </button> |
Contributor
There was a problem hiding this comment.
This section renders a dead form.
type="button" plus no form action/onSubmit means users cannot actually send anything. The placeholder-only fields also aren't sufficient labels, so the UI is inaccessible if it ships as a real form. Either wire a real submission flow or make the section clearly non-interactive.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyContactFormSection.tsx`
around lines 90 - 123, The form in Hs1AlbanyContactFormSection.tsx is
non-functional and inaccessible: change the button from type="button" to
type="submit", add an onSubmit handler on the form (e.g., handleContactSubmit)
that prevents default, validates fields, and sends data (fetch/axios) or calls a
provided prop; also give each input and the textarea name and required
attributes and add visible <label> elements or aria-label attributes (or
associate labels via htmlFor/id) so the fields are accessible; alternatively, if
no submission is intended, convert the inputs/button into non-interactive text
or disable them and update copy to indicate the form is not functional. Ensure
you edit the form element, the three <input> fields, the <textarea>, and the
Submit <button> in Hs1AlbanyContactFormSection.tsx accordingly.
Comment on lines
+106
to
+115
| {props.socialLinks.map((item) => { | ||
| const Icon = iconMap[item.icon]; | ||
| return ( | ||
| <Link | ||
| key={`${item.label}-${item.link}`} | ||
| cta={{ link: item.link, linkType: "URL" }} | ||
| className="flex h-9 w-9 items-center justify-center rounded-full bg-[#dcb65f] text-white no-underline" | ||
| > | ||
| <Icon /> | ||
| </Link> |
Contributor
There was a problem hiding this comment.
Icon-only social links need an accessible name.
These links render only SVGs, so assistive tech gets unlabeled controls. Add hidden text or an aria-label from item.label.
Suggested fix
<Link
key={`${item.label}-${item.link}`}
cta={{ link: item.link, linkType: "URL" }}
className="flex h-9 w-9 items-center justify-center rounded-full bg-[`#dcb65f`] text-white no-underline"
>
- <Icon />
+ <Icon aria-hidden="true" />
+ <span className="sr-only">{item.label}</span>
</Link>📝 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.
Suggested change
| {props.socialLinks.map((item) => { | |
| const Icon = iconMap[item.icon]; | |
| return ( | |
| <Link | |
| key={`${item.label}-${item.link}`} | |
| cta={{ link: item.link, linkType: "URL" }} | |
| className="flex h-9 w-9 items-center justify-center rounded-full bg-[#dcb65f] text-white no-underline" | |
| > | |
| <Icon /> | |
| </Link> | |
| {props.socialLinks.map((item) => { | |
| const Icon = iconMap[item.icon]; | |
| return ( | |
| <Link | |
| key={`${item.label}-${item.link}`} | |
| cta={{ link: item.link, linkType: "URL" }} | |
| className="flex h-9 w-9 items-center justify-center rounded-full bg-[`#dcb65f`] text-white no-underline" | |
| > | |
| <Icon aria-hidden="true" /> | |
| <span className="sr-only">{item.label}</span> | |
| </Link> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyFooterSection.tsx` around
lines 106 - 115, In Hs1AlbanyFooterSection, social link buttons rendered from
props.socialLinks currently only output SVG icons (Icon) inside Link, making
them inaccessible; update the Link to include an accessible name by adding an
aria-label={item.label} (or include visually-hidden text with item.label) on the
Link component so screen readers receive the label for each Icon, keeping the
existing key and cta props intact.
Comment on lines
+138
to
+144
| const resolvedHeroImage = resolveComponentData( | ||
| props.heroImage, | ||
| locale, | ||
| streamDocument, | ||
| ); | ||
| void resolvedHeroImage; | ||
| const heroImageUrl = capturedHeroImageUrl; |
Contributor
There was a problem hiding this comment.
Resolved hero image is discarded — entity-selected images won't render.
The resolvedHeroImage is computed from props.heroImage but immediately voided. The component then uses a hardcoded static asset (capturedHeroImageUrl) instead. This prevents users from seeing their entity-selected or configured hero images.
Based on context snippet 1, resolveComponentData returns a TranslatableAssetImage containing the url property that should be used.
🔧 Proposed fix to use the resolved image
const resolvedHeroImage = resolveComponentData(
props.heroImage,
locale,
streamDocument,
);
- void resolvedHeroImage;
- const heroImageUrl = capturedHeroImageUrl;
+ const heroImageUrl =
+ (resolvedHeroImage as { url?: string } | undefined)?.url ??
+ capturedHeroImageUrl;📝 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.
Suggested change
| const resolvedHeroImage = resolveComponentData( | |
| props.heroImage, | |
| locale, | |
| streamDocument, | |
| ); | |
| void resolvedHeroImage; | |
| const heroImageUrl = capturedHeroImageUrl; | |
| const resolvedHeroImage = resolveComponentData( | |
| props.heroImage, | |
| locale, | |
| streamDocument, | |
| ); | |
| const heroImageUrl = | |
| (resolvedHeroImage as { url?: string } | undefined)?.url ?? | |
| capturedHeroImageUrl; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyHeroSection.tsx` around
lines 138 - 144, The code computes resolvedHeroImage via
resolveComponentData(props.heroImage, locale, streamDocument) but immediately
discards it and always uses capturedHeroImageUrl; replace that by using the
resolved asset's url (e.g., heroImageUrl = (resolvedHeroImage as
TranslatableAssetImage)?.url ?? capturedHeroImageUrl) and remove the void
resolvedHeroImage; ensure you reference resolvedHeroImage and fall back to
capturedHeroImageUrl so entity-selected images render correctly.
Comment on lines
+255
to
+263
| {item.hours.includes(" - ") ? ( | ||
| <> | ||
| {item.hours.split(" - ")[0]} | ||
| <span> - </span> | ||
| {item.hours.split(" - ")[1]} | ||
| </> | ||
| ) : ( | ||
| item.hours | ||
| )} |
Contributor
There was a problem hiding this comment.
Edge case: Multiple intervals display incorrectly in desktop view.
When formatHoursForDay returns multiple intervals joined with ", " (e.g., "9:00 am - 12:00 pm, 1:00 pm - 5:00 pm"), splitting on " - " produces incorrect segments. The current logic would display "9:00 am" and "12:00 pm, 1:00 pm" instead of the full hours string.
The mobile view (line 286) correctly displays item.hours without splitting.
🔧 Proposed fix to handle multiple intervals
<p
className="mb-0 mt-3 text-[11px] text-[`#4f4e4e`]"
style={{ fontFamily: "Montserrat, Open Sans, sans-serif" }}
>
- {item.hours.includes(" - ") ? (
- <>
- {item.hours.split(" - ")[0]}
- <span> - </span>
- {item.hours.split(" - ")[1]}
- </>
- ) : (
- item.hours
- )}
+ {item.hours}
</p>Alternatively, if line breaking between start/end is needed, split only the first occurrence:
- {item.hours.includes(" - ") ? (
+ {item.hours.includes(" - ") && !item.hours.includes(", ") ? (
<>
{item.hours.split(" - ")[0]}
<span> - </span>
{item.hours.split(" - ")[1]}
</>
) : (
item.hours
)}📝 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.
Suggested change
| {item.hours.includes(" - ") ? ( | |
| <> | |
| {item.hours.split(" - ")[0]} | |
| <span> - </span> | |
| {item.hours.split(" - ")[1]} | |
| </> | |
| ) : ( | |
| item.hours | |
| )} | |
| {item.hours} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyHoursSection.tsx` around
lines 255 - 263, The desktop rendering in Hs1AlbanyHoursSection.tsx currently
splits item.hours on every " - ", which breaks multi-interval strings from
formatHoursForDay; update the conditional so if item.hours contains a comma (",
") you render item.hours as-is, otherwise split only the first " - " occurrence
(e.g., find the firstIndexOf " - " and use substring before/after) when
constructing the two parts; reference the item.hours usage in the JSX and adjust
the split logic to use indexOf/substring (or a single split with a limit)
instead of splitting on all " - "s.
Comment on lines
+133
to
+139
| <div className="mb-1 flex items-end gap-1"> | ||
| <span className="text-[26px] leading-none text-[#7b7b7b]"> | ||
| -- | ||
| </span> | ||
| <span className="text-[15px] font-bold uppercase tracking-[0.08em]"> | ||
| mi | ||
| </span> |
Contributor
There was a problem hiding this comment.
The distance badge is shipping a placeholder.
Nothing in this component ever replaces --, so the template will always render -- mi. Wire it to real data or remove the badge until that value exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyLocationSection.tsx`
around lines 133 - 139, The distance badge in Hs1AlbanyLocationSection currently
renders a hardcoded "--", so either wire it to the real distance value or hide
the badge when no distance exists: locate the Hs1AlbanyLocationSection component
and replace the static "--" text with the actual distance variable (e.g.,
props.distance or computed formattedDistance) and ensure you format it
(round/trim) and append "mi"; alternatively, wrap the entire <div
className="mb-1 flex items-end gap-1"> block in a conditional render (e.g., only
render when distance != null/undefined) so the placeholder never appears if the
data is absent.
Comment on lines
+91
to
+106
| <form className="grid gap-4 md:grid-cols-[1fr_1fr_auto]"> | ||
| <input | ||
| className="h-[44px] border border-white/80 bg-transparent px-3 text-[13px] text-white placeholder:text-white/80" | ||
| placeholder="Enter your name (Required)" | ||
| /> | ||
| <input | ||
| className="h-[44px] border border-white/80 bg-transparent px-3 text-[13px] text-white placeholder:text-white/80" | ||
| placeholder="Enter email (Required)" | ||
| /> | ||
| <button | ||
| type="button" | ||
| className="h-[44px] border border-white/80 px-8 text-[13px] font-bold uppercase tracking-[0.08em] text-white" | ||
| style={{ fontFamily: "Nunito Sans, Open Sans, sans-serif" }} | ||
| > | ||
| Submit | ||
| </button> |
Contributor
There was a problem hiding this comment.
The signup form can't be submitted.
This repeats the same type="button"/no submit handler pattern, so the CTA never does anything. Because the inputs rely only on placeholders, the published form would also miss proper labels. If this is demo chrome only, render it as a mock instead of a real form.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanySignupFormSection.tsx`
around lines 91 - 106, The form in Hs1AlbanySignupFormSection uses a plain
<form> with inputs that only have placeholders and a button set to
type="button", so it never submits and lacks accessible labels; change the
button in this component to type="submit" and add an onSubmit handler (e.g.,
handleSignupSubmit) on the <form> to process/validate the name/email, or if this
is purely demo chrome, replace the <form> with a non-interactive container (or
explicitly mark it as mock) and keep the CTA as a non-submitting element;
additionally, add proper <label> elements or visible/aria-label attributes for
the two inputs (name/email) and wire them to state or form refs so the submit
handler can read values.
Comment on lines
+104
to
+133
| const activeQuote = props.quotes[0]; | ||
| const quoteText = | ||
| activeQuote && | ||
| resolveComponentData(activeQuote.quote.text, locale, streamDocument); | ||
|
|
||
| return ( | ||
| <section className="bg-[#4f4e4e] px-6 py-12"> | ||
| <div className="mx-auto max-w-[1170px]"> | ||
| <h2 | ||
| className="mb-8 mt-0 text-center text-white" | ||
| style={{ | ||
| fontFamily: "Montserrat, Open Sans, sans-serif", | ||
| fontSize: `${props.heading.fontSize}px`, | ||
| fontWeight: props.heading.fontWeight, | ||
| letterSpacing: "1px", | ||
| textTransform: | ||
| props.heading.textTransform === "normal" | ||
| ? undefined | ||
| : props.heading.textTransform, | ||
| }} | ||
| > | ||
| {heading} | ||
| </h2> | ||
| <div className="mx-auto max-w-[840px] bg-white px-8 py-10 text-center"> | ||
| <div className="mx-auto mb-6 flex h-11 w-11 items-center justify-center rounded-full bg-[#d3a335] text-2xl text-white"> | ||
| “ | ||
| </div> | ||
| <p className="m-0 text-[22px] leading-8 text-[#4a4a4a]"> | ||
| {quoteText} | ||
| </p> |
Contributor
There was a problem hiding this comment.
Only the first testimonial is ever rendered.
quotes is modeled as an editable array of styled items, but this component always reads props.quotes[0] and prints it inside a hard-coded <p>. Every additional testimonial and all per-quote typography controls are ignored. Either render the array or collapse this to a single quote field so the editor matches runtime behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyTestimonialsSection.tsx`
around lines 104 - 133, The component currently only uses props.quotes[0]
(activeQuote) and always renders a single <p>, so additional testimonials and
per-quote typography are ignored; update Hs1AlbanyTestimonialsSection to iterate
over props.quotes (e.g., map) and for each quote call
resolveComponentData(quote.quote.text, locale, streamDocument) to compute
quoteText, render each testimonial block (including the quote mark, text, and
any per-quote typography props from the quote item) instead of the hard-coded
single <p>, and remove or repurpose the activeQuote variable if you keep a
single-view mode; ensure you reference props.quotes, resolveComponentData, and
any per-quote typography fields when implementing the mapping.
Comment on lines
+146
to
+160
| <p | ||
| className="mb-0 mt-5 text-[22px] text-[#d3a335]" | ||
| style={{ | ||
| fontFamily: "Montserrat, Open Sans, sans-serif", | ||
| lineHeight: "1.4", | ||
| }} | ||
| > | ||
| {subtitle} | ||
| </p> | ||
| <p | ||
| className="mb-0 mt-5 text-[14px] leading-7 text-[#7a7a7a]" | ||
| style={{ fontFamily: "Montserrat, Open Sans, sans-serif" }} | ||
| > | ||
| {body} | ||
| </p> |
Contributor
There was a problem hiding this comment.
Subtitle and body typography controls aren't wired up.
Both props are declared as StyledTextProps, but this render path never applies their fontSize, fontColor, fontWeight, or textTransform values. Edits to those controls in Puck won't show up until the styles come from props instead of fixed classes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyWelcomeSection.tsx`
around lines 146 - 160, The subtitle and body render blocks in
Hs1AlbanyWelcomeSection are using fixed classes/styles so the passed-in
StyledTextProps (subtitle and body) fontSize, fontColor, fontWeight, and
textTransform are ignored; update the subtitle and body render logic to consume
those props: compute style entries (fontSize -> fontSize, fontColor -> color,
fontWeight -> fontWeight, textTransform -> textTransform, plus existing
fontFamily/lineHeight where applicable) and merge them into the inline style
object, and/or compute className fragments from those prop values instead of
hardcoded text sizes/colors; target the JSX elements that reference subtitle and
body in Hs1AlbanyWelcomeSection.tsx and replace the fixed className/style usage
so the StyledTextProps values are applied at render time.
bryanreed
commented
Mar 30, 2026
| locale?: string; | ||
| hours?: HoursData; | ||
| }; | ||
| const entityFields = useEntityFields(); |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (13)
starter/localData/dev-location-stream__en__8573da7e3961090ddb59214a1f01e59a.json (1)
797-802: Ambiguousmonday: {}and inconsistent hours data.Two observations:
Line 797:
monday: {}is ambiguous—it lacks bothisClosed: trueandopenIntervals. The Hours.tsx constant field config expects one of these to be present. Consider explicitly setting"isClosed": trueif Monday is closed:"monday": { "isClosed": true }Lines 801-802: Saturday's
"start": "3:00"is an unusual business hour (3 AM) and uses non-zero-padded format. If intentional for edge-case testing, that's fine; otherwise, consider"03:00"or a more typical opening time.Additionally, the
hoursobject now conflicts with_schema.openingHourson line 62 which still references "Mo-" for Monday. If this fixture is meant to test edge cases with incomplete/malformed data, consider documenting that intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/localData/dev-location-stream__en__8573da7e3961090ddb59214a1f01e59a.json` around lines 797 - 802, The fixture has an ambiguous "monday" object and inconsistent Saturday time format: update the "monday" entry to explicitly declare closure or intervals (e.g., set "monday": { "isClosed": true } or provide "openIntervals") so it satisfies Hours.tsx expectations; normalize Saturday's start time to zero-padded "03:00" (or a more typical opening time) if 3 AM wasn't intentional; and reconcile the hours object with the _schema.openingHours reference to "Mo-" (ensure the Monday representation matches the schema, or add a comment in the fixture noting this is an intentional malformed/edge-case test).starter/src/registry/hs1-laguna/components/Hs1LagunaFeaturedBlocksSection.tsx (2)
185-191:resolveImageUrlis called twice; store result in a variable.The same
resolveImageUrl(card.image)call is made in both the condition check and the style. Extract to a variable in the map callback for efficiency.Suggested fix
- {cards.map((card) => ( + {cards.map((card) => { + const imageUrl = resolveImageUrl(card.image); + return ( <article key={`${card.title}-${card.cta.link}`} className="relative flex min-h-[520px] items-end justify-center overflow-hidden px-[15px] pb-[43px] pt-8 text-white md:min-h-[540px] md:px-5" > - {resolveImageUrl(card.image) ? ( + {imageUrl ? ( <div className="absolute inset-0 bg-cover bg-center" style={{ - backgroundImage: `url(${resolveImageUrl(card.image)})`, + backgroundImage: `url(${imageUrl})`, }} /> ) : null} ... </article> - ))} + ); + })}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-laguna/components/Hs1LagunaFeaturedBlocksSection.tsx` around lines 185 - 191, In Hs1LagunaFeaturedBlocksSection's map callback, avoid calling resolveImageUrl(card.image) twice by computing a const (e.g., imageUrl) at the top of the callback and using that variable for the conditional render and for the backgroundImage style; update the JSX that currently uses resolveImageUrl(card.image) in both the ternary and the style to reference imageUrl instead.
32-36: Props use fixedcardOne/cardTwo/cardThreeinstead of an array.This limits flexibility to exactly 3 cards. If the design requirement is always 3 cards, this is fine. If variable card counts may be needed, consider refactoring to a
cardsarray similar to other section components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-laguna/components/Hs1LagunaFeaturedBlocksSection.tsx` around lines 32 - 36, Hs1LagunaFeaturedBlocksSectionProps currently defines fixed cardOne/cardTwo/cardThree which prevents variable counts; change the prop shape to cards: FeaturedCardProps[] and update the Hs1LagunaFeaturedBlocksSection component to accept props.cards and render them via an array map (e.g., props.cards.map((card, idx) => ...)) instead of accessing cardOne/cardTwo/cardThree; also update any usage sites of Hs1LagunaFeaturedBlocksSection to pass an array `cards` and adjust TypeScript types/imports accordingly.starter/src/registry/hs1-chicago/components/Hs1ChicagoContactFormSection.tsx (2)
160-177: Form inputs lack accessibility associations.The form inputs use
placeholderfor labeling but lack proper<label>elements oraria-labelattributes. Screen readers benefit from explicit labels. Since this is a demo/preview component, this may be acceptable, but addingaria-labelwould be a low-effort accessibility improvement.Suggested improvement for one input
field.inputType === "textarea" ? ( <textarea key={`${field.placeholder}-${index}`} placeholder={field.placeholder} + aria-label={field.placeholder} rows={5} className="block w-full border border-white/80 bg-white/85 px-4 py-3 text-sm text-[`#5b5554`] placeholder:text-[`#8f8786`]" /> ) : ( <input key={`${field.placeholder}-${index}`} type={field.inputType} placeholder={field.placeholder} + aria-label={field.placeholder} className="block w-full border border-white/80 bg-white/85 px-4 py-3 text-sm text-[`#5b5554`] placeholder:text-[`#8f8786`]" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-chicago/components/Hs1ChicagoContactFormSection.tsx` around lines 160 - 177, Hs1ChicagoContactFormSection's form rendering maps props.fields to inputs/textarea but uses only placeholder text, lacking accessible labels; update the mapping in Hs1ChicagoContactFormSection to provide an accessible name for each control by adding an aria-label (e.g., aria-label={field.label || field.placeholder}) or rendering a visually-hidden <label> tied to the input via id, using props.fields and field.inputType/field.placeholder to generate unique keys/ids so screen readers can correctly associate labels with inputs.
191-200: Button usestype="button"— form will not submit.The submit button has
type="button"which prevents form submission. If this is intentional for a preview-only component, consider changing the label from "Submit" to clarify, or usetype="submit"withe.preventDefault()if form submission feedback is desired in the editor preview.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-chicago/components/Hs1ChicagoContactFormSection.tsx` around lines 191 - 200, The button in Hs1ChicagoContactFormSection.tsx is currently rendered with type="button" which prevents the form from submitting; update the button inside the Hs1ChicagoContactFormSection component so its role matches intent: if it should submit the form change the button to type="submit" (so native form submission works with the form's onSubmit handler), otherwise keep type="button" but change props.submitLabel to a preview-specific label like "Preview" or ensure the surrounding preview handler calls e.preventDefault() if you want to simulate submission; locate the JSX button that uses props.submitLabel to make this change.starter/src/registry/hs1-carmel/components/Hs1CarmelServicesSection.tsx (1)
11-17:textTransform: "none"is valid but inconsistent with other templates.Carmel components use
"none"while Chicago/Laguna use"normal". Both work in CSS (text-transform: noneis valid), but"normal"is more semantically correct sincenonetypically means "remove inherited transform." Consider standardizing across all templates for maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-carmel/components/Hs1CarmelServicesSection.tsx` around lines 11 - 17, The textTransform union in StyledTextProps uses "none" which is inconsistent with other templates; update the type for textTransform in StyledTextProps to use "normal" instead of "none" (i.e., change the union member referenced by the textTransform property) and update any usages of Hs1CarmelServicesSection/StyledTextProps that pass "none" to pass "normal" to keep behavior and semantics consistent across templates.starter/src/registry/hs1-carmel/components/Hs1CarmelFooterSection.tsx (2)
222-240: Consider using index in the key for uniqueness.The composite key
${item.label}-${item.link}could produce duplicates if the user adds two social links with identical label and link values. While unlikely, using the index as part of the key would guarantee uniqueness.Suggested fix
- {props.socialLinks.map((item) => { + {props.socialLinks.map((item, index) => { const Icon = iconMap[item.icon]; return ( <Link - key={`${item.label}-${item.link}`} + key={`${item.label}-${item.link}-${index}`} cta={{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-carmel/components/Hs1CarmelFooterSection.tsx` around lines 222 - 240, The key used in the props.socialLinks.map render (`${item.label}-${item.link}`) can collide if two items share label and link; update the map callback in Hs1CarmelFooterSection (the props.socialLinks.map(...) that returns <Link ... />) to include the iteration index (e.g., use a composite key like `${item.label}-${item.link}-${index}`) or otherwise incorporate the index variable to guarantee uniqueness; ensure you add the index parameter to the map callback signature so you can reference it when building the key.
14-20: InconsistenttextTransformvalue:"none"vs"normal".This file uses
"none"for the neutral text transform value, but other HS1 component files in this PR (e.g., Chicago, Laguna) use"normal". While both technically work (CSStext-transform: noneis valid), consistency across components would improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-carmel/components/Hs1CarmelFooterSection.tsx` around lines 14 - 20, The StyledTextProps type in Hs1CarmelFooterSection.tsx uses textTransform: "none" while other HS1 components use "normal"; update the type definition for StyledTextProps (symbol name: StyledTextProps) to use textTransform: "normal" instead of "none" so the prop's allowed values match other components and maintain consistency across the codebase.starter/src/registry/hs1-laguna/components/Hs1LagunaWelcomeSection.tsx (1)
165-204: Static content is hardcoded in the component.The welcome paragraphs contain hardcoded text that cannot be edited through the Visual Editor. This appears intentional for a demo template, but if this content should be configurable, consider adding additional
StyledTextPropsfields or a rich text field.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-laguna/components/Hs1LagunaWelcomeSection.tsx` around lines 165 - 204, The component Hs1LagunaWelcomeSection.tsx currently hardcodes the three welcome paragraphs making them uneditable in the Visual Editor; update the component to accept configurable text props (e.g., add fields to StyledTextProps such as welcomeParagraph, aboutParagraph, invitationParagraph or a single richTextHtml prop) and replace the hardcoded strings with those props (while preserving existing Link usage for props.emailLink and props.appointmentLink); also update the StyledTextProps/interface and any parent usages to pass the new fields or a rich text value so the content becomes editable.starter/src/registry/hs1-carmel/components/Hs1CarmelContactFormSection.tsx (1)
13-97: Consider extracting the styled-text field builder.This schema/helper block is now duplicated across several starter components (
Hs1CarmelWelcomeSection,Hs1ChicagoOfferSection,Hs1ChicagoTestimonialsSection,Hs1LagunaNewsletterSection). The copies are already drifting (textTransformis"none"in some files and"normal"in others), so a small shared helper would make future starter tweaks safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-carmel/components/Hs1CarmelContactFormSection.tsx` around lines 13 - 97, Extract the duplicated styled-text builder into a shared helper (e.g., export a createStyledTextField function and StyledTextProps type from a common module) and replace the local createStyledTextField definitions in Hs1CarmelContactFormSection and the other components (Hs1CarmelWelcomeSection, Hs1ChicagoOfferSection, Hs1ChicagoTestimonialsSection, Hs1LagunaNewsletterSection) to import and reuse that helper; ensure the shared builder defines objectFields for text, fontSize, fontColor, fontWeight and textTransform and standardize the textTransform option value to "none" (not "normal"), then update Hs1CarmelContactFormSectionFields to call the imported createStyledTextField("Heading") / createStyledTextField("Body") and remove the local duplicate types/definitions, adding any necessary imports.starter/src/registry/hs1-chicago/components/Hs1ChicagoStaffSection.tsx (1)
240-251: Guard against undefinedmember.ctafor defensive safety.While
defaultItemPropsanddefaultPropsboth definectawithlabelandlink, if a member entry is ever saved without actaobject (e.g., via manual JSON editing or migration), accessingmember.cta.linkwould throw a TypeError.Consider adding optional chaining:
Proposed fix
- {member.cta.link && ( + {member.cta?.link && ( <div className="mt-5"> - <Link cta={{ link: member.cta.link, linkType: "URL" }}> + <Link cta={{ link: member.cta?.link, linkType: "URL" }}> <span className="text-[15px] uppercase text-[`#815955`] underline underline-offset-4" style={{ fontFamily: "'Oswald', Verdana, sans-serif" }} > - {member.cta.label} + {member.cta?.label} </span> </Link> </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-chicago/components/Hs1ChicagoStaffSection.tsx` around lines 240 - 251, In Hs1ChicagoStaffSection, guard access to member.cta before reading member.cta.link to avoid a TypeError when cta is undefined; update the rendering condition around the Link (and any uses of member.cta.label) to use optional chaining or a safe default (e.g., check member.cta && member.cta.link or use member.cta?.link and member.cta?.label) so the Link is only rendered when a link exists and label access is safe.starter/src/registry/hs1-laguna/components/Hs1LagunaHeaderSection.tsx (2)
277-289: Consider using a more unique key for list items.The key
${item.label}-${item.link}could produce duplicates if the same label and link combination appears multiple times in the array (though unlikely in practice). Using the index as a fallback or combining with index would be safer.Proposed fix for primaryLinks
{props.primaryLinks.map((item, idx) => ( <li - key={`${item.label}-${item.link}`} + key={`${item.label}-${item.link}-${idx}`} className="border-b border-black/10" >Proposed fix for patientEducationLinks
{props.patientEducationLinks.map((item, idx) => ( <li - key={`${item.label}-${item.link}`} + key={`${item.label}-${item.link}-${idx}`} className="border-t border-black/10" >Also applies to: 313-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-laguna/components/Hs1LagunaHeaderSection.tsx` around lines 277 - 289, The list item keys in Hs1LagunaHeaderSection use `${item.label}-${item.link}` which can collide; update the key generation for both props.primaryLinks (the mapped block using key={`${item.label}-${item.link}`}) and props.patientEducationLinks (the similar mapped block around lines ~313-325) to include the array index as a fallback (e.g., `${item.label}-${item.link}-${index}`) or otherwise derive a guaranteed-unique id per item so keys are unique across duplicates.
293-299: Minor: Conflicting border styles on submenu toggle button.The button has
border-0which removes all borders, but thenborder-r border-black/10attempts to add a right border. Due to CSS specificity with Tailwind,border-0setsborder-width: 0pxon all sides, which may prevent theborder-rfrom appearing correctly.Proposed fix
<button type="button" onClick={() => setSubmenuOpen((open) => !open)} - className="w-[50px] border-0 border-r border-black/10 bg-transparent text-black/40" + className="w-[50px] border-0 border-r border-r-black/10 bg-transparent text-black/40" >Or more explicitly:
- className="w-[50px] border-0 border-r border-black/10 bg-transparent text-black/40" + className="w-[50px] border-y-0 border-l-0 border-r border-r-black/10 bg-transparent text-black/40"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-laguna/components/Hs1LagunaHeaderSection.tsx` around lines 293 - 299, The submenu toggle button in Hs1LagunaHeaderSection (the <button> with onClick={() => setSubmenuOpen((open) => !open)}) uses both "border-0" and "border-r border-black/10", causing the right border to be suppressed; remove "border-0" and keep "border-r border-black/10" (or explicitly set other sides to 0 with "border-t-0 border-l-0 border-b-0 border-r border-black/10") so the right border renders correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelCopyrightSection.tsx`:
- Around line 69-71: Replace the protocol-relative URL used for the default
copyright link with an explicit HTTPS URL: locate the object literal in
Hs1CarmelCopyrightSection (the entry with label "Copyright © 2026 MH Sub I, LLC
dba Officite" and property link) and change link from
"//www.henryscheinone.com/products/officite" to
"https://www.henryscheinone.com/products/officite" so the component always uses
a secure scheme.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelHeaderSection.tsx`:
- Around line 205-212: The code currently determines submenus by comparing
link.label which breaks on renames/localization; update the model to include a
non-editable discriminator like menuKey on each primaryLinks item and change the
checks in the desktop and mobile render paths (where props.primaryLinks.map is
used and the mobile loop handling the same logic) to switch on link.menuKey
(e.g., 'about', 'contact') to select props.aboutLinks or props.contactLinks
instead of comparing link.label; also ensure defaults/layout data set the new
menuKey values for the corresponding primaryLinks entries.
- Around line 228-240: The dropdown is only exposed via group-hover making
keyboard users unable to reach nested links; update the popover div (the element
rendering dropdownLinks in Hs1CarmelHeaderSection) to also become visible on
keyboard focus by adding the Tailwind utilities group-focus-within:visible and
group-focus-within:opacity-100 (or implement an explicit open state toggled on
focus/blur) and ensure the trigger element that wraps this group is focusable
(e.g., a button or has tabIndex) so renderLink/trigger receives keyboard focus;
also keep existing hover classes so both mouse and keyboard users can open the
menu.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelLocationSection.tsx`:
- Around line 118-123: Hs1CarmelLocationSection renders HoursStatus
unconditionally which can receive undefined; wrap the HoursStatus component in
the same conditional used for Address and mainPhone so it's only rendered when
streamDocument.hours exists. Locate HoursStatus and the existing checks around
Address/mainPhone in Hs1CarmelLocationSection and apply the same guard (e.g., if
(streamDocument.hours) { <HoursStatus ... /> }) to prevent passing undefined
into HoursStatus.
In `@starter/src/registry/hs1-chicago/components/Hs1ChicagoHeaderSection.tsx`:
- Around line 219-273: The education submenu is currently only visible via
desktop hover and hidden on small screens; update the <li className="group
relative"> trigger and the submenu <div className="... max-md:hidden"> so the
submenu is reachable via keyboard and mobile: (1) make the trigger focusable
(use the existing Link/span inside the group or wrap it in a button/link with
tabIndex) so it can receive focus and add the group-focus-within variants (e.g.,
group-focus-within:pointer-events-auto and group-focus-within:opacity-100) to
the panel to reveal it on keyboard focus (target the panel div that currently
has pointer-events-none/opacity-0/group-hover styles); (2) add a small-screen
expansion path by rendering a mobile-friendly disclosure (either a duplicated
stacked list of props.educationMenu.subLinks below the trigger visible on max-md
via responsive classes or toggle state) — ensure the mobile block is visible on
small screens (e.g., hidden md:block for desktop panel and block md:hidden for
mobile list) and reuse the same rendering of props.educationMenu.subLinks/items
so both Link and non-link branches are preserved.
In `@starter/src/registry/hs1-chicago/components/Hs1ChicagoOfferSection.tsx`:
- Around line 159-190: The form in Hs1ChicagoOfferSection renders inputs but
never submits or exposes their values; either make it visually inert or wire a
real submit flow: add an onSubmit prop (e.g., props.onSubmit: (data) => void) to
Hs1ChicagoOfferSection, change the <form> to <form onSubmit={handleSubmit}> and
the CTA button to type="submit", implement handleSubmit to collect values (use
controlled state per props.fields or new state like inputs[index] updated via
onChange) or build a FormData and call props.onSubmit(formDataOrObject) (and
call event.preventDefault() if needed); alternatively, if this is purely mock
UI, set inputs and button to readOnly/disabled and keep button type="button" to
prevent accidental data loss. Ensure references to props.fields,
props.submitLabel, and resolveText remain unchanged.
In
`@starter/src/registry/hs1-chicago/components/Hs1ChicagoTestimonialsSection.tsx`:
- Around line 165-205: Hs1ChicagoTestimonialsSection currently only renders
props.items[index] and the pagination dots are passive spans, so users cannot
switch testimonials; update the component to either render all testimonials or
(preferred) make the dots interactive: change the dot elements rendered from
<span> to buttons (or add role/button and keyboard handlers) and wire an
onClick/onKeyDown that updates the component's active index state (the same
state variable named index/activeItemIndex used to select props.items[index]);
ensure accessibility by setting aria-current/aria-label for each dot and prevent
relying on a prop-only read-only index by using useState for the active index or
exposing a callback prop to control it externally (modify
Hs1ChicagoTestimonialsSection so dot clicks update the displayed item via the
index state or controlled prop).
In
`@starter/src/registry/hs1-laguna/components/Hs1LagunaFeaturedArticlesSection.tsx`:
- Around line 44-46: The carousel leaves activeIndex pointing past the end when
articles are removed; add logic in Hs1LagunaFeaturedArticlesSection to clamp
activeIndex whenever articles change by computing a boundedIndex = Math.max(0,
Math.min(activeIndex, articles.length - 1)) and updating
setActiveIndex(boundedIndex) (or use the boundedIndex when deriving
activeArticle) so arrows and dots reflect the valid index; implement this in a
useEffect that watches articles (or articles.length) and ensure activeArticle
uses the clamped index rather than the stale activeIndex.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaFooterSection.tsx`:
- Around line 78-79: In Hs1LagunaFooterSection update the CTA passed to the Link
component so it receives the raw phone number instead of a tel: URI; change the
cta prop from cta={{ link: `tel:${phoneNumber}`, linkType: "PHONE" }} to use
link: phoneNumber (keep linkType: "PHONE") so `@yext/pages-components` will format
the tel: URI correctly—look for the cta prop in the Hs1LagunaFooterSection
component and adjust the link value accordingly.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaHeroSection.tsx`:
- Around line 42-43: The activeIndex state can become out-of-range when slides
change; update the component to always use a clamped index: create a clamp
helper and derive a boundedIndex = clamp(activeIndex, 0, slides.length - 1) (or
clamp/set activeIndex inside a useEffect when slides.length changes), replace
uses of activeIndex with boundedIndex for computing activeSlide and rendering
dots, and update navigation handlers (the arrow handlers and any setActiveIndex
calls) to compute the next index via clamp(currentOrPrevIndex ± 1, 0,
slides.length - 1) so state and UI remain consistent when slides are removed or
added.
In
`@starter/src/registry/hs1-laguna/components/Hs1LagunaLocationHoursSection.tsx`:
- Around line 3-16: The component currently only exposes title/caption props
(Hs1LagunaLocationHoursSectionProps and Hs1LagunaLocationHoursSectionFields)
while address, map tiles and the weekly hours are hard-coded; update the props
and fields to accept configurable office data (e.g., address lines, mapTiles
URL/string, and a weeklyHours structure or a resolvedHours prop) and use those
props inside the Hs1LagunaLocationHoursSection render logic instead of the fixed
Downers Grove values; also ensure the hours prop type matches the shared-hours
resolution format used elsewhere (or accept resolvedHours and render it) so the
component uses the shared-hours resolution work rather than bypassing it.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaNewsletterSection.tsx`:
- Around line 146-179: The form in Hs1LagunaNewsletterSection is a real <form>
with a type="submit" button and no handler, causing page navigation on submit;
stop the default submit by adding an onSubmit handler on the <form> that calls
event.preventDefault() (and optionally also change the Submit button in the
component to type="button") so clicking Submit or pressing Enter won't post back
the page—update the Hs1LagunaNewsletterSection form element and the Submit
button accordingly.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaOfferFormSection.tsx`:
- Around line 174-179: The submit button in Hs1LagunaOfferFormSection is using
type="submit" which will trigger a native browser form submission (page reload)
because there is no onSubmit handler preventing default; either change the
button to type="button" to match Hs1ChicagoContactFormSection or add an onSubmit
handler on the enclosing form in Hs1LagunaOfferFormSection that calls
e.preventDefault() and performs the intended submit logic; locate the JSX for
the button (the element with className "inline-flex min-w-[184px]...") and
implement one of these two fixes so clicking it doesn't cause a full page
reload.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaStaffSection.tsx`:
- Around line 49-50: The current state activeIndex can go out of range when
members changes; derive a bounded/clamped index and use it everywhere instead of
raw activeIndex: compute const clampedIndex = Math.max(0, Math.min(activeIndex,
members.length - 1)) (or clamp in a useEffect that calls setActiveIndex) and
replace usages of activeIndex for rendering activeMember and dot selection with
clampedIndex; update the arrow handlers (the next/prev functions that call
setActiveIndex) to base their math on members.length and set a clamped new index
via setActiveIndex(clampedNewIndex) so state never references an invalid member.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaTestimonialsSection.tsx`:
- Around line 76-88: The prev/next carousel buttons in
Hs1LagunaTestimonialsSection render only glyph text and need explicit accessible
names: update the button elements (the one that calls setActiveIndex with
activeIndex === 0 ? testimonials.length - 1 : activeIndex - 1 and the
corresponding next-button that advances the index) to include aria-label
attributes like aria-label="Previous testimonial" and aria-label="Next
testimonial"; apply the same change to the other button instance in this
component (the block around the second occurrence currently at the other range)
so screen readers get clear control names.
---
Nitpick comments:
In
`@starter/localData/dev-location-stream__en__8573da7e3961090ddb59214a1f01e59a.json`:
- Around line 797-802: The fixture has an ambiguous "monday" object and
inconsistent Saturday time format: update the "monday" entry to explicitly
declare closure or intervals (e.g., set "monday": { "isClosed": true } or
provide "openIntervals") so it satisfies Hours.tsx expectations; normalize
Saturday's start time to zero-padded "03:00" (or a more typical opening time) if
3 AM wasn't intentional; and reconcile the hours object with the
_schema.openingHours reference to "Mo-" (ensure the Monday representation
matches the schema, or add a comment in the fixture noting this is an
intentional malformed/edge-case test).
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelContactFormSection.tsx`:
- Around line 13-97: Extract the duplicated styled-text builder into a shared
helper (e.g., export a createStyledTextField function and StyledTextProps type
from a common module) and replace the local createStyledTextField definitions in
Hs1CarmelContactFormSection and the other components (Hs1CarmelWelcomeSection,
Hs1ChicagoOfferSection, Hs1ChicagoTestimonialsSection,
Hs1LagunaNewsletterSection) to import and reuse that helper; ensure the shared
builder defines objectFields for text, fontSize, fontColor, fontWeight and
textTransform and standardize the textTransform option value to "none" (not
"normal"), then update Hs1CarmelContactFormSectionFields to call the imported
createStyledTextField("Heading") / createStyledTextField("Body") and remove the
local duplicate types/definitions, adding any necessary imports.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelFooterSection.tsx`:
- Around line 222-240: The key used in the props.socialLinks.map render
(`${item.label}-${item.link}`) can collide if two items share label and link;
update the map callback in Hs1CarmelFooterSection (the
props.socialLinks.map(...) that returns <Link ... />) to include the iteration
index (e.g., use a composite key like `${item.label}-${item.link}-${index}`) or
otherwise incorporate the index variable to guarantee uniqueness; ensure you add
the index parameter to the map callback signature so you can reference it when
building the key.
- Around line 14-20: The StyledTextProps type in Hs1CarmelFooterSection.tsx uses
textTransform: "none" while other HS1 components use "normal"; update the type
definition for StyledTextProps (symbol name: StyledTextProps) to use
textTransform: "normal" instead of "none" so the prop's allowed values match
other components and maintain consistency across the codebase.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelServicesSection.tsx`:
- Around line 11-17: The textTransform union in StyledTextProps uses "none"
which is inconsistent with other templates; update the type for textTransform in
StyledTextProps to use "normal" instead of "none" (i.e., change the union member
referenced by the textTransform property) and update any usages of
Hs1CarmelServicesSection/StyledTextProps that pass "none" to pass "normal" to
keep behavior and semantics consistent across templates.
In
`@starter/src/registry/hs1-chicago/components/Hs1ChicagoContactFormSection.tsx`:
- Around line 160-177: Hs1ChicagoContactFormSection's form rendering maps
props.fields to inputs/textarea but uses only placeholder text, lacking
accessible labels; update the mapping in Hs1ChicagoContactFormSection to provide
an accessible name for each control by adding an aria-label (e.g.,
aria-label={field.label || field.placeholder}) or rendering a visually-hidden
<label> tied to the input via id, using props.fields and
field.inputType/field.placeholder to generate unique keys/ids so screen readers
can correctly associate labels with inputs.
- Around line 191-200: The button in Hs1ChicagoContactFormSection.tsx is
currently rendered with type="button" which prevents the form from submitting;
update the button inside the Hs1ChicagoContactFormSection component so its role
matches intent: if it should submit the form change the button to type="submit"
(so native form submission works with the form's onSubmit handler), otherwise
keep type="button" but change props.submitLabel to a preview-specific label like
"Preview" or ensure the surrounding preview handler calls e.preventDefault() if
you want to simulate submission; locate the JSX button that uses
props.submitLabel to make this change.
In `@starter/src/registry/hs1-chicago/components/Hs1ChicagoStaffSection.tsx`:
- Around line 240-251: In Hs1ChicagoStaffSection, guard access to member.cta
before reading member.cta.link to avoid a TypeError when cta is undefined;
update the rendering condition around the Link (and any uses of
member.cta.label) to use optional chaining or a safe default (e.g., check
member.cta && member.cta.link or use member.cta?.link and member.cta?.label) so
the Link is only rendered when a link exists and label access is safe.
In
`@starter/src/registry/hs1-laguna/components/Hs1LagunaFeaturedBlocksSection.tsx`:
- Around line 185-191: In Hs1LagunaFeaturedBlocksSection's map callback, avoid
calling resolveImageUrl(card.image) twice by computing a const (e.g., imageUrl)
at the top of the callback and using that variable for the conditional render
and for the backgroundImage style; update the JSX that currently uses
resolveImageUrl(card.image) in both the ternary and the style to reference
imageUrl instead.
- Around line 32-36: Hs1LagunaFeaturedBlocksSectionProps currently defines fixed
cardOne/cardTwo/cardThree which prevents variable counts; change the prop shape
to cards: FeaturedCardProps[] and update the Hs1LagunaFeaturedBlocksSection
component to accept props.cards and render them via an array map (e.g.,
props.cards.map((card, idx) => ...)) instead of accessing
cardOne/cardTwo/cardThree; also update any usage sites of
Hs1LagunaFeaturedBlocksSection to pass an array `cards` and adjust TypeScript
types/imports accordingly.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaHeaderSection.tsx`:
- Around line 277-289: The list item keys in Hs1LagunaHeaderSection use
`${item.label}-${item.link}` which can collide; update the key generation for
both props.primaryLinks (the mapped block using
key={`${item.label}-${item.link}`}) and props.patientEducationLinks (the similar
mapped block around lines ~313-325) to include the array index as a fallback
(e.g., `${item.label}-${item.link}-${index}`) or otherwise derive a
guaranteed-unique id per item so keys are unique across duplicates.
- Around line 293-299: The submenu toggle button in Hs1LagunaHeaderSection (the
<button> with onClick={() => setSubmenuOpen((open) => !open)}) uses both
"border-0" and "border-r border-black/10", causing the right border to be
suppressed; remove "border-0" and keep "border-r border-black/10" (or explicitly
set other sides to 0 with "border-t-0 border-l-0 border-b-0 border-r
border-black/10") so the right border renders correctly.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaWelcomeSection.tsx`:
- Around line 165-204: The component Hs1LagunaWelcomeSection.tsx currently
hardcodes the three welcome paragraphs making them uneditable in the Visual
Editor; update the component to accept configurable text props (e.g., add fields
to StyledTextProps such as welcomeParagraph, aboutParagraph, invitationParagraph
or a single richTextHtml prop) and replace the hardcoded strings with those
props (while preserving existing Link usage for props.emailLink and
props.appointmentLink); also update the StyledTextProps/interface and any parent
usages to pass the new fields or a rich text value so the content becomes
editable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5a7086fb-90b2-4b1b-bfb6-b0de2bf8dfd7
⛔ Files ignored due to path filters (2)
packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 64 static image.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 64 static image.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (40)
starter/localData/dev-location-stream__en__8573da7e3961090ddb59214a1f01e59a.jsonstarter/src/registry/hs1-albany/components/Hs1AlbanyHeroSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyHoursSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyLocationSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelContactFormSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelCopyrightSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelFooterSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelHeaderSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelHeroSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelLocationSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelQuickLinksSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelServicesSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelWelcomeSection.tsxstarter/src/registry/hs1-carmel/defaultLayout.jsonstarter/src/registry/hs1-chicago/components/Hs1ChicagoContactFormSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoCopyrightSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoFeaturedServicesSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoHeaderSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoHeroSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoInsuranceSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoLocationHoursSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoOfferSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoStaffSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoTestimonialsSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoWelcomeSection.tsxstarter/src/registry/hs1-chicago/defaultLayout.jsonstarter/src/registry/hs1-laguna/components/Hs1LagunaCopyrightSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaFeaturedArticlesSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaFeaturedBlocksSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaFooterSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaHeaderSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaHeroSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaLocationHoursSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaNewsletterSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaOfferFormSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaStaffSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaTestimonialsSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaWelcomeSection.tsxstarter/src/registry/hs1-laguna/defaultLayout.jsonstarter/src/ve.config.tsx
✅ Files skipped from review due to trivial changes (2)
- starter/src/registry/hs1-carmel/defaultLayout.json
- starter/src/registry/hs1-laguna/defaultLayout.json
🚧 Files skipped from review as they are similar to previous changes (2)
- starter/src/registry/hs1-albany/components/Hs1AlbanyLocationSection.tsx
- starter/src/registry/hs1-albany/components/Hs1AlbanyHoursSection.tsx
Comment on lines
+69
to
+71
| label: "Copyright © 2026 MH Sub I, LLC dba Officite", | ||
| link: "//www.henryscheinone.com/products/officite", | ||
| }, |
Contributor
There was a problem hiding this comment.
Prefer explicit HTTPS over protocol-relative URL in default link.
Using //... can inherit http in insecure contexts. Use https://... directly.
Proposed fix
- link: "//www.henryscheinone.com/products/officite",
+ link: "https://www.henryscheinone.com/products/officite",📝 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.
Suggested change
| label: "Copyright © 2026 MH Sub I, LLC dba Officite", | |
| link: "//www.henryscheinone.com/products/officite", | |
| }, | |
| label: "Copyright © 2026 MH Sub I, LLC dba Officite", | |
| link: "https://www.henryscheinone.com/products/officite", | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelCopyrightSection.tsx`
around lines 69 - 71, Replace the protocol-relative URL used for the default
copyright link with an explicit HTTPS URL: locate the object literal in
Hs1CarmelCopyrightSection (the entry with label "Copyright © 2026 MH Sub I, LLC
dba Officite" and property link) and change link from
"//www.henryscheinone.com/products/officite" to
"https://www.henryscheinone.com/products/officite" so the component always uses
a secure scheme.
Comment on lines
+205
to
+212
| {props.primaryLinks.map((link) => { | ||
| const isAbout = link.label === "About"; | ||
| const isContact = link.label === "Contact"; | ||
| const dropdownLinks = isAbout | ||
| ? props.aboutLinks | ||
| : isContact | ||
| ? props.contactLinks | ||
| : []; |
Contributor
There was a problem hiding this comment.
Use an explicit menu key instead of matching the label text.
Lines 206-212 and 284-290 decide whether a link owns aboutLinks/contactLinks by comparing link.label. If an editor renames or localizes “About” or “Contact”, the submenu wiring breaks even though the child links are still configured. Store a non-editable discriminator on the link model and switch on that instead.
Representative patch
type HeaderLink = {
label: string;
link: string;
+ menuKey?: "about" | "contact";
};
@@
- const isAbout = link.label === "About";
- const isContact = link.label === "Contact";
- const dropdownLinks = isAbout
- ? props.aboutLinks
- : isContact
- ? props.contactLinks
- : [];
+ const dropdownLinks =
+ link.menuKey === "about"
+ ? props.aboutLinks
+ : link.menuKey === "contact"
+ ? props.contactLinks
+ : [];Then set menuKey on the corresponding entries in primaryLinks defaults/layout data, and apply the same switch in the mobile loop.
Also applies to: 283-290
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelHeaderSection.tsx` around
lines 205 - 212, The code currently determines submenus by comparing link.label
which breaks on renames/localization; update the model to include a non-editable
discriminator like menuKey on each primaryLinks item and change the checks in
the desktop and mobile render paths (where props.primaryLinks.map is used and
the mobile loop handling the same logic) to switch on link.menuKey (e.g.,
'about', 'contact') to select props.aboutLinks or props.contactLinks instead of
comparing link.label; also ensure defaults/layout data set the new menuKey
values for the corresponding primaryLinks entries.
Comment on lines
+228
to
+240
| {dropdownLinks.length > 0 && ( | ||
| <div className="invisible absolute left-0 top-full z-10 mt-4 min-w-[220px] rounded-xl border border-white/10 bg-[#032a3d] p-3 opacity-0 shadow-xl transition group-hover:visible group-hover:opacity-100"> | ||
| <ul className="space-y-2"> | ||
| {dropdownLinks.map((item) => ( | ||
| <li key={`${item.label}-${item.link}`}> | ||
| {renderLink( | ||
| item, | ||
| "block rounded-lg px-3 py-2 text-sm text-white no-underline transition hover:bg-white/10 hover:text-[#7CB0D3]", | ||
| )} | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Desktop dropdowns need a focus path.
At Line 229, the popover becomes visible only on group-hover. On large screens that leaves the nested links unreachable for keyboard users. Add group-focus-within:visible/opacity-100 or manage the open state explicitly.
Suggested patch
- <div className="invisible absolute left-0 top-full z-10 mt-4 min-w-[220px] rounded-xl border border-white/10 bg-[`#032a3d`] p-3 opacity-0 shadow-xl transition group-hover:visible group-hover:opacity-100">
+ <div className="invisible absolute left-0 top-full z-10 mt-4 min-w-[220px] rounded-xl border border-white/10 bg-[`#032a3d`] p-3 opacity-0 shadow-xl transition group-hover:visible group-hover:opacity-100 group-focus-within:visible group-focus-within:opacity-100">📝 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.
Suggested change
| {dropdownLinks.length > 0 && ( | |
| <div className="invisible absolute left-0 top-full z-10 mt-4 min-w-[220px] rounded-xl border border-white/10 bg-[#032a3d] p-3 opacity-0 shadow-xl transition group-hover:visible group-hover:opacity-100"> | |
| <ul className="space-y-2"> | |
| {dropdownLinks.map((item) => ( | |
| <li key={`${item.label}-${item.link}`}> | |
| {renderLink( | |
| item, | |
| "block rounded-lg px-3 py-2 text-sm text-white no-underline transition hover:bg-white/10 hover:text-[#7CB0D3]", | |
| )} | |
| </li> | |
| ))} | |
| </ul> | |
| </div> | |
| {dropdownLinks.length > 0 && ( | |
| <div className="invisible absolute left-0 top-full z-10 mt-4 min-w-[220px] rounded-xl border border-white/10 bg-[`#032a3d`] p-3 opacity-0 shadow-xl transition group-hover:visible group-hover:opacity-100 group-focus-within:visible group-focus-within:opacity-100"> | |
| <ul className="space-y-2"> | |
| {dropdownLinks.map((item) => ( | |
| <li key={`${item.label}-${item.link}`}> | |
| {renderLink( | |
| item, | |
| "block rounded-lg px-3 py-2 text-sm text-white no-underline transition hover:bg-white/10 hover:text-[`#7CB0D3`]", | |
| )} | |
| </li> | |
| ))} | |
| </ul> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelHeaderSection.tsx` around
lines 228 - 240, The dropdown is only exposed via group-hover making keyboard
users unable to reach nested links; update the popover div (the element
rendering dropdownLinks in Hs1CarmelHeaderSection) to also become visible on
keyboard focus by adding the Tailwind utilities group-focus-within:visible and
group-focus-within:opacity-100 (or implement an explicit open state toggled on
focus/blur) and ensure the trigger element that wraps this group is focusable
(e.g., a button or has tabIndex) so renderLink/trigger receives keyboard focus;
also keep existing hover classes so both mouse and keyboard users can open the
menu.
Comment on lines
+118
to
+123
| <div className="mb-4"> | ||
| <HoursStatus | ||
| hours={streamDocument.hours} | ||
| timezone={streamDocument.timezone} | ||
| /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For @yext/pages-componentsv2.0.0, doesHoursStatussupporthours={undefined}safely, or is conditional rendering recommended whenhours is absent?
💡 Result:
Conditional rendering is recommended when hours is absent for the HoursStatus component in @yext/pages-components v2.0.0. No official documentation explicitly states support for hours={undefined}. Older related packages like @yext/react-components and @yext/components-hours pass hours directly without checks, but lack prop type details or handling for undefined. Given TypeScript usage and standard React practices for data from Yext Knowledge Graph (where fields can be missing), passing undefined risks errors if the prop is required. Best practice is to conditionally render: {hours && }.
Citations:
- 1: https://www.npmjs.com/package/@yext/pages-components
- 2: https://registry.npmjs.org/%40yext%2Fpages-components
- 3: https://github.com/yext/react-components
- 4: https://github.com/yext/pages
- 5: https://registry.npmjs.org/%40yext%2Fcomponents-hours
🏁 Script executed:
# Check if the file exists and get its total line count
wc -l starter/src/registry/hs1-carmel/components/Hs1CarmelLocationSection.tsxRepository: yext/visual-editor
Length of output: 137
🏁 Script executed:
# Read the specific lines mentioned in the review (118-123)
sed -n '110,130p' starter/src/registry/hs1-carmel/components/Hs1CarmelLocationSection.tsx | cat -nRepository: yext/visual-editor
Length of output: 976
🏁 Script executed:
# Check the entire file for any ID naming violations (should be Id instead of ID)
rg '\bID\b' starter/src/registry/hs1-carmel/components/Hs1CarmelLocationSection.tsx -iRepository: yext/visual-editor
Length of output: 44
Guard HoursStatus behind the same hours check used for Address and mainPhone.
HoursStatus is rendered unconditionally while other entity fields use conditional rendering. When streamDocument.hours is absent, passing undefined to the component is not recommended. Apply the same pattern as Address and mainPhone for consistency.
Proposed fix
- <div className="mb-4">
- <HoursStatus
- hours={streamDocument.hours}
- timezone={streamDocument.timezone}
- />
- </div>
+ {streamDocument.hours && (
+ <div className="mb-4">
+ <HoursStatus
+ hours={streamDocument.hours}
+ timezone={streamDocument.timezone}
+ />
+ </div>
+ )}📝 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.
Suggested change
| <div className="mb-4"> | |
| <HoursStatus | |
| hours={streamDocument.hours} | |
| timezone={streamDocument.timezone} | |
| /> | |
| </div> | |
| {streamDocument.hours && ( | |
| <div className="mb-4"> | |
| <HoursStatus | |
| hours={streamDocument.hours} | |
| timezone={streamDocument.timezone} | |
| /> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelLocationSection.tsx`
around lines 118 - 123, Hs1CarmelLocationSection renders HoursStatus
unconditionally which can receive undefined; wrap the HoursStatus component in
the same conditional used for Address and mainPhone so it's only rendered when
streamDocument.hours exists. Locate HoursStatus and the existing checks around
Address/mainPhone in Hs1CarmelLocationSection and apply the same guard (e.g., if
(streamDocument.hours) { <HoursStatus ... /> }) to prevent passing undefined
into HoursStatus.
Comment on lines
+219
to
+273
| <li className="group relative"> | ||
| {hasLink(props.educationMenu.link) ? ( | ||
| <Link | ||
| cta={{ | ||
| link: props.educationMenu.link, | ||
| linkType: "URL", | ||
| }} | ||
| > | ||
| <span className="text-[13px] uppercase tracking-[0.12em] text-[#3f3a39] transition-colors duration-150 hover:text-[#815955]"> | ||
| {props.educationMenu.label} | ||
| </span> | ||
| </Link> | ||
| ) : ( | ||
| <span className="text-[13px] uppercase tracking-[0.12em] text-[#3f3a39] transition-colors duration-150 hover:text-[#815955]"> | ||
| {props.educationMenu.label} | ||
| </span> | ||
| )} | ||
| <div className="pointer-events-none absolute left-1/2 top-full z-20 mt-4 w-[min(100vw-48px,1140px)] -translate-x-1/2 bg-[#815955] opacity-0 shadow-[0_20px_50px_rgba(0,0,0,0.18)] transition duration-200 group-hover:pointer-events-auto group-hover:opacity-100 max-md:hidden"> | ||
| <ul className="py-2"> | ||
| {props.educationMenu.subLinks.map((item, index) => ( | ||
| <li | ||
| key={`${item.label}-${index}`} | ||
| className="border-t border-white/30 first:border-t-0" | ||
| > | ||
| {hasLink(item.link) ? ( | ||
| <Link | ||
| cta={{ | ||
| link: item.link, | ||
| linkType: "URL", | ||
| }} | ||
| > | ||
| <span | ||
| className="block px-5 py-2 text-left text-[14px] leading-[22px] text-white transition-colors duration-150 hover:bg-white hover:text-[#815955]" | ||
| style={{ | ||
| fontFamily: | ||
| "'Hind', Arial, Helvetica, sans-serif", | ||
| }} | ||
| > | ||
| {item.label} | ||
| </span> | ||
| </Link> | ||
| ) : ( | ||
| <span | ||
| className="block px-5 py-2 text-left text-[14px] leading-[22px] text-white transition-colors duration-150 hover:bg-white hover:text-[#815955]" | ||
| style={{ | ||
| fontFamily: "'Hind', Arial, Helvetica, sans-serif", | ||
| }} | ||
| > | ||
| {item.label} | ||
| </span> | ||
| )} | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </div> |
Contributor
There was a problem hiding this comment.
The Education submenu is inaccessible outside desktop hover.
All of educationMenu.subLinks only render in the panel at Lines 236-273, but that panel is max-md:hidden and only revealed by group-hover. On small screens those links never appear anywhere, and on desktop they stay hidden when the trigger is focused via keyboard. Please add a small-screen expansion path and reveal the desktop panel on group-focus-within.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-chicago/components/Hs1ChicagoHeaderSection.tsx`
around lines 219 - 273, The education submenu is currently only visible via
desktop hover and hidden on small screens; update the <li className="group
relative"> trigger and the submenu <div className="... max-md:hidden"> so the
submenu is reachable via keyboard and mobile: (1) make the trigger focusable
(use the existing Link/span inside the group or wrap it in a button/link with
tabIndex) so it can receive focus and add the group-focus-within variants (e.g.,
group-focus-within:pointer-events-auto and group-focus-within:opacity-100) to
the panel to reveal it on keyboard focus (target the panel div that currently
has pointer-events-none/opacity-0/group-hover styles); (2) add a small-screen
expansion path by rendering a mobile-friendly disclosure (either a duplicated
stacked list of props.educationMenu.subLinks below the trigger visible on max-md
via responsive classes or toggle state) — ensure the mobile block is visible on
small screens (e.g., hidden md:block for desktop panel and block md:hidden for
mobile list) and reuse the same rendering of props.educationMenu.subLinks/items
so both Link and non-link branches are preserved.
Comment on lines
+3
to
+16
| export type Hs1LagunaLocationHoursSectionProps = { | ||
| locationTitle: string; | ||
| locationCaption: string; | ||
| hoursTitle: string; | ||
| hoursCaption: string; | ||
| }; | ||
|
|
||
| const Hs1LagunaLocationHoursSectionFields: Fields<Hs1LagunaLocationHoursSectionProps> = | ||
| { | ||
| locationTitle: { label: "Location Title", type: "text" }, | ||
| locationCaption: { label: "Location Caption", type: "text" }, | ||
| hoursTitle: { label: "Hours Title", type: "text" }, | ||
| hoursCaption: { label: "Hours Caption", type: "text" }, | ||
| }; |
Contributor
There was a problem hiding this comment.
Make the office data configurable.
Props only expose titles/captions, while the address, map tiles, and weekly hours are hard-coded. This starter will always render the same Downers Grove, IL office info and fixed schedule, and it also bypasses the shared-hours resolution work in this PR.
Also applies to: 21-29, 46-85, 99-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaLocationHoursSection.tsx`
around lines 3 - 16, The component currently only exposes title/caption props
(Hs1LagunaLocationHoursSectionProps and Hs1LagunaLocationHoursSectionFields)
while address, map tiles and the weekly hours are hard-coded; update the props
and fields to accept configurable office data (e.g., address lines, mapTiles
URL/string, and a weeklyHours structure or a resolvedHours prop) and use those
props inside the Hs1LagunaLocationHoursSection render logic instead of the fixed
Downers Grove values; also ensure the hours prop type matches the shared-hours
resolution format used elsewhere (or accept resolvedHours and render it) so the
component uses the shared-hours resolution work rather than bypassing it.
Comment on lines
+146
to
+179
| <form className="bg-[#6b3a2c] px-0 pb-0 pt-0"> | ||
| <div className="grid grid-cols-1 gap-[10px] md:grid-cols-3"> | ||
| <input | ||
| aria-label="Enter your name (Required)" | ||
| placeholder="Enter your name (Required)" | ||
| className="w-full rounded-none border border-white/20 bg-white px-3 py-2 text-[12px] text-[#4f4f4f] outline-none placeholder:text-[#8a8a8a]" | ||
| /> | ||
| <input | ||
| aria-label="Enter email (Required)" | ||
| placeholder="Enter email (Required)" | ||
| className="w-full rounded-none border border-white/20 bg-white px-3 py-2 text-[12px] text-[#4f4f4f] outline-none placeholder:text-[#8a8a8a]" | ||
| /> | ||
| <input | ||
| aria-label="(XXX)XXX-XXXX (Required)" | ||
| placeholder="(XXX)XXX-XXXX (Required)" | ||
| className="w-full rounded-none border border-white/20 bg-white px-3 py-2 text-[12px] text-[#4f4f4f] outline-none placeholder:text-[#8a8a8a]" | ||
| /> | ||
| </div> | ||
|
|
||
| <div className="mt-3 text-center"> | ||
| {renderStyledText( | ||
| props.disclaimer, | ||
| disclaimer, | ||
| "block text-[11px] leading-4 text-white/90", | ||
| )} | ||
| </div> | ||
|
|
||
| <div className="mt-2 text-center"> | ||
| <button | ||
| type="submit" | ||
| className="inline-flex min-w-[120px] items-center justify-center border border-white bg-white px-4 py-2 text-[11px] font-bold uppercase tracking-[0.08em] text-[#6b3a2c]" | ||
| > | ||
| Submit | ||
| </button> |
Contributor
There was a problem hiding this comment.
Prevent this mock form from submitting the page.
With a real <form> and a submit button but no handler, clicking Submit or pressing Enter will post back to the current page. That breaks the editor preview and still doesn't send the newsletter data anywhere.
Suggested fix
- <form className="bg-[`#6b3a2c`] px-0 pb-0 pt-0">
+ <form
+ className="bg-[`#6b3a2c`] px-0 pb-0 pt-0"
+ onSubmit={(event) => event.preventDefault()}
+ >
@@
- type="submit"
+ type="button"📝 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.
Suggested change
| <form className="bg-[#6b3a2c] px-0 pb-0 pt-0"> | |
| <div className="grid grid-cols-1 gap-[10px] md:grid-cols-3"> | |
| <input | |
| aria-label="Enter your name (Required)" | |
| placeholder="Enter your name (Required)" | |
| className="w-full rounded-none border border-white/20 bg-white px-3 py-2 text-[12px] text-[#4f4f4f] outline-none placeholder:text-[#8a8a8a]" | |
| /> | |
| <input | |
| aria-label="Enter email (Required)" | |
| placeholder="Enter email (Required)" | |
| className="w-full rounded-none border border-white/20 bg-white px-3 py-2 text-[12px] text-[#4f4f4f] outline-none placeholder:text-[#8a8a8a]" | |
| /> | |
| <input | |
| aria-label="(XXX)XXX-XXXX (Required)" | |
| placeholder="(XXX)XXX-XXXX (Required)" | |
| className="w-full rounded-none border border-white/20 bg-white px-3 py-2 text-[12px] text-[#4f4f4f] outline-none placeholder:text-[#8a8a8a]" | |
| /> | |
| </div> | |
| <div className="mt-3 text-center"> | |
| {renderStyledText( | |
| props.disclaimer, | |
| disclaimer, | |
| "block text-[11px] leading-4 text-white/90", | |
| )} | |
| </div> | |
| <div className="mt-2 text-center"> | |
| <button | |
| type="submit" | |
| className="inline-flex min-w-[120px] items-center justify-center border border-white bg-white px-4 py-2 text-[11px] font-bold uppercase tracking-[0.08em] text-[#6b3a2c]" | |
| > | |
| Submit | |
| </button> | |
| <form | |
| className="bg-[`#6b3a2c`] px-0 pb-0 pt-0" | |
| onSubmit={(event) => event.preventDefault()} | |
| > | |
| <div className="grid grid-cols-1 gap-[10px] md:grid-cols-3"> | |
| <input | |
| aria-label="Enter your name (Required)" | |
| placeholder="Enter your name (Required)" | |
| className="w-full rounded-none border border-white/20 bg-white px-3 py-2 text-[12px] text-[`#4f4f4f`] outline-none placeholder:text-[`#8a8a8a`]" | |
| /> | |
| <input | |
| aria-label="Enter email (Required)" | |
| placeholder="Enter email (Required)" | |
| className="w-full rounded-none border border-white/20 bg-white px-3 py-2 text-[12px] text-[`#4f4f4f`] outline-none placeholder:text-[`#8a8a8a`]" | |
| /> | |
| <input | |
| aria-label="(XXX)XXX-XXXX (Required)" | |
| placeholder="(XXX)XXX-XXXX (Required)" | |
| className="w-full rounded-none border border-white/20 bg-white px-3 py-2 text-[12px] text-[`#4f4f4f`] outline-none placeholder:text-[`#8a8a8a`]" | |
| /> | |
| </div> | |
| <div className="mt-3 text-center"> | |
| {renderStyledText( | |
| props.disclaimer, | |
| disclaimer, | |
| "block text-[11px] leading-4 text-white/90", | |
| )} | |
| </div> | |
| <div className="mt-2 text-center"> | |
| <button | |
| type="button" | |
| className="inline-flex min-w-[120px] items-center justify-center border border-white bg-white px-4 py-2 text-[11px] font-bold uppercase tracking-[0.08em] text-[`#6b3a2c`]" | |
| > | |
| Submit | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaNewsletterSection.tsx`
around lines 146 - 179, The form in Hs1LagunaNewsletterSection is a real <form>
with a type="submit" button and no handler, causing page navigation on submit;
stop the default submit by adding an onSubmit handler on the <form> that calls
event.preventDefault() (and optionally also change the Submit button in the
component to type="button") so clicking Submit or pressing Enter won't post back
the page—update the Hs1LagunaNewsletterSection form element and the Submit
button accordingly.
Comment on lines
+174
to
+179
| <button | ||
| type="submit" | ||
| className="inline-flex min-w-[184px] items-center justify-center border border-white bg-white px-6 py-3 text-[13px] font-bold uppercase tracking-[0.08em] text-[#6b3a2c]" | ||
| > | ||
| Submit | ||
| </button> |
Contributor
There was a problem hiding this comment.
Form submit button will trigger browser form submission.
Unlike Hs1ChicagoContactFormSection which uses type="button", this form uses type="submit". Without an onSubmit handler with e.preventDefault(), clicking submit will cause a browser form submission (page reload). Consider adding a handler or changing to type="button" for consistency with other demo forms.
Suggested fix
<div className="mt-3 text-center">
<button
- type="submit"
+ type="button"
className="inline-flex min-w-[184px] items-center justify-center border border-white bg-white px-6 py-3 text-[13px] font-bold uppercase tracking-[0.08em] text-[`#6b3a2c`]"
>
Submit
</button>📝 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.
Suggested change
| <button | |
| type="submit" | |
| className="inline-flex min-w-[184px] items-center justify-center border border-white bg-white px-6 py-3 text-[13px] font-bold uppercase tracking-[0.08em] text-[#6b3a2c]" | |
| > | |
| Submit | |
| </button> | |
| <button | |
| type="button" | |
| className="inline-flex min-w-[184px] items-center justify-center border border-white bg-white px-6 py-3 text-[13px] font-bold uppercase tracking-[0.08em] text-[`#6b3a2c`]" | |
| > | |
| Submit | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaOfferFormSection.tsx`
around lines 174 - 179, The submit button in Hs1LagunaOfferFormSection is using
type="submit" which will trigger a native browser form submission (page reload)
because there is no onSubmit handler preventing default; either change the
button to type="button" to match Hs1ChicagoContactFormSection or add an onSubmit
handler on the enclosing form in Hs1LagunaOfferFormSection that calls
e.preventDefault() and performs the intended submit logic; locate the JSX for
the button (the element with className "inline-flex min-w-[184px]...") and
implement one of these two fixes so clicking it doesn't cause a full page
reload.
Comment on lines
+49
to
+50
| const [activeIndex, setActiveIndex] = useState(0); | ||
| const activeMember = members[activeIndex] ?? members[0]; |
Contributor
There was a problem hiding this comment.
Use a bounded member index for render and navigation.
If the selected item is removed in the editor, Line 50 falls back to members[0] but the state from Line 49 stays out of range. That leaves the selected dot unset and the arrow handlers at Lines 105-132 working from an invalid index. Derive a bounded index (or clamp state when members.length changes) and use that everywhere.
Suggested patch
export const Hs1LagunaStaffSectionComponent: PuckComponent<
Hs1LagunaStaffSectionProps
> = ({ title, caption, members }) => {
const [activeIndex, setActiveIndex] = useState(0);
- const activeMember = members[activeIndex] ?? members[0];
+ const safeIndex = Math.max(0, Math.min(activeIndex, members.length - 1));
+ const activeMember = members[safeIndex];
if (!activeMember) {
return <></>;
}
@@
<button
type="button"
onClick={() =>
setActiveIndex(
- activeIndex === 0 ? members.length - 1 : activeIndex - 1,
+ safeIndex === 0 ? members.length - 1 : safeIndex - 1,
)
}
className="h-10 w-10 rounded-full border border-[`#755b53`] text-[`#755b53`]"
>
@@
type="button"
onClick={() => setActiveIndex(index)}
className={`h-2.5 w-2.5 rounded-full ${
- index === activeIndex ? "bg-[`#755b53`]" : "bg-[`#d5b8ae`]"
+ index === safeIndex ? "bg-[`#755b53`]" : "bg-[`#d5b8ae`]"
}`}
aria-label={`Show staff member ${index + 1}`}
/>
))}
@@
<button
type="button"
onClick={() =>
setActiveIndex(
- activeIndex === members.length - 1 ? 0 : activeIndex + 1,
+ safeIndex === members.length - 1 ? 0 : safeIndex + 1,
)
}
className="h-10 w-10 rounded-full border border-[`#755b53`] text-[`#755b53`]"
>Also applies to: 101-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaStaffSection.tsx` around
lines 49 - 50, The current state activeIndex can go out of range when members
changes; derive a bounded/clamped index and use it everywhere instead of raw
activeIndex: compute const clampedIndex = Math.max(0, Math.min(activeIndex,
members.length - 1)) (or clamp in a useEffect that calls setActiveIndex) and
replace usages of activeIndex for rendering activeMember and dot selection with
clampedIndex; update the arrow handlers (the next/prev functions that call
setActiveIndex) to base their math on members.length and set a clamped new index
via setActiveIndex(clampedNewIndex) so state never references an invalid member.
Comment on lines
+76
to
+88
| <button | ||
| type="button" | ||
| onClick={() => | ||
| setActiveIndex( | ||
| activeIndex === 0 | ||
| ? testimonials.length - 1 | ||
| : activeIndex - 1, | ||
| ) | ||
| } | ||
| className="h-10 w-10 rounded-full border border-[#755b53] text-[#755b53]" | ||
| > | ||
| ‹ | ||
| </button> |
Contributor
There was a problem hiding this comment.
Add explicit accessible names to carousel nav buttons.
The prev/next buttons only expose glyph text, which is weak for screen readers. Add aria-label for both controls.
Proposed fix
<button
type="button"
+ aria-label="Show previous testimonial"
onClick={() =>
setActiveIndex(
activeIndex === 0
? testimonials.length - 1
: activeIndex - 1,
@@
<button
type="button"
+ aria-label="Show next testimonial"
onClick={() =>
setActiveIndex(
activeIndex === testimonials.length - 1
? 0
: activeIndex + 1,Also applies to: 102-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-laguna/components/Hs1LagunaTestimonialsSection.tsx`
around lines 76 - 88, The prev/next carousel buttons in
Hs1LagunaTestimonialsSection render only glyph text and need explicit accessible
names: update the button elements (the one that calls setActiveIndex with
activeIndex === 0 ? testimonials.length - 1 : activeIndex - 1 and the
corresponding next-button that advances the index) to include aria-label
attributes like aria-label="Previous testimonial" and aria-label="Next
testimonial"; apply the same change to the other button instance in this
component (the block around the second occurrence currently at the other range)
so screen readers get clear control names.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
♻️ Duplicate comments (1)
starter/src/registry/hs1-albany/components/Hs1AlbanyLocationSection.tsx (1)
154-160:⚠️ Potential issue | 🟡 MinorDistance badge still hardcoded to
--.Same finding as prior review —
--is a static placeholder, so this will always render-- mi. Either wire it to a real distance value or hide the badge until one exists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany/components/Hs1AlbanyLocationSection.tsx` around lines 154 - 160, The distance badge in Hs1AlbanyLocationSection is hardcoded as "--" inside the div with className "mb-1 flex items-end gap-1" (the two spans rendering "--" and "mi"); change this to use the actual distance value (e.g., a prop or state like distance or computedDistance) and conditionally render the entire badge only when that value exists (or is a number). Locate the rendering of the two spans in Hs1AlbanyLocationSection, replace the static "--" with the dynamic distance variable (formatted as needed) and wrap the badge div in a conditional so it is hidden when no distance is available.
🟡 Minor comments (31)
starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffFooterSection.tsx-262-275 (1)
262-275:⚠️ Potential issue | 🟡 MinorDefault
socialLinkspoint to unrelated third-party URLs.The Facebook/Twitter/YouTube defaults reference "Anderson-Optometry", "InternetMatrix", and "webmarketingimatrix" — these look like leftover scraped placeholders unrelated to the HS1 Albany template and will be shipped as defaults if a user doesn't override them. Consider replacing with empty/neutral defaults (e.g.,
"#") or actual Albany-relevant URLs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffFooterSection.tsx` around lines 262 - 275, The default socialLinks array in Hs1AlbanyStaffFooterSection.tsx contains unrelated third-party URLs (platform entries for Facebook/Twitter/Youtube pointing to Anderson-Optometry/InternetMatrix/webmarketingimatrix); update the socialLinks default in the Hs1AlbanyStaffFooterSection component so it uses neutral placeholders (e.g., "#" or empty strings) or real Albany-specific URLs instead, ensuring the objects for platform "Facebook", "Twitter", and "Youtube" no longer reference external, unrelated sites.starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffLocationSection.tsx-175-180 (1)
175-180:⚠️ Potential issue | 🟡 MinorHardcoded "Phone:" label bypasses localization.
All other visible labels (
title,subtitle,locationLabel,addressLabel,contactLabel) are translatableYextEntityField<TranslatableString>values resolved per locale, but the literal"Phone:"rendered next to the number is not. For non-English locales this string will remain in English, producing a mixed-language UI. Consider either folding the phone prefix intocontactLabelrendering or adding a dedicated translatable field.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffLocationSection.tsx` around lines 175 - 180, The hardcoded "Phone:" string in Hs1AlbanyStaffLocationSection should be replaced with a translatable field; locate the Link that uses telHref and mainPhone and remove the literal "Phone:" prefix, instead render the existing contactLabel (or add a new YextEntityField<TranslatableString> phoneLabel prop if you need a distinct phone prefix) resolved for the current locale, so the phone prefix is localized consistently with title, subtitle, locationLabel, addressLabel; ensure you use the same resolution/rendering helper used elsewhere in the component for TranslatableString values.starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffLocationSection.tsx-127-128 (1)
127-128:⚠️ Potential issue | 🟡 MinorAvoid shipping a hardcoded fallback phone number; guard against non-string
mainPhone.Two concerns on these two lines:
"(877) 393-3348"is a real-looking fallback baked into the component. IfstreamDocument.mainPhoneis ever missing in production this number will render and generate a livetel:link for every such entity. Prefer rendering nothing (or hiding the contact block) whenmainPhoneis absent.streamDocument.mainPhoneis typed asany. If an upstream change ever yields a non-string (object/number),String(mainPhone)will silently coerce (e.g.,"[object Object]") and the displayed number will be garbage. Atypeof mainPhone === "string"check before rendering would be safer.🛡️ Suggested shape
- const mainPhone = streamDocument.mainPhone || "(877) 393-3348"; - const telHref = `tel:${String(mainPhone).replace(/[^\d+]/g, "")}`; + const mainPhone = + typeof streamDocument.mainPhone === "string" + ? streamDocument.mainPhone + : undefined; + const telHref = mainPhone + ? `tel:${mainPhone.replace(/[^\d+]/g, "")}` + : undefined;…and conditionally render the
<Link>only whenmainPhoneis defined.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffLocationSection.tsx` around lines 127 - 128, Remove the hardcoded fallback and guard against non-string mainPhone: stop assigning "(877) 393-3348" to mainPhone, only use streamDocument.mainPhone when typeof streamDocument.mainPhone === "string" and non-empty; compute telHref (using the existing replace regex) only in that branch and otherwise omit rendering the contact Link / contact block. Update references to mainPhone and telHref in Hs1AlbanyStaffLocationSection so the Link is conditionally rendered only when the validated string mainPhone is present.starter/src/registry/hs1-albany/components/Hs1AlbanyHeroSection.tsx-221-230 (1)
221-230:⚠️ Potential issue | 🟡 MinorStatic "pagination" dots suggest interactivity that doesn't exist.
The three dots visually imply a carousel/slider, but they're hardcoded (
[0, 1, 2].map(...)) with the first always active and no click handlers or aria semantics. This can confuse users (especially screen-reader users who will hear nothing) and mislead authors into thinking the hero is a slideshow. Consider either implementing real carousel behavior or removing the indicator until it's wired up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany/components/Hs1AlbanyHeroSection.tsx` around lines 221 - 230, The hardcoded pagination dots in Hs1AlbanyHeroSection (the {[0, 1, 2].map(...)} block rendering <span> dots) imply interactivity but have no behavior or accessibility semantics; either remove this block entirely, or make it an accessible, non-interactive decorative element by marking the container role="presentation" or aria-hidden="true" and removing the “active” styling so it doesn't imply state, or implement real carousel behavior by adding component state (e.g., currentIndex), clickable buttons (use <button> elements instead of spans), keyboard handlers, and proper ARIA attributes like aria-current on the active indicator and an accessible label describing the control.starter/src/registry/hs1-albany/components/Hs1AlbanyHeroSection.tsx-213-217 (1)
213-217:⚠️ Potential issue | 🟡 MinorEmpty
alton the hero image drops accessible context.
alt=""marks the image as purely decorative, but this is the primary hero visual paired with the heading/CTA — assistive tech users lose the content. Either derive alt from the resolved image entity (e.g.,ComplexImageType.alternateText/ImageType.alternateText) or from the heading text as a fallback.🧩 Sketch
- <img - src={heroImageUrl} - alt="" - className="h-full min-h-[300px] w-full object-cover" - /> + <img + src={heroImageUrl} + alt={getImageAlt(resolvedHeroImage) ?? resolvedHeading ?? ""} + className="h-full min-h-[300px] w-full object-cover" + />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany/components/Hs1AlbanyHeroSection.tsx` around lines 213 - 217, The hero image in Hs1AlbanyHeroSection.tsx currently uses alt="" which removes accessible context; update the <img> rendering to compute a meaningful alt value (use the resolved image entity's alternateText field if available, e.g., ComplexImageType.alternateText or ImageType.alternateText) and fall back to the heading/CTA text (the component's primary heading prop) when alternateText is empty, then pass that string into the img alt attribute instead of an empty string (keep using heroImageUrl for src).starter/src/registry/hs1-carmel/components/Hs1CarmelWelcomeSection.tsx-170-178 (1)
170-178:⚠️ Potential issue | 🟡 MinorExternal image URL in
defaultPropsis a fragile default.The default
constantValue.urlpoints to a third‑party host (cdcssl.ibsrv.net). If that asset is moved or removed, every new instance of this section will render a broken image on first drop. Prefer an asset hosted in this repo/registry (or a Yext-hosted asset) for defaults.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-carmel/components/Hs1CarmelWelcomeSection.tsx` around lines 170 - 178, The defaultProps currently sets image.constantValue.url to an external third‑party URL which is fragile; update Hs1CarmelWelcomeSection's default image by replacing image.constantValue.url with a repository-hosted or Yext-hosted asset (or a local placeholder asset added to the registry), keep image.constantValueEnabled: true, and ensure the new URL points to that stable internal asset (refer to the image, constantValue, constantValueEnabled fields in Hs1CarmelWelcomeSection to locate the change).starter/src/registry/hs1-carmel/components/Hs1CarmelWelcomeSection.tsx-149-157 (1)
149-157:⚠️ Potential issue | 🟡 MinorCTA
linkTypehardcoded to"URL".The Link is always rendered with
linkType: "URL", but the editor field exposes a free-formlinktext input. If a content editor enters a phone number or email incta.link, it will still be rendered as a URL anchor, producing a broken link. Consider either constraining the field to URLs, or inferringlinkTypefrom the value (e.g.,tel:/mailto:prefixes).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-carmel/components/Hs1CarmelWelcomeSection.tsx` around lines 149 - 157, The CTA Link currently hardcodes linkType: "URL" in Hs1CarmelWelcomeSection which breaks non-URL inputs; update the render to infer linkType from props.cta.link (or normalize the field) before passing to Link: detect "mailto:" prefix → "EMAIL", "tel:" or phone-number pattern → "PHONE" (or appropriate Link enum values used by your Link component), otherwise default to "URL"; ensure you reference props.cta.link and the Link component props so the chosen linkType and link value are passed together.starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffHeaderSection.tsx-302-347 (1)
302-347:⚠️ Potential issue | 🟡 MinorAdd
aria-expanded(and ideallyaria-controls) to disclosure buttons.The hamburger toggle (302-309), mobile "More" toggle (329-336), and nested toggle (340-347) all visually indicate open/closed state but don't expose it to assistive tech. Bind
aria-expanded={mobileOpen}/={mobileMoreOpen}/={mobileNestedOpen}on the corresponding buttons, and consideraria-controlspointing at the revealed region'sid.🛠️ Example for the hamburger
<button type="button" onClick={() => setMobileOpen((value) => !value)} className="flex h-10 w-10 items-center justify-center" aria-label="Toggle navigation" + aria-expanded={mobileOpen} + aria-controls="hs1-albany-staff-mobile-nav" > <Hamburger open={mobileOpen} /> </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffHeaderSection.tsx` around lines 302 - 347, The three disclosure buttons (the hamburger toggle using mobileOpen, the "More" toggle using mobileMoreOpen, and the nested toggle using mobileNestedOpen in Hs1AlbanyStaffHeaderSection) need ARIA state: add aria-expanded bound to the respective state variables (aria-expanded={mobileOpen}, aria-expanded={mobileMoreOpen}, aria-expanded={mobileNestedOpen}) and add aria-controls attributes that point to unique ids for the revealed regions (e.g., id="mobile-navigation", id="mobile-more-panel", id="mobile-nested-panel"); ensure the corresponding revealed containers (the mobile menu div, the More dropdown container that wraps SubmenuList, and the nested panel) include those matching id attributes so assistive tech can relate the buttons to the panels.starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffRosterSection.tsx-352-366 (1)
352-366:⚠️ Potential issue | 🟡 MinorDefault bio doesn't match the staff name and has typos.
The default
biofor "Dr. Nathan Anderson" reads as a biography for a different person ("Dr. John Smith ... University of Minnesota"). It also contains a couple of small typos that will ship into rendered template previews:
Master'Degree→ should beMaster's DegreeDr Smith entered→ should reference Dr. Anderson (and use a period:Dr.)Since these are baked-in defaults users will see when first dropping the section in, worth cleaning up.
📝 Suggested copy fix
- defaultValue: - "Dr. John Smith was born and raised in Minnesota. He received his degree from the University of Minnesota in 1990. He practiced in Minneapolis for ten years. Dr. Smith entered the Master'Degree program in at the University of Minnesota in 2000. Dr Smith enjoys spending time with his family, traveling, and exploring the wilderness of the Boundary Waters. He is a talented musician and avid reader.", + defaultValue: + "Dr. Nathan Anderson was born and raised in Minnesota. He received his degree from the University of Minnesota in 1990 and practiced in Minneapolis for ten years. Dr. Anderson entered the Master's Degree program at the University of Minnesota in 2000. He enjoys spending time with his family, traveling, and exploring the wilderness of the Boundary Waters. He is a talented musician and avid reader.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffRosterSection.tsx` around lines 352 - 366, The default bio text under the bio -> text -> constantValue -> defaultValue is wrong and contains typos; update that string to a clean, appropriate default for "Dr. Nathan Anderson" (mention his name, correct the degree phrasing to "Master's Degree" and use "Dr." with a period), and ensure the corrected copy replaces the current Dr. John Smith content in the Hs1AlbanyStaffRosterSection component (target the bio.text.constantValue.defaultValue field).starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffRosterSection.tsx-226-230 (1)
226-230:⚠️ Potential issue | 🟡 MinorImage truthiness check won't catch empty‑URL constants.
resolveComponentDatafor an image field withconstantValueEnabled: truereturns the constant object even when itsurlis empty, soresolvedImage ?is truthy and<Image image={...}>will render a broken/empty image instead of the grey placeholder fallback. Consider gating on a non‑empty URL (handling bothImageType/ComplexImageTypeshapes) before rendering<Image>.🛡️ Sketch of a stricter check
- {resolvedImage ? ( + {resolvedImage && + ("url" in resolvedImage + ? !!resolvedImage.url + : !!resolvedImage.image?.url) ? ( <Image image={resolvedImage} className="h-full w-full" /> ) : ( <div className="h-full min-h-[220px] w-full bg-[`#f0f0f0`]" /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffRosterSection.tsx` around lines 226 - 230, The current truthy check on resolvedImage allows constant image objects with empty url to render a broken <Image>; update Hs1AlbanyStaffRosterSection.tsx to gate rendering on a non-empty URL instead of just object truthiness: compute a helper (e.g. hasImageUrl) that checks for a non-empty string URL for both possible shapes (e.g. resolvedImage.url || resolvedImage.image?.url) and ensures trimmed length > 0, then render <Image image={resolvedImage}> only when hasImageUrl is true, otherwise render the grey placeholder div; update any places using resolvedImage directly to use this predicate so empty-URL constants fall back to the placeholder.starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsFooterSection.tsx-55-59 (1)
55-59:⚠️ Potential issue | 🟡 MinorSocial icons render as literal characters, not icons.
iconMapreturns"f","t", and"▶"which will display as plain glyphs rather than brand icons. The peer componentHs1AlbanyStaffFooterSection.tsxuses aSocialIconcomponent for this. Consider using the same SVG/icon component for visual parity and brand correctness, or at minimum use Unicode symbols / an SVG set rather than bare Latin letters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsFooterSection.tsx` around lines 55 - 59, iconMap currently maps SocialLink["icon"] values to plain characters ("f","t","▶") which render as literal glyphs; update the mapping to use the same icon component used in Hs1AlbanyStaffFooterSection (SocialIcon) or return SVG/icon identifiers instead of Latin letters. Replace the string values in iconMap in Hs1AlbanyNewPatientsFooterSection.tsx with references compatible with SocialIcon (or inline SVG markup identifiers) and ensure the footer rendering code calls <SocialIcon name={iconMap[link.icon]}> (or passes the mapped SVG/component) so brand icons display correctly and mirror Hs1AlbanyStaffFooterSection's behavior.starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesHoursSection.tsx-93-103 (1)
93-103:⚠️ Potential issue | 🟡 Minor
formatTimehas minor robustness gaps.Two small issues in the time parsing:
rawMinuteis passed through unchanged, so a source value like"9:5"renders as"9:5 am"instead of"9:05 am".- If
rawHouris non-numeric,Number.parseIntreturnsNaN, producing output like"NaN:00 am"rather than an empty string or fallback.♻️ Proposed hardening
const formatTime = (value?: string) => { if (!value) { return ""; } const [rawHour = "0", rawMinute = "00"] = value.split(":"); const hour = Number.parseInt(rawHour, 10); + if (Number.isNaN(hour)) { + return ""; + } const suffix = hour >= 12 ? "pm" : "am"; const normalizedHour = hour % 12 === 0 ? 12 : hour % 12; - return `${normalizedHour}:${rawMinute} ${suffix}`; + const minute = rawMinute.padStart(2, "0"); + return `${normalizedHour}:${minute} ${suffix}`; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesHoursSection.tsx` around lines 93 - 103, The formatTime function should properly handle non-numeric hours and single-digit minutes: when parsing rawHour use Number.parseInt(rawHour, 10) then if that produces NaN (Number.isNaN(hour) or !Number.isFinite(hour)) return an empty string (or other fallback) instead of formatting; also normalize/clamp the hour into a 0-23 range before computing suffix/normalizedHour. For minutes, pad rawMinute to two digits (e.g., padStart(2,"0") after defaulting to "00") so inputs like "9:5" become "9:05 am". Update the formatTime function to validate rawHour, clamp/normalize hour, and pad rawMinute before constructing the final string.starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsContentSection.tsx-15-19 (1)
15-19:⚠️ Potential issue | 🟡 MinorDeveloper-facing placeholder copy is user-visible.
The paragraph text ("This section is a temporary stub that restores the missing starter registry component so the dev server can boot again.") will render in any environment that loads this starter layout. Either replace it with the real New Patients content described in the captured plan, or at minimum use neutral placeholder copy so demos/previews don't leak internal dev notes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsContentSection.tsx` around lines 15 - 19, The paragraph in Hs1AlbanyNewPatientsContentSection currently contains an internal dev note; update the JSX inside the Hs1AlbanyNewPatientsContentSection component (the <p className="mb-0 mt-4"> block) to remove the dev-facing sentence and replace it with either the real "New Patients" content from the plan or a neutral placeholder such as a brief patient-facing sentence (e.g., "Information for new patients, including what to bring and what to expect.") so user-visible UI does not expose internal developer notes.starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesContactFormSection.tsx-141-165 (1)
141-165:⚠️ Potential issue | 🟡 MinorInputs lack accessible names.
Inputs rely solely on
placeholder, and there are no<label>,aria-label,name, oridattributes. Screen readers will announce these fields as unlabeled, and autofill won't work. Even for a demo template, adding a hidden<label>oraria-labelderived fromfield.placeholderwould be a low-cost accessibility win.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesContactFormSection.tsx` around lines 141 - 165, The inputs and textarea in Hs1AlbanyServicesContactFormSection.tsx lack accessible names; update the formFields mapping to generate an id/name and either a visually-hidden <label> (with htmlFor) or an aria-label derived from field.placeholder (use a slugified/trimmed value plus index to ensure uniqueness) and add name and id attributes to the <input> elements (refer to formFields and the mapped input) and likewise add id/name and aria-label or label for the <textarea> (refer to messagePlaceholder and the textarea block); ensure screen readers and autofill can identify each field by providing those attributes.starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeContentSection.tsx-620-669 (1)
620-669:⚠️ Potential issue | 🟡 MinorDefault copy references "Downers Grove" — wrong city for the Albany registry.
introTitle,appointmentsBodyBefore, and related default strings talk about "Downers Grove Dentist" / "Downers Grove, IL". Since this component lives underhs1-albany-office/, these defaults will ship as the initial editor content for the Albany template and look like leftover scaffolding from a previous template. Swap the copy to Albany (or make it generic) before merging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeContentSection.tsx` around lines 620 - 669, The default copy still references "Downers Grove" in fields created via createStyledTextDefault (notably introTitle, appointmentsBodyBefore, introBody and related strings like officeName); update those default string values to reference Albany (e.g., "Albany Dentist - Albany Dental Office") or make them generic (e.g., "Your Local Dentist") so the Hs1AlbanyOffice template's initial editor content matches the Albany registry; locate and replace the literal strings inside the introTitle, introBody, appointmentsBodyBefore, appointmentsBodyAfter and any other createStyledTextDefault calls in Hs1AlbanyOfficeContentSection.tsx.starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeContentSection.tsx-457-466 (1)
457-466:⚠️ Potential issue | 🟡 MinorPhone
Linkrenders even whenmainPhoneis empty.
mainPhoneis coerced withString(... ?? "")at Line 428, so when the stream lacksmainPhonethis renders an empty anchor inside the contact block (tel:with no number, no visible text). Consider guarding the<Link>on a truthymainPhone.🛠️ Suggested fix
- <Link - cta={{ - link: mainPhone, - linkType: "PHONE", - }} - className="inline-block" - > - {mainPhone} - </Link> + {mainPhone ? ( + <Link + cta={{ link: mainPhone, linkType: "PHONE" }} + className="inline-block" + > + {mainPhone} + </Link> + ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeContentSection.tsx` around lines 457 - 466, The Link with cta.link set to mainPhone is rendered even when mainPhone is empty; update Hs1AlbanyOfficeContentSection to conditionally render the Link component only when mainPhone is truthy (e.g., wrap the existing <Link ...>{mainPhone}</Link> in a guard like if (mainPhone) or {mainPhone && <Link ...>...}</Link>), ensuring no empty tel: anchor is emitted; keep the same cta shape and className so behavior/appearance remains unchanged when mainPhone exists.starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeContactFormSection.tsx-189-236 (1)
189-236:⚠️ Potential issue | 🟡 MinorForm is non-functional and has a11y gaps when placeholders resolve to empty strings.
Two concerns in the same block:
<form>has noonSubmitand the submit<button>usestype="button"(Line 225), so activating it does nothing. In the siblingHs1AlbanyServicesContactFormSectionthe equivalent button istype="submit"and the form doesevent.preventDefault(). Align on one approach — at minimum document that this is decorative/demo-only.- Inputs rely on
aria-label={namePlaceholder}/placeholder={...}. If an editor clears the label field,resolveStyledTextreturns""and screen-reader users lose any accessible name. Consider rendering a proper<label>(visually hidden if needed) and falling back to a stable default when the resolved value is empty.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeContactFormSection.tsx` around lines 189 - 236, The form in Hs1AlbanyOfficeContactFormSection is non-functional and inaccessible: change the <form> to handle submission (add an onSubmit handler on the form that at minimum calls event.preventDefault() and processes or documents/demo-returns) and change the button from type="button" to type="submit" (or explicitly document it as decorative if you intend no behavior) so the control activates; also fix the a11y gap by providing stable accessible names for the inputs (replace or augment aria-label={namePlaceholder}, aria-label={emailPlaceholder}, aria-label={phonePlaceholder}, and aria-label={notesPlaceholder} with explicit <label> elements or programmatic fallbacks that use a default string when resolveStyledText returns an empty string, keeping visual styling via a visually-hidden class if needed and preserving existing style usage for props.submitText and props.privacyNote).starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsx-158-260 (1)
158-260:⚠️ Potential issue | 🟡 MinorMap iframe renders with an empty query when
streamDocument.addressis missing.If
addressis undefined,mapQueryends up as""and the iframe loadshttps://www.google.com/maps?q=&output=embed, which shows Google Maps' default view rather than anything related to the entity. Gate the iframe on a non-emptymapQuery(or onaddress?.line1) and render a neutral placeholder in the panel otherwise.Additionally, the phone
Linkat Lines 242–250 has the same empty-mainPhoneproblem noted inHs1AlbanyOfficeContentSection.tsx— consider guarding here as well.🛠️ Suggested fix (map)
- <div className="min-h-[320px] border border-[`#dddddd`] bg-white"> - <iframe - title="Primary location map" - src={`https://www.google.com/maps?q=${encodeURIComponent(mapQuery)}&output=embed`} - className="h-[320px] w-full lg:h-[360px]" - loading="lazy" - referrerPolicy="no-referrer-when-downgrade" - /> - </div> + <div className="min-h-[320px] border border-[`#dddddd`] bg-white"> + {mapQuery ? ( + <iframe + title="Primary location map" + src={`https://www.google.com/maps?q=${encodeURIComponent(mapQuery)}&output=embed`} + className="h-[320px] w-full lg:h-[360px]" + loading="lazy" + referrerPolicy="no-referrer-when-downgrade" + /> + ) : null} + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsx` around lines 158 - 260, The iframe is rendered even when streamDocument.address is missing so mapQuery becomes empty and Google Maps shows a default view; update Hs1AlbanyOfficeLocationSection to only render the iframe when mapQuery (or address?.line1) is non-empty and otherwise render a neutral placeholder panel, and also guard the phone Link (the Link using mainPhone) to render only when mainPhone is truthy (or render a disabled/plain phone text) to avoid empty PHONE links; locate mapQuery, the iframe element with src using encodeURIComponent(mapQuery), and the Link that passes mainPhone to apply these guards.starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesLocationSection.tsx-77-88 (1)
77-88:⚠️ Potential issue | 🟡 MinorHardcoded phone fallback creates a real
tel:CTA to a third-party number.When
streamDocument.mainPhoneis missing,renderPhonereturns"(877) 393-3348"and Line 115 strips it intotel:8773933348. Rendering a functional call link to a hardcoded number that doesn't belong to the entity is a correctness/UX risk for any entity whose stream omitsmainPhone. Prefer returningundefined/empty and conditionally omitting the<Link>(or the whole "Phone:" row) instead.Note: the same pattern exists in
Hs1AlbanyServicesHeaderSection.tsx(seerenderPhoneat lines 145–160) — consider fixing at both sites or extracting to a shared helper.🛠️ Suggested fix
-const renderPhone = (phone?: string) => { - if (!phone) { - return "(877) 393-3348"; - } - - const digits = phone.replace(/\D/g, ""); +const renderPhone = (phone?: string) => { + if (!phone) { + return undefined; + } + + const digits = phone.replace(/\D/g, ""); if (digits.length === 10) { return `(${digits.slice(0, 3)}) ${digits.slice(3, 6)}-${digits.slice(6)}`; } - return phone; };And gate the rendering at lines 221–234 on
displayPhone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesLocationSection.tsx` around lines 77 - 88, The renderPhone function currently returns a hardcoded third‑party number when phone is missing, causing real tel: links to that number; change renderPhone (in Hs1AlbanyServicesLocationSection.tsx) to return undefined (or empty string) when no phone is provided, keep the existing formatting logic for valid numbers, and update the JSX that renders the Phone Link (the block that creates the tel: link / "Phone:" row) to conditionally render only when renderPhone returns a value (i.e., guard the Link/row on displayPhone). Apply the same change to the duplicate renderPhone in Hs1AlbanyServicesHeaderSection.tsx or extract a shared helper used by both.starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeFooterSection.tsx-147-147 (1)
147-147:⚠️ Potential issue | 🟡 MinorDefault Facebook URL points to the wrong business.
https://www.facebook.com/Anderson-Optometry-363713737059041/is from a different starter. For the Sunny Smiles / HS1 Albany office defaults, replace with the correct Facebook URL (or leave a placeholder likehttps://www.facebook.com/) so editors don't ship this demo template with a link to an unrelated business.Also applies to: 249-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeFooterSection.tsx` at line 147, The default Facebook URL in the Hs1AlbanyOfficeFooterSection component points to an unrelated business; update the `link` value(s) inside Hs1AlbanyOfficeFooterSection (the footer social-links/default data array entries) to the correct Sunny Smiles / HS1 Albany Facebook URL or replace with a neutral placeholder like `https://www.facebook.com/` so the shipped template doesn't link to another business; ensure you update all occurrences (the current `link` entry and the other entries in the same footer/social defaults).starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeHeaderSection.tsx-217-217 (1)
217-217:⚠️ Potential issue | 🟡 MinorPhone CTA renders even when
mainPhoneis missing.
String(streamDocument.mainPhone ?? "")produces"", so both the desktop and mobile ☎ links render with an empty href/label when the document has nomainPhone. Conditionally render the CTA only when a phone number is present.♻️ Proposed change
- const mainPhone = String(streamDocument.mainPhone ?? ""); + const mainPhoneRaw = streamDocument.mainPhone; + const mainPhone = typeof mainPhoneRaw === "string" ? mainPhoneRaw : "";Then guard the two
<Link cta={{ link: mainPhone, linkType: "PHONE" }} ...>blocks with{mainPhone && ( ... )}.Also applies to: 286-296, 315-323
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeHeaderSection.tsx` at line 217, The phone CTA is rendered even when mainPhone is empty because mainPhone is set via const mainPhone = String(streamDocument.mainPhone ?? "");—wrap the desktop and mobile Link components that use cta={{ link: mainPhone, linkType: "PHONE" }} with a conditional check so they only render when mainPhone is truthy (e.g., {mainPhone && ( ... )}); apply the same guard around the other two Link blocks that reference mainPhone (the blocks corresponding to the desktop and mobile phone CTAs) so no empty href/label is rendered.starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesFooterSection.tsx-145-149 (1)
145-149:⚠️ Potential issue | 🟡 MinorLogo container has no height, so
h-fullon the image collapses to 0.The wrapper is
w-[120px] shrink-0with no height, and<Image className="h-full w-full" />stretches the<img>to the parent's height — which is 0. The siblingHs1AlbanyOfficeFooterSectioncorrectly usesh-[78px] w-[120px]on the wrapper. Add a fixed height (or droph-full w-fulland let the image use its intrinsic sizing) so the logo is visible.🛡️ Proposed fix
- {resolvedLogoImage && ( - <div className="w-[120px] shrink-0"> - <Image image={resolvedLogoImage} className="h-full w-full" /> - </div> - )} + {resolvedLogoImage && ( + <div className="h-[78px] w-[120px] shrink-0"> + <Image + image={resolvedLogoImage} + className="h-full w-full object-contain" + /> + </div> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesFooterSection.tsx` around lines 145 - 149, The logo wrapper in Hs1AlbanyServicesFooterSection collapses to zero height so Image's h-full collapses; update the wrapper div that renders when resolvedLogoImage is truthy (the div with className "w-[120px] shrink-0") to provide a fixed height (e.g. add h-[78px] to match Hs1AlbanyOfficeFooterSection) or remove Image's "h-full w-full" and let the Image use intrinsic sizing; ensure you modify the conditional block around resolvedLogoImage and keep the Image component reference unchanged.starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHoursSection.tsx-86-92 (1)
86-92:⚠️ Potential issue | 🟡 Minor
formatTimecan yieldNaN:undefined am.If
interval.start/interval.endis an empty string or missing,value.split(":").map(Number)produces[NaN, undefined], andString(undefined).padStart(2, "0")renders as"undefined".formatHoursgates onopenIntervals.lengthbut does not validate interval shape. The sibling services hours component guards with sane defaults — consider mirroring:🛡️ Proposed hardening
-const formatTime = (value: string) => { - const [hoursValue, minutes] = value.split(":").map((part) => Number(part)); - const suffix = hoursValue >= 12 ? "pm" : "am"; - const twelveHour = hoursValue % 12 === 0 ? 12 : hoursValue % 12; - - return `${twelveHour}:${String(minutes).padStart(2, "0")} ${suffix}`; -}; +const formatTime = (value?: string) => { + if (!value) return ""; + const [rawHour = "0", rawMinute = "00"] = value.split(":"); + const hoursValue = Number.parseInt(rawHour, 10); + if (Number.isNaN(hoursValue)) return ""; + const suffix = hoursValue >= 12 ? "pm" : "am"; + const twelveHour = hoursValue % 12 === 0 ? 12 : hoursValue % 12; + return `${twelveHour}:${rawMinute.padStart(2, "0")} ${suffix}`; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHoursSection.tsx` around lines 86 - 92, formatTime currently assumes value is a "HH:MM" string and can produce "NaN:undefined am" if interval.start/interval.end are missing; update formatTime to validate and sanitize input before parsing (e.g., return an empty string or a clear fallback like "-" when value is falsy or doesn't match /^\d{1,2}:\d{2}$/), parse hours/minutes defensively (use Number with fallback values and check Number.isFinite), and ensure callers (e.g., formatHours and any code reading interval.start/interval.end) expect/handle the empty/fallback output; reference the formatTime function and the code paths that call it (formatHours, openIntervals and interval.start/interval.end) when making the changes.starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeFooterSection.tsx-141-159 (1)
141-159:⚠️ Potential issue | 🟡 MinorUse a
platformenum select for social links instead of matching by label.Selecting the icon via
socialIconByLabel[item.label.toLowerCase()] ?? FaFacebookFis fragile: any label change (rename, translation, typo) silently falls back to the Facebook icon even for Twitter/YouTube entries. The siblingHs1AlbanyServicesFooterSectionalready models this correctly with aplatform: "facebook" | "twitter" | "youtube"field plus a select input. Mirror that pattern here so icon and label stay decoupled.Also applies to: 201-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeFooterSection.tsx` around lines 141 - 159, The socialLinks array in Hs1AlbanyOfficeFooterSection.tsx uses label-based icon selection (socialIconByLabel[item.label.toLowerCase()] ?? FaFacebookF), which is brittle; change the link item schema produced by createLinkItemFields() for this component to include a platform enum (e.g., platform: "facebook" | "twitter" | "youtube") with a select UI, mirror the implementation used in Hs1AlbanyServicesFooterSection, update defaultItemProps for socialLinks to set platform values, and switch icon lookup to use the platform field instead of label so icons and labels are decoupled and robust.starter/src/registry/hs1-albany/components/Hs1AlbanyHeaderSection.tsx-180-186 (1)
180-186:⚠️ Potential issue | 🟡 MinorUse
linkType: "PHONE"for the phone CTA with a bare phone number.The current code uses
link: \tel:${phone}`withlinkType: "URL", which is inconsistent with other components in the codebase. The pattern in Hs1AlbanyOfficeHeaderSection and other templates is to uselinkType: "PHONE"paired with the bare phone number and let the Link component handle building thetel:` href. Align to this pattern for consistency.♻️ Proposed change
- <Link - cta={{ link: `tel:${phone}`, linkType: "URL" }} + <Link + cta={{ link: phone, linkType: "PHONE" }} className="justify-self-center text-[28px] font-medium text-[`#d3a335`] no-underline md:justify-self-end"Additionally, the phone number is hard-coded at line 99 as
const phone = "(877) 393-3348";while other link/logo values in this component come fromprops. Consider making this configurable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany/components/Hs1AlbanyHeaderSection.tsx` around lines 180 - 186, The Link CTA currently sets cta={{ link: `tel:${phone}`, linkType: "URL" }}; change it to cta={{ link: phone, linkType: "PHONE" }} so the Link component builds the tel: href consistently (see Link usage in Hs1AlbanyOfficeHeaderSection and the Link component itself); also stop using a hard-coded const phone and instead read phone from the component props (Hs1AlbanyHeaderSection) with a sensible default/fallback to preserve behavior if the prop is not provided.starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsLocationSection.tsx-192-202 (1)
192-202:⚠️ Potential issue | 🟡 MinorAddress
lines+separator=" "will produce extra spaces around commas.The literal
","entries are concatenated with theseparator=" ", so the rendered line becomes"city , region postalCode , countryCode"(space before each comma). Typical fix is to omit the literal commas and rely onAddress's own line/field joining, or set the separator to""and keep commas attached to the previous field via a custom formatter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsLocationSection.tsx` around lines 192 - 202, The Address rendering uses literal "," entries inside the lines prop combined with separator=" ", which creates unwanted spaces before commas; update the Hs1AlbanyNewPatientsLocationSection rendering so the Address call does not include separate comma tokens — either remove the literal "," entries from lines and let Address join fields, or change separator to "" and attach commas to the preceding field (adjust the lines array accordingly) when calling <Address address={streamDocument.address} lines={...} separator={...}> to eliminate stray spaces.starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHeaderSection.tsx-323-397 (1)
323-397:⚠️ Potential issue | 🟡 MinorMobile menu overlay lacks Esc-close and focus management.
The overlay closes on backdrop click but not on Escape, does not trap focus inside the drawer, and does not restore focus to the opener button on close. For a starter template this is survivable, but it's a keyboard-user blocker worth noting before this ships beyond the demo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHeaderSection.tsx` around lines 323 - 397, Mobile overlay currently only closes on backdrop click and misses keyboard accessibility; add Escape-to-close, focus trapping while open, and restore focus to the opener when closing. Use a ref for the menu opener button (e.g., openerRef) and for the drawer container (e.g., drawerRef), add a useEffect that when isMobileMenuOpen is true: (1) set focus to the first focusable element or the drawer (ensure drawer has role="dialog" and aria-modal="true"), (2) attach a keydown listener that closes on Escape by calling setIsMobileMenuOpen(false), and (3) implement a simple focus trap by intercepting Tab/Shift+Tab to cycle focus among focusable elements inside drawer; on cleanup (when closing) remove listeners and call openerRef.current?.focus() to restore focus. Ensure handlers reference isMobileMenuOpen and setIsMobileMenuOpen in Hs1AlbanyNewPatientsHeaderSection and that drawerRef is used on the div rendering the sliding panel.starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHeaderSection.tsx-346-393 (1)
346-393:⚠️ Potential issue | 🟡 MinorCombined-array keys and the string-match on "Patient Education" are brittle.
Two issues in the mobile nav:
[...primaryLinks, ...moreLinks].map(...)uses${item.label}-${item.link}as a React key. If an editor authors the same label+link in both arrays (e.g. a duplicate "Contact Us"), React will warn about duplicate keys and the second item may not update correctly. Prefixing by source list (primary-…/more-…) or including the array index fixes this.- The "Patient Education" sub‑menu is selected by exact label match (
item.label !== "Patient Education"). This silently breaks when the label is translated, renamed, or edited, and the sub‑menu stops expanding. Consider making this a structural property (e.g.hasChildren/childLinks) on the nav item, mirroring theHeaderDropdownLinkshape used inHs1AlbanyServicesHeaderSection.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHeaderSection.tsx` around lines 346 - 393, The mobile nav mapping over [...primaryLinks, ...moreLinks] uses brittle keys and a fragile label check for the patient-education submenu; update the map in Hs1AlbanyNewPatientsHeaderSection.tsx to generate stable unique keys (e.g. prefix each item with its source like `primary-${item.label}-${item.link}` or include the array index) instead of just `${item.label}-${item.link}`, and stop matching on the string "Patient Education" — add or use a structural flag/field on the nav item (e.g. childLinks or hasChildren, mirroring HeaderDropdownLink) and render the collapsible submenu when item.childLinks exists (iterate item.childLinks instead of patientEducationLinks), keeping the same toggle via setIsPatientEducationOpen and isPatientEducationOpen but keyed to the item identity.starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHeaderSection.tsx-278-285 (1)
278-285:⚠️ Potential issue | 🟡 Minor
tel:href contains parens/spaces from the display format.
phonehere is the output offormatPhone, which returns values like"(877) 393-3348". Embedding that directly intotel:${phone}producestel:(877) 393-3348, which is not a well-formed tel URI per RFC 3966 and some dialers/clients mishandle it. The sibling fileHs1AlbanyServicesHeaderSection.tsxalready handles this correctly viadisplayPhone.replace(/[^\d+]/g, "").🛠️ Proposed fix
const phone = formatPhone(streamDocument.mainPhone); + const phoneHref = phone.replace(/[^\d+]/g, ""); @@ - <a - href={`tel:${phone}`} + <a + href={`tel:${phoneHref}`} @@ - <a - href={`tel:${phone}`} + <a + href={`tel:${phoneHref}`}Also applies to: 315-320
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHeaderSection.tsx` around lines 278 - 285, The tel: link uses the formatted display string from formatPhone (e.g. "(877) 393-3348"), which yields an invalid tel URI; update the component to derive a sanitized tel value for the href by stripping all nondigit/non-plus characters (same approach used in Hs1AlbanyServicesHeaderSection.tsx), e.g. compute a local tel string from phone using phone.replace(/[^\d+]/g, "") and use that in href={`tel:${tel}`}; apply the same fix for the other occurrence around the 315-320 block.starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeHoursSection.tsx-95-119 (1)
95-119:⚠️ Potential issue | 🟡 MinorDay labels and hour formatting ignore
locale.The component reads
localefrom the stream document butorderedDaysis hardcoded to English labels andformatHourLabelpinsIntl.DateTimeFormatto"en-US". If this section is ever rendered for a non-English locale the surrounding UI will localize while this block stays English, producing a visibly mixed experience.Consider either (a) passing
localeintoIntl.DateTimeFormatand deriving weekday labels vianew Intl.DateTimeFormat(locale, { weekday: "long" }), or (b) explicitly documenting that this starter is US-only and removing the unusedlocaleplumbing for hours.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeHoursSection.tsx` around lines 95 - 119, orderedDays and formatHourLabel are hardcoded to English; update them to use the component's locale instead. Replace the static orderedDays array with generated weekday labels using new Intl.DateTimeFormat(locale, { weekday: "long" }) over representative dates for Monday–Sunday, and change formatHourLabel to pass the same locale into new Intl.DateTimeFormat rather than "en-US"; keep the existing UTC/time parsing logic but ensure locale is used consistently (refer to orderedDays and formatHourLabel and the component's locale prop/variable).starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsLocationSection.tsx-221-226 (1)
221-226:⚠️ Potential issue | 🟡 Minor
tel:href uses the formatted phone string.Same issue as in
Hs1AlbanyNewPatientsHeaderSection.tsx:phoneis"(xxx) xxx-xxxx"fromformatPhone, sotel:${phone}embeds parens and spaces. Build a sanitized href from digits (and+), and keepphoneonly for display.🛠️ Proposed fix
const phone = formatPhone(streamDocument.mainPhone); + const phoneHref = phone.replace(/[^\d+]/g, ""); @@ - <a - href={phone ? `tel:${phone}` : undefined} + <a + href={phoneHref ? `tel:${phoneHref}` : undefined}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsLocationSection.tsx` around lines 221 - 226, The anchor is using the formatted display string (phone) in the tel: href which includes parens/spaces; sanitize the number for the href by stripping all non-digit and non-plus characters (e.g., create a sanitizedPhone from phone or from formatPhone input) and use href={sanitizedPhone ? `tel:${sanitizedPhone}` : undefined} while leaving {phone} as the visible text; update Hs1AlbanyNewPatientsLocationSection's anchor and mirror the same fix applied in Hs1AlbanyNewPatientsHeaderSection.tsx (use a sanitized variable name like sanitizedPhone to build the href).
Comment on lines
+166
to
+177
| {socialLinks.map((item) => ( | ||
| <Link | ||
| key={`${item.label}-${item.link}`} | ||
| cta={{ link: item.link, linkType: "URL" }} | ||
| className="no-underline" | ||
| target="_blank" | ||
| > | ||
| <span className="flex h-[27px] w-[27px] items-center justify-center rounded-full bg-[#dcb65f] text-sm font-bold text-white"> | ||
| {iconMap[item.icon]} | ||
| </span> | ||
| </Link> | ||
| ))} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -e tsx -e ts SafeLink starter/src/shared
fd SafeLink --exec cat {}Repository: yext/visual-editor
Length of output: 843
🏁 Script executed:
#!/bin/bash
# Check Hs1AlbanyStaffFooterSection.tsx for the pattern mentioned
find . -name "Hs1AlbanyStaffFooterSection.tsx" -type f | head -5Repository: yext/visual-editor
Length of output: 144
🏁 Script executed:
#!/bin/bash
# Search for Link usage with target="_blank" in the codebase to see patterns
rg 'target="_blank"' -A 3 -B 1 starter/src --type tsx --type ts | head -100Repository: yext/visual-editor
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Check if PagesLink is documented or check package.json for version
cat package.json | jq '.dependencies."@yext/pages-components"' 2>/dev/null || echo "Not found in root package.json"
# Try finding it in relevant package.json files
find . -name "package.json" -exec grep -l "pages-components" {} \; | head -5Repository: yext/visual-editor
Length of output: 127
🏁 Script executed:
#!/bin/bash
# Read the Hs1AlbanyStaffFooterSection.tsx to see the claimed pattern
cat "./starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffFooterSection.tsx"Repository: yext/visual-editor
Length of output: 8743
🏁 Script executed:
#!/bin/bash
# Search for target="_blank" in tsx files using rg without type flag
rg 'target="_blank"' starter/src -A 2 -B 2Repository: yext/visual-editor
Length of output: 2126
🏁 Script executed:
#!/bin/bash
# Check what version of pages-components is used
cat ./starter/package.json | grep -A 2 -B 2 "pages-components"Repository: yext/visual-editor
Length of output: 238
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or code about PagesLink auto-injecting rel
# Search in node_modules or package files for pages-components info
find . -path "*/node_modules/@yext/pages-components*" -name "*.d.ts" -o -name "*.js" 2>/dev/null | head -20Repository: yext/visual-editor
Length of output: 294
🏁 Script executed:
#!/bin/bash
# Search for other external links in the codebase to see if any are missing rel
rg 'target="_blank"' starter/src --no-heading -B 1 | grep -E '(target="_blank"|rel=)'Repository: yext/visual-editor
Length of output: 404
🏁 Script executed:
#!/bin/bash
# Get more context around target="_blank" in each file to see rel attribute presence
echo "=== Hs1CarmelFooterSection.tsx ===" && rg 'target="_blank"' starter/src/registry/hs1-carmel/components/Hs1CarmelFooterSection.tsx -B 1 -A 1
echo ""
echo "=== Hs1AlbanyStaffFooterSection.tsx ===" && rg 'target="_blank"' starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffFooterSection.tsx -B 1 -A 1
echo ""
echo "=== Hs1AlbanyNewPatientsFooterSection.tsx ===" && rg 'target="_blank"' starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsFooterSection.tsx -B 2 -A 2Repository: yext/visual-editor
Length of output: 804
Add rel="noreferrer" to external link with target="_blank".
The social media link opens in a new tab without the rel attribute, leaving the opened page with access to window.opener and leaking referrer info. The SafeLink component does not auto-inject rel, so it must be passed explicitly. Similar components in the codebase (Hs1AlbanyStaffFooterSection.tsx and Hs1CarmelFooterSection.tsx) explicitly include rel="noreferrer" on external links.
Fix
<Link
key={`${item.label}-${item.link}`}
cta={{ link: item.link, linkType: "URL" }}
className="no-underline"
target="_blank"
+ rel="noreferrer"
>📝 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.
Suggested change
| {socialLinks.map((item) => ( | |
| <Link | |
| key={`${item.label}-${item.link}`} | |
| cta={{ link: item.link, linkType: "URL" }} | |
| className="no-underline" | |
| target="_blank" | |
| > | |
| <span className="flex h-[27px] w-[27px] items-center justify-center rounded-full bg-[#dcb65f] text-sm font-bold text-white"> | |
| {iconMap[item.icon]} | |
| </span> | |
| </Link> | |
| ))} | |
| {socialLinks.map((item) => ( | |
| <Link | |
| key={`${item.label}-${item.link}`} | |
| cta={{ link: item.link, linkType: "URL" }} | |
| className="no-underline" | |
| target="_blank" | |
| rel="noreferrer" | |
| > | |
| <span className="flex h-[27px] w-[27px] items-center justify-center rounded-full bg-[`#dcb65f`] text-sm font-bold text-white"> | |
| {iconMap[item.icon]} | |
| </span> | |
| </Link> | |
| ))} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsFooterSection.tsx`
around lines 166 - 177, The social media Link elements rendered inside the
socialLinks.map in Hs1AlbanyNewPatientsFooterSection currently use
target="_blank" without a rel attribute; update the Link component (the JSX
inside the map that uses key={`${item.label}-${item.link}`} and cta={{ link:
item.link, linkType: "URL" }}) to include rel="noreferrer" (same approach used
in Hs1AlbanyStaffFooterSection.tsx and Hs1CarmelFooterSection.tsx) so external
pages opened in a new tab cannot access window.opener or receive referrer
information.
Comment on lines
+247
to
+282
| <li className="group relative"> | ||
| <button | ||
| type="button" | ||
| className="flex items-center gap-1 uppercase transition-colors hover:text-[#d3a335]" | ||
| > | ||
| {renderStyledText(props.moreLabel, moreLabel)} | ||
| <span className="text-[10px]">▾</span> | ||
| </button> | ||
| <div className="invisible absolute right-0 top-full z-20 mt-5 w-[320px] border border-[#e8e2d6] bg-white p-5 opacity-0 shadow-[0_18px_48px_rgba(0,0,0,0.12)] transition-all group-hover:visible group-hover:opacity-100"> | ||
| <ul className="space-y-3 border-b border-[#eee4cf] pb-4 text-[12px] uppercase tracking-[0.08em] text-[#4a4a4a]"> | ||
| {props.secondaryLinks.map((item) => ( | ||
| <li key={`${item.label}-${item.link}`}> | ||
| <HeaderLink | ||
| item={item} | ||
| className="transition-colors hover:text-[#d3a335]" | ||
| /> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| <div className="pt-4"> | ||
| <HeaderLink | ||
| item={props.educationParentLink} | ||
| className="text-[12px] uppercase tracking-[0.08em] text-[#4a4a4a] transition-colors hover:text-[#d3a335]" | ||
| /> | ||
| <ul className="mt-3 space-y-2 text-[11px] tracking-[0.06em] text-[#6d6d6d]"> | ||
| {props.educationLinks.map((item) => ( | ||
| <li key={`${item.label}-${item.link}`}> | ||
| <HeaderLink | ||
| item={item} | ||
| className="transition-colors hover:text-[#d3a335]" | ||
| /> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </div> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Hover-only "More" dropdown is not keyboard accessible.
The secondary/education menu opens exclusively via group-hover, so keyboard users tabbing through the nav cannot open it, and the <button> has no aria-expanded/aria-haspopup, no click handler, and no focus-within styling. This blocks task completion for keyboard and many assistive-tech users. Consider:
- toggling visibility on
:focus-withinin addition to:hover, or - managing open/close state with a stateful disclosure (button +
aria-expanded) and revealing on click/Enter/Space.
The mobile <details> branch already handles this correctly; the desktop branch should too.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeHeaderSection.tsx`
around lines 247 - 282, The "More" dropdown in Hs1AlbanyOfficeHeaderSection.tsx
is only revealed via CSS group-hover so it is not keyboard accessible; update
the component to manage open/closed state for the desktop menu (e.g., add a
boolean like isMoreOpen in the Hs1AlbanyOfficeHeaderSection component), replace
the hover-only reveal with conditional rendering/classes driven by that state
for the dropdown container, add a click handler on the button (and keyboard
handlers for Enter/Space) to toggle the state, set proper ARIA attributes on the
button (aria-haspopup="menu" and aria-expanded={isMoreOpen}), and ensure focus
styling/closing logic (escape to close and close on blur/focusout) while keeping
existing renderStyledText and HeaderLink usage for menu items (props.moreLabel,
props.secondaryLinks, props.educationParentLink, props.educationLinks).
Comment on lines
+125
to
+210
| return ( | ||
| <section className="relative overflow-hidden bg-transparent"> | ||
| <div className="absolute inset-0 bg-[#e5c989]" /> | ||
| <div className="relative mx-auto max-w-[1140px] px-[15px] py-[50px]"> | ||
| <h2 | ||
| className="mb-[14px] mt-0 text-center" | ||
| style={{ | ||
| fontFamily: '"Montserrat", "Open Sans", sans-serif', | ||
| fontSize: `${title.fontSize}px`, | ||
| color: title.fontColor, | ||
| fontWeight: title.fontWeight, | ||
| lineHeight: "28px", | ||
| letterSpacing: "1px", | ||
| textTransform: toCssTextTransform(title.textTransform), | ||
| }} | ||
| > | ||
| {resolvedTitle} | ||
| </h2> | ||
| <div className="mt-[30px] grid gap-[30px] lg:grid-cols-[48%_48%] lg:justify-between"> | ||
| <div className="flex flex-col gap-[15px]"> | ||
| {inputFields.map((field, index) => { | ||
| const resolvedLabel = resolveStyledText( | ||
| field.label, | ||
| locale, | ||
| streamDocument, | ||
| ); | ||
|
|
||
| return ( | ||
| <div | ||
| key={`${resolvedLabel}-${index}`} | ||
| className="flex h-[50px] items-center bg-white px-[15px]" | ||
| style={{ | ||
| fontFamily: | ||
| '"Lato", "Open Sans", Arial, Helvetica, sans-serif', | ||
| fontSize: `${field.label.fontSize}px`, | ||
| color: field.label.fontColor, | ||
| fontWeight: field.label.fontWeight, | ||
| lineHeight: "26px", | ||
| textTransform: toCssTextTransform( | ||
| field.label.textTransform, | ||
| ), | ||
| }} | ||
| > | ||
| {resolvedLabel} | ||
| </div> | ||
| ); | ||
| })} | ||
| </div> | ||
| <div | ||
| className="min-h-[190px] bg-white px-[15px] py-[10px]" | ||
| style={{ | ||
| fontFamily: '"Lato", "Open Sans", Arial, Helvetica, sans-serif', | ||
| fontSize: `${notesField.fontSize}px`, | ||
| color: notesField.fontColor, | ||
| fontWeight: notesField.fontWeight, | ||
| lineHeight: "26px", | ||
| textTransform: toCssTextTransform(notesField.textTransform), | ||
| }} | ||
| > | ||
| {resolvedNotes} | ||
| </div> | ||
| </div> | ||
| <p | ||
| className="mb-[16px] mt-[30px] text-center" | ||
| style={{ | ||
| fontFamily: | ||
| '-apple-system, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif', | ||
| fontSize: `${disclaimer.fontSize}px`, | ||
| color: disclaimer.fontColor, | ||
| fontWeight: disclaimer.fontWeight, | ||
| lineHeight: "21px", | ||
| textTransform: toCssTextTransform(disclaimer.textTransform), | ||
| }} | ||
| > | ||
| {resolvedDisclaimer} | ||
| </p> | ||
| <button | ||
| type="button" | ||
| className="h-[50px] w-full border border-white bg-transparent text-center text-[15px] font-bold uppercase tracking-[1px] text-white [font-family:'Nunito_Sans','Open_Sans',sans-serif]" | ||
| > | ||
| {submitLabel} | ||
| </button> | ||
| </div> | ||
| </section> | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
"Contact Form" renders no actual form controls.
Despite the name, the rendered markup contains no <form>, <input>, or <textarea> elements — only styled <div>s displaying the label/notes text, and a type="button" that has no onClick. End users cannot enter data or submit anything. If this is intentional as a demo/visual mockup, consider renaming to clarify (e.g., ...ContactFormMockup) or adding a comment; otherwise real inputs, a form element, and a submit handler are needed for the component to serve its advertised purpose. Also note the accessibility gap: there are no <label htmlFor> associations and the "submit" button conveys no operable intent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffContactFormSection.tsx`
around lines 125 - 210, Hs1AlbanyStaffContactFormSection currently renders
static divs (inputFields mapped to divs, notesField as div, and a button with
submitLabel) instead of real form controls; replace the static markup with a
proper <form> that wraps inputs: for each item in inputFields render a labeled
<input> (or <textarea> when field.type === 'textarea') with unique id (use index
or a stable key), connect label via htmlFor to that id (use resolvedLabel for
label text), and wire controlled value/onChange handlers (or accept
onChange/onSubmit props) so the form can collect data; convert the button to
type="submit" and implement an onSubmit handler (or call an onSubmit prop) to
handle form submission and validation, and ensure accessibility attributes
(aria-invalid, required, etc.) as needed in Hs1AlbanyStaffContactFormSection.
Comment on lines
+199
to
+209
| {socialLinks.map((socialLink, index) => ( | ||
| <Link | ||
| key={`${socialLink.platform}-${index}`} | ||
| href={socialLink.href} | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| className="flex h-8 w-8 items-center justify-center rounded-full bg-[#dcb65f] text-white no-underline" | ||
| > | ||
| <SocialIcon platform={socialLink.platform} /> | ||
| </Link> | ||
| ))} |
Contributor
There was a problem hiding this comment.
Icon-only social links lack an accessible name.
Each <Link> contains only an SVG with aria-hidden="true", so screen readers announce no name for the link. Add an aria-label (or visually hidden text) identifying the platform.
♿ Proposed fix
{socialLinks.map((socialLink, index) => (
<Link
key={`${socialLink.platform}-${index}`}
href={socialLink.href}
target="_blank"
rel="noreferrer"
+ aria-label={socialLink.platform}
className="flex h-8 w-8 items-center justify-center rounded-full bg-[`#dcb65f`] text-white no-underline"
>
<SocialIcon platform={socialLink.platform} />
</Link>
))}📝 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.
Suggested change
| {socialLinks.map((socialLink, index) => ( | |
| <Link | |
| key={`${socialLink.platform}-${index}`} | |
| href={socialLink.href} | |
| target="_blank" | |
| rel="noreferrer" | |
| className="flex h-8 w-8 items-center justify-center rounded-full bg-[#dcb65f] text-white no-underline" | |
| > | |
| <SocialIcon platform={socialLink.platform} /> | |
| </Link> | |
| ))} | |
| {socialLinks.map((socialLink, index) => ( | |
| <Link | |
| key={`${socialLink.platform}-${index}`} | |
| href={socialLink.href} | |
| target="_blank" | |
| rel="noreferrer" | |
| aria-label={socialLink.platform} | |
| className="flex h-8 w-8 items-center justify-center rounded-full bg-[`#dcb65f`] text-white no-underline" | |
| > | |
| <SocialIcon platform={socialLink.platform} /> | |
| </Link> | |
| ))} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffFooterSection.tsx`
around lines 199 - 209, The social icon links rendered in the socialLinks.map
callback (Link components wrapping SocialIcon) lack accessible names; update the
Link (in the map where socialLinks.map(...) creates Link with key
`${socialLink.platform}-${index}`) to include an aria-label or visually hidden
text that identifies the platform (e.g., aria-label={socialLink.platform} or a
translated label) while keeping the SVG inside SocialIcon aria-hidden; ensure
the label uses the platform value or a human-readable string so screen readers
announce the link purpose.
Comment on lines
+259
to
+282
| <li className="group relative"> | ||
| <button | ||
| type="button" | ||
| className="inline-flex items-center gap-[8px] px-[11px] py-[11px] text-[15px] font-bold leading-[15px] text-[#4a4a4a] [font-family:'Nunito_Sans','Open_Sans',sans-serif]" | ||
| > | ||
| {moreMenu.label} | ||
| <Chevron /> | ||
| </button> | ||
| <div className="absolute right-0 top-full hidden min-w-[260px] bg-[#4f4e4e] shadow-lg group-hover:block group-focus-within:block"> | ||
| <SubmenuList links={moreMenu.links} /> | ||
| <div className="group/nested relative"> | ||
| <Link | ||
| href={moreMenu.nestedMenuHref} | ||
| className="flex items-center justify-between whitespace-nowrap px-[18px] py-[10px] text-[15px] font-bold leading-[20px] text-white no-underline [font-family:'Nunito_Sans','Open_Sans',sans-serif]" | ||
| > | ||
| {moreMenu.nestedMenuLabel} | ||
| <span className="text-white">›</span> | ||
| </Link> | ||
| <div className="absolute right-full top-0 hidden min-w-[290px] bg-[#4f4e4e] shadow-lg group-hover/nested:block group-focus-within/nested:block"> | ||
| <SubmenuList links={moreMenu.nestedLinks} /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </li> |
Contributor
There was a problem hiding this comment.
Desktop "More" menu is not reachable via click/tap.
The button on Line 260-266 has type="button" but no onClick. The dropdown on Line 267 is shown only via group-hover:block / group-focus-within:block. On touch devices (or any environment without real hover — e.g., tablets in desktop viewport, Windows touch laptops), tapping the button does nothing and users can't open the submenu. Consider wiring a state toggle (like the mobile variant already has) and binding aria-expanded / aria-haspopup="menu" to the button so it works via click and is announced correctly by assistive tech.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffHeaderSection.tsx`
around lines 259 - 282, The desktop "More" button currently relies only on CSS
hover/focus to reveal the dropdown so touch/click users can't open it; update
the Hs1AlbanyStaffHeaderSection component to add a local state (e.g.,
isMoreOpen) tied to the moreMenu button, add an onClick handler on that button
to toggle isMoreOpen, set aria-haspopup="menu" and aria-expanded={isMoreOpen} on
the button, and switch the dropdown container (the div that wraps SubmenuList
and nested menu) to render/show when either isMoreOpen is true OR the existing
group-hover/group-focus-within conditions apply so hover still works. Ensure the
nested menu behavior (moreMenu.nestedLinks / nestedMenuHref) continues to work
and that focus/blur or an outside click handler closes the menu for
accessibility.
Comment on lines
+24
to
+46
| const mapTileUrls = [ | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8370!3i12183!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=56293", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8371!3i12183!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=21693", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8372!3i12183!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=118164", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8373!3i12183!4i256!2m3!1e0!2sm!3i772535988!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=95558", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8374!3i12183!4i256!2m3!1e0!2sm!3i772535988!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=60958", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8375!3i12183!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=14364", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8370!3i12184!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=103773", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8371!3i12184!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=69173", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8372!3i12184!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=34573", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8373!3i12184!4i256!2m3!1e0!2sm!3i772535988!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=11967", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8374!3i12184!4i256!2m3!1e0!2sm!3i772535988!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=108438", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8375!3i12184!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=61844", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8370!3i12185!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=20182", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8371!3i12185!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=116653", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8372!3i12185!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=82053", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8373!3i12185!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=47453", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8374!3i12185!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=12853", | ||
| "https://maps.google.com/maps/vt?pb=!1m5!1m4!1i15!2i8375!3i12185!4i256!2m3!1e0!2sm!3i772536204!3m13!2sen-US!3sUS!5e18!12m5!1e68!2m2!1sset!2sRoadmap!4e2!12m3!1e37!2m1!1ssmartmaps!4e0!5m1!1e3!23i46991212!23i47054750!23i47083502&key=AIzaSyCPi18lG_rd7of67jBJsJxGESX-KiXqcj4&token=109324", | ||
| ] as const; | ||
|
|
||
| const mapPinUrl = | ||
| "https://maps.gstatic.com/mapfiles/api-3/images/spotlight-poi3.png"; |
Contributor
There was a problem hiding this comment.
Hardcoded Google Maps API key embedded in tile URLs.
The mapTileUrls array bakes a Google Maps API key (AIzaSy...) directly into the source. Even if this key is client-visible at runtime, committing it to the repo makes key rotation painful and leaks it to anyone browsing the source. Additionally, these tiles point to a fixed lat/long (Downers Grove, IL) regardless of the entity being rendered, so every location page will show the same map. Consider replacing with either a parameterized Static Maps URL driven by the entity's yextDisplayCoordinate, or a proper map component, and move the key to a configurable/env-scoped value with appropriate HTTP referrer restrictions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyLocationSection.tsx`
around lines 24 - 46, The array mapTileUrls (and mapPinUrl) currently embed a
hardcoded Google Maps API key and fixed tile coordinates; remove the baked-in
key and static coordinates and instead build tile/static-map URLs at runtime
using a configurable key (from env/config) and the entity's
yextDisplayCoordinate (or replace with a proper map component such as
Leaflet/Google Maps React). Update any code that reads mapTileUrls to accept a
function or prop that composes the URL using process.env (or app config) for the
API key and the entity's yextDisplayCoordinate, and ensure the key is not
committed to source; also add/refactor usage sites of mapTileUrls/mapPinUrl
(e.g., Hs1AlbanyLocationSection) to use the new runtime URL generator.
Comment on lines
+118
to
+119
| const phone = "(877) 393-3348"; | ||
| const addressLines = ["3010 Highland Parkway", "Downers Grove, IL 60515, US"]; |
Contributor
There was a problem hiding this comment.
Hardcoded phone and address in a "Location" section.
phone and addressLines are literals pointing to a single specific location, so this section will render the same contact info on every entity's page. For a location section, these should come from the stream document (e.g., mainPhone, address) — either as resolved entity fields or via YextEntityFieldSelector like heading/subtitle. If this is intentionally a static demo stub, consider adding a TODO so it isn't shipped as-is.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyLocationSection.tsx`
around lines 118 - 119, Hs1AlbanyLocationSection currently hardcodes contact
info via the phone and addressLines constants, causing every entity page to show
the same contact; replace those literals with values sourced from the stream
entity (e.g., use the entity's mainPhone and address fields or wire up
YextEntityFieldSelector for heading/subtitle) inside the
Hs1AlbanyLocationSection component so each location renders its own data, or if
this is intentionally a demo stub add a clear TODO comment indicating it must be
replaced before shipping.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
♻️ Duplicate comments (4)
starter/src/registry/hs1-albany/components/Hs1AlbanyWelcomeSection.tsx (1)
174-188:⚠️ Potential issue | 🟡 MinorSubtitle and body still ignore their
StyledTextPropscontrols.
props.subtitleandprops.bodyexposefontSize,fontColor,fontWeight, andtextTransform, but these paragraphs hard-codetext-[22px] text-[#d3a335]/text-[14px] ... text-[#7a7a7a]and only setfontFamily/lineHeightinline. Editing any of those Puck controls will have no visible effect. Wire the prop values into the inlinestyle(and drop the conflicting fixed Tailwind classes), similar to how the heading is rendered at lines 159-172.♻️ Proposed fix
- <p - className="mb-0 mt-5 text-[22px] text-[`#d3a335`]" - style={{ - fontFamily: "Montserrat, Open Sans, sans-serif", - lineHeight: "1.4", - }} - > + <p + className="mb-0 mt-5" + style={{ + fontFamily: "Montserrat, Open Sans, sans-serif", + lineHeight: "1.4", + fontSize: `${props.subtitle.fontSize}px`, + color: props.subtitle.fontColor, + fontWeight: props.subtitle.fontWeight, + textTransform: toCssTextTransform(props.subtitle.textTransform), + }} + > {subtitle} </p> - <p - className="mb-0 mt-5 text-[14px] leading-7 text-[`#7a7a7a`]" - style={{ fontFamily: "Montserrat, Open Sans, sans-serif" }} - > + <p + className="mb-0 mt-5 leading-7" + style={{ + fontFamily: "Montserrat, Open Sans, sans-serif", + fontSize: `${props.body.fontSize}px`, + color: props.body.fontColor, + fontWeight: props.body.fontWeight, + textTransform: toCssTextTransform(props.body.textTransform), + }} + > {body} </p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany/components/Hs1AlbanyWelcomeSection.tsx` around lines 174 - 188, The subtitle and body <p> elements ignore StyledTextProps because they hard-code Tailwind classes (e.g., text-[22px], text-[`#d3a335`], text-[14px], text-[`#7a7a7a`]) instead of using the prop values; remove those conflicting Tailwind classes and merge the StyledTextProps (fontSize, fontColor, fontWeight, textTransform) into the inline style for the subtitle and body elements (keeping fontFamily and lineHeight), following the same pattern used for the heading rendering (use the subtitle and body variables and apply styles derived from StyledTextProps).starter/src/registry/hs1-carmel/components/Hs1CarmelCopyrightSection.tsx (1)
93-95:⚠️ Potential issue | 🟡 MinorProtocol-relative URL in default link.
Previously flagged: prefer explicit
https://over//to avoid inheritinghttpin insecure contexts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-carmel/components/Hs1CarmelCopyrightSection.tsx` around lines 93 - 95, The default link uses a protocol-relative URL ("//www.henryscheinone.com/products/officite") which can inherit http; update the link in the Hs1CarmelCopyrightSection component to use an explicit HTTPS URL ("https://www.henryscheinone.com/products/officite"). Locate the object with label "Copyright © 2026 MH Sub I, LLC dba Officite" and replace the link property value accordingly so the component always uses secure https.starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffHeaderSection.tsx (1)
286-309:⚠️ Potential issue | 🟠 MajorDesktop "More" menu still opens only via hover/focus-within.
The
<button>on Lines 287-293 has noonClick, and the dropdown on Line 294 is revealed solely throughgroup-hover:block/group-focus-within:block. On touch devices without a real hover state (tablets, touch laptops in desktop viewport), tapping the button does nothing. Wire a localuseStatetoggle (mirroring the mobile pattern already in this file), bindaria-haspopup="menu"andaria-expandedon the button, and OR the state into the dropdown visibility. The same applies to the nested Patient Education submenu (Lines 296-307).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffHeaderSection.tsx` around lines 286 - 309, The "More" desktop dropdown in Hs1AlbanyStaffHeaderSection is only opened via CSS hover/focus and lacks click/touch support; add a local useState (e.g., moreOpen) in Hs1AlbanyStaffHeaderSection mirroring the mobile pattern, toggle it from an onClick handler on the <button> that currently renders {moreMenu.label}, add aria-haspopup="menu" and aria-expanded={moreOpen} to that button, and incorporate moreOpen into the dropdown class/visibility logic (OR it with the existing group-hover/group-focus checks) for the outer menu; do the same for the nested Patient Education submenu (use a nested state like nestedMoreOpen, add onClick, aria attributes to the Link/button that shows nestedMenuLabel, and OR nestedMoreOpen into the nested submenu visibility so touch taps open the nested panel).starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffFooterSection.tsx (1)
226-235:⚠️ Potential issue | 🟠 MajorIcon-only social links still need an accessible name.
Each link contains only an
aria-hiddenSVG, so assistive tech has no link name. Addaria-label={socialLink.platform}or equivalent hidden text.♿ Proposed fix
<Link key={`${socialLink.platform}-${index}`} href={socialLink.href} target="_blank" rel="noreferrer" + aria-label={socialLink.platform} className="flex h-8 w-8 items-center justify-center rounded-full bg-[`#dcb65f`] text-white no-underline" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffFooterSection.tsx` around lines 226 - 235, The social link anchors rendered in socialLinks.map currently contain only an aria-hidden SVG (SocialIcon) and lack accessible names; update the Link element rendered in the map (the JSX inside socialLinks.map) to provide an accessible name by adding aria-label={socialLink.platform} (or aria-label with a more descriptive string derived from socialLink.platform), or alternatively insert visually hidden text inside the Link alongside <SocialIcon />; keep the SocialIcon SVG aria-hidden and ensure the Link uses the platform value for its label so screen readers can announce each link.
🧹 Nitpick comments (11)
starter/src/registry/hs1-carmel/components/Hs1CarmelServicesSection.tsx (1)
16-33: Consider tightening theLinkwrapper's typing.The
as anycast discards the discriminated-union typing ofPagesLinkProps(which distinguishes thectavariant from thehrefvariant). Since both branches are checked at runtime via"cta" in safeProps/"href" in safeProps, you can keep the type safety by narrowing on the union instead of widening toany.♻️ Proposed refactor
-const Link = (props: PagesLinkProps) => { - const safeProps = { ...props } as any; - - if ("cta" in safeProps && safeProps.cta) { - safeProps.cta = { - ...safeProps.cta, - link: getSafeHref(safeProps.cta.link), - }; - } - - if ("href" in safeProps) { - safeProps.href = getSafeHref(safeProps.href); - } - - return <PagesLink {...safeProps} />; -}; +const Link = (props: PagesLinkProps) => { + if ("cta" in props && props.cta) { + return ( + <PagesLink + {...props} + cta={{ ...props.cta, link: getSafeHref(props.cta.link) }} + /> + ); + } + if ("href" in props) { + return <PagesLink {...props} href={getSafeHref(props.href)} />; + } + return <PagesLink {...props} />; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-carmel/components/Hs1CarmelServicesSection.tsx` around lines 16 - 33, The Link wrapper currently casts props to any and loses the PagesLinkProps discriminated-union type; instead, keep props typed as PagesLinkProps and narrow using the existing runtime checks: use "if ('cta' in props && props.cta)" to create a new safeCta with getSafeHref(props.cta.link) and pass it back into <PagesLink ... cta={safeCta}>, and use "else if ('href' in props)" to pass href={getSafeHref(props.href)}; this preserves type safety for PagesLinkProps, avoids the as any cast, and uses the existing symbols Link, PagesLinkProps, PagesLink, and getSafeHref.starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffCopyrightSection.tsx (2)
74-81:rel="noreferrer"alone ontarget="_blank"links — consider addingnoopener.When opening in a new tab, it's conventional to include
noopeneras well (rel="noopener noreferrer") to guarantee the new context cannot accesswindow.opener, regardless of the underlyingLinkcomponent's behavior. Modern browsers implynoopenerfortarget="_blank", but explicit is safer for older engines and for any intermediary that might stripnoreferrer.Proposed fix
- rel={link.newTab ? "noreferrer" : undefined} + rel={link.newTab ? "noopener noreferrer" : undefined}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffCopyrightSection.tsx` around lines 74 - 81, The Link rendered in Hs1AlbanyStaffCopyrightSection uses rel="noreferrer" when link.newTab is true but should include "noopener" too; update the Link's rel prop to use "noopener noreferrer" whenever target="_blank" (e.g., compute relValue = link.newTab ? "noopener noreferrer" : undefined and pass that into the Link) so new-tab links explicitly prevent access to window.opener.
28-36: Property naming inconsistency across sibling components.This file models link items with
href, while the Carmel variant (Hs1CarmelCopyrightSection.tsx) and Chicago variant (Hs1ChicagoCopyrightSection.tsx, seeFooterLink { link: string }) uselink. If these items are ever shared, migrated, or copy-pasted between templates, the divergence will complicate tooling and data reuse. Consider standardizing on one property name across the HS1 registry copyright sections.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffCopyrightSection.tsx` around lines 28 - 36, The FooterLink type in Hs1AlbanyStaffCopyrightSection.tsx uses href while sibling components Hs1CarmelCopyrightSection.tsx and Hs1ChicagoCopyrightSection.tsx use link; standardize the property name across these components to avoid confusion: pick one canonical property (e.g., link), update the FooterLink type in Hs1AlbanyStaffCopyrightSection (and its consumer type Hs1AlbanyStaffCopyrightSectionProps) to use that property name, and update any JSX/usage in the Hs1AlbanyStaffCopyrightSection component to reference the chosen property; also scan and align the other two components (Hs1CarmelCopyrightSection and Hs1ChicagoCopyrightSection) so all three use the same property name.starter/src/registry/hs1-carmel/components/Hs1CarmelCopyrightSection.tsx (1)
4-26: Consider extracting thegetSafeHref+Linkwrapper into a shared module.This exact helper (identical implementation) is duplicated across multiple registry components (e.g.,
Hs1AlbanyCopyrightSection.tsx,Hs1AlbanyStaffCopyrightSection.tsx). Extracting to a shared utility (e.g.,starter/src/registry/_shared/SafeLink.tsx) would reduce drift and keep sanitization consistent if the rules evolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-carmel/components/Hs1CarmelCopyrightSection.tsx` around lines 4 - 26, Extract getSafeHref and the Link wrapper into a shared module (e.g., SafeLink.tsx) that exports getSafeHref and a Link component which accepts PagesLinkProps and normalizes cta.link and href using getSafeHref before rendering PagesLink; update this component to preserve the existing type alias PagesLinkProps (importing ComponentProps<typeof PagesLink>) and keep the same runtime behavior, then replace the duplicated implementations in Hs1CarmelCopyrightSection.tsx and the other registry components to import { getSafeHref, Link } from the new shared module.starter/src/registry/hs1-albany/components/Hs1AlbanyServicesSection.tsx (1)
21-38:as anycast defeats the typedPagesLinkProps.
PagesLinkPropsis derived only to then be discarded viaas any, so mistakes incta/hrefshapes won't surface at compile time. SincePagesLinksupports eitherctaorhref(discriminated), you can narrow withoutany:♻️ Proposed refactor
-const Link = (props: PagesLinkProps) => { - const safeProps = { ...props } as any; - - if ("cta" in safeProps && safeProps.cta) { - safeProps.cta = { - ...safeProps.cta, - link: getSafeHref(safeProps.cta.link), - }; - } - - if ("href" in safeProps) { - safeProps.href = getSafeHref(safeProps.href); - } - - return <PagesLink {...safeProps} />; -}; +const Link = (props: PagesLinkProps) => { + if ("cta" in props && props.cta) { + return ( + <PagesLink + {...props} + cta={{ ...props.cta, link: getSafeHref(props.cta.link) }} + /> + ); + } + if ("href" in props) { + return <PagesLink {...props} href={getSafeHref(props.href)} />; + } + return <PagesLink {...props} />; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany/components/Hs1AlbanyServicesSection.tsx` around lines 21 - 38, The Link component currently casts props to any which defeats type-checking; remove the as any cast and use TypeScript narrowing on PagesLinkProps to preserve discriminated union checks: accept props: PagesLinkProps, then check and narrow with if ("cta" in props && props.cta) to create a new cta object with link: getSafeHref(props.cta.link) and return <PagesLink {...propsWithSafeCta} /> (similarly handle the "href" branch by creating propsWithSafeHref using getSafeHref(props.href)); keep references to PagesLink, PagesLinkProps, Link, getSafeHref, cta and href so the compiler validates shapes instead of bypassing types.starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsBreadcrumbsSection.tsx (1)
43-49: InconsistenthomeLinkshape across sibling breadcrumb components.This component uses
homeLink.link, whileHs1AlbanyStaffBreadcrumbsSectionandHs1AlbanyServicesBreadcrumbsSectionusehomeLink.href. Having two different property names for the same concept across sibling components in the same template family makes future consolidation harder and is error-prone for editor users managing layouts. Recommend aligning onhref(matches the underlyingPagesLink/anchor prop name).Proposed alignment
export type Hs1AlbanyNewPatientsBreadcrumbsSectionProps = { homeLink: { label: string; - link: string; + href: string; }; currentPage: StyledTextProps; };…and update the field schema, default props, and the
<Link href={homeLink.link}>render site accordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsBreadcrumbsSection.tsx` around lines 43 - 49, The prop type Hs1AlbanyNewPatientsBreadcrumbsSectionProps uses homeLink.link which is inconsistent with sibling components that use homeLink.href; update the prop shape to use homeLink.href everywhere (change Hs1AlbanyNewPatientsBreadcrumbsSectionProps to have homeLink: { label: string; href: string } ), update any default props/schema for this component, and update the render site where Link/PagesLink is used to pass href={homeLink.href} instead of href={homeLink.link} (search for Hs1AlbanyNewPatientsBreadcrumbsSection, homeLink.link, and the JSX Link usage to locate all places to change).starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffBreadcrumbsSection.tsx (1)
18-33: Consider extracting theLinkwrapper +getSafeHrefinto a shared helper.The exact same
getSafeHref+Linkwrapper (Lines 11-33) is duplicated verbatim inHs1AlbanyNewPatientsBreadcrumbsSection.tsx,Hs1AlbanyServicesBreadcrumbsSection.tsx, andHs1AlbanyStaffHeaderSection.tsx. Since the PR description notes an intentional move to self-contained components, this may be by design — but if not, a single shared module would reduce drift risk. Also,as anydefeats the typing ofPagesLinkProps; narrowing via a discriminated check on"cta" in propswould preserve types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffBreadcrumbsSection.tsx` around lines 18 - 33, The Link wrapper duplicates getSafeHref logic and uses unsafe casting; extract a shared helper (e.g., export getSafeHref and a SafePagesLink or normalizeLinkProps function) and replace the inline Link in Hs1AlbanyStaffBreadcrumbsSection (and the other components) to import and reuse that helper; preserve typing by using a proper type guard (narrow props with "cta" in props rather than casting to any) and update the wrapper to map cta.link and href through getSafeHref while keeping the original PagesLinkProps type.starter/src/registry/hs1-carmel/components/Hs1CarmelQuickLinksSection.tsx (1)
23-37: Avoid dropping link prop typing withany.The wrapper is doing useful normalization, but
as anyhides futurePagesLinkprop-shape changes. Prefer a typed copy or narrow only the mutable fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-carmel/components/Hs1CarmelQuickLinksSection.tsx` around lines 23 - 37, The Link wrapper is losing type-safety by casting safeProps to any; instead create a properly typed copy (e.g., const safeProps: PagesLinkProps = { ...props }) or only narrow the mutable fields (use Partial<PagesLinkProps> or pick the cta/href fields) and preserve the original PagesLinkProps type for the rest so you don't drop future prop shape checks; then mutate safeProps.cta.link and safeProps.href using getSafeHref as currently implemented and return <PagesLink {...safeProps} />.starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesHeaderSection.tsx (1)
26-43: Avoidas anyin theLinkwrapper.
PagesLinkPropsis a discriminated union of{ cta: ... }vs{ href: ... }, so casting toanyerases the type checking this wrapper is otherwise preserving. You can keep the runtime behavior and recover types with a narrowed copy:-const Link = (props: PagesLinkProps) => { - const safeProps = { ...props } as any; - - if ("cta" in safeProps && safeProps.cta) { - safeProps.cta = { - ...safeProps.cta, - link: getSafeHref(safeProps.cta.link), - }; - } - - if ("href" in safeProps) { - safeProps.href = getSafeHref(safeProps.href); - } - - return <PagesLink {...safeProps} />; -}; +const Link = (props: PagesLinkProps) => { + if ("cta" in props && props.cta) { + return ( + <PagesLink + {...props} + cta={{ ...props.cta, link: getSafeHref(props.cta.link) }} + /> + ); + } + if ("href" in props) { + return <PagesLink {...props} href={getSafeHref(props.href)} />; + } + return <PagesLink {...props} />; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesHeaderSection.tsx` around lines 26 - 43, The wrapper Link currently uses "as any" on safeProps which erases the discriminated-union types from PagesLinkProps; remove the cast and instead narrow props by the discriminant and create a correctly typed copy for each branch. For example, use the runtime checks ("cta" in props) and ("href" in props) against the original props to build new objects like const withCta: PagesLinkProps = { ...props, cta: { ...props.cta, link: getSafeHref(props.cta.link) } } or const withHref: PagesLinkProps = { ...props, href: getSafeHref(props.href) } (or return directly from each branch), then return <PagesLink {...withCta} /> or <PagesLink {...withHref} /> so you preserve type safety without using any; reference functions/types: Link, PagesLink, PagesLinkProps, getSafeHref.starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeHeroSection.tsx (1)
16-33: Drop theas anycast in theLinkwrapper.
PagesLinkPropsis already a discriminated union betweencta-style andhref-style link props. Theas anycast here erases TS coverage for the whole prop bag (not just the branch you're mutating), so future prop drift onPagesLinkwon't be caught. A narrower approach works withoutany:♻️ Proposed refactor
-const Link = (props: PagesLinkProps) => { - const safeProps = { ...props } as any; - - if ("cta" in safeProps && safeProps.cta) { - safeProps.cta = { - ...safeProps.cta, - link: getSafeHref(safeProps.cta.link), - }; - } - - if ("href" in safeProps) { - safeProps.href = getSafeHref(safeProps.href); - } - - return <PagesLink {...safeProps} />; -}; +const Link = (props: PagesLinkProps) => { + if ("cta" in props && props.cta) { + return ( + <PagesLink + {...props} + cta={{ ...props.cta, link: getSafeHref(props.cta.link) }} + /> + ); + } + if ("href" in props) { + return <PagesLink {...props} href={getSafeHref(props.href)} />; + } + return <PagesLink {...props} />; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeHeroSection.tsx` around lines 16 - 33, Remove the any cast by narrowing the discriminated PagesLinkProps union and returning in separate branches: inside the Link function, check 'cta' in props and if present create a newProps const = { ...props, cta: { ...props.cta, link: getSafeHref(props.cta.link) } } and return <PagesLink {...newProps} />; else if 'href' in props create newProps = { ...props, href: getSafeHref(props.href) } and return <PagesLink {...newProps} />; finally return <PagesLink {...props} /> for the fallback—this preserves the PagesLinkProps types without using as any while still applying getSafeHref to cta.link or href.starter/src/registry/hs1-albany/components/Hs1AlbanyHeaderSection.tsx (1)
152-153: Avoid editable content as React keys.These keys collide when editors add multiple default links like
{ label: "Link", link: "#" }, which can cause React to reuse the wrong item during edits. Prefer a stable item id if Puck provides one; otherwise include the index as a fallback.🔑 Fallback approach if no stable item id is available
- {props.primaryLinks.map((item) => ( - <li key={`${item.label}-${item.link}`}> + {props.primaryLinks.map((item, index) => ( + <li key={`${item.label}-${item.link}-${index}`}> ... - {props.moreLinks.map((item) => ( + {props.moreLinks.map((item, index) => ( <Link - key={`${item.label}-${item.link}`} + key={`${item.label}-${item.link}-${index}`} ... - {props.patientEducation.items.map((item) => ( + {props.patientEducation.items.map((item, index) => ( <Link - key={`${item.label}-${item.link}`} + key={`${item.label}-${item.link}-${index}`}Also applies to: 171-174, 191-194
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/src/registry/hs1-albany/components/Hs1AlbanyHeaderSection.tsx` around lines 152 - 153, The list item keys currently use editable fields (key={`${item.label}-${item.link}`}) which can collide; update the key generation in the Hs1AlbanyHeaderSection render maps (the props.primaryLinks.map occurrences and the other two map usages noted) to prefer a stable identifier (e.g., item.id or whatever stable id Puck provides) and only fall back to combining a safe fallback plus the index when no stable id exists (e.g., use item.id ?? `${item.label}-${index}`), ensuring each <li> key is deterministic and unique across renders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHeaderSection.tsx`:
- Around line 350-423: The mobile sheet overlay lacks dialog semantics and focus
management; update Hs1AlbanyNewPatientsHeaderSection so the overlay container
(rendered when isMobileMenuOpen is true) includes role="dialog" and
aria-modal="true", add an Escape key handler that calls
setIsMobileMenuOpen(false) (attach/remove on open/close), implement focus
trapping while open (move focus into the sheet and prevent tabbing to
background) and restore focus to the element that opened the menu when it
closes; use existing state handlers setIsMobileMenuOpen and
isPatientEducationOpen and ensure click-on-backdrop still closes the dialog
while clicks inside call event.stopPropagation().
- Around line 254-295: The dropdowns open only on hover so keyboard users can't
reach them; update the "More" button and the patient submenu so they also open
on focus/keyboard: either add CSS focus-within utilities to the parent list
items (the li with class "group" that contains the "More" button and the nested
li with class "group/patient") to toggle the submenu visibility (same classes
used for group-hover but with focus-within), or implement a small stateful
toggle on the "More" button (and the patient submenu trigger) that manages
aria-expanded and conditionally applies the visible/opacity classes for the
submenu; ensure the "More" button and the patient trigger are real buttons with
aria-expanded and submenus maintain keyboard focusability for the items rendered
from moreLinks and patientEducationLinks.
- Around line 118-129: The formatted phone string produced by formatPhone is
being reused inside tel: hrefs (in Hs1AlbanyNewPatientsHeaderSection), which
yields invalid hrefs like tel:(877) 393-3348; fix by separating display
formatting from the dialable value: keep formatPhone (or rename to
formatPhoneDisplay) to return the human-readable string, and add a small helper
(e.g., getDialablePhone or extractDigits) that returns only digits
(phone.replace(/\D/g, "") or prefixed with + if needed) and use that helper when
building tel: links in the component (instead of the formatted display value).
In
`@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsx`:
- Around line 181-188: The Google Maps iframe is being rendered even when
mapQuery is an empty string; update the Hs1AlbanyOfficeLocationSection render
logic to only render the iframe (or the entire map block) when mapQuery is
truthy/non-empty (derived from address?.line1, address?.city, address?.region,
address?.postalCode). In practice, wrap the iframe JSX that uses
`https://www.google.com/maps?q=${mapQuery}&output=embed` in a conditional check
like `if (mapQuery)` (or render `{mapQuery && <iframe .../>}`) so the map is not
requested/shown when there is no address; apply the same conditional guard to
the other map rendering location referenced in the file.
- Line 172: The component Hs1AlbanyOfficeLocationSection currently normalizes
mainPhone to an empty string (const mainPhone = String(streamDocument.mainPhone
?? "")) but then always renders the contact Link and label even when mainPhone
is falsy; change rendering so the contact label and the Link (<Link ...>) are
only rendered when mainPhone is truthy (non-empty). Similarly, build a boolean
like hasAddress from the address parts on streamDocument (street, city, state,
postalCode) and conditionally render the Google Maps iframe block (the iframe
and its container currently at lines ~295–307) only when hasAddress is true to
avoid creating an invalid embed URL; update any href/src constructors to only
run when those values exist.
In
`@starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesHeaderSection.tsx`:
- Around line 247-295: The "More" dropdown in Hs1AlbanyServicesHeaderSection is
only opened by CSS group-hover so keyboard and touch users can't access it;
update the component to manage open state (e.g., const [moreOpen, setMoreOpen] =
useState(false)) and wire the trigger button (currently rendering {moreLabel})
with onClick to toggle, onKeyDown to handle Escape/Arrow keys, and set
aria-haspopup="menu" and aria-expanded={moreOpen}; render the outer panel and
each submenu conditionally (or add focus-within:block classes) based on moreOpen
and per-item submenu state (use item-level state or a focused index) so nested
submenus (item.childLinks) also become keyboard-focusable, and ensure outside
clicks/blur close the menu (or use a headless primitive like Radix/Headless UI
to replace the CSS-only behavior).
- Around line 210-211: The tel: href is being built from the formatted
displayPhone (via displayPhone.replace(/[^\d+]/g, "")) which strips the leading
+, so derive phoneHref from the raw input instead: use streamDocument.mainPhone
(or a safe empty string) and strip all non-digits except a single leading +
(e.g. phoneHref = (streamDocument.mainPhone || "").trim().replace(/(?!^\+)\D/g,
"") , with a fallback to remove all non-digits if needed) and leave
renderPhone(streamDocument.mainPhone) only for display.
- Around line 172-183: The renderPhone function currently returns a hardcoded
"(877) 393-3348" when phone is missing; change renderPhone(phone?: string,
fallback?: string) to return undefined/null when no phone is provided (or use
the passed-in fallback prop instead of the hardcoded number), keep the 10-digit
formatting logic intact, and update the header render to compute a displayPhone
variable from streamDocument.mainPhone or the configured fallback and wrap the
phone CTA block in a conditional like {displayPhone && (...)} so no hardcoded
number leaks into tenants; refer to renderPhone and the streamDocument.mainPhone
usage and the phone CTA render block to locate where to apply these changes.
In
`@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffCopyrightSection.tsx`:
- Around line 98-102: Replace the protocol-relative URL in the
Hs1AlbanyStaffCopyrightSection component with an explicit HTTPS URL: update the
href value currently set to "//www.henryscheinone.com/products/officite" to
"https://www.henryscheinone.com/products/officite" while keeping the same label
and newTab settings so the link always uses secure HTTPS.
In
`@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffHeaderSection.tsx`:
- Around line 242-243: The code uses streamDocument.mainPhone directly when
building telHref which can be a non-string and produce invalid values; update
the logic around mainPhone and telHref so you first verify mainPhone is a string
(e.g., typeof streamDocument.mainPhone === "string"), otherwise use a validated
fallback (avoid the hardcoded demo unless intentional—consider empty string or a
config/default phone constant), then create a sanitizedPhone by stripping
non-digits/plus from that string and build telHref = `tel:${sanitizedPhone}`;
ensure the changes touch the mainPhone variable assignment and telHref
construction in Hs1AlbanyStaffHeaderSection to guarantee only a valid string is
sanitized for the href.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyHeaderSection.tsx`:
- Around line 162-204: The dropdowns in Hs1AlbanyHeaderSection are only
hover-activated (props.moreTrigger / props.patientEducation and moreLinks) which
blocks keyboard users; update the markup so the trigger is keyboard-focusable
and the menus become visible on focus as well — e.g., change the Link used for
the “More” trigger to a focusable button or anchor with role/button and
aria-expanded, add focus-within/focus-visible Tailwind classes to the parent
group container (replace group-hover:visible/group-hover:opacity-100 with
group-hover:... plus group-focus-within:visible/group-focus-within:opacity-100
or add focus-visible equivalents), and ensure the nested patientEducation
submenu container uses group/patient-focus-within variants too; also add
appropriate aria-haspopup and aria-controls attributes and manage aria-expanded
state if you convert to a stateful menu.
- Around line 126-212: The tel: link currently uses the formatted const phone
"(877) 393-3348" which introduces parentheses/spaces into the URI; update
Hs1AlbanyHeaderSection to keep the human-friendly phone for display (e.g.
phoneDisplay) and compute a normalized dial string (e.g. phoneDial) by stripping
non-digit characters (and optionally prefixing +1 if needed), then change the
Link cta to use `tel:${phoneDial}` while leaving the visible content as the
formatted phoneDisplay; update the const phone usage and the Link that builds
cta={{ link: `tel:${phone}`, ... }} accordingly.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyServicesSection.tsx`:
- Around line 113-122: The list item key in Hs1AlbanyServicesSection's
props.cards.map uses `${card.title.label}-${card.title.link}`, which can collide
for duplicate cards; update the key to include the iteration index (e.g., use
`${card.title.label}-${card.title.link}-${index}`) when generating keys inside
the props.cards.map callback so each rendered <div> has a unique key; ensure you
reference the same callback where resolvedImage is computed and the return <div
key=...> is set.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelFooterSection.tsx`:
- Around line 250-265: The social link rendering can crash or be inaccessible
when item.icon is unknown or item.label is empty; update the
props.socialLinks.map block so the resolved Icon uses a safe fallback (e.g.,
const Icon = iconMap[item.icon] || FallbackIcon) and provide a fallback
accessible name for the visually hidden span (e.g., item.label || item.link ||
'Social link'); ensure the link markup still renders the fallback Icon component
and the sr-only text uses the fallback label so the anchor remains navigable and
accessible when persisted data drifts.
- Around line 18-39: getSafeHref currently returns any non-empty string and can
forward unsafe schemes (e.g., javascript:, data:) to PagesLink; update
getSafeHref to enforce a scheme allowlist (e.g., http, https, mailto, tel) by
parsing the href (or lowercasing and checking prefix) and returning "#" when the
scheme is not allowed or href is empty, and ensure Link still uses getSafeHref
for both safeProps.cta.link and safeProps.href so editor-controlled values like
socialLinks.link are normalized and unsafe schemes are rejected before
forwarding to PagesLink.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelQuickLinksSection.tsx`:
- Around line 16-19: getSafeHref currently only checks for empty strings and
allows unsafe schemes; update getSafeHref to whitelist safe protocols and
relative URLs: trim the input (as now), allow hrefs that start with "/" or "#"
(relative/internal), otherwise parse the href (e.g., via new URL(href,
"http://example") to handle absolute URLs) and only allow protocols "http:",
"https:", "mailto:", "tel:". If parsing fails or the protocol is not in the
whitelist, return "#" so links like "javascript:" or "data:" are rejected.
- Around line 106-115: The map over props.cards in Hs1CarmelQuickLinksSection
uses a non-unique key `${card.title}-${card.link}`, causing duplicate React keys
for new placeholder cards; change the Link key to include a stable unique
identifier (prefer an existing unique field like card._key or card.id if
present) and fall back to combining the current key with the map index (e.g.,
use card._key || `${card.title}-${card.link}-${index}`) so each rendered card
has a truly unique key.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelServicesSection.tsx`:
- Around line 152-162: The "Learn More" CTA in Hs1CarmelServicesSection is
hardcoded; instead pass it through the same localization flow used for the
heading (e.g., use resolveComponentData or the TranslatableString type) so it
renders per-locale. Update the Link usage to supply a localized label (e.g.,
resolveComponentData(card.ctaLabel || defaultCta, locale) or convert
card.title/description/cta to TranslatableString and resolve them before render)
and ensure the component uses the same locale variable and resolver as the
heading.
---
Duplicate comments:
In
`@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffFooterSection.tsx`:
- Around line 226-235: The social link anchors rendered in socialLinks.map
currently contain only an aria-hidden SVG (SocialIcon) and lack accessible
names; update the Link element rendered in the map (the JSX inside
socialLinks.map) to provide an accessible name by adding
aria-label={socialLink.platform} (or aria-label with a more descriptive string
derived from socialLink.platform), or alternatively insert visually hidden text
inside the Link alongside <SocialIcon />; keep the SocialIcon SVG aria-hidden
and ensure the Link uses the platform value for its label so screen readers can
announce each link.
In
`@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffHeaderSection.tsx`:
- Around line 286-309: The "More" desktop dropdown in
Hs1AlbanyStaffHeaderSection is only opened via CSS hover/focus and lacks
click/touch support; add a local useState (e.g., moreOpen) in
Hs1AlbanyStaffHeaderSection mirroring the mobile pattern, toggle it from an
onClick handler on the <button> that currently renders {moreMenu.label}, add
aria-haspopup="menu" and aria-expanded={moreOpen} to that button, and
incorporate moreOpen into the dropdown class/visibility logic (OR it with the
existing group-hover/group-focus checks) for the outer menu; do the same for the
nested Patient Education submenu (use a nested state like nestedMoreOpen, add
onClick, aria attributes to the Link/button that shows nestedMenuLabel, and OR
nestedMoreOpen into the nested submenu visibility so touch taps open the nested
panel).
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyWelcomeSection.tsx`:
- Around line 174-188: The subtitle and body <p> elements ignore StyledTextProps
because they hard-code Tailwind classes (e.g., text-[22px], text-[`#d3a335`],
text-[14px], text-[`#7a7a7a`]) instead of using the prop values; remove those
conflicting Tailwind classes and merge the StyledTextProps (fontSize, fontColor,
fontWeight, textTransform) into the inline style for the subtitle and body
elements (keeping fontFamily and lineHeight), following the same pattern used
for the heading rendering (use the subtitle and body variables and apply styles
derived from StyledTextProps).
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelCopyrightSection.tsx`:
- Around line 93-95: The default link uses a protocol-relative URL
("//www.henryscheinone.com/products/officite") which can inherit http; update
the link in the Hs1CarmelCopyrightSection component to use an explicit HTTPS URL
("https://www.henryscheinone.com/products/officite"). Locate the object with
label "Copyright © 2026 MH Sub I, LLC dba Officite" and replace the link
property value accordingly so the component always uses secure https.
---
Nitpick comments:
In
`@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsBreadcrumbsSection.tsx`:
- Around line 43-49: The prop type Hs1AlbanyNewPatientsBreadcrumbsSectionProps
uses homeLink.link which is inconsistent with sibling components that use
homeLink.href; update the prop shape to use homeLink.href everywhere (change
Hs1AlbanyNewPatientsBreadcrumbsSectionProps to have homeLink: { label: string;
href: string } ), update any default props/schema for this component, and update
the render site where Link/PagesLink is used to pass href={homeLink.href}
instead of href={homeLink.link} (search for
Hs1AlbanyNewPatientsBreadcrumbsSection, homeLink.link, and the JSX Link usage to
locate all places to change).
In
`@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeHeroSection.tsx`:
- Around line 16-33: Remove the any cast by narrowing the discriminated
PagesLinkProps union and returning in separate branches: inside the Link
function, check 'cta' in props and if present create a newProps const = {
...props, cta: { ...props.cta, link: getSafeHref(props.cta.link) } } and return
<PagesLink {...newProps} />; else if 'href' in props create newProps = {
...props, href: getSafeHref(props.href) } and return <PagesLink {...newProps}
/>; finally return <PagesLink {...props} /> for the fallback—this preserves the
PagesLinkProps types without using as any while still applying getSafeHref to
cta.link or href.
In
`@starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesHeaderSection.tsx`:
- Around line 26-43: The wrapper Link currently uses "as any" on safeProps which
erases the discriminated-union types from PagesLinkProps; remove the cast and
instead narrow props by the discriminant and create a correctly typed copy for
each branch. For example, use the runtime checks ("cta" in props) and ("href" in
props) against the original props to build new objects like const withCta:
PagesLinkProps = { ...props, cta: { ...props.cta, link:
getSafeHref(props.cta.link) } } or const withHref: PagesLinkProps = { ...props,
href: getSafeHref(props.href) } (or return directly from each branch), then
return <PagesLink {...withCta} /> or <PagesLink {...withHref} /> so you preserve
type safety without using any; reference functions/types: Link, PagesLink,
PagesLinkProps, getSafeHref.
In
`@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffBreadcrumbsSection.tsx`:
- Around line 18-33: The Link wrapper duplicates getSafeHref logic and uses
unsafe casting; extract a shared helper (e.g., export getSafeHref and a
SafePagesLink or normalizeLinkProps function) and replace the inline Link in
Hs1AlbanyStaffBreadcrumbsSection (and the other components) to import and reuse
that helper; preserve typing by using a proper type guard (narrow props with
"cta" in props rather than casting to any) and update the wrapper to map
cta.link and href through getSafeHref while keeping the original PagesLinkProps
type.
In
`@starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffCopyrightSection.tsx`:
- Around line 74-81: The Link rendered in Hs1AlbanyStaffCopyrightSection uses
rel="noreferrer" when link.newTab is true but should include "noopener" too;
update the Link's rel prop to use "noopener noreferrer" whenever target="_blank"
(e.g., compute relValue = link.newTab ? "noopener noreferrer" : undefined and
pass that into the Link) so new-tab links explicitly prevent access to
window.opener.
- Around line 28-36: The FooterLink type in Hs1AlbanyStaffCopyrightSection.tsx
uses href while sibling components Hs1CarmelCopyrightSection.tsx and
Hs1ChicagoCopyrightSection.tsx use link; standardize the property name across
these components to avoid confusion: pick one canonical property (e.g., link),
update the FooterLink type in Hs1AlbanyStaffCopyrightSection (and its consumer
type Hs1AlbanyStaffCopyrightSectionProps) to use that property name, and update
any JSX/usage in the Hs1AlbanyStaffCopyrightSection component to reference the
chosen property; also scan and align the other two components
(Hs1CarmelCopyrightSection and Hs1ChicagoCopyrightSection) so all three use the
same property name.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyHeaderSection.tsx`:
- Around line 152-153: The list item keys currently use editable fields
(key={`${item.label}-${item.link}`}) which can collide; update the key
generation in the Hs1AlbanyHeaderSection render maps (the props.primaryLinks.map
occurrences and the other two map usages noted) to prefer a stable identifier
(e.g., item.id or whatever stable id Puck provides) and only fall back to
combining a safe fallback plus the index when no stable id exists (e.g., use
item.id ?? `${item.label}-${index}`), ensuring each <li> key is deterministic
and unique across renders.
In `@starter/src/registry/hs1-albany/components/Hs1AlbanyServicesSection.tsx`:
- Around line 21-38: The Link component currently casts props to any which
defeats type-checking; remove the as any cast and use TypeScript narrowing on
PagesLinkProps to preserve discriminated union checks: accept props:
PagesLinkProps, then check and narrow with if ("cta" in props && props.cta) to
create a new cta object with link: getSafeHref(props.cta.link) and return
<PagesLink {...propsWithSafeCta} /> (similarly handle the "href" branch by
creating propsWithSafeHref using getSafeHref(props.href)); keep references to
PagesLink, PagesLinkProps, Link, getSafeHref, cta and href so the compiler
validates shapes instead of bypassing types.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelCopyrightSection.tsx`:
- Around line 4-26: Extract getSafeHref and the Link wrapper into a shared
module (e.g., SafeLink.tsx) that exports getSafeHref and a Link component which
accepts PagesLinkProps and normalizes cta.link and href using getSafeHref before
rendering PagesLink; update this component to preserve the existing type alias
PagesLinkProps (importing ComponentProps<typeof PagesLink>) and keep the same
runtime behavior, then replace the duplicated implementations in
Hs1CarmelCopyrightSection.tsx and the other registry components to import {
getSafeHref, Link } from the new shared module.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelQuickLinksSection.tsx`:
- Around line 23-37: The Link wrapper is losing type-safety by casting safeProps
to any; instead create a properly typed copy (e.g., const safeProps:
PagesLinkProps = { ...props }) or only narrow the mutable fields (use
Partial<PagesLinkProps> or pick the cta/href fields) and preserve the original
PagesLinkProps type for the rest so you don't drop future prop shape checks;
then mutate safeProps.cta.link and safeProps.href using getSafeHref as currently
implemented and return <PagesLink {...safeProps} />.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelServicesSection.tsx`:
- Around line 16-33: The Link wrapper currently casts props to any and loses the
PagesLinkProps discriminated-union type; instead, keep props typed as
PagesLinkProps and narrow using the existing runtime checks: use "if ('cta' in
props && props.cta)" to create a new safeCta with getSafeHref(props.cta.link)
and pass it back into <PagesLink ... cta={safeCta}>, and use "else if ('href' in
props)" to pass href={getSafeHref(props.href)}; this preserves type safety for
PagesLinkProps, avoids the as any cast, and uses the existing symbols Link,
PagesLinkProps, PagesLink, and getSafeHref.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 15935b94-2cea-4b2f-8666-e02ddc626655
⛔ Files ignored due to path filters (3)
packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 64 static image.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/StaticMapSection/[desktop] default props with coordinate - with api key.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/StaticMapSection/[mobile] default props with coordinate - with api key.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (48)
starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsBreadcrumbsSection.tsxstarter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsCopyrightSection.tsxstarter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsFooterSection.tsxstarter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHeaderSection.tsxstarter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeFooterSection.tsxstarter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeHeaderSection.tsxstarter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeHeroSection.tsxstarter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsxstarter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesBreadcrumbsSection.tsxstarter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesContentSection.tsxstarter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesCopyrightSection.tsxstarter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesFooterSection.tsxstarter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesHeaderSection.tsxstarter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesLocationSection.tsxstarter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffBreadcrumbsSection.tsxstarter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffCopyrightSection.tsxstarter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffFooterSection.tsxstarter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffHeaderSection.tsxstarter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffLocationSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyCopyrightSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyFooterSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyHeaderSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyHeroSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyLocationSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyServicesSection.tsxstarter/src/registry/hs1-albany/components/Hs1AlbanyWelcomeSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelContactFormSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelCopyrightSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelFooterSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelHeaderSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelHeroSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelLocationSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelQuickLinksSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelServicesSection.tsxstarter/src/registry/hs1-carmel/components/Hs1CarmelWelcomeSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoCopyrightSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoFeaturedServicesSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoHeaderSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoHeroSection.tsxstarter/src/registry/hs1-chicago/components/Hs1ChicagoStaffSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaCopyrightSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaFeaturedArticlesSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaFeaturedBlocksSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaFooterSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaHeaderSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaHeroSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaStaffSection.tsxstarter/src/registry/hs1-laguna/components/Hs1LagunaWelcomeSection.tsx
✅ Files skipped from review due to trivial changes (3)
- starter/src/registry/hs1-albany/components/Hs1AlbanyHeroSection.tsx
- starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesContentSection.tsx
- starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeHeaderSection.tsx
🚧 Files skipped from review as they are similar to previous changes (18)
- starter/src/registry/hs1-albany/components/Hs1AlbanyFooterSection.tsx
- starter/src/registry/hs1-albany/components/Hs1AlbanyCopyrightSection.tsx
- starter/src/registry/hs1-chicago/components/Hs1ChicagoFeaturedServicesSection.tsx
- starter/src/registry/hs1-chicago/components/Hs1ChicagoCopyrightSection.tsx
- starter/src/registry/hs1-carmel/components/Hs1CarmelContactFormSection.tsx
- starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsFooterSection.tsx
- starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesCopyrightSection.tsx
- starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesFooterSection.tsx
- starter/src/registry/hs1-carmel/components/Hs1CarmelLocationSection.tsx
- starter/src/registry/hs1-albany/components/Hs1AlbanyLocationSection.tsx
- starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeFooterSection.tsx
- starter/src/registry/hs1-albany-staff/components/Hs1AlbanyStaffLocationSection.tsx
- starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesBreadcrumbsSection.tsx
- starter/src/registry/hs1-carmel/components/Hs1CarmelHeaderSection.tsx
- starter/src/registry/hs1-carmel/components/Hs1CarmelWelcomeSection.tsx
- starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsCopyrightSection.tsx
- starter/src/registry/hs1-carmel/components/Hs1CarmelHeroSection.tsx
- starter/src/registry/hs1-albany-services/components/Hs1AlbanyServicesLocationSection.tsx
Comment on lines
+118
to
+129
| const formatPhone = (phone: string | undefined) => { | ||
| if (!phone) { | ||
| return "(877) 393-3348"; | ||
| } | ||
|
|
||
| const digits = phone.replace(/\D/g, ""); | ||
| if (digits.length === 10) { | ||
| return `(${digits.slice(0, 3)}) ${digits.slice(3, 6)}-${digits.slice(6)}`; | ||
| } | ||
|
|
||
| return phone; | ||
| }; |
Contributor
There was a problem hiding this comment.
Separate the display phone from the tel: href.
Line 212 stores a formatted display string, then Lines 307 and 343 use it in tel: URLs. For a 10-digit number this produces values like tel:(877) 393-3348, including spaces/formatting. Keep the rendered text formatted, but build the href from dialable digits.
☎️ Proposed fix
+const DEFAULT_PHONE = "8773933348";
+const DEFAULT_PHONE_DISPLAY = "(877) 393-3348";
+
+const getPhoneDigits = (phone: string | undefined) => {
+ const digits = phone?.replace(/\D/g, "") ?? "";
+ return digits || DEFAULT_PHONE;
+};
+
const formatPhone = (phone: string | undefined) => {
- if (!phone) {
- return "(877) 393-3348";
+ if (!phone?.trim()) {
+ return DEFAULT_PHONE_DISPLAY;
}
const digits = phone.replace(/\D/g, "");
if (digits.length === 10) {
return `(${digits.slice(0, 3)}) ${digits.slice(3, 6)}-${digits.slice(6)}`;
@@
- const phone = formatPhone(streamDocument.mainPhone);
+ const phone = formatPhone(streamDocument.mainPhone);
+ const phoneHref = `tel:${getPhoneDigits(streamDocument.mainPhone)}`;
@@
- href={`tel:${phone}`}
+ href={phoneHref}
@@
- href={`tel:${phone}`}
+ href={phoneHref}Also applies to: 212-212, 305-311, 342-347
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHeaderSection.tsx`
around lines 118 - 129, The formatted phone string produced by formatPhone is
being reused inside tel: hrefs (in Hs1AlbanyNewPatientsHeaderSection), which
yields invalid hrefs like tel:(877) 393-3348; fix by separating display
formatting from the dialable value: keep formatPhone (or rename to
formatPhoneDisplay) to return the human-readable string, and add a small helper
(e.g., getDialablePhone or extractDigits) that returns only digits
(phone.replace(/\D/g, "") or prefixed with + if needed) and use that helper when
building tel: links in the component (instead of the formatted display value).
Comment on lines
+254
to
+295
| <li className="group relative"> | ||
| <button | ||
| type="button" | ||
| className="font-['Lato','Arial',sans-serif] text-[10px] uppercase text-[#4a4a4a] transition-colors hover:text-[#d3a335] lg:text-[11px]" | ||
| > | ||
| More | ||
| </button> | ||
| <div className="invisible absolute right-0 top-full z-20 mt-4 min-w-[240px] border border-[#e6e6e6] bg-white opacity-0 shadow-[0_10px_30px_rgba(0,0,0,0.12)] transition-all group-hover:visible group-hover:opacity-100"> | ||
| <ul className="py-2"> | ||
| {moreLinks.map((item) => { | ||
| if (item.label !== "Patient Education") { | ||
| return ( | ||
| <li key={`${item.label}-${item.link}`}> | ||
| <MenuItem | ||
| item={item} | ||
| className="block px-4 py-2 text-left text-[12px] normal-case text-[#4a4a4a] no-underline transition-colors hover:bg-[#f7f7f7] hover:text-[#d3a335]" | ||
| /> | ||
| </li> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <li | ||
| key={`${item.label}-${item.link}`} | ||
| className="group/patient relative" | ||
| > | ||
| <MenuItem | ||
| item={item} | ||
| className="block px-4 py-2 text-left text-[12px] normal-case text-[#4a4a4a] no-underline transition-colors hover:bg-[#f7f7f7] hover:text-[#d3a335]" | ||
| /> | ||
| <div className="invisible absolute left-full top-0 z-20 min-w-[280px] border border-[#e6e6e6] bg-white opacity-0 shadow-[0_10px_30px_rgba(0,0,0,0.12)] transition-all group-hover/patient:visible group-hover/patient:opacity-100"> | ||
| <ul className="py-2"> | ||
| {patientEducationLinks.map((subItem) => ( | ||
| <li key={`${subItem.label}-${subItem.link}`}> | ||
| <MenuItem | ||
| item={subItem} | ||
| className="block px-4 py-2 text-left text-[12px] normal-case text-[#4a4a4a] no-underline transition-colors hover:bg-[#f7f7f7] hover:text-[#d3a335]" | ||
| /> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Make the desktop dropdown keyboard-accessible.
The “More” and nested “Patient Education” menus only open on hover, so keyboard users can focus the button/link but cannot reveal or reach the submenu items. Add focus-within visibility at minimum; a stateful button with aria-expanded would be even better.
♿ Minimal CSS-state fix
- <div className="invisible absolute right-0 top-full z-20 mt-4 min-w-[240px] border border-[`#e6e6e6`] bg-white opacity-0 shadow-[0_10px_30px_rgba(0,0,0,0.12)] transition-all group-hover:visible group-hover:opacity-100">
+ <div className="invisible absolute right-0 top-full z-20 mt-4 min-w-[240px] border border-[`#e6e6e6`] bg-white opacity-0 shadow-[0_10px_30px_rgba(0,0,0,0.12)] transition-all group-hover:visible group-hover:opacity-100 group-focus-within:visible group-focus-within:opacity-100">
@@
- <div className="invisible absolute left-full top-0 z-20 min-w-[280px] border border-[`#e6e6e6`] bg-white opacity-0 shadow-[0_10px_30px_rgba(0,0,0,0.12)] transition-all group-hover/patient:visible group-hover/patient:opacity-100">
+ <div className="invisible absolute left-full top-0 z-20 min-w-[280px] border border-[`#e6e6e6`] bg-white opacity-0 shadow-[0_10px_30px_rgba(0,0,0,0.12)] transition-all group-hover/patient:visible group-hover/patient:opacity-100 group-focus-within/patient:visible group-focus-within/patient:opacity-100">📝 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.
Suggested change
| <li className="group relative"> | |
| <button | |
| type="button" | |
| className="font-['Lato','Arial',sans-serif] text-[10px] uppercase text-[#4a4a4a] transition-colors hover:text-[#d3a335] lg:text-[11px]" | |
| > | |
| More | |
| </button> | |
| <div className="invisible absolute right-0 top-full z-20 mt-4 min-w-[240px] border border-[#e6e6e6] bg-white opacity-0 shadow-[0_10px_30px_rgba(0,0,0,0.12)] transition-all group-hover:visible group-hover:opacity-100"> | |
| <ul className="py-2"> | |
| {moreLinks.map((item) => { | |
| if (item.label !== "Patient Education") { | |
| return ( | |
| <li key={`${item.label}-${item.link}`}> | |
| <MenuItem | |
| item={item} | |
| className="block px-4 py-2 text-left text-[12px] normal-case text-[#4a4a4a] no-underline transition-colors hover:bg-[#f7f7f7] hover:text-[#d3a335]" | |
| /> | |
| </li> | |
| ); | |
| } | |
| return ( | |
| <li | |
| key={`${item.label}-${item.link}`} | |
| className="group/patient relative" | |
| > | |
| <MenuItem | |
| item={item} | |
| className="block px-4 py-2 text-left text-[12px] normal-case text-[#4a4a4a] no-underline transition-colors hover:bg-[#f7f7f7] hover:text-[#d3a335]" | |
| /> | |
| <div className="invisible absolute left-full top-0 z-20 min-w-[280px] border border-[#e6e6e6] bg-white opacity-0 shadow-[0_10px_30px_rgba(0,0,0,0.12)] transition-all group-hover/patient:visible group-hover/patient:opacity-100"> | |
| <ul className="py-2"> | |
| {patientEducationLinks.map((subItem) => ( | |
| <li key={`${subItem.label}-${subItem.link}`}> | |
| <MenuItem | |
| item={subItem} | |
| className="block px-4 py-2 text-left text-[12px] normal-case text-[#4a4a4a] no-underline transition-colors hover:bg-[#f7f7f7] hover:text-[#d3a335]" | |
| /> | |
| </li> | |
| ))} | |
| </ul> | |
| </div> | |
| <li className="group relative"> | |
| <button | |
| type="button" | |
| className="font-['Lato','Arial',sans-serif] text-[10px] uppercase text-[`#4a4a4a`] transition-colors hover:text-[`#d3a335`] lg:text-[11px]" | |
| > | |
| More | |
| </button> | |
| <div className="invisible absolute right-0 top-full z-20 mt-4 min-w-[240px] border border-[`#e6e6e6`] bg-white opacity-0 shadow-[0_10px_30px_rgba(0,0,0,0.12)] transition-all group-hover:visible group-hover:opacity-100 group-focus-within:visible group-focus-within:opacity-100"> | |
| <ul className="py-2"> | |
| {moreLinks.map((item) => { | |
| if (item.label !== "Patient Education") { | |
| return ( | |
| <li key={`${item.label}-${item.link}`}> | |
| <MenuItem | |
| item={item} | |
| className="block px-4 py-2 text-left text-[12px] normal-case text-[`#4a4a4a`] no-underline transition-colors hover:bg-[`#f7f7f7`] hover:text-[`#d3a335`]" | |
| /> | |
| </li> | |
| ); | |
| } | |
| return ( | |
| <li | |
| key={`${item.label}-${item.link}`} | |
| className="group/patient relative" | |
| > | |
| <MenuItem | |
| item={item} | |
| className="block px-4 py-2 text-left text-[12px] normal-case text-[`#4a4a4a`] no-underline transition-colors hover:bg-[`#f7f7f7`] hover:text-[`#d3a335`]" | |
| /> | |
| <div className="invisible absolute left-full top-0 z-20 min-w-[280px] border border-[`#e6e6e6`] bg-white opacity-0 shadow-[0_10px_30px_rgba(0,0,0,0.12)] transition-all group-hover/patient:visible group-hover/patient:opacity-100 group-focus-within/patient:visible group-focus-within/patient:opacity-100"> | |
| <ul className="py-2"> | |
| {patientEducationLinks.map((subItem) => ( | |
| <li key={`${subItem.label}-${subItem.link}`}> | |
| <MenuItem | |
| item={subItem} | |
| className="block px-4 py-2 text-left text-[12px] normal-case text-[`#4a4a4a`] no-underline transition-colors hover:bg-[`#f7f7f7`] hover:text-[`#d3a335`]" | |
| /> | |
| </li> | |
| ))} | |
| </ul> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHeaderSection.tsx`
around lines 254 - 295, The dropdowns open only on hover so keyboard users can't
reach them; update the "More" button and the patient submenu so they also open
on focus/keyboard: either add CSS focus-within utilities to the parent list
items (the li with class "group" that contains the "More" button and the nested
li with class "group/patient") to toggle the submenu visibility (same classes
used for group-hover but with focus-within), or implement a small stateful
toggle on the "More" button (and the patient submenu trigger) that manages
aria-expanded and conditionally applies the visible/opacity classes for the
submenu; ensure the "More" button and the patient trigger are real buttons with
aria-expanded and submenus maintain keyboard focusability for the items rendered
from moreLinks and patientEducationLinks.
Comment on lines
+350
to
+423
| {isMobileMenuOpen ? ( | ||
| <div | ||
| className="fixed inset-0 z-50 bg-black/40" | ||
| onClick={() => setIsMobileMenuOpen(false)} | ||
| > | ||
| <div | ||
| className="h-full w-[86%] max-w-[420px] overflow-y-auto bg-[#f3f3f3] text-[#4a4a4a] shadow-2xl" | ||
| onClick={(event) => event.stopPropagation()} | ||
| > | ||
| <div className="flex items-center justify-between border-b border-[#d7d7d7] px-5 py-4"> | ||
| <span className="font-['Montserrat','Open_Sans',sans-serif] text-sm uppercase tracking-[1px]"> | ||
| Menu | ||
| </span> | ||
| <button | ||
| type="button" | ||
| aria-label="Close menu" | ||
| className="text-2xl leading-none" | ||
| onClick={() => setIsMobileMenuOpen(false)} | ||
| > | ||
| × | ||
| </button> | ||
| </div> | ||
|
|
||
| <ul className="text-[14px]"> | ||
| {[...primaryLinks, ...moreLinks].map((item) => { | ||
| if (item.label !== "Patient Education") { | ||
| return ( | ||
| <li | ||
| key={`${item.label}-${item.link}`} | ||
| className="border-b border-[#dddddd]" | ||
| > | ||
| <MenuItem | ||
| item={item} | ||
| className="block px-5 py-3 text-[#4a4a4a] no-underline" | ||
| /> | ||
| </li> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <li | ||
| key={`${item.label}-${item.link}`} | ||
| className="border-b border-[#dddddd]" | ||
| > | ||
| <button | ||
| type="button" | ||
| className="flex w-full items-center justify-between px-5 py-3 text-left" | ||
| onClick={() => | ||
| setIsPatientEducationOpen( | ||
| (currentValue) => !currentValue, | ||
| ) | ||
| } | ||
| > | ||
| <span>{item.label}</span> | ||
| <span>{isPatientEducationOpen ? "−" : "+"}</span> | ||
| </button> | ||
| {isPatientEducationOpen ? ( | ||
| <ul className="border-t border-[#dddddd] bg-white py-1"> | ||
| {patientEducationLinks.map((subItem) => ( | ||
| <li key={`${subItem.label}-${subItem.link}`}> | ||
| <MenuItem | ||
| item={subItem} | ||
| className="block px-8 py-2 text-[13px] text-[#4a4a4a] no-underline" | ||
| /> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| ) : null} | ||
| </li> | ||
| ); | ||
| })} | ||
| </ul> | ||
| </div> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Add dialog semantics and focus handling for the mobile menu.
The full-screen mobile sheet behaves like a modal, but it has no dialog semantics, Escape close handling, or focus containment. This can send screen-reader and keyboard users into background page content while the menu is visually open.
Consider adding role="dialog", aria-modal="true", Escape handling, and a focus trap/restore helper around this overlay.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@starter/src/registry/hs1-albany-new-patients/components/Hs1AlbanyNewPatientsHeaderSection.tsx`
around lines 350 - 423, The mobile sheet overlay lacks dialog semantics and
focus management; update Hs1AlbanyNewPatientsHeaderSection so the overlay
container (rendered when isMobileMenuOpen is true) includes role="dialog" and
aria-modal="true", add an Escape key handler that calls
setIsMobileMenuOpen(false) (attach/remove on open/close), implement focus
trapping while open (move focus into the sheet and prevent tabbing to
background) and restore focus to the element that opened the menu when it
closes; use existing state handlers setIsMobileMenuOpen and
isPatientEducationOpen and ensure click-on-backdrop still closes the dialog
while clicks inside call event.stopPropagation().
| locale, | ||
| streamDocument, | ||
| ); | ||
| const mainPhone = String(streamDocument.mainPhone ?? ""); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "Hs1AlbanyOfficeLocationSection.tsx" | head -20Repository: yext/visual-editor
Length of output: 148
🏁 Script executed:
git ls-files | grep -i hs1albanyofficeRepository: yext/visual-editor
Length of output: 647
🏁 Script executed:
fd "Hs1AlbanyOffice" --type fRepository: yext/visual-editor
Length of output: 647
🏁 Script executed:
wc -l ./starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsxRepository: yext/visual-editor
Length of output: 152
🏁 Script executed:
sed -n '150,200p' ./starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsxRepository: yext/visual-editor
Length of output: 1681
🏁 Script executed:
sed -n '240,280p' ./starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsxRepository: yext/visual-editor
Length of output: 1572
🏁 Script executed:
find . -type f -name "*.tsx" -o -name "*.ts" | xargs grep -l "interface.*Link.*cta" | head -5Repository: yext/visual-editor
Length of output: 44
🏁 Script executed:
fd -t f "Link" --type f -path "*/components/*" | grep -i link | head -10Repository: yext/visual-editor
Length of output: 230
🏁 Script executed:
rg "export.*Link" -t ts -t tsx --max-count 20 | head -30Repository: yext/visual-editor
Length of output: 89
🏁 Script executed:
sed -n '1,50p' ./starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsxRepository: yext/visual-editor
Length of output: 1378
🏁 Script executed:
rg "const Link|function Link|export.*Link" --max-count 20Repository: yext/visual-editor
Length of output: 11078
🏁 Script executed:
rg "linkType.*PHONE" --max-count 20Repository: yext/visual-editor
Length of output: 3199
🏁 Script executed:
sed -n '160,180p' ./starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsxRepository: yext/visual-editor
Length of output: 530
🏁 Script executed:
sed -n '295,310p' ./starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsxRepository: yext/visual-editor
Length of output: 407
🏁 Script executed:
sed -n '190,210p' ./starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsxRepository: yext/visual-editor
Length of output: 899
🏁 Script executed:
sed -n '280,310p' ./starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsxRepository: yext/visual-editor
Length of output: 801
🏁 Script executed:
sed -n '245,275p' ./starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsxRepository: yext/visual-editor
Length of output: 1229
Avoid rendering an empty phone link when mainPhone is missing.
Line 172 normalizes missing phone to an empty string, then the unconditional Link renders with href="#" and empty child text. Gate the contact label and link on a truthy phone value.
The same issue applies to the map iframe: it renders even when all address parts are missing, producing an invalid Google Maps embed URL.
Proposed fix
- const mainPhone = String(streamDocument.mainPhone ?? "");
+ const mainPhone = String(streamDocument.mainPhone ?? "").trim();
...
- <p
- className="mb-1 mt-5 text-[13px] text-[`#767676`]"
- style={{
- fontSize: `${props.contactLabel.fontSize}px`,
- color: props.contactLabel.fontColor,
- fontWeight: props.contactLabel.fontWeight,
- textTransform: cssTextTransform(
- props.contactLabel.textTransform,
- ),
- }}
- >
- {contactLabel}
- </p>
- <Link
- cta={{
- link: mainPhone,
- linkType: "PHONE",
- }}
- className="font-['Arial','Helvetica',sans-serif] text-[14px] text-[`#4a4a4a`]"
- >
- {mainPhone}
- </Link>
+ {mainPhone ? (
+ <>
+ <p
+ className="mb-1 mt-5 text-[13px] text-[`#767676`]"
+ style={{
+ fontSize: `${props.contactLabel.fontSize}px`,
+ color: props.contactLabel.fontColor,
+ fontWeight: props.contactLabel.fontWeight,
+ textTransform: cssTextTransform(
+ props.contactLabel.textTransform,
+ ),
+ }}
+ >
+ {contactLabel}
+ </p>
+ <Link
+ cta={{
+ link: mainPhone,
+ linkType: "PHONE",
+ }}
+ className="font-['Arial','Helvetica',sans-serif] text-[14px] text-[`#4a4a4a`]"
+ >
+ {mainPhone}
+ </Link>
+ </>
+ ) : null}Also applies to map iframe rendering at lines 295–307 when address is missing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsx`
at line 172, The component Hs1AlbanyOfficeLocationSection currently normalizes
mainPhone to an empty string (const mainPhone = String(streamDocument.mainPhone
?? "")) but then always renders the contact Link and label even when mainPhone
is falsy; change rendering so the contact label and the Link (<Link ...>) are
only rendered when mainPhone is truthy (non-empty). Similarly, build a boolean
like hasAddress from the address parts on streamDocument (street, city, state,
postalCode) and conditionally render the Google Maps iframe block (the iframe
and its container currently at lines ~295–307) only when hasAddress is true to
avoid creating an invalid embed URL; update any href/src constructors to only
run when those values exist.
Comment on lines
+181
to
+188
| const mapQuery = [ | ||
| address?.line1, | ||
| address?.city, | ||
| address?.region, | ||
| address?.postalCode, | ||
| ] | ||
| .filter(Boolean) | ||
| .join(", "); |
Contributor
There was a problem hiding this comment.
Don’t render a Google Maps iframe for an empty address query.
If all address parts are missing, mapQuery is "", but Line 278 still loads https://www.google.com/maps?q=&output=embed, which can show an irrelevant default map. Render the map only when there is a query.
Proposed fix
- <div className="min-h-[320px] border border-[`#dddddd`] bg-white">
- <iframe
- title="Primary location map"
- src={`https://www.google.com/maps?q=${encodeURIComponent(mapQuery)}&output=embed`}
- className="h-[320px] w-full lg:h-[360px]"
- loading="lazy"
- referrerPolicy="no-referrer-when-downgrade"
- />
- </div>
+ {mapQuery ? (
+ <div className="min-h-[320px] border border-[`#dddddd`] bg-white">
+ <iframe
+ title="Primary location map"
+ src={`https://www.google.com/maps?q=${encodeURIComponent(mapQuery)}&output=embed`}
+ className="h-[320px] w-full lg:h-[360px]"
+ loading="lazy"
+ referrerPolicy="no-referrer-when-downgrade"
+ />
+ </div>
+ ) : null}Also applies to: 275-282
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@starter/src/registry/hs1-albany-office/components/Hs1AlbanyOfficeLocationSection.tsx`
around lines 181 - 188, The Google Maps iframe is being rendered even when
mapQuery is an empty string; update the Hs1AlbanyOfficeLocationSection render
logic to only render the iframe (or the entire map block) when mapQuery is
truthy/non-empty (derived from address?.line1, address?.city, address?.region,
address?.postalCode). In practice, wrap the iframe JSX that uses
`https://www.google.com/maps?q=${mapQuery}&output=embed` in a conditional check
like `if (mapQuery)` (or render `{mapQuery && <iframe .../>}`) so the map is not
requested/shown when there is no address; apply the same conditional guard to
the other map rendering location referenced in the file.
Comment on lines
+18
to
+39
| const getSafeHref = (href?: string): string => { | ||
| const trimmedHref = href?.trim(); | ||
| return trimmedHref ? trimmedHref : "#"; | ||
| }; | ||
|
|
||
| type PagesLinkProps = import("react").ComponentProps<typeof PagesLink>; | ||
|
|
||
| const Link = (props: PagesLinkProps) => { | ||
| const safeProps = { ...props } as any; | ||
|
|
||
| if ("cta" in safeProps && safeProps.cta) { | ||
| safeProps.cta = { | ||
| ...safeProps.cta, | ||
| link: getSafeHref(safeProps.cta.link), | ||
| }; | ||
| } | ||
|
|
||
| if ("href" in safeProps) { | ||
| safeProps.href = getSafeHref(safeProps.href); | ||
| } | ||
|
|
||
| return <PagesLink {...safeProps} />; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Print the footer link normalizer and its call sites.
sed -n '18,40p;250,263p' starter/src/registry/hs1-carmel/components/Hs1CarmelFooterSection.tsxRepository: yext/visual-editor
Length of output: 1181
🏁 Script executed:
fd -type f PagesLink --iglob "*pages*link*" --iglob "*component*"Repository: yext/visual-editor
Length of output: 232
🏁 Script executed:
rg -l "PagesLink" --type tsx --type ts -g "*/components/*" | head -5Repository: yext/visual-editor
Length of output: 89
🏁 Script executed:
rg "PagesLink" starter/src/registry/hs1-carmel/components/Hs1CarmelFooterSection.tsx -B 3 -A 3Repository: yext/visual-editor
Length of output: 610
🏁 Script executed:
rg "socialLinks" starter/src/registry/hs1-carmel/ -A 5 -B 2Repository: yext/visual-editor
Length of output: 3838
🏁 Script executed:
rg "SAFE.*HREF|SAFE.*URL|sanitize.*url|URL.*PATTERN" --type js --type ts -iRepository: yext/visual-editor
Length of output: 18360
🏁 Script executed:
rg "@yext/pages-components" -A 3 | head -30Repository: yext/visual-editor
Length of output: 3330
🏁 Script executed:
rg "FooterSocialLinksSlot" -A 20 -B 5 packages/visual-editor/src/components/footer/Repository: yext/visual-editor
Length of output: 47503
🏁 Script executed:
grep -r "link.pattern" packages/visual-editor/src/components/footer/ -A 2 -B 2Repository: yext/visual-editor
Length of output: 512
🏁 Script executed:
rg "validPatterns" packages/visual-editor/src/components/footer/ExpandedFooter.tsx -A 30Repository: yext/visual-editor
Length of output: 1033
🏁 Script executed:
cat packages/visual-editor/src/components/footer/FooterSocialLinksSlot.tsx | head -100Repository: yext/visual-editor
Length of output: 2917
Reject unsafe URL schemes before forwarding links.
getSafeHref currently allows any non-empty string, including javascript:/data: URLs, through to PagesLink. Since socialLinks.link is editor-controlled, keep the fallback but add a scheme allowlist.
🛡️ Proposed URL allowlist
+const SAFE_HREF_PATTERN =
+ /^(https?:\/\/|\/\/|mailto:|tel:|\/(?!\/)|#|\{[A-Za-z0-9_.]+\}$)/i;
+
const getSafeHref = (href?: string): string => {
const trimmedHref = href?.trim();
- return trimmedHref ? trimmedHref : "#";
+ return trimmedHref && SAFE_HREF_PATTERN.test(trimmedHref) ? trimmedHref : "#";
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelFooterSection.tsx` around
lines 18 - 39, getSafeHref currently returns any non-empty string and can
forward unsafe schemes (e.g., javascript:, data:) to PagesLink; update
getSafeHref to enforce a scheme allowlist (e.g., http, https, mailto, tel) by
parsing the href (or lowercasing and checking prefix) and returning "#" when the
scheme is not allowed or href is empty, and ensure Link still uses getSafeHref
for both safeProps.cta.link and safeProps.href so editor-controlled values like
socialLinks.link are normalized and unsafe schemes are rejected before
forwarding to PagesLink.
Comment on lines
+250
to
+265
| {props.socialLinks.map((item) => { | ||
| const Icon = iconMap[item.icon]; | ||
|
|
||
| return ( | ||
| <Link | ||
| key={`${item.label}-${item.link}`} | ||
| cta={{ | ||
| link: item.link, | ||
| linkType: "URL", | ||
| }} | ||
| className="inline-flex h-12 w-12 items-center justify-center rounded-xl bg-white text-[#212121] no-underline transition hover:bg-[#c1bcbc]" | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| > | ||
| <span className="sr-only">{item.label}</span> | ||
| <Icon className="h-5 w-5" /> |
Contributor
There was a problem hiding this comment.
Add runtime fallbacks for social icon and accessible label.
Persisted/editor data can drift from the TypeScript union. An unknown icon will render an undefined component, and an empty label leaves the icon-only link without an accessible name.
♿ Proposed fallback handling
- {props.socialLinks.map((item) => {
- const Icon = iconMap[item.icon];
+ {props.socialLinks.map((item, index) => {
+ const Icon = iconMap[item.icon as keyof typeof iconMap] ?? Rss;
+ const accessibleLabel =
+ item.label?.trim() || `${item.icon || "social"} link`;
return (
<Link
- key={`${item.label}-${item.link}`}
+ key={`${accessibleLabel}-${item.link}-${index}`}
cta={{
link: item.link,
linkType: "URL",
}}
@@
>
- <span className="sr-only">{item.label}</span>
+ <span className="sr-only">{accessibleLabel}</span>
<Icon className="h-5 w-5" />
</Link>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelFooterSection.tsx` around
lines 250 - 265, The social link rendering can crash or be inaccessible when
item.icon is unknown or item.label is empty; update the props.socialLinks.map
block so the resolved Icon uses a safe fallback (e.g., const Icon =
iconMap[item.icon] || FallbackIcon) and provide a fallback accessible name for
the visually hidden span (e.g., item.label || item.link || 'Social link');
ensure the link markup still renders the fallback Icon component and the sr-only
text uses the fallback label so the anchor remains navigable and accessible when
persisted data drifts.
Comment on lines
+16
to
+19
| const getSafeHref = (href?: string): string => { | ||
| const trimmedHref = href?.trim(); | ||
| return trimmedHref ? trimmedHref : "#"; | ||
| }; |
Contributor
There was a problem hiding this comment.
Reject unsafe URL schemes, not just empty strings.
getSafeHref still passes through values like javascript:alert(1) or data:text/html,... from the editable link field. Since these become user-facing links, restrict to expected protocols/relative URLs before rendering.
🛡️ Proposed URL validation
const getSafeHref = (href?: string): string => {
const trimmedHref = href?.trim();
- return trimmedHref ? trimmedHref : "#";
+ if (!trimmedHref) {
+ return "#";
+ }
+
+ try {
+ const parsedUrl = new URL(trimmedHref, "https://example.com");
+ const allowedProtocols = new Set(["http:", "https:", "mailto:", "tel:"]);
+ return allowedProtocols.has(parsedUrl.protocol) ? trimmedHref : "#";
+ } catch {
+ return "#";
+ }
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelQuickLinksSection.tsx`
around lines 16 - 19, getSafeHref currently only checks for empty strings and
allows unsafe schemes; update getSafeHref to whitelist safe protocols and
relative URLs: trim the input (as now), allow hrefs that start with "/" or "#"
(relative/internal), otherwise parse the href (e.g., via new URL(href,
"http://example") to handle absolute URLs) and only allow protocols "http:",
"https:", "mailto:", "tel:". If parsing fails or the protocol is not in the
whitelist, return "#" so links like "javascript:" or "data:" are rejected.
Comment on lines
+106
to
+115
| {props.cards.map((card) => { | ||
| const resolvedImage = resolveComponentData( | ||
| card.image, | ||
| locale, | ||
| streamDocument, | ||
| ); | ||
|
|
||
| return ( | ||
| <Link | ||
| key={`${card.title}-${card.link}`} |
Contributor
There was a problem hiding this comment.
Make card keys unique for duplicate placeholder cards.
Newly added cards can share the default title and link, producing duplicate React keys and unstable reconciliation in the editor.
🔑 Proposed key fix
- {props.cards.map((card) => {
+ {props.cards.map((card, index) => {
const resolvedImage = resolveComponentData(
card.image,
locale,
streamDocument,
);
return (
<Link
- key={`${card.title}-${card.link}`}
+ key={`${card.title}-${card.link}-${index}`}📝 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.
Suggested change
| {props.cards.map((card) => { | |
| const resolvedImage = resolveComponentData( | |
| card.image, | |
| locale, | |
| streamDocument, | |
| ); | |
| return ( | |
| <Link | |
| key={`${card.title}-${card.link}`} | |
| {props.cards.map((card, index) => { | |
| const resolvedImage = resolveComponentData( | |
| card.image, | |
| locale, | |
| streamDocument, | |
| ); | |
| return ( | |
| <Link | |
| key={`${card.title}-${card.link}-${index}`} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelQuickLinksSection.tsx`
around lines 106 - 115, The map over props.cards in Hs1CarmelQuickLinksSection
uses a non-unique key `${card.title}-${card.link}`, causing duplicate React keys
for new placeholder cards; change the Link key to include a stable unique
identifier (prefer an existing unique field like card._key or card.id if
present) and fall back to combining the current key with the map index (e.g.,
use card._key || `${card.title}-${card.link}-${index}`) so each rendered card
has a truly unique key.
Comment on lines
+152
to
+162
| <div className="mt-6"> | ||
| <Link | ||
| cta={{ | ||
| link: card.link, | ||
| linkType: "URL", | ||
| }} | ||
| className="text-sm font-semibold uppercase tracking-[0.16em] text-[#0384D7] no-underline hover:text-black" | ||
| > | ||
| Learn More | ||
| </Link> | ||
| </div> |
Contributor
There was a problem hiding this comment.
"Learn More" is hardcoded in English while the heading is localized.
The section goes through resolveComponentData + locale resolution for the heading, but the CTA label here is a literal English string and card title/description are plain strings (not TranslatableString). For a locale-aware section, this will render "Learn More" in all locales. If this is intentional for the HS1 Carmel starter (single-locale demo), feel free to ignore; otherwise consider running the CTA label (and ideally card text) through the same translation path used for heading.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@starter/src/registry/hs1-carmel/components/Hs1CarmelServicesSection.tsx`
around lines 152 - 162, The "Learn More" CTA in Hs1CarmelServicesSection is
hardcoded; instead pass it through the same localization flow used for the
heading (e.g., use resolveComponentData or the TranslatableString type) so it
renders per-locale. Update the Link usage to supply a localized label (e.g.,
resolveComponentData(card.ctaLabel || defaultCta, locale) or convert
card.title/description/cta to TranslatableString and resolve them before render)
and ensure the component uses the same locale variable and resolver as the
heading.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification