Conversation
WalkthroughAdds a visual search feature and custom directory support to the visual-editor package. New components: SearchComponent (with SearchBarSlot and SearchResultsSlot), MapComponent, Cards, GDAResponse, SourceCard, CustomDirectoryComponent, and CustomBreadcrumbs. Introduces many platform/component locale keys across languages, search utilities (config, visual autocomplete, URL helpers, typing-effect hook), fetch utilities for custom directory entities, and shared types/defaults for search layouts. CTA and CTAWrapper gain an optional textColor prop. Template metadata and publish logic replace multi-state headDeployStatus with a boolean deploymentInProgress. Several category/slot registries and docs/types were updated to export the new pieces. Sequence Diagram(s)mermaid Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ❌ 2❌ Failed checks (2 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (21)
packages/visual-editor/src/components/atoms/cta.tsx-253-257 (1)
253-257:⚠️ Potential issue | 🟠 MajorUse the selected override color for CTA text.
textColorcurrently resolvesoverrideTextColor.contrastingColor, so picking a custom text color will render the opposite/accessibility pair instead of the color the editor selected. That breaks the new override behavior for primary CTAs.🛠️ Minimal fix
- const textColor = getThemeColorCssValue( - overrideTextColor - ? overrideTextColor.contrastingColor - : color?.contrastingColor - ); + const textColor = getThemeColorCssValue( + overrideTextColor?.selectedColor ?? color?.contrastingColor + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/atoms/cta.tsx` around lines 253 - 257, textColor currently uses overrideTextColor.contrastingColor so a chosen custom text color is replaced with its contrast pair; update the expression that computes textColor (the getThemeColorCssValue call) to use the selected override color itself (e.g., overrideTextColor.color or overrideTextColor.value depending on the property name on overrideTextColor) when overrideTextColor is present, otherwise fall back to color?.contrastingColor; adjust only the conditional operand passed to getThemeColorCssValue in the textColor definition.packages/visual-editor/src/components/pageSections/SearchSection/SourceCard.tsx-20-23 (1)
20-23:⚠️ Potential issue | 🟠 MajorEnsure CTA always has a non-empty label.
When a link exists but
searchResult.nameis missing, the CTA can render without meaningful text. That’s an accessibility/UX blocker for an interactive element.Suggested fix
const SourceCard = (props: CitationProps) => { - let rawData: RawData = props.searchResult.rawData; - let link = rawData?.landingPageUrl || rawData?.c_primaryCTA?.link || ""; - const name = props.searchResult?.name; const { t } = useTranslation(); + const rawData: RawData = props.searchResult.rawData ?? {}; + const link = rawData.landingPageUrl || rawData.c_primaryCTA?.link || ""; + const name = props.searchResult?.name; + const displayLabel = + typeof name === "string" && name.trim() + ? name + : t("learnMore", "Learn more"); return ( <div className="px-5 py-2.5 rounded-md"> {link ? ( <CTA link={link} - label={name} + label={displayLabel} variant={"primary"} className="!w-fit justify-center text-xs" normalizeLink={true} /> ) : ( <p> - {name}{" "} + {displayLabel}{" "} <span className="text-xs"> ({t("noLinkAvailable", "no link available")}) </span> </p> )} </div> ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/SourceCard.tsx` around lines 20 - 23, The CTA rendering can end up with an empty label when link exists but searchResult.name (used as name/label) is missing; update the SourceCard where CTA is used (the CTA component call with props link and label/name) to ensure label is never empty by providing a fallback such as label={name || searchResult.url || 'Open source'} or compute a fallbackDisplayName before rendering; alternatively set an explicit aria-label with that fallback if visual text is undesired so the interactive element always has non-empty accessible text.packages/visual-editor/src/components/pageSections/SearchSection/GDAResponse.tsx-13-43 (1)
13-43:⚠️ Potential issue | 🟠 MajorAvoid rendering the final GDA block while loading
Line 13 onward shows a loading skeleton, but Line 35 still renders
GenerativeDirectAnswerat the same time. This can surface stale/duplicated content during fetch transitions.✅ Proposed fix
- {loading && ( + {loading ? ( <div className="p-6 border border-gray-200 rounded-lg shadow-sm my-4 animate-pulse"> <div className="text-xl"> {t("generatingAIAnswer", "Generating AI Answer...")} </div> @@ </div> </div> - )} - <GenerativeDirectAnswer - CitationCard={SourceCard} - customCssClasses={{ - container: "my-4", - divider: "!py-5", - citationsContainer: "grid grid-cols-1 md:grid-cols-3 gap-2 md:gap-4", - }} - /> + ) : ( + <GenerativeDirectAnswer + CitationCard={SourceCard} + customCssClasses={{ + container: "my-4", + divider: "!py-5", + citationsContainer: "grid grid-cols-1 md:grid-cols-3 gap-2 md:gap-4", + }} + /> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/GDAResponse.tsx` around lines 13 - 43, The GenerativeDirectAnswer is rendered while the loading skeleton is also shown, causing stale/duplicated content; update the render logic so that GenerativeDirectAnswer (the component instance with props CitationCard={SourceCard} and customCssClasses) is only rendered when loading is false (i.e., wrap or move the GenerativeDirectAnswer behind a !loading check or into the else branch of the existing loading conditional) to prevent the final GDA block from appearing during fetch transitions.packages/visual-editor/src/components/pageSections/CustomDirectory/utils.ts-25-37 (1)
25-37:⚠️ Potential issue | 🟠 MajorFailure paths should return a stable value (or throw), not implicit
undefined.Current error branches return
undefined, while success returns an array (json.response ?? []). This makes downstream handling fragile and can mask fetch failures as “no data”.🛠️ Proposed fix
-export const fetchData = async ({ +export const fetchData = async ({ endpoint, apiKey, entityIds, -}: FetchDataProps) => { +}: FetchDataProps): Promise<unknown[]> => { @@ const res = await fetch(url.toString()); if (!res.ok) { console.error("Failed to fetch entities:", res.status); - return; + return []; } @@ return fetchedEntities; } catch (error) { console.error("Entity fetch error:", error); + return []; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/CustomDirectory/utils.ts` around lines 25 - 37, The fetch utility currently returns an array on success but yields implicit undefined on failure (in the !res.ok branch and the catch), which breaks callers; update the !res.ok branch and the catch to return a stable empty array instead of falling through, and keep logging the error (i.e., when checking res.ok and in the catch that currently logs "Failed to fetch entities:" / "Entity fetch error:", return [] so callers always receive an array); ensure json.response ?? [] is still returned on success and that the function signature/typing reflects Promise<Array<...>>.packages/visual-editor/src/components/pageSections/SearchSection/useTypeEffect.ts-42-68 (1)
42-68:⚠️ Potential issue | 🟠 MajorAdd
enabledto the dependency array and reset animation state when disabled or prompts change.The second
useEffect(lines 42–68) lacksenabledin its dependency array, so disabling the feature doesn't stop the animation loop or clear the placeholder. Additionally, whenqueryPromptschanges to a shorter list,indexRefcan remain out of bounds, causing the interval to silently stall without updating the placeholder. Reset all refs and the placeholder each time the effect runs to ensure clean state.Suggested fix
useEffect(() => { - if (queryPrompts.length === 0) return; + if (!enabled || queryPrompts.length === 0) { + setPlaceholder(""); + indexRef.current = 0; + charIndexRef.current = 0; + isDeletingRef.current = false; + return; + } + + setPlaceholder(""); + indexRef.current = 0; + charIndexRef.current = 0; + isDeletingRef.current = false; const interval = setInterval(() => { const currentWord = queryPrompts[indexRef.current]; if (!currentWord) return; @@ return () => clearInterval(interval); - }, [queryPrompts]); + }, [enabled, queryPrompts]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/useTypeEffect.ts` around lines 42 - 68, The typing-animation effect (useEffect) must include the enabled flag in its dependency array and must reset state when disabled or when queryPrompts change: add enabled to the deps for the useEffect that references queryPrompts, and at the start of that effect reset indexRef.current = 0, charIndexRef.current = 0, isDeletingRef.current = false, and call setPlaceholder('') so the animation always starts clean; also clamp indexRef.current to indexRef.current % queryPrompts.length whenever queryPrompts is non-empty to avoid out-of-bounds lookups, and ensure the interval is cleared and placeholder cleared when enabled becomes false.packages/visual-editor/src/components/pageSections/SearchSection/useTypeEffect.ts-23-40 (1)
23-40:⚠️ Potential issue | 🟠 MajorAdd missing dependencies and guard against undefined credentials.
Lines 28–40 read
localeanddocument?._envproperties but the dependency array only includesenvandenabled. Whenlocalechanges or ifdocument._envvalues are undefined, the effect won't refetch, leaving the typing effect stale or empty. Additionally, undefined credentials will silently fail in the fetch without retry.Suggested fix
useEffect(() => { - if (!enabled) return; + const apiKey = document?._env?.YEXT_PUBLIC_ADV_SEARCH_API_KEY; + const experienceKey = document?._env?.YEXT_PUBLIC_ADV_SEARCH_EXP_KEY; + if (!enabled || !apiKey || !experienceKey) { + setQueryPrompts([]); + return; + } const fetchPrompts = async () => { const base = env === "PRODUCTION" ? "cdn" : "sbx-cdn"; - const url = `https://${base}.yextapis.com/v2/accounts/me/search/autocomplete?v=20250101&api_key=${document?._env?.YEXT_PUBLIC_ADV_SEARCH_API_KEY}&sessionTrackingEnabled=false&experienceKey=${document?._env?.YEXT_PUBLIC_ADV_SEARCH_EXP_KEY}&locale=${locale}&input=`; + const url = `https://${base}.yextapis.com/v2/accounts/me/search/autocomplete?v=20250101&api_key=${apiKey}&sessionTrackingEnabled=false&experienceKey=${experienceKey}&locale=${locale}&input=`; try { const res = await fetch(url); const data = await res.json(); setQueryPrompts(data.response.results.map((i: any) => i.value)); @@ - fetchPrompts(); - }, [env, enabled]); + void fetchPrompts(); + }, [ + document?._env?.YEXT_PUBLIC_ADV_SEARCH_API_KEY, + document?._env?.YEXT_PUBLIC_ADV_SEARCH_EXP_KEY, + env, + enabled, + locale, + ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/useTypeEffect.ts` around lines 23 - 40, The effect in useTypeEffect.ts (useEffect -> fetchPrompts -> setQueryPrompts) is missing dependencies and doesn't guard against undefined credentials; update the dependency array for the useEffect (currently [env, enabled]) to include locale and the environment credential values accessed (document?._env?.YEXT_PUBLIC_ADV_SEARCH_API_KEY and document?._env?.YEXT_PUBLIC_ADV_SEARCH_EXP_KEY or a single stable object reference) so the effect reruns when those change, and add an early-return/guard in fetchPrompts to check that both API key and experience key are present (log or handle the missing credentials) before constructing the URL and calling fetch to avoid silent failures.packages/visual-editor/src/components/pageSections/SearchSection/MapComponent.tsx-19-26 (1)
19-26:⚠️ Potential issue | 🟠 MajorNormalize the non-universal pin index before passing it to
MapPinIcon.This path forwards
result.indexas-is, but the related pin components are expected to receiveresultIndex >= 1. Please clamp/default it here instead of trusting upstream data.Based on learnings: ensure that the `resultIndex` prop used by `MapPinIcon` (and related components) is always >= 1; enforce it via type definitions, default values, runtime checks, and tests.🔧 Minimal fix
return ( <MapPinIcon color={backgroundColors.background6.value} - resultIndex={result.index} + resultIndex={Math.max(1, result.index ?? 1)} /> ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/MapComponent.tsx` around lines 19 - 26, The LocatorPin component forwards result.index directly to MapPinIcon but MapPinIcon expects resultIndex >= 1; update LocatorPin to normalize/clamp the value before passing it: compute a safeIndex = Math.max(1, Number(result?.index) || 0) (or similar runtime check) and pass that as the resultIndex prop to MapPinIcon, ensuring non-numeric/undefined values default to 1; adjust any PropTypes/types if present for LocatorPin/MapPinIcon to reflect the >=1 invariant and add a unit test that verifies LocatorPin passes 1 when result.index is missing or zero.packages/visual-editor/src/components/pageSections/SearchSection/LayoutSections.tsx-19-23 (1)
19-23:⚠️ Potential issue | 🟠 MajorApply
resultsCountwhen rendering cards.
resultsCountis only written toconsole.log, so this section always renders the full result set. That makes the per-vertical limit ineffective and leaves a debug statement in the new layout path.💡 Minimal fix
export const LayoutSection = ({ layoutType, results, CardComponent, header, resultsCount = 4, }: LayoutSectionProps) => { if (!CardComponent) return null; - console.log(resultsCount); + const visibleResults = results.slice(0, resultsCount); const layoutClasses = layoutType === "Grid" ? GRID_LAYOUT_CLASSES : FLEX_LAYOUT_CLASSES; @@ - {results.map((result, index) => ( + {visibleResults.map((result, index) => ( <CardComponent key={index} result={result} /> ))}Also applies to: 41-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/LayoutSections.tsx` around lines 19 - 23, The LayoutSections component currently logs resultsCount and doesn't enforce the per-vertical limit; remove the console.log and use the resultsCount prop to limit the rendered cards (e.g., slice the results/children array) before mapping to CardComponent so only up to resultsCount cards are rendered; apply the same change where resultsCount is currently just logged (the other occurrence around the block that references CardComponent at lines noted) to ensure the per-vertical limit is honored consistently.packages/visual-editor/src/components/pageSections/CustomDirectory/CustomDirectory.tsx-80-107 (1)
80-107:⚠️ Potential issue | 🟠 MajorClear
entitieswhen there are no child ids or the request fails.Both the callback and the effect currently no-op on empty input, so old directory cards remain rendered after
dm_childEntityIdsbecomes empty. Failed fetches also leave the previous list in place.💡 One way to reset the stale state
const fetchEntities = useCallback( async (entityIds: string[]) => { - if (!entityIds?.length) return; + if (!entityIds?.length) { + setEntities([]); + return; + } setLoading(true); @@ - const fetchedEntities = res?.entities ?? []; - setEntities(fetchedEntities); + setEntities(res?.entities ?? []); } catch (error) { + setEntities([]); console.error("Entity fetch error:", error); } finally { setLoading(false); } }, @@ useEffect(() => { const childIds = streamDocument?.dm_childEntityIds; - if (childIds?.length) fetchEntities(childIds); + if (!childIds?.length) { + setEntities([]); + return; + } + void fetchEntities(childIds); }, [streamDocument?.dm_childEntityIds, fetchEntities]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/CustomDirectory/CustomDirectory.tsx` around lines 80 - 107, The component currently leaves stale entities when dm_childEntityIds becomes empty or fetch fails; update the fetchEntities function and the effect to clear state: inside fetchEntities, if entityIds is empty immediately call setEntities([]) and setLoading(false) (instead of returning), and in the catch block also call setEntities([]) before rethrowing/logging so failures clear the list; additionally, in the useEffect, when streamDocument?.dm_childEntityIds is falsy or has length 0 call setEntities([]) (and ensure fetchEntities is only called when there are ids) so the UI is reset when child ids disappear (references: fetchEntities, setEntities, useEffect, streamDocument?.dm_childEntityIds).packages/visual-editor/src/components/pageSections/SearchSection/Search.tsx-84-140 (1)
84-140:⚠️ Potential issue | 🟠 MajorMove hooks above the early return guards to comply with React hook rules.
Hooks at lines 128–140 are invoked after conditional early returns (lines 84–126). This violates React's rule that hooks must be called in the same order and number on every render. If
apiKeyorexperienceKeytransition from missing to present across renders, the component will error due to mismatched hook counts.Move the
useMemoanduseEffectcalls above the guard and let their bodies short-circuit when config is missing:Example fix
const searchConfig = React.useMemo( () => buildSearchConfigFromDocument(streamDocument), [streamDocument.id, streamDocument.locale] ); const searcher = React.useMemo(() => { return provideHeadless(searchConfig); }, [searchConfig]); React.useEffect(() => { if (!searcher) return; searcher.setSessionTrackingEnabled(true); }, [searcher]); if (!apiKey || !experienceKey) { // guard and early returns here }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/Search.tsx` around lines 84 - 140, Move the React hooks (the React.useMemo that builds searchConfig via buildSearchConfigFromDocument, the React.useMemo that creates searcher via provideHeadless, and the React.useEffect that calls searcher.setSessionTrackingEnabled) above the early-return block that checks apiKey/experienceKey so hooks execute unconditionally on every render; inside each hook body short-circuit when inputs are missing (e.g., return null or no-op if streamDocument, apiKey, or experienceKey are not present) and keep the existing guard-only JSX/console.warn/returns after the hooks.packages/visual-editor/src/components/pageSections/SearchSection/searchVisualAutoComplete.tsx-60-66 (1)
60-66:⚠️ Potential issue | 🟠 MajorUse the dropdown's selection flow for preview clicks.
The custom
onClickhandler skipsdropdownItemProps.onClick, andhistory.pushStatealone only mutates the URL without notifying listeners. This breaks the dropdown's selection flow and leaves navigation incomplete. Compare with the pattern used elsewhere in SearchBarSlot.tsx, which couplespushStatewithPopStateEventdispatch. Call the provided callback and follow the established navigation pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/searchVisualAutoComplete.tsx` around lines 60 - 66, The DropdownItem click currently bypasses the dropdown's selection flow by calling history.pushState directly; update the handler in searchVisualAutoComplete.tsx so it invokes the provided dropdownItemProps.onClick (to trigger selection behavior) and then perform navigation using history.pushState with the result.landingPageUrl followed by dispatching a PopStateEvent (matching the pattern used in SearchBarSlot.tsx) so listeners are notified; ensure you reference DropdownItem, dropdownItemProps.onClick, result.landingPageUrl, history.pushState and PopStateEvent when making this change.packages/visual-editor/src/components/pageSections/CustomDirectory/CustomBreadcrumbs.tsx-152-186 (1)
152-186:⚠️ Potential issue | 🟠 MajorDon't leave this section in a permanent loading state on fetch failure.
Both the early return and the
catchpath leavefetchedBreadcrumbsempty, so the skeleton can render forever instead of falling back to a minimal breadcrumb trail or error state.🐛 Proposed fix
- const [fetchedBreadcrumbs, setFetchedBreadcrumbs] = useState< - CustomBreadcrumbItem[] - >([]); + const [fetchedBreadcrumbs, setFetchedBreadcrumbs] = useState< + CustomBreadcrumbItem[] + >([]); + const [hasLoaded, setHasLoaded] = useState(false); const fetchBreadcrumbs = useCallback(async () => { - if (!streamDocument?.uid) return; + if (!streamDocument?.uid) { + setHasLoaded(true); + return; + } try { const json = await fetchData({ endpoint: `${customEndpointURL}/${streamDocument.uid}`, apiKey, @@ setFetchedBreadcrumbs([ ...mapped, { id: streamDocument.id, name: streamDocument.name, slug: streamDocument.slug, }, ]); } catch (error) { console.error("Breadcrumb fetch failed:", error); + setFetchedBreadcrumbs([ + { + id: streamDocument.id, + name: streamDocument.name, + slug: streamDocument.slug, + }, + ]); + } finally { + setHasLoaded(true); } }, [streamDocument, customEndpointURL, apiKey]); - if (!fetchedBreadcrumbs?.length) { + if (!hasLoaded) {Also applies to: 193-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/CustomDirectory/CustomBreadcrumbs.tsx` around lines 152 - 186, The fetchBreadcrumbs function currently returns early or leaves fetchedBreadcrumbs empty on errors, causing the UI skeleton to hang; update fetchBreadcrumbs to setFetchedBreadcrumbs to a minimal fallback breadcrumb trail whenever streamDocument?.uid is missing or when the try/catch fails (use streamDocument's id/name/slug as the sole breadcrumb) and also consider setting a boolean or single-item error breadcrumb in the catch branch so the component can exit the loading state; refer to fetchBreadcrumbs, setFetchedBreadcrumbs, streamDocument, and fetchedBreadcrumbs when making this change.packages/visual-editor/src/components/pageSections/SearchSection/SearchResultsSlot.tsx-250-268 (1)
250-268:⚠️ Potential issue | 🟠 MajorKeep the URL in sync when search state falls back or clears.
Invalid
verticalparams are only rewritten when there is no universal tab, and the later effect skips updates oncecommittedSearchTermbecomes empty. Both cases leave stale query params behind.🐛 Proposed fix
- if (!validVertical && !hasUniversalTab && firstVerticalKey) { - updateSearchUrl({ vertical: firstVerticalKey, searchTerm }); - } + if (!validVertical) { + updateSearchUrl({ vertical: nextVertical, searchTerm }); + } ... - if (committedSearchTerm) { - updateSearchUrl({ - vertical: activeVerticalKey, - searchTerm: committedSearchTerm, - }); - } + updateSearchUrl({ + vertical: activeVerticalKey, + searchTerm: committedSearchTerm, + });Also applies to: 303-312
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/SearchResultsSlot.tsx` around lines 250 - 268, The effect that sets verticalKey and runs a search (using puck, verticals, urlParams, setVerticalKey, runSearch) currently only rewrites invalid vertical params when hasUniversalTab is false, and another effect skips URL updates when committedSearchTerm becomes empty, leaving stale query params; modify this effect to always call updateSearchUrl whenever the resolved nextVertical does not match urlParams.vertical (including when nextVertical is null for the universal tab fall-back) and also ensure the other effect that watches committedSearchTerm still calls updateSearchUrl to clear the searchTerm param when committedSearchTerm becomes empty; specifically update the logic around nextVertical and the call sites of updateSearchUrl so invalid or cleared state always synchronizes the URL.packages/visual-editor/src/components/pageSections/SearchSection/SearchResultsSlot.tsx-323-327 (1)
323-327:⚠️ Potential issue | 🟠 MajorUse the selected token for the active tab color.
contrastingColoris the inverse token; on light backgrounds it can make the active text/border effectively invisible.🎨 Proposed fix
- const activeColor = - styles?.activeVerticalColor?.color?.contrastingColor; + const activeColor = + styles?.activeVerticalColor?.color?.selectedColor;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/SearchResultsSlot.tsx` around lines 323 - 327, The code is using the inverse token (styles?.activeVerticalColor?.color?.contrastingColor) which can make active text/border disappear on light backgrounds; change it to use the selected token instead — reference the active color via styles?.activeVerticalColor?.color?.selectedColor (or the equivalent "selected" token field on activeVerticalColor) and keep building activeColorValue with getThemeColorCssValue as before so the active tab uses the selected token rather than the contrasting/inverse token.packages/visual-editor/src/components/pageSections/SearchSection/SearchResultsSlot.tsx-331-355 (1)
331-355:⚠️ Potential issue | 🟠 MajorUse real buttons for the vertical tabs.
<a>without anhrefis not keyboard-focusable by default, so the tab strip isn't accessible.♿ Proposed fix
- <a + <button + type="button" onClick={() => { const nextVertical = item.pageType === "universal" ? null : (item.verticalKey ?? null); setVerticalKey(nextVertical); !puck.isEditing && runSearch(nextVertical, committedSearchTerm); }} className={`px-5 pt-1.5 pb-3 tracking-[1.1px] mb-0 hover:cursor-pointer ${ isActive ? "border-b-2 " : "" }`} style={ isActive ? { color: activeColorValue, borderColor: activeColorValue, } : undefined } > {item.label} - </a> + </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/SearchResultsSlot.tsx` around lines 331 - 355, The tab strip uses an anchor tag without href which is not keyboard-focusable; change the <a> to a semantic <button type="button"> (preserving the onClick handler that calls setVerticalKey and runSearch using item, puck, and committedSearchTerm) and keep the same className and inline style logic (isActive / activeColorValue) but add appropriate accessibility attributes for a tab: role="tab" and aria-selected={isActive} so keyboard users and assistive tech can interact with the vertical tabs.packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx-168-175 (1)
168-175:⚠️ Potential issue | 🟠 MajorAccordion cards ignore
questiontitles.This branch reads
result.rawData?.nameinstead of the sharednamevalue, so FAQ-style results render the"name"fallback instead of the actual question text.🐛 Proposed fix
- {(result.rawData?.name as any) ?? "name"} + {name ?? "name"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx` around lines 168 - 175, The Accordion card is using result.rawData?.name which causes FAQ items to show the literal "name" fallback; update the Heading inside the AccordionTrigger in Cards.tsx to use the shared title variable (e.g., name or result.name) instead of (result.rawData?.name as any) so the actual question text is shown; locate the Heading within the AccordionTrigger and replace the rawData access with the shared name value used elsewhere in this component.packages/visual-editor/src/components/pageSections/SearchSection/utils.tsx-143-165 (1)
143-165:⚠️ Potential issue | 🟠 MajorLet
updateSearchUrl()clear stale params.The early return exits before the delete branches run, so callers cannot remove both
verticalandsearchTermonce they become empty.🐛 Proposed fix
export const updateSearchUrl = (params: { vertical?: string | null; searchTerm?: string | null; }) => { if (typeof window === "undefined") return; const url = new URL(window.location.href); - - if (!params.searchTerm && !params.vertical) return; if (params.vertical && params.vertical.trim()) { url.searchParams.set("vertical", params.vertical); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/utils.tsx` around lines 143 - 165, The function updateSearchUrl currently returns early when both params are falsy, preventing the delete branches from clearing URL params; change the early-return to only skip when both keys are explicitly undefined (i.e., params.searchTerm === undefined && params.vertical === undefined) so that empty/null/empty-string values still run the delete logic; locate updateSearchUrl in utils.tsx and update that conditional while preserving the rest of the url.searchParams.set/delete handling and the window check.packages/visual-editor/src/components/pageSections/SearchSection/VerticalResultsSection.tsx-135-139 (1)
135-139:⚠️ Potential issue | 🟠 MajorWire
Clear Allor hide it until it works.This renders as a real filter action, but the handler is empty, so users cannot reset selected facets from the modal.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/VerticalResultsSection.tsx` around lines 135 - 139, The "Clear All" button in VerticalResultsSection.tsx has an empty onClick handler; wire it to actually reset filters or hide it until implemented. Add a handler (e.g., handleClearAll or clearSelectedFacets) that calls the existing state updater or prop callback responsible for facets (for example setSelectedFacets([]) or props.onClearFilters()) to clear selected facets and close/reset the modal as appropriate; if no clearing API exists yet, conditionally render the button only when an onClearFilters prop or local state setter (e.g., setSelectedFacets) is available so users don't see a non-functional action. Ensure you reference the VerticalResultsSection component and the button's onClick when making the change.packages/visual-editor/src/components/pageSections/CustomDirectory/CustomBreadcrumbs.tsx-216-233 (1)
216-233:⚠️ Potential issue | 🟠 MajorUse the fetched slug for breadcrumb links.
The fetch maps
slug, but the renderer builds/${b.id}. That points at entity ids instead of the published directory URLs.🐛 Proposed fix
- const href = `/${b.id}`; + const href = + b.slug && !b.slug.startsWith("/") ? `/${b.slug}` : b.slug;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/CustomDirectory/CustomBreadcrumbs.tsx` around lines 216 - 233, The breadcrumb links are built using entity ids (`/${b.id}`) but the fetched breadcrumb objects include the published slug; update the href construction used by the renderer (where fetchedBreadcrumbs is mapped and href is defined for each b) to use the fetched slug (e.g., `/${b.slug}`) so MaybeLink points at the published directory URLs; keep the existing isLast logic so the last breadcrumb still renders without a link and preserve directoryRoot rendering for the first item.packages/visual-editor/src/components/pageSections/SearchSection/SearchResultsSlot.tsx-214-217 (1)
214-217:⚠️ Potential issue | 🟠 MajorUniversal results ignore the configured GDA toggle.
When the universal tab is active,
currentVerticalConfigis alwaysundefined, soenableGDAfalls back totrueeven if the universal config disabled it.🐛 Proposed fix
+ const universalConfig = React.useMemo( + () => verticals.find((v) => v.pageType === "universal"), + [verticals] + ); ... - enableGDA={ - currentVerticalConfig?.enableGenerativeDirectAnswer ?? true - } + enableGDA={ + universalConfig?.enableGenerativeDirectAnswer ?? true + }Also applies to: 385-388
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/SearchResultsSlot.tsx` around lines 214 - 217, The currentVerticalConfig lookup ignores the universal tab because when resolvedVerticalKey is falsy it returns undefined; update the React.useMemo for currentVerticalConfig to handle the universal case by returning the vertical whose verticalKey matches resolvedVerticalKey OR, when resolvedVerticalKey is falsy (universal active), the vertical marked as universal (e.g., a property like isUniversal or a known key such as 'universal'); adjust the same logic at the other occurrence (lines ~385-388) so enableGDA reads the correct vertical config instead of falling back to true.packages/visual-editor/src/components/pageSections/SearchSection/SearchResultsSlot.tsx-407-413 (1)
407-413:⚠️ Potential issue | 🟠 MajorDon't call
usePuck()from inside atry/catchblock.React hooks are not supported in
try/catch, and swallowing the thrown context error can also hide real render failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/SearchResultsSlot.tsx` around lines 407 - 413, The current useOptionalPuckStore wraps the hook usePuck() in a try/catch which is invalid for React hooks; remove the try/catch and instead access the puck context directly (or provide a safe optional hook) by using useContext on the PuckContext value and returning undefined when the context is not present. Concretely, replace useOptionalPuckStore's try/catch approach with a call to useContext(PuckContext) (or change usePuck to expose the underlying context via useContext) so you call hooks unconditionally and simply check for a null/undefined context to return undefined; update references to useOptionalPuckStore and keep usePuck usage intact elsewhere.
🟡 Minor comments (17)
starter/.npmrc-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorConsider removing
legacy-peer-deps=trueif no peer dependency conflicts exist.Verification with
npm install --dry-run(without the legacy-peer-deps flag) shows no peer dependency conflicts. If no conflicts are preventing installation, this flag may be unnecessary and should be removed.While
legacy-peer-deps=truecan help bypass peer dependency resolution errors, it's a workaround rather than a proper solution. Using it when conflicts don't exist introduces unnecessary technical debt. If conflicts do reappear in the future, identify and resolve the specific versions causing incompatibility instead of relying on this flag.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@starter/.npmrc` at line 1, Remove the unnecessary legacy-peer-deps=true entry from the .npmrc file (the single line present) if npm install --dry-run confirms no peer dependency conflicts; update any CI or deployment configs that rely on .npmrc to ensure they no longer pass the legacy-peer-deps behavior and document or pin specific package versions if future peer conflicts arise instead of keeping the flag.packages/visual-editor/locales/platform/zh/visual-editor.json-186-186 (1)
186-186:⚠️ Potential issue | 🟡 MinorReuse the existing Chinese translation for
base.
fields.baseis"根据"here, but the same file already uses"基础"forbaseat Line 26. The current value changes the meaning from a UI noun to “according to.”💡 Minimal fix
- "base": "根据", + "base": "基础",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/locales/platform/zh/visual-editor.json` at line 186, The translation for fields.base in the visual-editor locale is inconsistent and uses "根据" (meaning "according to") instead of the noun "基础"; update the value of the JSON key fields.base in packages/visual-editor/locales/platform/zh/visual-editor.json to the existing noun translation "基础" to match the other occurrence and preserve UI meaning (ensure you edit the entry referenced as "base" under fields).packages/visual-editor/locales/platform/fi/visual-editor.json-523-523 (1)
523-523:⚠️ Potential issue | 🟡 Minor
universalLimitis still untranslated.
"Universal Limit"will leak English directly into the Finnish editor UI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/locales/platform/fi/visual-editor.json` at line 523, The key "universalLimit" in the Finnish locale file is still in English; update the value for universalLimit in packages/visual-editor/locales/platform/fi/visual-editor.json to a proper Finnish translation (e.g., "Yleinen enimmäisraja" or another context-appropriate Finnish phrase) so the editor UI no longer shows the English text.packages/visual-editor/locales/platform/it/visual-editor.json-65-65 (1)
65-65:⚠️ Potential issue | 🟡 Minor
customBreadcrumbsis using the food term.
"Pangrattato personalizzato"refers to bread crumbs, not navigation breadcrumbs, so this will read obviously wrong in the editor UI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/locales/platform/it/visual-editor.json` at line 65, The translation for the key customBreadcrumbs uses the food term ("Pangrattato personalizzato"); update the Italian string for the key customBreadcrumbs to a navigation-appropriate term such as "Breadcrumb personalizzati" or "Percorso di navigazione personalizzato" so the UI shows navigation breadcrumbs rather than food; locate the key customBreadcrumbs in the visual-editor.json and replace the value accordingly.packages/visual-editor/locales/platform/pl/visual-editor.json-65-65 (1)
65-65:⚠️ Potential issue | 🟡 Minor
customBreadcrumbsis mistranslated.
"Niestandardowe bułki tartej"reads as bread crumbs rather than navigation breadcrumbs, so this will ship as broken UI copy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/locales/platform/pl/visual-editor.json` at line 65, The translation for the key "customBreadcrumbs" is incorrect (it currently reads as literal bread crumbs); update the string value for the "customBreadcrumbs" key to a proper navigation-oriented Polish phrase such as "Niestandardowe okruszki nawigacji" or "Niestandardowe ścieżki nawigacji" so the UI reflects navigation breadcrumbs rather than food.packages/visual-editor/locales/platform/ja/visual-editor.json-257-257 (1)
257-257:⚠️ Potential issue | 🟡 Minor
fields.heightshould use the layout term, not a human-height term.
"身長"means a person's height. For a size control, this will read incorrectly to Japanese users in the editor UI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/locales/platform/ja/visual-editor.json` at line 257, The translation key fields.height in packages/visual-editor/locales/platform/ja/visual-editor.json currently uses "身長" (human height); update the value to the appropriate layout/size term for UI controls (e.g., "高さ" or another agreed layout-specific term) so the editor displays the correct meaning for a size control; locate the "height" entry under the fields object in visual-editor.json and replace the string accordingly.packages/visual-editor/src/components/contentBlocks/CtaWrapper.tsx-362-367 (1)
362-367:⚠️ Potential issue | 🟡 MinorHide
textColorwhen CTA type ispresetImage.
textColorvisibility currently depends only on variant. WheneffectiveCtaType === "presetImage", this control can still appear even though it’s not meaningful in that mode.Suggested fix
- const showTextColor = ctaVariant === "primary"; + const showTextColor = + ctaVariant === "primary" && effectiveCtaType !== "presetImage";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/contentBlocks/CtaWrapper.tsx` around lines 362 - 367, The visibility logic for textColor is incomplete: instead of using only ctaVariant, update the showTextColor condition to also ensure effectiveCtaType !== "presetImage" (e.g., compute showTextColor = ctaVariant === "primary" && effectiveCtaType !== "presetImage"), then call setDeep(updatedFields, "styles.objectFields.textColor.visible", showTextColor) so the textColor control is hidden when effectiveCtaType is "presetImage".packages/visual-editor/locales/platform/en/visual-editor.json-602-603 (1)
602-603:⚠️ Potential issue | 🟡 MinorCorrect typo in the missing custom endpoint message
Line 602 includes two typos in UI copy (
"Add you"and"sectiom").✏️ Proposed fix
- "missingCustomEndpointApiKey": "Add you custom Content endpoint API key to view this sectiom", + "missingCustomEndpointApiKey": "Add your custom Content endpoint API key to view this section",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/locales/platform/en/visual-editor.json` around lines 602 - 603, Update the two locale strings for the keys missingCustomEndpointApiKey and missingCustomEndpointURL to fix typos: change "Add you" to "Add your" and "sectiom" to "section" so the messages read correctly (update the values for missingCustomEndpointApiKey and missingCustomEndpointURL in the visual-editor.json).packages/visual-editor/locales/platform/en-GB/visual-editor.json-602-603 (1)
602-603:⚠️ Potential issue | 🟡 MinorFix typo in missing endpoint API key message
Line 602 has user-facing typos (
"Add you"and"sectiom"), which will ship directly in the editor UI.✏️ Proposed fix
- "missingCustomEndpointApiKey": "Add you custom Content endpoint API key to view this sectiom", + "missingCustomEndpointApiKey": "Add your custom Content endpoint API key to view this section",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/locales/platform/en-GB/visual-editor.json` around lines 602 - 603, The string for the localization key missingCustomEndpointApiKey contains typos; update its value to correct grammar and spelling (e.g., "Add your custom Content endpoint API key to view this section") and also verify the related key missingCustomEndpointURL uses consistent capitalization/spelling ("Add your custom Content endpoint URL to view this section"); edit the JSON entries for missingCustomEndpointApiKey and missingCustomEndpointURL to these corrected user-facing messages.packages/visual-editor/src/components/pageSections/CustomDirectory/utils.ts-1-22 (1)
1-22:⚠️ Potential issue | 🟡 MinorRemove dead code branch and fix interface naming.
Line 1 should use PascalCase:
interface FetchDataProps(notfetchDataProps).Line 21's
Array.isArray()branch is dead code—callers normalizeentityIdsto a string via.join(",")before passing it, so the ternary always takes the right side. Remove the unused array branch:🔧 Proposed fix
-interface fetchDataProps { +interface FetchDataProps { endpoint: string; apiKey: string; entityIds?: string; } export const fetchData = async ({ endpoint, apiKey, entityIds, -}: fetchDataProps) => { +}: FetchDataProps) => { try { const url = new URL(endpoint); url.searchParams.set("api_key", apiKey); url.searchParams.set("v", "20250101"); if (entityIds?.length) { url.searchParams.set( "entityIds", - Array.isArray(entityIds) ? entityIds.join(",") : entityIds + entityIds ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/CustomDirectory/utils.ts` around lines 1 - 22, Rename the interface from fetchDataProps to PascalCase FetchDataProps and update the fetchData parameter type accordingly; inside fetchData (function name) remove the dead Array.isArray(entityIds) branch and simply set url.searchParams.set("entityIds", entityIds) when entityIds is present (keeping the existing check if (entityIds?.length) and other URL param logic intact).packages/visual-editor/locales/platform/hu/visual-editor.json-113-113 (1)
113-113:⚠️ Potential issue | 🟡 MinorTranslate
searchBarSlotfor the Hungarian locale.
SearchBar Slotis still English, so this new editor label will render as mixed-language copy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/locales/platform/hu/visual-editor.json` at line 113, Replace the English value for the JSON key "searchBarSlot" with a Hungarian translation so the editor label is fully localized; update the value from "SearchBar Slot" to a Hungarian string such as "Keresősáv helye" for the key searchBarSlot in the locale file.packages/visual-editor/locales/platform/da/visual-editor.json-113-113 (1)
113-113:⚠️ Potential issue | 🟡 MinorTranslate
searchBarSlotfor the Danish locale.
SearchBar Slotis still English, so this new editor label will show up as mixed-language copy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/locales/platform/da/visual-editor.json` at line 113, The key searchBarSlot currently has the English value "SearchBar Slot"; update its value to the Danish translation (e.g., "Søgefelt-plads" or another approved Danish phrase) so the locale file uses consistent Danish text; locate the searchBarSlot entry in the visual-editor.json and replace the string value with the chosen Danish translation.packages/visual-editor/locales/platform/sv/visual-editor.json-113-113 (1)
113-113:⚠️ Potential issue | 🟡 MinorFinish translating the new Swedish editor labels.
searchBarSlotis currentlySökbar Slot, andvisualAutoCompleteVerticalKeyis still fully English. Both labels surface directly in the editor UI.Also applies to: 534-534
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/locales/platform/sv/visual-editor.json` at line 113, Update the Swedish translations for the editor keys: change "searchBarSlot" from "Sökbar Slot" to a proper Swedish label and translate "visualAutoCompleteVerticalKey" (currently English) into Swedish; ensure you update both occurrences of these keys (searchBarSlot and visualAutoCompleteVerticalKey) in this locale file so the editor UI shows the translated labels consistently.packages/visual-editor/locales/platform/nl/visual-editor.json-65-67 (1)
65-67:⚠️ Potential issue | 🟡 MinorRegenerate the i18n artifacts after adding these locale keys.
This locale update should be followed by
pnpm --dir packages/visual-editor run i18n:updateand any generated diff should be committed, otherwise the editor's derived i18n metadata can drift from the source JSON.Based on learnings: If locale strings change, run
pnpm --dir packages/visual-editor run i18n:update.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/locales/platform/nl/visual-editor.json` around lines 65 - 67, You added three locale keys ("customBreadcrumbs", "customCodeSection", "CustomDirectory") to the visual-editor NL JSON but didn't update the derived i18n artifacts; run the i18n update task and commit its output: execute pnpm --dir packages/visual-editor run i18n:update, review the generated diffs, and commit any changed files so the editor's i18n metadata stays in sync with the JSON keys you added.packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx-147-150 (1)
147-150:⚠️ Potential issue | 🟡 MinorReplace
space-betweenwithjustify-between.
space-betweenis not a Tailwind utility, so these wrappers fall back to the default flex alignment and the CTA block won't sit at the far edge as intended.🐛 Proposed fix
- <div className="w-full flex space-between"> + <div className="w-full flex justify-between"> ... - <div className={`${FLEX_LAYOUT_CLASSES} space-between`}> + <div className={`${FLEX_LAYOUT_CLASSES} justify-between`}>Also applies to: 179-182
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx` around lines 147 - 150, The wrapper divs in the Cards.tsx JSX (the div with className "w-full flex space-between" that contains MaybeRTF and the CTA block, and the similar occurrence later) use an invalid Tailwind class "space-between"; replace "space-between" with the correct Tailwind utility "justify-between" so the flex container properly pushes the CTA block to the far edge.packages/visual-editor/src/components/pageSections/SearchSection/VerticalResultsSection.tsx-176-176 (1)
176-176:⚠️ Potential issue | 🟡 MinorFix the
flex-flex-coltypo.
flex-flex-colis not a Tailwind utility, so this container never gets the intended flex-column layout.🐛 Proposed fix
- <div className="flex-flex-col"> + <div className="flex flex-col">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/VerticalResultsSection.tsx` at line 176, The div in VerticalResultsSection (the element with className "flex-flex-col") uses an invalid Tailwind utility; change the className on that div in VerticalResultsSection.tsx from "flex-flex-col" to "flex flex-col" so it correctly applies a flex container with column direction (locate the div in the VerticalResultsSection component and update its className).packages/visual-editor/locales/platform/hr/visual-editor.json-65-65 (1)
65-65:⚠️ Potential issue | 🟡 MinorTranslate the remaining English Croatian strings.
components.customBreadcrumbsandfields.visualAutoCompleteVerticalKeyare still English, so they'll surface untranslated in the localized UI.Also applies to: 534-534
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/locales/platform/hr/visual-editor.json` at line 65, Replace the remaining English values with Croatian translations: change components.customBreadcrumbs value from "Custom Breadcrumbs" to "Prilagođene krušne mrvice" (or "Prilagođena navigacija" if preferred) and change fields.visualAutoCompleteVerticalKey from its English text to "Vertikalni ključ automatskog dovršavanja" (ensure both occurrences, including the other instance noted around line 534, are updated so the localized UI shows Croatian strings).
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (4)
packages/visual-editor/src/components/pageSections/SearchSection/LayoutSections.tsx (1)
29-29: Minor: Extra whitespace inclassName.There's a double space between
border-gray-300andborder.✨ Fix spacing
- <div className="px-5 py-2.5 flex items-end border-gray-300 border rounded-t-md"> + <div className="px-5 py-2.5 flex items-end border-gray-300 border rounded-t-md">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/LayoutSections.tsx` at line 29, In the JSX div inside the LayoutSections component, there's an extra space in the className string between "border-gray-300" and "border"; update that className (in the div with "px-5 py-2.5 flex items-end ...") to use a single space so it reads "... border-gray-300 border rounded-t-md" to keep class names consistent.packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx (2)
92-94: Avoid@ts-ignore— fix the type error or add an explanation.The
@ts-ignoresuppresses type checking for thelinesprop. If theAddresscomponent's types are incorrect or incomplete, consider using@ts-expect-errorwith a comment explaining why, or file an issue upstream to fix the types.♻️ Prefer `@ts-expect-error` with explanation
lines={[ ["line1"], - //@ts-ignore + // `@ts-expect-error` - Address component types don't support tuple format for city/region/postalCode ["city", ", ", "region", " ", "postalCode"], ]}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx` around lines 92 - 94, Replace the blanket //@ts-ignore on the lines prop for the Address component in Cards.tsx: either correct the type mismatch by casting the value to the expected type (e.g. ensure the array is string[] or update the AddressProps type to accept (string | undefined)[]), or use //@ts-expect-error with a one‑sentence explanation referencing the upstream typing bug; update the Address usage (prop name "lines") and/or AddressProps so TypeScript knows the shape instead of silencing the error.
26-26: Consider using a specific type instead ofanyforCardProps.Using
CardProps<any>means all accesses toresult.rawData(e.g.,question,name,slug,address,hours,distance) are untyped and won't catch typos or missing fields at compile time. Define an interface for the expected result shape.♻️ Suggested approach
// Define the expected shape of search result data interface SearchResultData { question?: string; name?: string; slug?: string; address?: Address; mainPhone?: string; hours?: Hours; answerV2?: unknown; richTextDescriptionV2?: unknown; bodyV2?: unknown; description?: unknown; } interface CardsProps extends CardProps<SearchResultData> { // ...existing props }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx` at line 26, Replace the use of CardProps<any> with a concrete typed interface describing the shape of result.rawData: create an interface (e.g., SearchResultData) listing optional fields you access (question, name, slug, address, mainPhone, hours, answerV2, richTextDescriptionV2, bodyV2, description, distance, etc.), then change the CardsProps declaration from CardProps<any> to CardProps<SearchResultData> and update any result.rawData usages in Cards (and related components) to rely on the new typed properties so the compiler can catch typos/missing fields (refer to CardsProps and result.rawData in your edits).packages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsx (1)
355-356:resolveDataFromParentis being overridden here.
resolveDataFromParentinpackages/visual-editor/src/editor/ParentData.tsx:4-34only rewrites thedatafield. Line 389 immediately forcesdatatofalse, so the inherited-data message can never surface from this resolver. If the intent is to hidedatacompletely, dropping the helper call would make that clearer.Also applies to: 389-389
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsx` around lines 355 - 356, resolveDataFromParent(searchBarSlotFields, data) is being overridden by forcing the data field to false immediately after, which prevents the inherited-data message from ever surfacing; either remove the resolveDataFromParent call if your intent is to completely hide data, or stop forcing data to false so the helper can preserve/resolve the inherited-data behavior — update the resolveFields function that references resolveDataFromParent and searchBarSlotFields accordingly (i.e., drop the helper call when hiding data, or remove the explicit data = false override to keep inherited-data).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/visual-editor/src/components/pageSections/CustomDirectory/CustomDirectory.tsx`:
- Around line 68-71: Update the user-facing copy passed to pt in
CustomDirectory.tsx: fix the two typos in the second argument of the pt call
with key "missingCustomEndpointApiKey" so the message reads "Add your custom
Content endpoint API key to view this section"; keep the translation key
unchanged and only replace the string literal to correct "you" → "your" and
"sectiom" → "section".
- Around line 125-149: The list rendering produces invalid HTML because
Background renders as a div wrapping an li; update the map so each list item is
a semantic li: either render Background as an li by passing as="li" (e.g.,
Background with as="li" and keep the inner li content removed) or move the <li>
outside so the map returns <li><Background ...>...</Background></li>; ensure the
key remains on the outer list item (use item.id ?? item.uid) and keep MaybeLink,
Body, and puck.isEditing logic unchanged.
- Around line 80-111: The fetchEntities flow can be overwritten by stale
responses; modify fetchEntities and the surrounding useEffect to ignore
responses from prior requests by using an AbortController or a local request
token: create a unique token/AbortController when calling fetchEntities (inside
the useEffect before invoking fetchEntities), pass the signal or token into
fetchEntities, and inside fetchEntities check the token/signal before calling
setEntities or setLoading (and handle fetch cancellation errors). Also ensure
the useEffect cleanup cancels the controller or invalidates the token so any
in-flight response is ignored and won't call setEntities/setLoading after a
newer request starts.
In `@packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx`:
- Line 76: The JSX currently uses a literal placeholder fallback {name ??
"name"} which should be replaced with a meaningful default or suppressed; update
the Card rendering in SearchSection/Cards.tsx to either show an empty string or
a localized fallback (e.g., t("untitled", "Untitled")) instead of "name", or
conditionally omit the heading when the name prop is falsy; apply the same
change to every occurrence of the name fallback in this file (the other
instances of {name ?? "name"}).
- Line 167: The AccordionItem key currently uses result.index which may be
undefined; replace it to use the already-computed fallback index (the local
variable used to derive a stable index) instead of result.index so the key is
stable; locate the AccordionItem JSX and change the key from
`result-${result.index}` to use the fallback index variable (e.g.,
`result-${index}`) to avoid duplicate "result-undefined" keys.
- Around line 121-129: The CTA call is passing getDirections a single arg but
getDirections requires five; update the call in Cards.tsx to call
getDirections(address, listings, coordinate, options, coordinate) using the
available data on the card (e.g. result.rawData.address for address, the
component/parent listings array or result.listings for listings, the location
coordinate from result.rawData (or props) for coordinate, an options object or
undefined for options, and the coordinate again for the final param) so the
signature matches the expected (getDirections) parameters used in cta.tsx.
In
`@packages/visual-editor/src/components/pageSections/SearchSection/LayoutSections.tsx`:
- Around line 41-43: The code calls results.map(...) which will throw if results
is null/undefined; guard or provide a fallback before mapping. Update the
rendering around results.map in LayoutSections.tsx (the expression that renders
CardComponent for each result) to either conditionally render only when results
is an array (e.g. Array.isArray(results) && results.map(...)) or use a fallback
array ((results ?? []).map(...)); keep CardComponent and key={result.id}
semantics unchanged.
- Around line 19-22: The resultsCount prop is currently only logged and not
used; either remove it from LayoutSectionProps and the default value if it's not
needed, or enforce it by using resultsCount to limit the rendered items before
mapping to CardComponent (e.g., slice the source array to resultsCount) in the
LayoutSections component; update any place that maps over the data to use the
sliced array and remove the console.log(resultsCount) debug line.
- Line 22: Remove the leftover debug console.log call for resultsCount in
LayoutSections.tsx; locate the console.log(resultsCount) statement and delete it
(or replace with proper logging via the app logger if persistent logging is
required), and scan the surrounding function/component (LayoutSections) to
ensure no other stray debug console statements remain.
In
`@packages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsx`:
- Around line 219-223: heightClass is computed via getHeight(height) but the
actual search bar container is hardcoded to "h-16"/"h-12", so the editor's
styles.height only affects the wrapper and not the input; replace the hardcoded
height classes on the search bar element (the JSX element that currently uses
"h-16"/"h-12") with the computed heightClass (preserving the showResults logic),
e.g. conditionally include heightClass when showResults is false or otherwise
use an appropriate full-height class, and ensure getHeight(height) returns the
correct Tailwind height string used in className.
---
Nitpick comments:
In `@packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx`:
- Around line 92-94: Replace the blanket //@ts-ignore on the lines prop for the
Address component in Cards.tsx: either correct the type mismatch by casting the
value to the expected type (e.g. ensure the array is string[] or update the
AddressProps type to accept (string | undefined)[]), or use //@ts-expect-error
with a one‑sentence explanation referencing the upstream typing bug; update the
Address usage (prop name "lines") and/or AddressProps so TypeScript knows the
shape instead of silencing the error.
- Line 26: Replace the use of CardProps<any> with a concrete typed interface
describing the shape of result.rawData: create an interface (e.g.,
SearchResultData) listing optional fields you access (question, name, slug,
address, mainPhone, hours, answerV2, richTextDescriptionV2, bodyV2, description,
distance, etc.), then change the CardsProps declaration from CardProps<any> to
CardProps<SearchResultData> and update any result.rawData usages in Cards (and
related components) to rely on the new typed properties so the compiler can
catch typos/missing fields (refer to CardsProps and result.rawData in your
edits).
In
`@packages/visual-editor/src/components/pageSections/SearchSection/LayoutSections.tsx`:
- Line 29: In the JSX div inside the LayoutSections component, there's an extra
space in the className string between "border-gray-300" and "border"; update
that className (in the div with "px-5 py-2.5 flex items-end ...") to use a
single space so it reads "... border-gray-300 border rounded-t-md" to keep class
names consistent.
In
`@packages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsx`:
- Around line 355-356: resolveDataFromParent(searchBarSlotFields, data) is being
overridden by forcing the data field to false immediately after, which prevents
the inherited-data message from ever surfacing; either remove the
resolveDataFromParent call if your intent is to completely hide data, or stop
forcing data to false so the helper can preserve/resolve the inherited-data
behavior — update the resolveFields function that references
resolveDataFromParent and searchBarSlotFields accordingly (i.e., drop the helper
call when hiding data, or remove the explicit data = false override to keep
inherited-data).
🪄 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: 52803370-cca9-469f-835d-cfa11ff8e265
📒 Files selected for processing (7)
packages/visual-editor/src/components/pageSections/CustomDirectory/CustomDirectory.tsxpackages/visual-editor/src/components/pageSections/CustomDirectory/utils.tspackages/visual-editor/src/components/pageSections/SearchSection/Cards.tsxpackages/visual-editor/src/components/pageSections/SearchSection/LayoutSections.tsxpackages/visual-editor/src/components/pageSections/SearchSection/Search.tsxpackages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsxpackages/visual-editor/src/components/pageSections/SearchSection/search.css
✅ Files skipped from review due to trivial changes (1)
- packages/visual-editor/src/components/pageSections/SearchSection/search.css
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/visual-editor/src/components/pageSections/SearchSection/Search.tsx
| {pt( | ||
| "missingCustomEndpointApiKey", | ||
| "Add you custom Content endpoint API key to view this sectiom" | ||
| )} |
There was a problem hiding this comment.
Fix the editor empty-state copy.
There are two typos in this setup message: "Add you" and "sectiom". This is editor-visible text, so it will look unfinished during configuration.
✏️ Proposed fix
- "Add you custom Content endpoint API key to view this sectiom"
+ "Add your custom Content endpoint API key to view this section"📝 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.
| {pt( | |
| "missingCustomEndpointApiKey", | |
| "Add you custom Content endpoint API key to view this sectiom" | |
| )} | |
| {pt( | |
| "missingCustomEndpointApiKey", | |
| "Add your custom Content endpoint API key to view this section" | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/visual-editor/src/components/pageSections/CustomDirectory/CustomDirectory.tsx`
around lines 68 - 71, Update the user-facing copy passed to pt in
CustomDirectory.tsx: fix the two typos in the second argument of the pt call
with key "missingCustomEndpointApiKey" so the message reads "Add your custom
Content endpoint API key to view this section"; keep the translation key
unchanged and only replace the string literal to correct "you" → "your" and
"sectiom" → "section".
| const fetchEntities = useCallback( | ||
| async (entityIds: string[]) => { | ||
| if (!entityIds?.length) return; | ||
|
|
||
| setLoading(true); | ||
|
|
||
| try { | ||
| const res = await fetchData({ | ||
| endpoint: "https://cdn.yextapis.com/v2/accounts/me/entities", | ||
| apiKey: apiKey, | ||
| entityIds: entityIds.join(","), | ||
| }); | ||
|
|
||
| const fetchedEntities = res?.entities ?? []; | ||
| setEntities(fetchedEntities); | ||
| } catch (error) { | ||
| console.error("Entity fetch error:", error); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }, | ||
| [apiKey] | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| const childIds = streamDocument?.dm_childEntityIds; | ||
| if (childIds?.length) { | ||
| void fetchEntities(childIds); | ||
| } else { | ||
| setEntities([]); | ||
| } | ||
| }, [streamDocument?.dm_childEntityIds, fetchEntities]); |
There was a problem hiding this comment.
Guard against stale fetches overwriting newer directory data.
If dm_childEntityIds changes while a request is in flight, the older response can still call setEntities/setLoading after the newer one starts. That can briefly render the wrong cards when switching documents or child IDs.
💡 One way to ignore stale responses
- const fetchEntities = useCallback(
- async (entityIds: string[]) => {
+ const fetchEntities = useCallback(
+ async (entityIds: string[], isActive: () => boolean) => {
if (!entityIds?.length) return;
setLoading(true);
@@
- setEntities(fetchedEntities);
+ if (!isActive()) return;
+ setEntities(fetchedEntities);
@@
- setLoading(false);
+ if (isActive()) {
+ setLoading(false);
+ }
}
},
[apiKey]
);
useEffect(() => {
+ let active = true;
const childIds = streamDocument?.dm_childEntityIds;
if (childIds?.length) {
- void fetchEntities(childIds);
+ void fetchEntities(childIds, () => active);
} else {
+ setLoading(false);
setEntities([]);
}
+ return () => {
+ active = false;
+ };
}, [streamDocument?.dm_childEntityIds, fetchEntities]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/visual-editor/src/components/pageSections/CustomDirectory/CustomDirectory.tsx`
around lines 80 - 111, The fetchEntities flow can be overwritten by stale
responses; modify fetchEntities and the surrounding useEffect to ignore
responses from prior requests by using an AbortController or a local request
token: create a unique token/AbortController when calling fetchEntities (inside
the useEffect before invoking fetchEntities), pass the signal or token into
fetchEntities, and inside fetchEntities check the token/signal before calling
setEntities or setLoading (and handle fetch cancellation errors). Also ensure
the useEffect cleanup cancels the controller or invalidates the token so any
in-flight response is ignored and won't call setEntities/setLoading after a
newer request starts.
| <ul className="grid md:grid-cols-3 sm:grid-cols-2 grid-cols-1 gap-4 w-full"> | ||
| {entities.map((item, index) => { | ||
| return ( | ||
| <Background | ||
| key={item.id ?? item.uid} | ||
| className="h-full flex flex-col p-1" | ||
| background={styles.backgroundColor} | ||
| > | ||
| <li> | ||
| <MaybeLink | ||
| variant="directoryLink" | ||
| eventName={`link${index}`} | ||
| href={`/${item.meta?.id}`} | ||
| className="text-wrap break-words block w-full flex items-center" | ||
| disabled={puck.isEditing} | ||
| > | ||
| <div key={item.meta?.id}> | ||
| <Body variant="lg">{item.name}</Body> | ||
| </div> | ||
| </MaybeLink> | ||
| </li> | ||
| </Background> | ||
| ); | ||
| })} | ||
| </ul> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
bg_file="$(fd -i 'background\.tsx$' packages/visual-editor/src/components | head -n 1)"
if [[ -z "$bg_file" ]]; then
echo "background.tsx not found under packages/visual-editor/src/components" >&2
exit 1
fi
sed -n '1,220p' "$bg_file"
rg -nC2 'export (const|function) Background|<(div|section)\b|as\??:' "$bg_file"Repository: yext/visual-editor
Length of output: 3677
Move the <li> outside the Background component or pass as="li" to preserve list semantics.
The Background component renders as a div by default (when no as prop is provided), which creates invalid HTML: <ul><div><li>.... Since Background supports an as prop that accepts semantic elements including list items, either restructure to make Background render as li via as="li", or move the li outside the loop to wrap the Background component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/visual-editor/src/components/pageSections/CustomDirectory/CustomDirectory.tsx`
around lines 125 - 149, The list rendering produces invalid HTML because
Background renders as a div wrapping an li; update the map so each list item is
a semantic li: either render Background as an li by passing as="li" (e.g.,
Background with as="li" and keep the inner li content removed) or move the <li>
outside so the map returns <li><Background ...>...</Background></li>; ensure the
key remains on the outer list item (use item.id ?? item.uid) and keep MaybeLink,
Body, and puck.isEditing logic unchanged.
| className="font-bold text-palette-primary-dark" | ||
| level={headingStyles?.headingLevel ?? 3} | ||
| > | ||
| {name ?? "name"} |
There was a problem hiding this comment.
Replace placeholder fallback "name" with meaningful default or empty string.
The fallback name ?? "name" appears to be a debug placeholder. In production, displaying the literal string "name" when no name exists would confuse users. Consider using an empty string, a localized fallback like t("untitled", "Untitled"), or omitting the heading entirely when name is missing.
🔧 Suggested fix
- {name ?? "name"}
+ {name ?? ""}Or with localization:
- {name ?? "name"}
+ {name ?? t("untitled", "Untitled")}Also applies to: 144-144, 174-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx`
at line 76, The JSX currently uses a literal placeholder fallback {name ??
"name"} which should be replaced with a meaningful default or suppressed; update
the Card rendering in SearchSection/Cards.tsx to either show an empty string or
a localized fallback (e.g., t("untitled", "Untitled")) instead of "name", or
conditionally omit the heading when the name prop is falsy; apply the same
change to every occurrence of the name fallback in this file (the other
instances of {name ?? "name"}).
| <CTA | ||
| color={ctaStyles?.background} | ||
| textColor={ctaStyles?.textColor} | ||
| link={getDirections(result.rawData.address)} | ||
| label={t("getDirections", "Get Directions")} | ||
| variant={"primary"} | ||
| className="!w-52 justify-center" | ||
| normalizeLink={true} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check getDirections function signature and all usages in the codebase
# Find getDirections usages to compare call patterns
echo "=== All getDirections usages ==="
rg -n 'getDirections\(' --type ts --type tsx -C 2
# Check if there's a type definition or export for getDirections
echo "=== getDirections exports/definitions ==="
ast-grep --pattern 'export function getDirections($$$)'
echo "=== getDirections in `@yext/pages-components` types ==="
fd -t f 'pages-components' node_modules --exec grep -l 'getDirections' {} \; 2>/dev/null | head -5 | xargs cat 2>/dev/null | head -50Repository: yext/visual-editor
Length of output: 235
🏁 Script executed:
#!/bin/bash
# Description: Search for getDirections with corrected approach (tsx issue)
echo "=== All getDirections usages (no type filter) ==="
rg -n 'getDirections' --type ts -C 2 | head -80
echo "=== Check Cards.tsx file ==="
cat -n packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx | sed -n '110,140p'
echo "=== Check imports in Cards.tsx ==="
head -50 packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx | grep -n 'import\|getDirections'Repository: yext/visual-editor
Length of output: 9215
🏁 Script executed:
#!/bin/bash
echo "=== CTA component file ==="
cat -n packages/visual-editor/src/components/atoms/cta.tsx | sed -n '1,150p'
echo "=== Check if cta.tsx has getDirections calls ==="
grep -n 'getDirections' packages/visual-editor/src/components/atoms/cta.tsxRepository: yext/visual-editor
Length of output: 6881
Fix getDirections call signature in Cards.tsx.
The call getDirections(result.rawData.address) at line 124 passes a single argument, but based on usage in cta.tsx (lines 125-138), getDirections expects 5 parameters: (address, listings, coordinate, options, coordinate). Update the call to match the expected signature.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx`
around lines 121 - 129, The CTA call is passing getDirections a single arg but
getDirections requires five; update the call in Cards.tsx to call
getDirections(address, listings, coordinate, options, coordinate) using the
available data on the card (e.g. result.rawData.address for address, the
component/parent listings array or result.listings for listings, the location
coordinate from result.rawData (or props) for coordinate, an options object or
undefined for options, and the coordinate again for the final param) so the
signature matches the expected (getDirections) parameters used in cta.tsx.
| } else { | ||
| content = ( | ||
| <Accordion className="w-full"> | ||
| <AccordionItem key={`result-${result.index}`}> |
There was a problem hiding this comment.
result.index may be undefined, causing an unstable React key.
When result.index is undefined, the key becomes "result-undefined", which could cause React reconciliation issues if multiple results lack an index. Use the fallback index already computed.
🔧 Suggested fix
- <AccordionItem key={`result-${result.index}`}>
+ <AccordionItem key={`result-${nIndex}`}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/visual-editor/src/components/pageSections/SearchSection/Cards.tsx`
at line 167, The AccordionItem key currently uses result.index which may be
undefined; replace it to use the already-computed fallback index (the local
variable used to derive a stable index) instead of result.index so the key is
stable; locate the AccordionItem JSX and change the key from
`result-${result.index}` to use the fallback index variable (e.g.,
`result-${index}`) to avoid duplicate "result-undefined" keys.
| resultsCount = 4, | ||
| }: LayoutSectionProps) => { | ||
| if (!CardComponent) return null; | ||
| console.log(resultsCount); |
There was a problem hiding this comment.
resultsCount prop is unused.
The resultsCount prop is defined with a default of 4 and documented in the AI summary as controlling the number of results, but it's only referenced in a debug console.log. If this prop should limit the displayed results, the implementation is missing. Otherwise, consider removing the prop entirely.
🔧 If `resultsCount` should limit results, apply this fix
resultsCount = 4,
}: LayoutSectionProps) => {
if (!CardComponent) return null;
- console.log(resultsCount);
const layoutClasses =
layoutType === "Grid" ? GRID_LAYOUT_CLASSES : FLEX_LAYOUT_CLASSES;And in the map call:
- {results.map((result) => (
+ {results.slice(0, resultsCount).map((result) => (
<CardComponent key={result.id} result={result} />
))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/visual-editor/src/components/pageSections/SearchSection/LayoutSections.tsx`
around lines 19 - 22, The resultsCount prop is currently only logged and not
used; either remove it from LayoutSectionProps and the default value if it's not
needed, or enforce it by using resultsCount to limit the rendered items before
mapping to CardComponent (e.g., slice the source array to resultsCount) in the
LayoutSections component; update any place that maps over the data to use the
sliced array and remove the console.log(resultsCount) debug line.
| resultsCount = 4, | ||
| }: LayoutSectionProps) => { | ||
| if (!CardComponent) return null; | ||
| console.log(resultsCount); |
There was a problem hiding this comment.
Remove debug console.log statement.
This appears to be leftover debug code that should be removed before merging.
🧹 Proposed fix
- console.log(resultsCount);📝 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.
| console.log(resultsCount); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/visual-editor/src/components/pageSections/SearchSection/LayoutSections.tsx`
at line 22, Remove the leftover debug console.log call for resultsCount in
LayoutSections.tsx; locate the console.log(resultsCount) statement and delete it
(or replace with proper logging via the app logger if persistent logging is
required), and scan the surrounding function/component (LayoutSections) to
ensure no other stray debug console statements remain.
| {results.map((result) => ( | ||
| <CardComponent key={result.id} result={result} /> | ||
| ))} |
There was a problem hiding this comment.
Add defensive check for results to prevent runtime error.
results.map() will throw if results is undefined or null. Consider adding a fallback or guard.
🛡️ Proposed fix
- {results.map((result) => (
+ {results?.map((result) => (
<CardComponent key={result.id} result={result} />
- ))}
+ )) ?? null}Or use a fallback array:
- {results.map((result) => (
+ {(results ?? []).map((result) => (
<CardComponent key={result.id} result={result} />
))}📝 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.
| {results.map((result) => ( | |
| <CardComponent key={result.id} result={result} /> | |
| ))} | |
| {(results ?? []).map((result) => ( | |
| <CardComponent key={result.id} result={result} /> | |
| ))} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/visual-editor/src/components/pageSections/SearchSection/LayoutSections.tsx`
around lines 41 - 43, The code calls results.map(...) which will throw if
results is null/undefined; guard or provide a fallback before mapping. Update
the rendering around results.map in LayoutSections.tsx (the expression that
renders CardComponent for each result) to either conditionally render only when
results is an array (e.g. Array.isArray(results) && results.map(...)) or use a
fallback array ((results ?? []).map(...)); keep CardComponent and
key={result.id} semantics unchanged.
| const heightClass = !showResults ? getHeight(height) : ""; | ||
| const layoutClasses = !showResults | ||
| ? `${getWidth(width)} ${getAlignment(align)}` | ||
| : "w-full"; | ||
| const recognitionRef = React.useRef<BrowserSpeechRecognition | null>(null); |
There was a problem hiding this comment.
The height control is currently ineffective.
Line 219 computes heightClass, but Lines 316-321 pin the rendered control to h-16/h-12. That makes the editor’s styles.height setting change the wrapper while the actual search bar stays at the base size.
💡 Proposed fix
- const heightClass = !showResults ? getHeight(height) : "";
+ const heightClass = showResults ? getHeight("base") : getHeight(height);
...
- <div className={`relative ${layoutClasses} ${heightClass} `}>
+ <div className={`relative ${layoutClasses}`}>
...
- searchBarContainer: `h-16 ${getRounded(rounded)} !mb-0 relative ${
+ searchBarContainer: `${heightClass} ${getRounded(rounded)} !mb-0 relative ${
isTypingEffect ? "isTypingEffect" : ""
} w-full`,
...
- inputElement: `text-lg h-12 outline-none focus:outline-none focus:ring-0 focus:border-none px-5 py-2.5 rounded-[inherit]`,
+ inputElement: `text-lg h-full outline-none focus:outline-none focus:ring-0 focus:border-none px-5 py-2.5 rounded-[inherit]`,Also applies to: 316-321
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsx`
around lines 219 - 223, heightClass is computed via getHeight(height) but the
actual search bar container is hardcoded to "h-16"/"h-12", so the editor's
styles.height only affects the wrapper and not the input; replace the hardcoded
height classes on the search bar element (the JSX element that currently uses
"h-16"/"h-12") with the computed heightClass (preserving the showResults logic),
e.g. conditionally include heightClass when showResults is false or otherwise
use an appropriate full-height class, and ensure getHeight(height) returns the
correct Tailwind height string used in className.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsx (1)
219-223:⚠️ Potential issue | 🟠 MajorWire the height control into the rendered search bar.
Line 219 computes
heightClass, but Lines 332-337 still pin the actual bar toh-16/h-12, so the editor’sstyles.heightonly resizes the wrapper and not the input itself.💡 Proposed fix
- const heightClass = !showResults ? getHeight(height) : ""; + const heightClass = showResults ? getHeight("base") : getHeight(height); - <div className={`relative ${layoutClasses} ${heightClass} `}> + <div className={`relative ${layoutClasses}`}> - searchBarContainer: `h-16 ${getRounded(rounded)} !mb-0 relative ${ + searchBarContainer: `${heightClass} ${getRounded(rounded)} !mb-0 relative ${ isTypingEffect ? "isTypingEffect" : "" } w-full`, - inputElement: `text-lg h-12 outline-none focus:outline-none focus:ring-0 focus:border-none px-5 py-2.5 rounded-[inherit]`, + inputElement: `text-lg h-full outline-none focus:outline-none focus:ring-0 focus:border-none px-5 py-2.5 rounded-[inherit]`,Also applies to: 326-337
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsx` around lines 219 - 223, The computed heightClass (from getHeight(height)) is not applied to the actual search bar/input element—only the wrapper—so replace the hardcoded size classes ("h-16"/"h-12") in the rendered search bar with the heightClass (or include heightClass alongside existing classes) so the input itself respects editor styles; update the JSX in SearchBarSlot (the element rendered around lines ~326-337 where h-16/h-12 are used) to include heightClass when showResults is false (or always include heightClass) so the input and wrapper both use getHeight(height).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsx`:
- Around line 198-202: The placeholder from useTypingEffect can be empty
initially or on failure; update SearchBarSlot to keep a localized fallback
placeholder when placeholder is falsy by using the app's i18n/translation helper
(or a small locale->string map) keyed by document.locale and pass that fallback
into the rendered input instead of an empty string; specifically, where
useTypingEffect is called (useTypingEffect({ env:"PRODUCTION",
enabled:isTypingEffect, locale:document.locale })) and where placeholder is used
in the input render, replace direct usage with (placeholder ||
translatedDefaultPlaceholder) so the component always shows a translated default
while the async typing effect resolves or fails.
- Around line 134-138: The limit field currently allows 0 which is invalid
because createVisualAutocompleteConfig returns undefined for limit < 1; update
the YextField definition for limit (the "limit" property in SearchBarSlot) to
enforce a minimum of 1 (e.g., set min: 1 or add a validator) and/or add runtime
validation before calling createVisualAutocompleteConfig to ensure the
configured limit is >= 1 so the editor cannot show visual-autocomplete as
enabled while it will be disabled at runtime.
---
Duplicate comments:
In
`@packages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsx`:
- Around line 219-223: The computed heightClass (from getHeight(height)) is not
applied to the actual search bar/input element—only the wrapper—so replace the
hardcoded size classes ("h-16"/"h-12") in the rendered search bar with the
heightClass (or include heightClass alongside existing classes) so the input
itself respects editor styles; update the JSX in SearchBarSlot (the element
rendered around lines ~326-337 where h-16/h-12 are used) to include heightClass
when showResults is false (or always include heightClass) so the input and
wrapper both use getHeight(height).
🪄 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: b7f90b19-ebf3-4660-ad4b-027b54e9daa1
📒 Files selected for processing (1)
packages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsx
| limit: YextField(msg("fields.limit", "Limit"), { | ||
| type: "number", | ||
| min: 0, | ||
| max: 5, | ||
| }), |
There was a problem hiding this comment.
Don’t allow a visual-autocomplete limit that silently disables the feature.
Lines 134-138 allow 0, but createVisualAutocompleteConfig returns undefined for limit < 1, so the editor can show visual autocomplete as enabled while turning it off at runtime.
💡 Proposed fix
- min: 0,
+ min: 1,📝 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.
| limit: YextField(msg("fields.limit", "Limit"), { | |
| type: "number", | |
| min: 0, | |
| max: 5, | |
| }), | |
| limit: YextField(msg("fields.limit", "Limit"), { | |
| type: "number", | |
| min: 1, | |
| max: 5, | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsx`
around lines 134 - 138, The limit field currently allows 0 which is invalid
because createVisualAutocompleteConfig returns undefined for limit < 1; update
the YextField definition for limit (the "limit" property in SearchBarSlot) to
enforce a minimum of 1 (e.g., set min: 1 or add a validator) and/or add runtime
validation before calling createVisualAutocompleteConfig to ensure the
configured limit is >= 1 so the editor cannot show visual-autocomplete as
enabled while it will be disabled at runtime.
| const { placeholder } = useTypingEffect({ | ||
| env: "PRODUCTION", | ||
| enabled: isTypingEffect, | ||
| locale: document.locale, | ||
| }); |
There was a problem hiding this comment.
Keep a localized fallback placeholder when typing effect is enabled.
useTypingEffect starts empty and only fills placeholder after its async fetch completes, so Line 330 renders a blank placeholder on first load and stays blank on fetch failure. It also hardcodes English. Fall back to a translated default when placeholder is empty.
💡 Proposed fix
- placeholder={isTypingEffect ? placeholder : "Search here...."}
+ placeholder={
+ isTypingEffect && placeholder
+ ? placeholder
+ : t("searchHere", "Search here…")
+ }Also applies to: 330-330
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/visual-editor/src/components/pageSections/SearchSection/SearchBarSlot.tsx`
around lines 198 - 202, The placeholder from useTypingEffect can be empty
initially or on failure; update SearchBarSlot to keep a localized fallback
placeholder when placeholder is falsy by using the app's i18n/translation helper
(or a small locale->string map) keyed by document.locale and pass that fallback
into the rendered input instead of an empty string; specifically, where
useTypingEffect is called (useTypingEffect({ env:"PRODUCTION",
enabled:isTypingEffect, locale:document.locale })) and where placeholder is used
in the input render, replace direct usage with (placeholder ||
translatedDefaultPlaceholder) so the component always shows a translated default
while the async typing effect resolves or fails.
1 participant
Dev release to test pages publishing