feat: inject locator display fields into puck field dropdowns#912
feat: inject locator display fields into puck field dropdowns#912mkouzel-yext merged 73 commits intolocator-result-cardsfrom
Conversation
…nto add-field-dropdowns
…nto add-field-dropdowns
…ues; add liveVisibility setting for secondary/tertiary headers
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/visual-editor/src/components/LocatorResultCard.tsx (1)
1028-1090: Parsing helpers look solid with one optional improvement.The parsing utilities cleanly handle data extraction:
parseStringFromLocationcorrectly treats null/undefined as undefined (avoiding the "undefined" string issue)parseArrayFromLocationsafely validates array typesresolveProjectedFieldimplements dot-notation field resolution with proper early returnsOptional: Consider using
Object.hasOwnfor better robustness.At line 1083,
hasOwnPropertyworks correctly here but could be more robust in edge cases (objects with null prototype or shadowedhasOwnProperty):- if (!current?.hasOwnProperty(fieldId)) { + if (!Object.hasOwn(current, fieldId)) { return undefined; }Note: Requires the optional chaining to be moved since
Object.hasOwnhandles null/undefined checking:+ if (current === null || current === undefined) { + return undefined; + } + if (!Object.hasOwn(current, fieldId)) {Or use the traditional approach:
- if (!current?.hasOwnProperty(fieldId)) { + if (!current || !Object.prototype.hasOwnProperty.call(current, fieldId)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/visual-editor/src/components/testing/screenshots/StaticMapSection/[desktop] default props with coordinate - with api key.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (1)
packages/visual-editor/src/components/LocatorResultCard.tsx(25 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/LocatorResultCard.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/components/LocatorResultCard.tsx
🧬 Code graph analysis (1)
packages/visual-editor/src/components/LocatorResultCard.tsx (5)
packages/visual-editor/src/editor/DynamicOptionsSelector.tsx (2)
DynamicOptionsSingleSelectorType(27-31)DynamicOption(33-36)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/editor/TranslatableStringField.tsx (1)
TranslatableStringField(20-94)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: call_unit_test / unit_tests (18.x)
- GitHub Check: semgrep/ci
🔇 Additional comments (2)
packages/visual-editor/src/components/LocatorResultCard.tsx (2)
18-18: LGTM! Well-structured type system for dynamic field selection.The migration from plain
stringfields toDynamicOptionsSingleSelectorType<string>enables template-metadata-driven field selection. The updated DEFAULT_LOCATOR_RESULT_CARD_PROPS correctly uses the new{ selection: { value: "<field>" } }shape. The Image component import from @yext/pages-components will provide CDN optimization benefits.Also applies to: 28-29, 55-56, 65-236
815-1026: Excellent modularization with robust null-safety.The new section components (ImageSection, HeadingTextSection, HoursSection, PhoneSection, EmailSection, ServicesSection) provide clear separation of concerns and improve maintainability. Key strengths:
- Consistent optional chaining (
field?.selection?.value) prevents runtime errors- Type guards in EmailSection and ServicesSection validate array element types before rendering
- All sections respect
liveVisibilityflags- ImageSection now uses
Imagefrom @yext/pages-components for CDN optimization (addresses prior feedback)
Use display fields for the secondary CTA label/link J=[WAT-5225](https://yext.atlassian.net/browse/WAT-5225?atlOrigin=eyJpIjoiZWVmODAyNzliOGNmNGE5OWFlM2I2NjMwNmJhNjcwNzgiLCJwIjoiaiJ9) https://github.com/user-attachments/assets/dd38c2af-0334-4269-b66f-aa11e2f5c59a --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/visual-editor/src/editor/TranslatableStringField.tsx (2)
71-110: Consider extracting the duplicated onChange logic.The onChange handler logic on lines 74-82 and 98-106 is identical. Extracting this into a helper function would reduce duplication and improve maintainability.
Example refactor:
+ const handleLocaleChange = (val: any) => { + return onChange({ + ...(typeof value === "object" && !Array.isArray(value) + ? value + : {}), + [locale]: val, + hasLocalizedValue: "true", + } as Record<string, string> as T); + }; + const fieldEditor = ( <> {getOptions ? ( <EmbeddedFieldStringInputFromOptions value={resolvedValue} - onChange={(val: any) => { - return onChange({ - ...(typeof value === "object" && !Array.isArray(value) - ? value - : {}), - [locale]: val, - hasLocalizedValue: "true", - } as Record<string, string> as T); - }} + onChange={handleLocaleChange} options={getOptions?.() .filter( (opt): opt is { label: string; value: string } => opt.value !== undefined ) .map((opt) => ({ label: opt.label, value: opt.value, }))} showFieldSelector={showFieldSelector ?? true} useOptionValueSublabel={true} /> ) : ( <EmbeddedFieldStringInputFromEntity value={resolvedValue} - onChange={(val: any) => { - return onChange({ - ...(typeof value === "object" && !Array.isArray(value) - ? value - : {}), - [locale]: val, - hasLocalizedValue: "true", - } as Record<string, string> as T); - }} + onChange={handleLocaleChange} filter={filter ?? { types: ["type.string"] }} showFieldSelector={showFieldSelector ?? true} /> )}
83-83: Unnecessary optional chaining.The optional chaining
getOptions?.()on line 83 is redundant sincegetOptionsis already verified to be truthy on line 71. You can safely usegetOptions()instead.packages/visual-editor/src/editor/YextEntityFieldSelector.tsx (1)
46-47: EmbeddedFieldStringInputFromEntity wiring for single-string constants looks solidThe switch to
EmbeddedFieldStringInputFromEntityin the single-string constant path preserves the previous behavior (entity-field-backed insertion + localized value handling) and cleanly adopts the new API. TheonChangemerge logic forconstantValueandhasLocalizedValueremains correct.If you want to trim a tiny bit of noise, you could rely on the component’s default and omit the explicit
showFieldSelector={true}prop.Also applies to: 342-364
packages/visual-editor/src/editor/EmbeddedFieldStringInput.tsx (2)
28-62: Good split between FromEntity and FromOptions; small prop-typing nitThe split into
EmbeddedFieldStringInputFromEntity(entity-field aware) andEmbeddedFieldStringInputFromOptions(generic options-driven) is clean and makes reuse straightforward. The wrapper’s use ofgetFieldsForSelector+ mapping to{ label, value }matches other selector patterns in the codebase.One small ergonomic tweak: both components treat
showFieldSelectoras required in the prop type but also give it a default oftruein the parameter destructuring. If you intend it to be optional, consider typing it asshowFieldSelector?: booleanin both places so callers aren’t forced to pass it explicitly.-export const EmbeddedFieldStringInputFromEntity = <T extends Record<string, any>>({ - value, - onChange, - filter, - showFieldSelector = true, -}: { - value: string; - onChange: (value: string) => void; - filter: RenderEntityFieldFilter<T>; - showFieldSelector: boolean; -}) => { +export const EmbeddedFieldStringInputFromEntity = <T extends Record<string, any>>({ + value, + onChange, + filter, + showFieldSelector = true, +}: { + value: string; + onChange: (value: string) => void; + filter: RenderEntityFieldFilter<T>; + showFieldSelector?: boolean; +}) => {(and similarly for
EmbeddedFieldStringInputFromOptions).Also applies to: 64-76
101-188: Selector UI conditionality and options handling look correct; consider dropping trivial useMemoThe new selector UI gating on
showFieldSelectorand consuming theoptionsprop viafieldOptions.map(...)is straightforward and keeps the input usable even when the selector is hidden.
fieldOptionsis just auseMemowrapper that returnsoptionsunchanged:const fieldOptions = React.useMemo(() => { return options; }, [options]);Since
optionsis already a prop and you’re not doing any expensive work here, you could simplify this to useoptionsdirectly without theuseMemo, which removes a bit of noise without changing behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/visual-editor/src/components/LocatorResultCard.tsx(26 hunks)packages/visual-editor/src/docs/components.md(1 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/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/visual-editor/src/docs/components.md
🧰 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/LocatorResultCard.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/components/LocatorResultCard.tsxpackages/visual-editor/src/editor/YextEntityFieldSelector.tsx
🧬 Code graph analysis (4)
packages/visual-editor/src/editor/TranslatableStringField.tsx (4)
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/EmbeddedFieldStringInput.tsx (2)
EmbeddedFieldStringInputFromOptions(64-191)EmbeddedFieldStringInputFromEntity(28-62)
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)
packages/visual-editor/src/components/LocatorResultCard.tsx (6)
packages/visual-editor/src/editor/DynamicOptionsSelector.tsx (2)
DynamicOptionsSingleSelectorType(27-31)DynamicOption(33-36)packages/visual-editor/src/types/types.ts (1)
TranslatableString(205-205)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/editor/TranslatableStringField.tsx (1)
TranslatableStringField(25-126)
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)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: call_unit_test / unit_tests (18.x)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: semgrep/ci
🔇 Additional comments (8)
packages/visual-editor/src/editor/TranslatableStringField.tsx (1)
22-24: Documentation properly updated.The JSDoc now correctly documents the new parameters, addressing the previous review feedback.
packages/visual-editor/src/editor/index.ts (1)
23-26: Barrel exports correctly updated for embedded inputs and dynamic single-selectorsThe editor barrel now exposes
EmbeddedFieldStringInputFromEntity/EmbeddedFieldStringInputFromOptionsand the newDynamicOptionsSingleSelector/DynamicOptionsSingleSelectorType, which lines up with the refactors in the rest of the PR. No issues from an API-surface perspective.Also applies to: 29-35
packages/visual-editor/src/editor/EmbeddedFieldStringInput.tsx (1)
193-247: useOptionValue flag in CommandItemWithResolvedValue behaves as intendedThe
useOptionValueflag cleanly skipsresolveComponentDatawork when you just want to display the rawoption.value(e.g., for locator display-field IDs), and the render condition:{(resolvedValue || useOptionValue) && ( <div className="ve-text-xs ve-text-gray-500 ..."> {useOptionValue ? option.value : resolvedValue} </div> )}correctly shows either the resolved value or the raw option value as appropriate. The updated effect dependency list including
useOptionValueensures you won’t accidentally resolve values when you’ve opted into the cheaper code path.packages/visual-editor/src/components/LocatorResultCard.tsx (5)
18-31: Dynamic single-select field typing and defaults align with locator display-field usageSwitching the various card field props (
primaryHeading.field,secondaryHeading.field,tertiaryHeading.field,hours.field,phone.field,email.field,services.field,image.field) toDynamicOptionsSingleSelectorType<string>and updating the defaults to{ selection: { value: "<field>" } }matches the new dynamic single-select infrastructure and keeps defaults consistent with the prior hard-coded fields.Using
TranslatableStringforsecondaryCTA.label/secondaryCTA.linkalso fits naturally with the embedded-field resolution later viaresolveComponentData, so editors can embed locator display fields directly into those strings. No issues here.Also applies to: 59-178, 180-237
239-257: getDisplayFieldOptions and dynamicSingleSelect configs are consistent and defensive
getDisplayFieldOptionspulls fromtemplateMetadata.locatorDisplayFields, filters byfield_type_id(supporting a single string or a small set viastring | string[]), and returnsDynamicOption<string>objects. Returning an empty array whenlocatorDisplayFieldsis missing is a good safe default and prevents runtime errors if metadata isn’t wired yet.Each of the
YextFieldconfigurations forfield(headings, hours, phone, email, services, image) correctly:
- Uses
type: "dynamicSingleSelect",- Points
dropdownLabelto the localized “Field” label, and- Supplies
getOptions: () => getDisplayFieldOptions("<appropriate type>")with the expectedfield_type_id.The
TranslatableStringFieldwiring forsecondaryCTA.labeland.linkusinggetDisplayFieldOptions("type.string")should give editors the same display-field-driven experience for those strings as for the headings, which is exactly what the PR is aiming for.Also applies to: 263-372, 399-571
819-885: ImageSection and HeadingTextSection refactor improves clarity and fixes earlier edge casesPulling the image and heading logic into
ImageSectionandHeadingTextSectionmakes the card layout easier to follow and consolidates field resolution concerns:
ImageSectionnow respectsimage.liveVisibilityand only renders when both a record is present and the block is visible, while still falling back tolocation.namefor alt text and providing safe default dimensions.HeadingTextSectionusesparseStringFromLocationwith optional chaining onfield?.selection?.value, and the primary heading falls back tolocation.namewhen the selected field is unset or missing.This also benefits from the new
parseStringFromLocationbehavior (no moreString(undefined)), so you won’t see the literal"undefined"leak into headings.
887-1030: Hours/Phone/Email/Services sections correctly use selected display fields and are defensively gatedThe new sections make the per-block behavior explicit and robust:
HoursSectionnow bases rendering solely on the selected hours field (hoursProps.field?.selection?.value) andhoursProps.liveVisibility, usingparseHoursFromLocation. This avoids the previous hard-codedlocation.hoursshort-circuit and properly supports alternate hours display fields.PhoneSectionresolves the selected phone field to a string viaparseStringFromLocationand requires both a configured field andphone.liveVisibilitybefore renderingPhoneAtom. UsingeventName={phone${index}}and the configuredphoneFormat/includePhoneHyperlinkpreserves analytics and UX behavior.EmailSectionandServicesSectionboth useparseArrayFromLocation, and then explicitly check for non-empty arrays and string element types before rendering. That gives a nice guardrail against mis-typed data and prevents rendering junk values.Overall, these sections are a clear improvement in readability and should behave well even with partially configured or evolving locator display-field data.
1032-1094: Centralized parse helpers and resolveProjectedField are simple and reasonably safeThe helper functions:
parseStringFromLocation— only stringifies non-null/undefined values, avoiding the previous"undefined"bug.parseArrayFromLocation— returns an array only whenArray.isArray(fieldValue)is true.parseRecordFromLocation/parseHoursFromLocation— thin wrappers aroundresolveProjectedField, tailored to the expected downstream usage.resolveProjectedField— walks a"foo.bar.baz"path with a simplehasOwnPropertyguard and returnsundefinedif any hop is missing.Given that
Locationhas an index signature and you know the shape of search results here, this level of validation is appropriate and keeps the helpers lightweight. If you ever find yourself needing stricter type guarantees (e.g., validating that the “image” field really is an object with aurl), you can easily extend these helpers with additionaltypeof/shape checks, but nothing in the current usage requires it.
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](https://yext.atlassian.net/browse/PB-26919) --------- Co-authored-by: Kyle Gerner <kgerner@yext.com> Co-authored-by: Kyle Gerner <49618240+k-gerner@users.noreply.github.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>
5 participants
Updates the locator settings panel so that the result card fields are read from the locator search experience display fields.
J=WAT-5198
Primary new functionality:
Screen.Recording.2025-11-20.at.3.04.46.PM.mov
Secondary CTA new behavior:
Screen.Recording.2025-11-21.at.9.44.37.AM.mov