Skip to content

Conversation

@mkouzel-yext
Copy link
Contributor

Adds all the new functionality to support customizable result cards in the locator. This functionality was broken into #905 and #912 (which also included #915), and now all the new features are jointly being merged into main.

J=PB-26919

k-gerner and others added 5 commits November 10, 2025 16:03
Adds lots of new properties to the locator config for styling the result
cards.

We still need to do work to plumb field options to the e.g. header
fields (i.e. besides just name), but the styling should be pretty much
there.




J=[WAT-5196](https://yexttest.atlassian.net/browse/WAT-5196?atlOrigin=eyJpIjoiZmRmNzIzYzFjODAyNDA4NmI3YmI2MjA1N2UyNDgwNjIiLCJwIjoiaiJ9),
[WAT-5199](https://yexttest.atlassian.net/browse/WAT-5199?atlOrigin=eyJpIjoiZDQzMzEwOWU3ZjI1NDQ4ODhmNTVmYWJlZTVjZTZmNGEiLCJwIjoiaiJ9)


https://github.com/user-attachments/assets/03682427-4543-483c-9826-e4f02c5c08f8


https://github.com/user-attachments/assets/861fce24-2a9d-4a87-ab3d-ead9302da643

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Updates the locator settings panel so that the result card fields are
read from the locator search experience display fields.

J=[WAT-5198](https://yext.atlassian.net/browse/WAT-5198)

Primary new functionality:

https://github.com/user-attachments/assets/2287f2b7-1d81-4a74-bcad-1537ed3c8768

Secondary CTA new behavior:


https://github.com/user-attachments/assets/cc15ab1f-16ce-4d9c-9e03-0aafd8b90cb4

---------

Co-authored-by: Kyle Gerner <kgerner@yext.com>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Kyle Gerner <49618240+k-gerner@users.noreply.github.com>
@mkouzel-yext mkouzel-yext added the create-dev-release Triggers dev release workflow label Nov 21, 2025
@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 21, 2025

pages-visual-editor-starter

npm i https://pkg.pr.new/yext/visual-editor/@yext/visual-editor@916

commit: f6660e1

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 21, 2025

Walkthrough

This change adds many new localization keys across visual-editor locales; introduces a new LocatorResultCard component (with props, types, editor fields, defaults); exposes resultCard props on Locator and replaces LocationCard with LocatorResultCard; adds a migration to merge default resultCard props; updates tests for showDistanceOptions/resultCard; extends editor tooling (DynamicOptionsSingleSelector, split EmbeddedFieldStringInputFromEntity/FromOptions); adds onClick handling to CTA and Phone atoms; updates public exports and template metadata types.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User
    participant Locator
    participant Migration as MigrationRegistry
    participant Config as resultCard Props
    participant LocatorCard as LocatorResultCard
    participant Data as LocationData

    Note over Migration,Config: Migration ensures defaults merged into resultCard props
    User->>Locator: trigger search / render
    Locator->>Migration: ensure migrations applied (merge default resultCard)
    Migration-->>Config: merged resultCard props
    Locator->>LocatorCard: render each result with LocationData + Config
    LocatorCard->>Data: resolve fields (name, phone, hours, services, image)
    Data-->>LocatorCard: return resolved values
    LocatorCard->>User: render headings, image, contact, CTAs
    User->>LocatorCard: click CTA / tap phone
    LocatorCard->>Locator: emit analytics / navigate (onClick handlers forwarded)
Loading

Possibly related PRs

Suggested reviewers

  • colton-demetriou
  • benlife5
  • mkilpatrick
  • asanehisa

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding customizable locator result cards as a new feature.
Description check ✅ Passed The description relates to the changeset by explaining that it adds new functionality for customizable result cards in the locator component, consolidating prior PRs.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch locator-result-cards

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (10)
packages/visual-editor/src/components/atoms/accordion.tsx (1)

7-8: Consider updating for consistency.

The Accordion component still uses string interpolation for className, while AccordionItem, AccordionTrigger, and AccordionContent now use themeManagerCn. Consider applying the same pattern here for consistency.

Apply this diff if consistency is desired:

-  return <div ref={ref} className={`flex flex-col ${className}`} {...props} />;
+  return <div ref={ref} className={themeManagerCn("flex flex-col", className)} {...props} />;
packages/visual-editor/src/components/atoms/cta.tsx (1)

159-174: Simplify redundant condition check.

The outer ctaType !== "presetImage" check on line 159 is redundant since showCaret (computed on line 115) already excludes preset images. Additionally, the nested ternary on lines 166-171 is complex and reduces readability.

Consider this simplification:

-  const caretIcon = ctaType !== "presetImage" && (
+  const caretIcon = (
     <FaAngleRight
       size="12px"
       // For directoryLink, the theme value for caret is ignored
       className={variant === "directoryLink" ? "block sm:hidden" : ""}
       // display does not support custom Tailwind utilities so the property must be set directly
       style={{
-        display:
-          variant === "directoryLink"
-            ? undefined
-            : showCaret
-              ? "inline-block"
-              : "none",
+        display: variant === "directoryLink" 
+          ? undefined 
+          : showCaret ? "inline-block" : "none",
       }}
     />
   );

Or even cleaner with early returns or simplified logic:

+  const getCaretDisplay = () => {
+    if (variant === "directoryLink") return undefined;
+    return showCaret ? "inline-block" : "none";
+  };
+
-  const caretIcon = ctaType !== "presetImage" && (
+  const caretIcon = (
     <FaAngleRight
       size="12px"
       className={variant === "directoryLink" ? "block sm:hidden" : ""}
-      style={{
-        display:
-          variant === "directoryLink"
-            ? undefined
-            : showCaret
-              ? "inline-block"
-              : "none",
-      }}
+      style={{ display: getCaretDisplay() }}
     />
   );
packages/visual-editor/src/internal/types/templateMetadata.ts (2)

20-27: FieldTypeData looks good; consider tightening field_type for safer usage

The new locatorDisplayFields surface and FieldTypeData shape make sense for exposing locator field metadata off TemplateMetadata. The only thing I’d consider is the field_type: object typing: it’s very loose and makes it hard to safely access any properties without type assertions.

If the backend already has a defined shape for this payload (or a finite set of variants), modeling that here (or at least using unknown and narrowing at call sites) would give better type-safety and self-documentation.

Example (optional) tweak:

-export type FieldTypeData = {
-  field_id: string;
-  field_name: string;
-  field_type: object;
-  field_type_id: string;
-};
+export type FieldTypeData = {
+  field_id: string;
+  field_name: string;
+  field_type: unknown; // or a more specific FieldType union when available
+  field_type_id: string;
+};

48-63: Keep dev locatorDisplayFields fixture aligned with real metadata shape

The sample locatorDisplayFields entries for name and mainPhone are useful, but two minor things to double‑check:

  • The structure of field_type differs ({ variant_type: 1 } vs {}); if downstream logic ever assumes variant_type exists, this fixture might not catch issues for non-string fields.
  • Locator result cards likely reference additional fields (e.g., address, services, etc.). If this dev metadata is used in any kind of field picker UI, you may want to expand this object or share a common fixture with whatever backs production metadata so the dev experience stays representative.

If the current minimal fixture is intentional and only used in very narrow flows, then this is fine as-is; otherwise, consider centralizing/updating it to mirror the backend response more closely.

packages/visual-editor/src/components/Locator.test.tsx (1)

104-164: Result card and filter test coverage is thorough and consistent

The new/updated cases:

  • Add showDistanceOptions to filters for both latest and v22 props.
  • Exercise a fully populated resultCard in the “latest version non‑default props” case.
  • Introduce a “version 24 default props” scenario that aligns with the new default locator card structure and migration versioning.

These provide solid regression coverage across versions and around the new result‑card configuration. The repeated document/env setup is a bit verbose but acceptable for clarity.

Also applies to: 351-397, 401-498

packages/visual-editor/src/editor/TranslatableStringField.tsx (1)

25-33: New getOptions/showFieldSelector path looks good; minor cleanup opportunity

The extended signature and conditional use of EmbeddedFieldStringInputFromOptions versus EmbeddedFieldStringInputFromEntity are consistent with the docs: when getOptions is provided, the entity-field filter is ignored, and the selector is populated from DynamicOption<string>[] after filtering out undefined values. This should be backward compatible for existing call sites that only pass label, filter, and showApplyAllOption.

One small clarity tweak: inside the getOptions branch you’re already guarded by getOptions ? (...) : (...), so the extra optional call is redundant:

-              options={getOptions?.()
+              options={getOptions()
                 .filter(
                   (opt): opt is { label: string; value: string } =>
                     opt.value !== undefined
                 )
                 .map((opt) => ({
                   label: opt.label,
                   value: opt.value,
                 }))}

Not required, but it tightens up the types and makes it clear that getOptions is expected whenever that branch is rendered.

Also applies to: 69-110

packages/visual-editor/src/editor/YextField.tsx (1)

122-137: Dynamic single-select wiring is correct; consider small type/API refinements

The new dynamicSingleSelect path is hooked up coherently:

  • YextDynamicSingleSelectField<T> matches the DynamicOptionsSingleSelector props (getOptions, dropdownLabel, placeholderOptionLabel).
  • The overload YextField< T extends DynamicOptionsSingleSelectorType<U>, U ...> plus the config.type === "dynamicSingleSelect" branch correctly surface a field.selection.value string for consumers like LocatorResultCardProps.

A couple of follow‑ups you might want to consider (not blockers):

  1. Return type alignment

    DynamicOptionsSingleSelector currently returns Field<DynamicOptionsSingleSelectorType<T> | undefined>, but the overload advertises Field<T>. In practice this all goes through the any implementation, but for downstream typings it may be clearer to mirror the multi‑select overload and allow undefined:

    export function YextField<
      T extends DynamicOptionsSingleSelectorType<U>,
      U extends DynamicOptionValueTypes,
    >(
      fieldName: MsgString,
      config: YextDynamicSingleSelectField<U>
  • ): Field;
  • ): Field<T | undefined>;
    
    
  1. Exposing showFieldSelector/getOptions for translatable strings

    Now that TranslatableStringField supports showFieldSelector and getOptions, you may eventually want to extend YextTranslatableStringField and the config.type === "translatableString" branch to pass those through as well, so callers using YextField don’t have to drop down and call TranslatableStringField directly for this capability.

Both are type/API niceties rather than functional issues, so they can be deferred if you prefer.

Also applies to: 160-163, 175-187, 296-312

packages/visual-editor/src/components/LocatorResultCard.tsx (1)

1032-1093: Improve robustness of resolveProjectedField property traversal

resolveProjectedField is a simple and effective helper for projected field IDs like "foo.bar.baz", but it currently relies on current?.hasOwnProperty(fieldId):

if (!current?.hasOwnProperty(fieldId)) {
  return undefined;
}
current = current[fieldId];

Two small robustness tweaks you might consider:

  • Guard explicitly against null/undefined before dereferencing.
  • Use Object.prototype.hasOwnProperty.call to avoid surprises if a value in the chain shadows hasOwnProperty.

For example:

const resolveProjectedField = (
  object: Record<string, any>,
  projectFieldId: string | undefined
): any => {
  if (!projectFieldId) {
    return undefined;
  }
-  // We can get away with this simple implementation for now since we know exactly what fields
-  // can be in the locator response
-  let current = object;
-  for (const fieldId of projectFieldId.split(".")) {
-    if (!current?.hasOwnProperty(fieldId)) {
-      return undefined;
-    }
-    current = current[fieldId];
-  }
+  let current: any = object;
+  for (const fieldId of projectFieldId.split(".")) {
+    if (!current || !Object.prototype.hasOwnProperty.call(current, fieldId)) {
+      return undefined;
+    }
+    current = current[fieldId];
+  }
   return current;
};

This keeps behavior the same for well-formed locator responses while being a bit more defensive against unexpected shapes.

packages/visual-editor/src/editor/EmbeddedFieldStringInput.tsx (2)

28-40: EmbeddedFieldStringInputFromEntity abstraction is clean and type-safe

Deriving entityFieldOptions via getFieldsForSelector and delegating into EmbeddedFieldStringInputFromOptions keeps the entity-aware logic separate from the generic options-based input. The generic T extends Record<string, any> and RenderEntityFieldFilter<T> wiring are appropriate, and defaulting showFieldSelector = true gives a sensible baseline.

If you expect many callers not to pass showFieldSelector, you might consider making its prop type optional (showFieldSelector?: boolean) to better match the defaulted parameter.

Also applies to: 41-52, 53-62


64-76: Options-based string input and selector gating behave as intended

EmbeddedFieldStringInputFromOptions correctly:

  • Keeps local state in sync with value and debounces onChange.
  • Uses showFieldSelector to fully gate the popover UI while always rendering the underlying input.
  • Reuses options via fieldOptions memo (even if slightly redundant).

The insertion logic for [[fieldName]] respects existing embedded segments and cursor position, preserving previous behavior.

You could inline fieldOptions and drop the useMemo here, since it currently just passes through options without transformation.

Also applies to: 101-104, 140-152, 153-188

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d59311e and 38bf4b3.

⛔ Files ignored due to path filters (29)
  • packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] latest version default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] latest version non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 10 default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 10 non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 21 default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 21 non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 22 default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 22 non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] version 24 default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] latest version default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] latest version non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 10 default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 10 non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 21 default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 21 non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 22 default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 22 non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[mobile] version 24 default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] latest version default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] latest version non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 10 default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 10 non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 21 default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 21 non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 22 default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 22 non-default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] version 24 default props.png is 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.png is 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.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (43)
  • packages/visual-editor/locales/cs/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/da/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/de/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/en-GB/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/en/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/es/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/et/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/fi/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/fr/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/hr/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/hu/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/it/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/ja/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/lt/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/lv/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/nb/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/nl/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/pl/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/pt/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/ro/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/sk/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/sv/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/tr/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/zh-TW/visual-editor.json (9 hunks)
  • packages/visual-editor/locales/zh/visual-editor.json (9 hunks)
  • packages/visual-editor/src/components/Locator.test.tsx (3 hunks)
  • packages/visual-editor/src/components/Locator.tsx (11 hunks)
  • packages/visual-editor/src/components/LocatorResultCard.tsx (1 hunks)
  • packages/visual-editor/src/components/atoms/accordion.tsx (4 hunks)
  • packages/visual-editor/src/components/atoms/cta.tsx (3 hunks)
  • packages/visual-editor/src/components/atoms/phone.tsx (4 hunks)
  • packages/visual-editor/src/components/index.ts (1 hunks)
  • packages/visual-editor/src/components/migrations/0024_locator_card_default_props.ts (1 hunks)
  • packages/visual-editor/src/components/migrations/migrationRegistry.ts (2 hunks)
  • packages/visual-editor/src/docs/ai/components.d.ts (1 hunks)
  • packages/visual-editor/src/docs/components.md (1 hunks)
  • packages/visual-editor/src/editor/DynamicOptionsSelector.tsx (3 hunks)
  • packages/visual-editor/src/editor/EmbeddedFieldStringInput.tsx (7 hunks)
  • packages/visual-editor/src/editor/TranslatableStringField.tsx (2 hunks)
  • packages/visual-editor/src/editor/YextEntityFieldSelector.tsx (3 hunks)
  • packages/visual-editor/src/editor/YextField.tsx (6 hunks)
  • packages/visual-editor/src/editor/index.ts (1 hunks)
  • packages/visual-editor/src/internal/types/templateMetadata.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-29T22:00:03.843Z
Learnt from: mkouzel-yext
Repo: yext/visual-editor PR: 833
File: packages/visual-editor/src/components/Locator.tsx:1050-1057
Timestamp: 2025-10-29T22:00:03.843Z
Learning: In packages/visual-editor/src/components/Locator.tsx, the AppliedFilters component is intentionally rendered in two locations (inside the filter modal and outside it) as per the design requirements. This dual rendering should not be flagged as a duplicate issue.

Applied to files:

  • packages/visual-editor/src/components/index.ts
  • packages/visual-editor/src/components/Locator.test.tsx
  • packages/visual-editor/src/components/LocatorResultCard.tsx
  • packages/visual-editor/src/components/Locator.tsx
📚 Learning: 2025-11-06T14:55:12.395Z
Learnt from: benlife5
Repo: yext/visual-editor PR: 862
File: packages/visual-editor/src/utils/schema/resolveSchema.ts:118-135
Timestamp: 2025-11-06T14:55:12.395Z
Learning: In `packages/visual-editor/src/utils/schema/resolveSchema.ts`, the `OpeningHoursSchema` and `PhotoGallerySchema` functions from `yext/pages-components` contain internal type validation and handle invalid inputs gracefully (returning empty objects or undefined) rather than throwing TypeErrors, so no pre-validation guards are needed before calling them.

Applied to files:

  • packages/visual-editor/src/editor/YextEntityFieldSelector.tsx
  • packages/visual-editor/src/docs/ai/components.d.ts
🧬 Code graph analysis (8)
packages/visual-editor/src/editor/TranslatableStringField.tsx (5)
packages/visual-editor/src/types/types.ts (1)
  • TranslatableString (205-205)
packages/visual-editor/src/utils/i18n/platform.ts (1)
  • MsgString (40-40)
packages/visual-editor/src/internal/utils/getFilteredEntityFields.ts (1)
  • RenderEntityFieldFilter (34-35)
packages/visual-editor/src/editor/DynamicOptionsSelector.tsx (1)
  • DynamicOption (33-36)
packages/visual-editor/src/editor/EmbeddedFieldStringInput.tsx (2)
  • EmbeddedFieldStringInputFromOptions (64-191)
  • EmbeddedFieldStringInputFromEntity (28-62)
packages/visual-editor/src/editor/YextEntityFieldSelector.tsx (2)
packages/visual-editor/src/editor/EmbeddedFieldStringInput.tsx (1)
  • EmbeddedFieldStringInputFromEntity (28-62)
packages/visual-editor/src/editor/index.ts (1)
  • EmbeddedFieldStringInputFromEntity (24-24)
packages/visual-editor/src/components/migrations/migrationRegistry.ts (1)
packages/visual-editor/src/components/migrations/0024_locator_card_default_props.ts (1)
  • locatorCardDefaultProps (94-157)
packages/visual-editor/src/components/atoms/cta.tsx (1)
packages/visual-editor/src/components/atoms/button.tsx (1)
  • Button (89-115)
packages/visual-editor/src/editor/YextField.tsx (2)
packages/visual-editor/src/editor/DynamicOptionsSelector.tsx (5)
  • DynamicOptionValueTypes (10-10)
  • DynamicOption (33-36)
  • DynamicOptionsSelectorType (23-25)
  • DynamicOptionsSingleSelectorType (27-31)
  • DynamicOptionsSingleSelector (88-122)
packages/visual-editor/src/utils/i18n/platform.ts (1)
  • MsgString (40-40)
packages/visual-editor/src/components/LocatorResultCard.tsx (9)
packages/visual-editor/src/editor/DynamicOptionsSelector.tsx (2)
  • DynamicOptionsSingleSelectorType (27-31)
  • DynamicOption (33-36)
packages/visual-editor/src/components/atoms/cta.tsx (1)
  • CTA (140-190)
packages/visual-editor/src/internal/hooks/useMessageReceivers.ts (1)
  • useTemplateMetadata (167-167)
packages/visual-editor/src/internal/types/templateMetadata.ts (1)
  • FieldTypeData (23-28)
packages/visual-editor/src/editor/YextField.tsx (1)
  • YextField (188-318)
packages/visual-editor/src/components/contentBlocks/HoursTable.tsx (1)
  • HoursTableStyleFields (49-78)
packages/visual-editor/src/editor/TranslatableStringField.tsx (1)
  • TranslatableStringField (25-126)
packages/visual-editor/src/components/atoms/accordion.tsx (4)
  • Accordion (4-9)
  • AccordionItem (11-25)
  • AccordionTrigger (27-53)
  • AccordionContent (55-62)
packages/visual-editor/src/components/atoms/phone.tsx (1)
  • PhoneAtom (17-57)
packages/visual-editor/src/components/Locator.tsx (3)
packages/visual-editor/src/components/LocatorResultCard.tsx (4)
  • LocatorResultCardProps (59-178)
  • DEFAULT_LOCATOR_RESULT_CARD_PROPS (180-237)
  • Location (575-588)
  • LocatorResultCard (590-806)
packages/visual-editor/src/editor/YextField.tsx (1)
  • YextField (188-318)
packages/visual-editor/src/editor/DynamicOptionsSelector.tsx (1)
  • DynamicOptionsSelectorType (23-25)
packages/visual-editor/src/editor/EmbeddedFieldStringInput.tsx (3)
packages/visual-editor/src/editor/index.ts (2)
  • EmbeddedFieldStringInputFromEntity (24-24)
  • EmbeddedFieldStringInputFromOptions (25-25)
packages/visual-editor/src/internal/utils/getFilteredEntityFields.ts (1)
  • RenderEntityFieldFilter (34-35)
packages/visual-editor/src/editor/YextEntityFieldSelector.tsx (1)
  • getFieldsForSelector (396-416)
🔇 Additional comments (30)
packages/visual-editor/src/components/atoms/accordion.tsx (2)

2-2: LGTM!

The import of themeManagerCn is correctly structured for integrating theme management into the accordion components.


18-21: LGTM!

The refactor correctly applies themeManagerCn to merge base classes with incoming className props, ensuring consistent theme management across these components.

Also applies to: 34-37, 59-61

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

41-41: Localization additions look well-coordinated and complete across all locales. All new translation keys (callToAction, CTAVariant, icons, mainPhone, phone, primaryHeading, resultCard, secondaryHeading, tertiaryHeading, services) are present and consistently positioned across all 8 locale files (en, et, zh-TW, da, pt, fr, es, pl). The English translations are clear, follow existing conventions, and JSON syntax is valid.

Also applies to: 126-127, 199-200, 230-231, 344-345, 357-358, 362-363, 371-372, 375-376, 394-395, 560-561

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

41-41: Translations are consistent with the English locale additions. All expected new keys are present and properly positioned within the Estonian locale structure. JSON formatting is valid.

Also applies to: 127-127, 200-200, 231-231, 345-345, 358-358, 363-363, 372-372, 376-376, 395-395, 561-561

packages/visual-editor/locales/zh-TW/visual-editor.json (1)

41-41: Translations are consistent with the English locale additions. All expected new keys are present and properly positioned within the Traditional Chinese locale structure. JSON formatting is valid.

Also applies to: 127-127, 200-200, 231-231, 345-345, 358-358, 363-363, 372-372, 376-376, 395-395, 561-561

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

41-41: Translations are consistent with the English locale additions. All expected new keys are present and properly positioned within the Danish locale structure. JSON formatting is valid.

Also applies to: 128-128, 201-201, 232-232, 346-346, 359-359, 364-364, 373-373, 377-377, 396-396, 562-562

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

41-41: Translations are consistent with the English locale additions. All expected new keys are present and properly positioned within the Portuguese locale structure. JSON formatting is valid.

Also applies to: 127-127, 200-200, 231-231, 345-345, 358-358, 363-363, 372-372, 376-376, 395-395, 561-561

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

41-41: Translations are consistent with the English locale additions. All expected new keys are present and properly positioned within the French locale structure. JSON formatting is valid.

Also applies to: 126-126, 199-199, 230-230, 344-344, 357-357, 362-362, 371-371, 375-375, 394-394, 560-560

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

41-41: Translations are consistent with the English locale additions. All expected new keys are present and properly positioned within the Spanish locale structure. JSON formatting is valid.

Also applies to: 126-126, 199-199, 230-230, 344-344, 357-357, 362-362, 371-371, 375-375, 394-394, 560-560

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

41-41: Translations are consistent with the English locale additions. All expected new keys are present and properly positioned within the Polish locale structure. JSON formatting is valid.

Also applies to: 128-128, 201-201, 232-232, 346-346, 359-359, 364-364, 373-373, 377-377, 396-396, 562-562

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

41-41: Localization additions are consistent and complete across all 23 locales.

Verification confirms that all new translation keys (callToAction, CTAVariant, icons, mainPhone, primaryHeading, resultCard, secondaryHeading, tertiaryHeading) are present in all 25 locale files. The Hungarian translations are appropriately integrated as part of a coordinated localization update with no missing keys or inconsistencies detected.

packages/visual-editor/src/components/atoms/cta.tsx (3)

29-29: LGTM! Clean addition of onClick handler support.

The optional onClick prop is correctly typed for anchor element events, maintaining backward compatibility.


186-186: LGTM! Cleaner JSX with extracted caret logic.

Moving the caret rendering into a constant improves readability of the main render logic.


183-183: The Link component has an onClick handler that will run before the link redirects, so passing onClick to the Link component from @yext/pages-components is valid and will work as expected. No changes are needed.

packages/visual-editor/src/components/atoms/phone.tsx (2)

14-14: LGTM! Clean implementation of onClick support.

The additions properly support custom click handlers by:

  • Adding the optional onClick prop
  • Conditionally sanitizing phone numbers for tel: links when onClick is present
  • Passing onClick through to the CTA component

The comment clearly explains why sanitization is needed when a custom handler is provided.

Also applies to: 23-28, 45-45, 50-50


26-28: No issues found—the code is correct as written.

The CTA component's link prop is defined as link?: string, which accepts string | undefined. When phoneNumberLink is undefined (from sanitizePhoneForTelHref returning undefined for an empty string), it is safely handled. The CTA component has a fallback that converts falsy link values to "#", so no error occurs.

packages/visual-editor/src/components/index.ts (1)

15-19: Public API surfacing of LocatorResultCard looks correct

Re‑exporting LocatorResultCard, Location, and LocatorResultCardProps from the components index cleanly exposes the new result card surface without changing existing behavior. No issues from my side.

packages/visual-editor/src/docs/ai/components.d.ts (1)

1-12: Confirm ListingType availability in @yext/pages-components

The new ListingType import looks fine, but it does rely on your installed @yext/pages-components exposing that symbol. Please confirm the dependency version you’re targeting includes ListingType so the type declarations continue to compile cleanly.

packages/visual-editor/src/docs/components.md (1)

431-447: LocatorComponent resultCard/filter docs align with new API

The updated table for filters and resultCard.* (dynamic selector types, heading levels, hours table, address/phone/email/services, and CTAs/image) matches the new Locator result‑card configuration and is consistent with the rest of the docs style. Looks good.

packages/visual-editor/src/editor/YextEntityFieldSelector.tsx (1)

46-47: EmbeddedFieldStringInputFromEntity integration looks sound

Using EmbeddedFieldStringInputFromEntity for the single‑string constant path keeps the previous behavior (entity‑driven field options, locale‑aware value) while aligning with the new embedded‑field input split. The generic filter type and onChange wiring look correct; showFieldSelector={true} is explicit but consistent with the intended UX.

Also applies to: 347-364

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

25-25: New locator card default‑props migration correctly registered

Importing locatorCardDefaultProps and appending it as the final entry in migrationRegistry keeps the migration ordering consistent with the existing versioning scheme (tests using migrationRegistry.length/version 24). No issues here.

Also applies to: 56-57

packages/visual-editor/src/components/migrations/0024_locator_card_default_props.ts (1)

3-92: Deep merge behavior looks correct; verify defaults vs runtime expectations

The migration’s merge strategy is solid:

  • resultCard is built from DEFAULT_RESULT_CARD overlaid with existingResultCard, then key nested sections (headings, hours.table, address/phone/email/services, CTAs, image) are deep‑merged so existing user overrides win while new fields get sensible defaults.
  • hours.table specifically preserves any prior table config while filling in new properties.

Two things worth double‑checking:

  1. Icons default for migrated configs

    • DEFAULT_RESULT_CARD.icons is false, while the “version 24 default props” test config sets icons: true as the default.
    • If the intent is that new locators show icons by default but migrated locators do not (to avoid visual changes), this is fine. If not, you may want DEFAULT_RESULT_CARD.icons to match whatever LocatorComponent.defaultProps uses so defaults stay consistent across new vs migrated content.
  2. field shape vs DynamicOptionsSingleSelectorType

    • Here, defaults use field: { selection: { value: "…" } }, while some tests use plain strings (e.g., field: "name").
    • Please confirm that both shapes are valid for the DynamicOptionsSingleSelectorType<string> used by the Locator result card, and that this migration’s shape matches what the runtime and DEFAULT_LOCATOR_RESULT_CARD_PROPS expect.

Overall, the migration logic itself looks correct; this is mainly about validating that the baked‑in defaults line up with your intended UX and type contracts.

Also applies to: 94-157

packages/visual-editor/src/editor/index.ts (1)

23-26: New editor exports look consistent and correctly wired

The additional exports for the embedded field string inputs and the single-options selector line up with the underlying module names and types, keeping the editor’s public surface in sync with the new components.

Also applies to: 29-35

packages/visual-editor/src/components/LocatorResultCard.tsx (1)

180-237: Result card config and dynamic options integration look solid

The LocatorResultCardProps, DEFAULT_LOCATOR_RESULT_CARD_PROPS, and LocatorResultCardFields wiring is coherent:

  • Each configurable field (headings, hours, phone, email, services, image) uses DynamicOptionsSingleSelectorType<string> with field.selection.value as the projected path, and the defaults are consistent with those types.
  • getDisplayFieldOptions correctly derives DynamicOption<string>[] from templateMetadata.locatorDisplayFields, filtered by field_type_id ("type.string", "type.hours", "type.phone", "type.image"). When metadata is missing, it safely returns an empty array.
  • The various YextField uses for dynamicSingleSelect and the two TranslatableStringField instances (for secondary CTA label/link) all point to () => getDisplayFieldOptions("type.string"), which matches how the runtime code later dereferences selection.value via resolveProjectedField.

Just one constraint to keep in mind: getDisplayFieldOptions calls useTemplateMetadata, so it must only be invoked from functions that are themselves executed during React render (as you’re doing via getOptions and getOptions-backed TranslatableStringFields). Avoid reusing it from non-React contexts (e.g., plain utility modules or migrations) to stay within the rules of hooks.

Also applies to: 239-257, 259-573

packages/visual-editor/src/components/Locator.tsx (5)

2-3: New imports and type wiring look consistent

The added imports for WithPuckProps, React, Location, LocatorResultCard, LocatorResultCardProps, DEFAULT_LOCATOR_RESULT_CARD_PROPS, LocatorResultCardFields, and the typed mapbox-gl symbols are all used and align with the new result-card flow; no issues from a typing or module-path perspective.

Also applies to: 31-43, 48-52


442-447: Result card prop plumbed cleanly through Locator surface

The new resultCard: LocatorResultCardProps prop, its field config (LocatorResultCardFields), and default (DEFAULT_LOCATOR_RESULT_CARD_PROPS) are wired end-to-end into LocatorInternal and LocatorResultCard. This gives a clear, single configuration surface for result cards without breaking runtime behavior, assuming callers rely on defaultProps when not specifying resultCard explicitly.

Also applies to: 534-535, 547-548, 591-597


503-517: Dynamic facet field selector integration looks correct

Using YextField<DynamicOptionsSelectorType<string>, string> with type: "dynamicSelect" and mapping facetFields?.selections into selectedFacets for setFacetAllowList matches the DynamicOptionsSelectorType contract and is safely guarded against undefined. This should make facet configuration much more flexible without introducing obvious runtime hazards.

Also applies to: 680-690


826-827: Vertical limit of 50 results: confirm intended UX

Calling searchActions.setVerticalLimit(50); before the initial query will cap results at 50. That’s a reasonable default, but it is a behavioral change; just confirm this matches the expected locator UX and analytics assumptions.


1056-1056: Layout tweaks appear safe but may affect small/medium breakpoints

The updated left-panel width/min-width (md:w-2/5 lg:w-[40rem] md:min-w-[24rem]) and the right-panel responsive classes (md:flex-1 md:flex hidden) are consistent with Tailwind usage and shouldn’t introduce functional issues; just sanity-check on a few common viewport sizes to ensure the map and list maintain the desired proportions.

Also applies to: 1133-1133

packages/visual-editor/src/editor/EmbeddedFieldStringInput.tsx (1)

173-181: CommandItemWithResolvedValue correctly handles raw vs resolved sublabels

The new useOptionValue flag:

  • Skips the resolution effect when true, avoiding unnecessary resolveComponentData work.
  • Drives the render path to show option.value instead of a resolved value via the conditional on (resolvedValue || useOptionValue).

This matches the intended behavior for cases where options already carry user-facing text versus entity-field-based values. The dependencies include useOptionValue, so toggling the flag won’t leave the effect in a stale state.

Also applies to: 193-203, 211-215, 231-247

* adds resultCardProps to CardComponent callback deps
* checks for "typeof distance === "number"" instead of just "distance" to handle distance = 0 case correctly
@mkouzel-yext mkouzel-yext requested review from a team November 21, 2025 15:57
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
packages/visual-editor/src/components/Locator.tsx (1)

793-802: Dependency array now correctly includes resultCardProps.

The previous issue flagging the missing resultCardProps dependency has been resolved. The callback will now properly update when result card configuration changes in the editor.

packages/visual-editor/src/components/LocatorResultCard.tsx (1)

607-612: Verify t() function signature compatibility.

The distance type-guard typeof distance === "number" correctly handles zero values and is more precise than a nullish check.

However, the t() calls at lines 687 and 766 use a three-argument form: t("distanceInUnit", \${distanceInMiles} mi`, { distanceInMiles, distanceInKilometers }). According to react-i18next documentation, the standard signatures are t(key), t(key, options), or using defaultValue` within the options object. Verify this signature works with your i18next configuration.

Run the following to check current i18next usage patterns in the codebase:

#!/bin/bash
# Check other t() usage patterns to see if three-arg form is used elsewhere
rg -n 'this\.t\(|useTranslation.*t.*\(' packages/visual-editor/src -A 2 | rg '^\s+t\(' -A 1 | head -30

If the three-argument form is not standard in your setup, refactor to:

-  t("distanceInUnit", `${distanceInMiles} mi`, {
-    distanceInMiles,
-    distanceInKilometers,
-  })
+  t("distanceInUnit", {
+    defaultValue: `${distanceInMiles} mi`,
+    distanceInMiles,
+    distanceInKilometers,
+  })

Also applies to: 681-692, 759-771

🧹 Nitpick comments (1)
packages/visual-editor/src/components/LocatorResultCard.tsx (1)

1072-1094: Consider safer property access pattern.

The resolveProjectedField function uses hasOwnProperty which works but could be modernized. While not a security issue here (field IDs are controlled), consider using Object.hasOwn() or optional chaining for cleaner code:

 const resolveProjectedField = (
   object: Record<string, any>,
   projectFieldId: string | undefined
 ): any => {
   if (!projectFieldId) {
     return undefined;
   }
   let current = object;
   for (const fieldId of projectFieldId.split(".")) {
-    if (!current?.hasOwnProperty(fieldId)) {
+    if (!Object.hasOwn(current ?? {}, fieldId)) {
       return undefined;
     }
     current = current[fieldId];
   }
   return current;
 };

Alternatively, consider a more modern approach with optional chaining:

const resolveProjectedField = (
  object: Record<string, any>,
  projectFieldId: string | undefined
): any => {
  if (!projectFieldId) return undefined;
  
  return projectFieldId.split(".").reduce((current, fieldId) => {
    return current?.[fieldId];
  }, object);
};
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be4694a and eef252d.

📒 Files selected for processing (2)
  • packages/visual-editor/src/components/Locator.tsx (11 hunks)
  • packages/visual-editor/src/components/LocatorResultCard.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-29T22:00:03.843Z
Learnt from: mkouzel-yext
Repo: yext/visual-editor PR: 833
File: packages/visual-editor/src/components/Locator.tsx:1050-1057
Timestamp: 2025-10-29T22:00:03.843Z
Learning: In packages/visual-editor/src/components/Locator.tsx, the AppliedFilters component is intentionally rendered in two locations (inside the filter modal and outside it) as per the design requirements. This dual rendering should not be flagged as a duplicate issue.

Applied to files:

  • packages/visual-editor/src/components/Locator.tsx
  • packages/visual-editor/src/components/LocatorResultCard.tsx
📚 Learning: 2025-11-21T15:34:06.038Z
Learnt from: mkouzel-yext
Repo: yext/visual-editor PR: 916
File: packages/visual-editor/src/components/atoms/phone.tsx:94-94
Timestamp: 2025-11-21T15:34:06.038Z
Learning: In packages/visual-editor/src/components/atoms/phone.tsx, the sanitizePhoneForTelHref function assumes that the '+' character only appears as a prefix in phone numbers (for international dialing codes), never in the middle of the phone number string. This is consistent with standard phone number formats.

Applied to files:

  • packages/visual-editor/src/components/Locator.tsx
🧬 Code graph analysis (2)
packages/visual-editor/src/components/Locator.tsx (3)
packages/visual-editor/src/components/LocatorResultCard.tsx (5)
  • LocatorResultCardProps (59-178)
  • LocatorResultCardFields (259-573)
  • DEFAULT_LOCATOR_RESULT_CARD_PROPS (180-237)
  • Location (575-588)
  • LocatorResultCard (590-806)
packages/visual-editor/src/editor/YextField.tsx (1)
  • YextField (188-318)
packages/visual-editor/src/editor/DynamicOptionsSelector.tsx (1)
  • DynamicOptionsSelectorType (23-25)
packages/visual-editor/src/components/LocatorResultCard.tsx (9)
packages/visual-editor/src/editor/DynamicOptionsSelector.tsx (2)
  • DynamicOptionsSingleSelectorType (27-31)
  • DynamicOption (33-36)
packages/visual-editor/src/components/atoms/cta.tsx (1)
  • CTA (140-190)
packages/visual-editor/src/internal/hooks/useMessageReceivers.ts (1)
  • useTemplateMetadata (167-167)
packages/visual-editor/src/internal/types/templateMetadata.ts (1)
  • FieldTypeData (23-28)
packages/visual-editor/src/editor/YextField.tsx (1)
  • YextField (188-318)
packages/visual-editor/src/components/contentBlocks/HoursTable.tsx (1)
  • HoursTableStyleFields (49-78)
packages/visual-editor/src/editor/TranslatableStringField.tsx (1)
  • TranslatableStringField (25-126)
packages/visual-editor/src/components/atoms/accordion.tsx (4)
  • Accordion (4-9)
  • AccordionItem (11-25)
  • AccordionTrigger (27-53)
  • AccordionContent (55-62)
packages/visual-editor/src/components/atoms/phone.tsx (1)
  • PhoneAtom (17-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: call_unit_test / unit_tests (18.x)
  • GitHub Check: call_unit_test / unit_tests (20.x)
  • GitHub Check: semgrep/ci
  • GitHub Check: create-dev-release
🔇 Additional comments (8)
packages/visual-editor/src/components/Locator.tsx (3)

442-446: LGTM! LocatorResultCard integration is well-structured.

The new resultCard prop is properly typed, has a default value, and is correctly wired into the component's field configuration. The integration follows the existing patterns in the codebase.

Also applies to: 534-534, 547-547


503-517: LGTM! Facet fields typing is correct.

The typing for facetFields correctly reflects the dynamic selector pattern with DynamicOptionsSelectorType<string>, and the field configuration properly uses dynamicSelect type with appropriate options retrieval.


826-826: Hardcoded result limit is reasonable for current use case.

The limit of 50 results is a sensible default for a locator search. If configurability becomes necessary in the future, this could be extracted to a prop.

packages/visual-editor/src/components/LocatorResultCard.tsx (5)

59-178: LGTM! Interface definitions are comprehensive and well-documented.

The LocatorResultCardProps interface provides extensive customization options with clear JSDoc comments and default values. The Location interface appropriately uses an index signature to allow flexibility for custom fields while maintaining type safety for required properties.

Also applies to: 180-237, 575-588


590-806: LGTM! Main component structure is well-organized.

The use of React.memo is appropriate for list rendering performance. Analytics callbacks are properly configured for all user interactions (directions, visit page, CTA clicks, phone taps), and the component cleanly separates concerns into logical subsections.


819-844: LGTM! ImageSection handles edge cases properly.

The component provides sensible fallbacks for missing image metadata (dimensions default to 80×80, alternate text falls back to location name) and correctly gates rendering with the liveVisibility flag.


846-885: LGTM! Heading section provides appropriate fallbacks.

The primary heading correctly falls back to location.name ensuring content is always displayed, while secondary and tertiary headings are properly gated by both data presence and liveVisibility settings.


887-940: LGTM! Hours section provides good UX with accordion.

The collapsible accordion pattern is appropriate for hours display in a compact card layout. The component properly integrates existing hour display atoms and respects all configuration flags.

benlife5
benlife5 previously approved these changes Nov 21, 2025
Copy link
Contributor

@benlife5 benlife5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting that we'll need to re-number the slots migrations

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (1)
packages/visual-editor/src/components/LocatorResultCard.tsx (1)

687-690: Fix t() function call signature at lines 687-690 and 766-769.

The three-argument form t(key, defaultValue, options) is not supported by react-i18next v15.7.1 (which uses i18next). i18next's documented t API accepts either t(key) or t(key, options) where the options object can include defaultValue. The current code passes the options as a third argument, which JavaScript ignores, breaking interpolation variable passing.

Apply at lines 687-690:

-                 {t("distanceInUnit", `${distanceInMiles} mi`, {
-                   distanceInMiles,
-                   distanceInKilometers,
-                 })}
+                 {t("distanceInUnit", {
+                   defaultValue: `${distanceInMiles} mi`,
+                   distanceInMiles,
+                   distanceInKilometers,
+                 })}

Apply the same change at lines 766-769.

🧹 Nitpick comments (2)
packages/visual-editor/src/components/LocatorResultCard.tsx (2)

607-612: Extract magic numbers to named constants.

The conversion factors 1609.344 (meters to miles) and 1000 (meters to kilometers) are magic numbers that reduce readability and maintainability.

Define constants at the top of the file:

+const METERS_TO_MILES = 1609.344;
+const METERS_TO_KILOMETERS = 1000;
+
 export const LocatorResultCard = React.memo(
   ({
     result,
     resultCardProps: props,
     puck,
   }: {
     result: CardProps<Location>["result"];
     resultCardProps: LocatorResultCardProps;
     puck: PuckContext;
   }): React.JSX.Element => {
     const { document: streamDocument, relativePrefixToRoot } =
       useTemplateProps();
     const { t, i18n } = useTranslation();
 
     const location = result.rawData;
     const distance = result.distance;
 
     const distanceInMiles =
       typeof distance === "number"
-        ? (distance / 1609.344).toFixed(1)
+        ? (distance / METERS_TO_MILES).toFixed(1)
         : undefined;
     const distanceInKilometers =
-      typeof distance === "number" ? (distance / 1000).toFixed(1) : undefined;
+      typeof distance === "number" ? (distance / METERS_TO_KILOMETERS).toFixed(1) : undefined;

1076-1094: Replace hasOwnProperty with safer alternative.

Line 1087 uses hasOwnProperty directly on the object, which can fail if the object has its own hasOwnProperty property or if the object is created with Object.create(null).

Use the safer Object.hasOwn() (ES2022+) or Object.prototype.hasOwnProperty.call():

 const resolveProjectedField = (
   object: Record<string, any>,
   projectFieldId: string | undefined
 ): any => {
   if (!projectFieldId) {
     return undefined;
   }
   // We can get away with this simple implementation for now since we know exactly what fields
   // can be in the locator response
   let current = object;
   for (const fieldId of projectFieldId.split(".")) {
-    if (!current?.hasOwnProperty(fieldId)) {
+    if (!Object.hasOwn(current ?? {}, fieldId)) {
       return undefined;
     }
     current = current[fieldId];
   }
 
   return current;
 };

If targeting environments without ES2022 support, use:

if (!Object.prototype.hasOwnProperty.call(current ?? {}, fieldId)) {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eef252d and 0eda3d3.

📒 Files selected for processing (1)
  • packages/visual-editor/src/components/LocatorResultCard.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-29T22:00:03.843Z
Learnt from: mkouzel-yext
Repo: yext/visual-editor PR: 833
File: packages/visual-editor/src/components/Locator.tsx:1050-1057
Timestamp: 2025-10-29T22:00:03.843Z
Learning: In packages/visual-editor/src/components/Locator.tsx, the AppliedFilters component is intentionally rendered in two locations (inside the filter modal and outside it) as per the design requirements. This dual rendering should not be flagged as a duplicate issue.

Applied to files:

  • packages/visual-editor/src/components/LocatorResultCard.tsx
🧬 Code graph analysis (1)
packages/visual-editor/src/components/LocatorResultCard.tsx (8)
packages/visual-editor/src/editor/DynamicOptionsSelector.tsx (2)
  • DynamicOptionsSingleSelectorType (27-31)
  • DynamicOption (33-36)
packages/visual-editor/src/components/atoms/cta.tsx (1)
  • CTA (140-190)
packages/visual-editor/src/internal/hooks/useMessageReceivers.ts (1)
  • useTemplateMetadata (167-167)
packages/visual-editor/src/internal/types/templateMetadata.ts (1)
  • FieldTypeData (23-28)
packages/visual-editor/src/editor/YextField.tsx (1)
  • YextField (188-318)
packages/visual-editor/src/components/contentBlocks/HoursTable.tsx (1)
  • HoursTableStyleFields (49-78)
packages/visual-editor/src/editor/TranslatableStringField.tsx (1)
  • TranslatableStringField (25-126)
packages/visual-editor/src/components/atoms/phone.tsx (1)
  • PhoneAtom (17-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: call_unit_test / unit_tests (18.x)
  • GitHub Check: call_unit_test / unit_tests (20.x)
  • GitHub Check: semgrep/ci
  • GitHub Check: create-dev-release

@mkouzel-yext mkouzel-yext merged commit 019f599 into main Nov 21, 2025
16 checks passed
@mkouzel-yext mkouzel-yext deleted the locator-result-cards branch November 21, 2025 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

create-dev-release Triggers dev release workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants