feat: add support for linked entity list fields#1178
Open
jwartofsky-yext wants to merge 28 commits intomainfrom
Open
feat: add support for linked entity list fields#1178jwartofsky-yext wants to merge 28 commits intomainfrom
jwartofsky-yext wants to merge 28 commits intomainfrom
Conversation
Contributor
|
Warning: Component files have been updated but no migrations have been added. See https://github.com/yext/visual-editor/blob/main/packages/visual-editor/src/components/migrations/README.md for more information. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds a list-source editor field and runtime helpers: Sequence Diagram(s)sequenceDiagram
participant User
participant Editor as ListSourceField (UI)
participant Utils as listSourceFieldUtils
participant Stream as StreamDocument
participant Component as Section Component
User->>Editor: select source (constant or linked list) and item mappings
Editor->>Utils: getListSourceSelectorOptions()
Utils-->>Editor: selector options (legacy + linked list fields)
Editor->>Editor: compute resolveDefaultListItemMappings()
User->>Editor: confirm selection
Component->>Stream: request streamDocument at runtime
Stream-->>Component: streamDocument
Component->>Utils: resolveListSectionItems / resolveMappedListItems(streamDocument, sourceField, mappings)
Utils-->>Component: resolved item list + required length
Component->>Utils: buildListSectionCards(cards, items, requiredLength)
Utils-->>Component: updated card array (decorated with index/parentData)
Component->>Component: render/update cards
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ 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 |
commit: |
auto-screenshot-update: true
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
packages/visual-editor/src/editor/YextEntityFieldSelector.test.tsx (1)
169-194: Consider restoring a negative assertion for non-list direct fields.The rewritten test only asserts that the linked list field shows up. With
includeListsOnly: true, the direct fields indefaultEntityFields(name,description,photo— all non-list) should still be filtered out. Adding a negative check protects against regressions in the list-only filter for direct entity fields.🧪 Proposed addition
expect(screen.getByText("Linked Location > Emails")).toBeDefined(); + // includeListsOnly should still exclude non-list direct entity fields. + expect(screen.queryByText("Name")).toBeNull(); + expect(screen.queryByText("Description")).toBeNull(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/editor/YextEntityFieldSelector.test.tsx` around lines 169 - 194, The test currently only asserts the linked list field appears when renderEntityFieldInput is called with filter.includeListsOnly: true; add a negative assertion to ensure non-list direct fields from defaultEntityFields (e.g. "name", "description", "photo") are filtered out — use screen.queryByText("name"/"description"/"photo") or similar after opening the combobox and assert those return null/are not found so the includeListsOnly behavior in renderEntityFieldInput is protected against regressions.packages/visual-editor/src/editor/yextEntityFieldUtils.ts (1)
31-54: LGTM — linked field filtering is now consistent with direct field filtering.Using
filterto filterlinkedEntityStreamFields(instead of suppressing them whenincludeListsOnlyis set) correctly surfaces linked list fields, and tightening the fallback to require both field arrays to be empty avoids clobbering valid linked matches.Minor optional nit: the
directChildrenOffallback recomputeslinkedEntityFieldsusingallowList: [filter.directChildrenOf]. For linked stream field names (e.g.,c_linkedLocation.emails), thisallowListwill only match ifdirectChildrenOfis itself the linked reference path; otherwise this recomputation will always produce[]. Not a bug today, just a small amount of wasted work you could skip if you knowdirectChildrenOfis never a linked path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/editor/yextEntityFieldUtils.ts` around lines 31 - 54, The current fallback recomputes linkedEntityFields with allowList: [filter.directChildrenOf] even when directChildrenOf is a linked stream path (e.g., "c_linkedLocation.emails"), which will never match and wastes work; update the fallback inside the if-block that builds fallbackFilter to only recompute linkedEntityFields when linkedEntityStreamFields is present AND filter.directChildrenOf is not a linked path (e.g., contains a dot) or when it exactly matches the linked reference path; use the existing symbols linkedEntityFields, linkedEntityStreamFields, filter.directChildrenOf and fallbackFilter to add this guard so you skip the recomputation when directChildrenOf is clearly a linked stream field.packages/visual-editor/src/utils/listSourceFieldUtils.ts (1)
202-218: Relative-path extraction handles two distinct naming shapes — add a brief comment.The
startsWithbranch accommodates direct-stream children (whosefield.nameis already the full dotted path likec_faqSection.items) while theelsebranch covers linked-entity schema children (whosefield.nameis already the bare relative name likequestion). Future maintainers will have a hard time reasoning about this without a one-line note.📝 Proposed comment
).map((field) => ({ label: field.displayName ?? field.name, + // Stream children are returned prefixed with the source path; linked-entity + // schema children are returned already relative. Normalize to relative. value: field.name.startsWith(`${sourceField}.`) ? field.name.slice(sourceField.length + 1) : field.name, }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/utils/listSourceFieldUtils.ts` around lines 202 - 218, The relative-path extraction in the mappingConfigs.forEach loop is handling two different naming shapes but lacks explanation; add a one-line comment above the value calculation inside mappingConfigs.forEach explaining that getFieldsForSelector can return either full dotted paths for directChildren (e.g. "c_faqSection.items") or bare relative names for linked-entity schema children (e.g. "question"), and that the conditional on field.name.startsWith(`${sourceField}.`) strips the prefix for the former while leaving the latter intact; reference mappingConfigs.forEach, getFieldsForSelector, sourceField, field.name and mappingOptions in the comment to make intent explicit for future readers.packages/visual-editor/src/editor/ListSourceField.tsx (2)
193-199: Redundant default onconstantValueEnabled.
normalizedValue.constantValueEnabledis already defaulted totruein theuseMemoat line 93, so the extra?? truehere is dead code.🧹 Proposed cleanup
- constantValueEnabled={normalizedValue.constantValueEnabled ?? true} + constantValueEnabled={normalizedValue.constantValueEnabled}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/editor/ListSourceField.tsx` around lines 193 - 199, Remove the redundant fallback on constantValueEnabled when rendering ConstantValueModeToggler: use normalizedValue.constantValueEnabled directly (it is already defaulted to true where normalizedValue is computed in the useMemo that builds normalizedValue) and delete the `?? true` to avoid dead code; keep the prop name and other props (ConstantValueModeToggler, normalizedValue.constantValueEnabled, toggleConstantValueEnabled, fieldTypeFilter, label) unchanged.
42-42: UseCustomField<ListSourceFieldValue>inRenderPropstype for improved type safety.The
RenderPropstype currently extracts fromCustomField<any>, which causesvalueandonChangeparameters inListSourceFieldInputto lose their concrete type. SincecreateListSourceFieldcorrectly returnsCustomField<ListSourceFieldValue>, threading that type through toRenderPropsensures type consistency across the render function and its parameters.♻️ Proposed refactor
-type RenderProps = Parameters<CustomField<any>["render"]>[0]; +type RenderProps = Parameters<CustomField<ListSourceFieldValue>["render"]>[0];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/editor/ListSourceField.tsx` at line 42, The RenderProps type is currently extracted from CustomField<any>, losing concrete types for value/onChange; change its definition to use CustomField<ListSourceFieldValue> so RenderProps = Parameters<CustomField<ListSourceFieldValue>["render"]>[0], then ensure ListSourceFieldInput and its usage (including createListSourceField) accept the narrowed RenderProps so value and onChange carry the ListSourceFieldValue type throughout.packages/visual-editor/src/utils/listSourceFieldUtils.test.ts (2)
66-149: Consider broader coverage for the new utilities.The suite covers only the linked-schemas happy paths. Given this is a new core utility used by FAQ and Product sections, a few more cases would harden it:
resolveDefaultListItemMappings(preserve existing, preferred-name match, single-option fallback)getListSourceSelectorOptionswith legacy-only and list-only entityFields (covers thesourceOptionGroups === undefinedbranch)resolveMappedListItemsnegative paths: missingitemFieldMappings, non-array source, items that are non-objects / null, and missingfieldPathfor a keyhasRequiredMappingsexclusion: list source missing a required mapping (e.g., notype.stringfield) should not appear inlistSourceFieldNames🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/utils/listSourceFieldUtils.test.ts` around lines 66 - 149, Add unit tests to cover edge cases for the new utilities: for resolveDefaultListItemMappings add tests for (a) preserving an existing mapping, (b) preferring a mapping that matches the preferred name, and (c) falling back to a single available option; for getListSourceSelectorOptions add tests where entityFields contains only legacy fields and only linked-list fields to hit the sourceOptionGroups === undefined branch and ensure listSourceFieldNames is correct; for resolveMappedListItems add negative-path tests for missing itemFieldMappings, when the resolved source at fieldPath is not an array, when array items are non-objects/null, and when a mapping key has no fieldPath to ensure the mapper skips or throws as expected; and for hasRequiredMappings add a test where a list source lacks a required type (e.g., no type.string) so it is excluded from listSourceFieldNames. Use the existing test helpers and reference the functions resolveDefaultListItemMappings, getListSourceSelectorOptions, resolveMappedListItems, hasRequiredMappings, listSourceFieldNames, sourceOptionGroups, and itemFieldMappings to locate code under test.
88-110: Test's assumption about sort order is backed by code—no flakiness risk.The test correctly assumes alphabetical ordering of mapping options by
displayName. ThegetFieldsForSelectorfunction explicitly sorts results at line 56 ofyextEntityFieldUtils.tsusing a case-insensitive lexicographic comparator ondisplayName(with fallback toname), guaranteeing ascending order. The test will only fail if the sort logic changes, which would be an intentional code modification, not a flaky dependency.If preferred for better test isolation, you could optionally use
expect.arrayContaining([...])or assertions on array length and individual membership to decouple from ordering. However, this is not necessary to prevent flakiness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/utils/listSourceFieldUtils.test.ts` around lines 88 - 110, The test currently asserts exact ordering for mappingOptionsBySourceField["c_linkedLocation.linkedFAQs"] which relies on getFieldsForSelector's sorting behavior; to make the test less brittle change the assertions in packages/visual-editor/src/utils/listSourceFieldUtils.test.ts to check membership rather than exact order (e.g., use expect.arrayContaining for the question and answer arrays or assert array length plus presence of each expected option) while still referencing the same keys so the relationship between the test and getFieldsForSelector in yextEntityFieldUtils.ts remains covered.
🤖 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/FAQsSection/FAQsSection.tsx`:
- Around line 216-235: The resolvedFAQs fallback never runs because
resolveMappedListItems can return an array of objects with all fields undefined
(truthy); update the logic so that after calling resolveMappedListItems in
FAQsSection (and similarly in ProductCardsWrapper), you detect when the returned
array exists but every item has all-mapped fields undefined and treat that as
undefined to allow the resolveYextEntityField fallback to run; implement this by
checking the array returned by resolveMappedListItems<FAQStruct> (reference
resolvedFAQs / resolveMappedListItems) and set resolvedFAQs = undefined if every
item’s mapped properties (question/answer) are strictly === undefined, or
alternatively change resolveMappedListItems to return undefined in that
all-empty-items case so the existing coalescing works.
In
`@packages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.tsx`:
- Around line 132-138: The slots schema for ProductCardsWrapper defines CardSlot
without an explicit allow array which is inconsistent with the runtime usage and
other components; update the slots.objectFields.CardSlot definition in
ProductCardsWrapper (the CardSlot slot schema) to include an explicit allow: []
property so the schema reflects the one-card-per-slot contract used at runtime
and matches peer implementations.
- Around line 201-214: The mapping for the product name in ProductCardsWrapper
(the object returned from the resolvedItemFields arrow function) incorrectly
narrows name to strings and returns undefined for LocalizedValues; replace the
typeof === "string" guard with the same cast pattern used by other fields (e.g.,
price/description/category) so name is assigned as ProductStruct["name"] (a
TranslatableString) by casting resolvedItemFields.name, preserving localized
values.
In `@packages/visual-editor/src/editor/ListSourceField.tsx`:
- Around line 164-173: The first option group added in ListSourceField (when
selectorOptions.sourceOptionGroups exists) creates an untitled group by
prepending { options: [defaultOption] } to optionGroups; update that logic in
ListSourceField.tsx so the default option is either placed as a standalone
non-group option (merge into the first existing group) or the prepended group
includes a neutral title (e.g., title: "General") to avoid an untitled heading;
modify the branch that builds optionGroups (using selectorOptions,
defaultOption, and optionGroups) and ensure BasicSelectorField receives the
adjusted structure so the default option displays without an empty heading or
layout glitch.
In `@packages/visual-editor/src/utils/listSourceFieldUtils.ts`:
- Around line 95-102: The group titles in sourceOptionGroups are hardcoded
("Section Fields", "List Fields"); replace these with localized strings using
the project's translation helper (e.g., pt or msg) so they go through the
translation pipeline; update the creation of sourceOptionGroups to call
pt("sectionFields") and pt("listFields") (or the equivalent msg(...) keys used
elsewhere) while leaving the options payload (legacyOptions, listOptions)
unchanged so the UI shows translated titles for non-English locales.
---
Nitpick comments:
In `@packages/visual-editor/src/editor/ListSourceField.tsx`:
- Around line 193-199: Remove the redundant fallback on constantValueEnabled
when rendering ConstantValueModeToggler: use
normalizedValue.constantValueEnabled directly (it is already defaulted to true
where normalizedValue is computed in the useMemo that builds normalizedValue)
and delete the `?? true` to avoid dead code; keep the prop name and other props
(ConstantValueModeToggler, normalizedValue.constantValueEnabled,
toggleConstantValueEnabled, fieldTypeFilter, label) unchanged.
- Line 42: The RenderProps type is currently extracted from CustomField<any>,
losing concrete types for value/onChange; change its definition to use
CustomField<ListSourceFieldValue> so RenderProps =
Parameters<CustomField<ListSourceFieldValue>["render"]>[0], then ensure
ListSourceFieldInput and its usage (including createListSourceField) accept the
narrowed RenderProps so value and onChange carry the ListSourceFieldValue type
throughout.
In `@packages/visual-editor/src/editor/YextEntityFieldSelector.test.tsx`:
- Around line 169-194: The test currently only asserts the linked list field
appears when renderEntityFieldInput is called with filter.includeListsOnly:
true; add a negative assertion to ensure non-list direct fields from
defaultEntityFields (e.g. "name", "description", "photo") are filtered out — use
screen.queryByText("name"/"description"/"photo") or similar after opening the
combobox and assert those return null/are not found so the includeListsOnly
behavior in renderEntityFieldInput is protected against regressions.
In `@packages/visual-editor/src/editor/yextEntityFieldUtils.ts`:
- Around line 31-54: The current fallback recomputes linkedEntityFields with
allowList: [filter.directChildrenOf] even when directChildrenOf is a linked
stream path (e.g., "c_linkedLocation.emails"), which will never match and wastes
work; update the fallback inside the if-block that builds fallbackFilter to only
recompute linkedEntityFields when linkedEntityStreamFields is present AND
filter.directChildrenOf is not a linked path (e.g., contains a dot) or when it
exactly matches the linked reference path; use the existing symbols
linkedEntityFields, linkedEntityStreamFields, filter.directChildrenOf and
fallbackFilter to add this guard so you skip the recomputation when
directChildrenOf is clearly a linked stream field.
In `@packages/visual-editor/src/utils/listSourceFieldUtils.test.ts`:
- Around line 66-149: Add unit tests to cover edge cases for the new utilities:
for resolveDefaultListItemMappings add tests for (a) preserving an existing
mapping, (b) preferring a mapping that matches the preferred name, and (c)
falling back to a single available option; for getListSourceSelectorOptions add
tests where entityFields contains only legacy fields and only linked-list fields
to hit the sourceOptionGroups === undefined branch and ensure
listSourceFieldNames is correct; for resolveMappedListItems add negative-path
tests for missing itemFieldMappings, when the resolved source at fieldPath is
not an array, when array items are non-objects/null, and when a mapping key has
no fieldPath to ensure the mapper skips or throws as expected; and for
hasRequiredMappings add a test where a list source lacks a required type (e.g.,
no type.string) so it is excluded from listSourceFieldNames. Use the existing
test helpers and reference the functions resolveDefaultListItemMappings,
getListSourceSelectorOptions, resolveMappedListItems, hasRequiredMappings,
listSourceFieldNames, sourceOptionGroups, and itemFieldMappings to locate code
under test.
- Around line 88-110: The test currently asserts exact ordering for
mappingOptionsBySourceField["c_linkedLocation.linkedFAQs"] which relies on
getFieldsForSelector's sorting behavior; to make the test less brittle change
the assertions in packages/visual-editor/src/utils/listSourceFieldUtils.test.ts
to check membership rather than exact order (e.g., use expect.arrayContaining
for the question and answer arrays or assert array length plus presence of each
expected option) while still referencing the same keys so the relationship
between the test and getFieldsForSelector in yextEntityFieldUtils.ts remains
covered.
In `@packages/visual-editor/src/utils/listSourceFieldUtils.ts`:
- Around line 202-218: The relative-path extraction in the
mappingConfigs.forEach loop is handling two different naming shapes but lacks
explanation; add a one-line comment above the value calculation inside
mappingConfigs.forEach explaining that getFieldsForSelector can return either
full dotted paths for directChildren (e.g. "c_faqSection.items") or bare
relative names for linked-entity schema children (e.g. "question"), and that the
conditional on field.name.startsWith(`${sourceField}.`) strips the prefix for
the former while leaving the latter intact; reference mappingConfigs.forEach,
getFieldsForSelector, sourceField, field.name and mappingOptions in the comment
to make intent explicit for future readers.
🪄 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: b35b5ee6-6fec-4bf0-85b3-28b8d8b972a7
📒 Files selected for processing (8)
packages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsxpackages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.tsxpackages/visual-editor/src/editor/ListSourceField.tsxpackages/visual-editor/src/editor/YextEntityFieldSelector.test.tsxpackages/visual-editor/src/editor/YextEntityFieldSelector.tsxpackages/visual-editor/src/editor/yextEntityFieldUtils.tspackages/visual-editor/src/utils/listSourceFieldUtils.test.tspackages/visual-editor/src/utils/listSourceFieldUtils.ts
auto-screenshot-update: true
auto-screenshot-update: true
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/visual-editor/src/utils/linkedEntityWarningUtils.ts (1)
16-29: Optional: short-circuit on the dedup cache before callingresolveField.Two small clean-ups you could consider:
resolveFieldruns on every invocation, even after we've already warned for this(streamDocument, fieldPath). Since this helper is invoked from auseEffectin bothYextEntityFieldSelectorandListSourceFieldand re-runs on dependency changes, checking the cache first avoids repeated tree walks.- When
warnedFieldsalready exists,warnedFields.add(fieldPath)mutates the sameSetthat's already stored in theWeakMap, so the subsequentwarnedFieldsByDocument.set(...)call is redundant.♻️ Proposed refactor
export const warnOnMultiValueLinkedEntityTraversal = ( streamDocument: StreamDocument, fieldPath: string ): void => { + const warnedFields = warnedFieldsByDocument.get(streamDocument); + if (warnedFields?.has(fieldPath)) { + return; + } + const resolution = resolveField(streamDocument, fieldPath); if (!resolution.traversedMultiValueReference) { return; } - const warnedFields = warnedFieldsByDocument.get(streamDocument); - if (warnedFields?.has(fieldPath)) { - return; + if (warnedFields) { + warnedFields.add(fieldPath); + } else { + warnedFieldsByDocument.set(streamDocument, new Set([fieldPath])); } - - warnedFieldsByDocument.set( - streamDocument, - warnedFields ? warnedFields.add(fieldPath) : new Set([fieldPath]) - ); toast.warning(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/utils/linkedEntityWarningUtils.ts` around lines 16 - 29, Short-circuit the dedup cache before calling resolveField: first retrieve warnedFields = warnedFieldsByDocument.get(streamDocument) and if warnedFields?.has(fieldPath) return immediately to avoid the expensive resolveField tree walk; only call resolveField if not already warned and, when adding the path, avoid redundant WeakMap.set by mutating the existing Set in-place (warnedFields.add(fieldPath) and return) or creating and setting a new Set only when warnedFields is undefined (warnedFieldsByDocument.set(streamDocument, new Set([fieldPath]))).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/visual-editor/src/utils/linkedEntityWarningUtils.ts`:
- Around line 16-29: Short-circuit the dedup cache before calling resolveField:
first retrieve warnedFields = warnedFieldsByDocument.get(streamDocument) and if
warnedFields?.has(fieldPath) return immediately to avoid the expensive
resolveField tree walk; only call resolveField if not already warned and, when
adding the path, avoid redundant WeakMap.set by mutating the existing Set
in-place (warnedFields.add(fieldPath) and return) or creating and setting a new
Set only when warnedFields is undefined
(warnedFieldsByDocument.set(streamDocument, new Set([fieldPath]))).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 90f8dd4f-a5d1-49f3-9a86-a3d1113ff622
📒 Files selected for processing (4)
packages/visual-editor/src/editor/ListSourceField.tsxpackages/visual-editor/src/editor/YextEntityFieldSelector.test.tsxpackages/visual-editor/src/editor/YextEntityFieldSelector.tsxpackages/visual-editor/src/utils/linkedEntityWarningUtils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/visual-editor/src/editor/YextEntityFieldSelector.test.tsx
- packages/visual-editor/src/editor/ListSourceField.tsx
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsx (1)
217-263:⚠️ Potential issue | 🟠 MajorBlock stale mapped FAQ results before rendering.
If the mapped FAQ path resolves to empty objects, this branch still treats them as valid and renders blank cards instead of falling back to the legacy entity shape. This is the same stale-mapping edge case as before; reject all-empty mapped results before calling
buildListSectionCards.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsx` around lines 217 - 263, The mapped FAQ branch currently accepts resolvedFAQs even when every item is an empty object; add a guard after obtaining resolvedFAQs/requiredLength to detect and reject all-empty mapped results (e.g., check resolvedFAQs.every(f => !f || Object.values(f).every(v => v == null || v === ""))). If that guard is true, fall back the same way as when requiredLength/resolvedFAQs are missing (return setDeep(data, "props.slots.CardSlot", [])) instead of calling buildListSectionCards; place the check immediately before the call to buildListSectionCards in the block that uses resolveListSectionItems and reference resolvedFAQs, requiredLength, and buildListSectionCards when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@packages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsx`:
- Around line 217-263: The mapped FAQ branch currently accepts resolvedFAQs even
when every item is an empty object; add a guard after obtaining
resolvedFAQs/requiredLength to detect and reject all-empty mapped results (e.g.,
check resolvedFAQs.every(f => !f || Object.values(f).every(v => v == null || v
=== ""))). If that guard is true, fall back the same way as when
requiredLength/resolvedFAQs are missing (return setDeep(data,
"props.slots.CardSlot", [])) instead of calling buildListSectionCards; place the
check immediately before the call to buildListSectionCards in the block that
uses resolveListSectionItems and reference resolvedFAQs, requiredLength, and
buildListSectionCards when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 216e9c1d-2a83-4209-8f24-975f2d0a3aee
📒 Files selected for processing (6)
packages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsxpackages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.test.tsxpackages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.tsxpackages/visual-editor/src/utils/cardSlots/listSectionData.test.tspackages/visual-editor/src/utils/cardSlots/listSectionData.tspackages/visual-editor/src/utils/listSourceFieldUtils.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.tsx
auto-screenshot-update: true
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.tsx (1)
205-220:⚠️ Potential issue | 🟠 MajorDon't narrow mapped
nameto plain strings.
ProductStruct["name"]is translatable, so this guard drops localized-object values andisValidItemthen filters out otherwise valid mapped products.Proposed fix
resolveListSectionItems<ProductStruct>({ buildMappedItem: (resolvedItemFields) => ({ image: resolvedItemFields.image as ProductStruct["image"], brow: resolvedItemFields.brow as ProductStruct["brow"], - name: - typeof resolvedItemFields.name === "string" - ? resolvedItemFields.name - : undefined, + name: resolvedItemFields.name as ProductStruct["name"], price: resolvedItemFields.price as ProductStruct["price"], description: resolvedItemFields.description as ProductStruct["description"],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.tsx` around lines 205 - 220, The mapped name is being narrowed to a plain string which drops localized (translatable) objects; in buildMappedItem (resolvedItemFields.name) assign the value directly as ProductStruct["name"] instead of forcing a string, and update isValidItem (product.name) to accept localized-name shapes (e.g., check for non-null/undefined or presence of a text value rather than typeof === "string") so localized objects are not filtered out.packages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsx (1)
219-236:⚠️ Potential issue | 🟡 MinorTreat all-empty mapped FAQs as unresolved.
If the list itself resolves but every mapped path is stale,
resolveMappedListItemsstill produces a truthy array of empty FAQ objects. That preventsresolveLegacyItems()from running here, so this branch builds blank FAQ cards instead of falling back to the legacyfaqsfield. Please collapse the “all mapped fields undefined” case toundefinedbefore usingresolvedFAQs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsx` around lines 219 - 236, The mapped FAQ array returned by resolveListSectionItems (resolvedFAQs) can be a truthy array of empty objects which blocks the resolveLegacyItems fallback; after calling resolveListSectionItems in the FAQsSection component, detect when resolvedFAQs is an array where every item has both question and answer undefined/empty and collapse it to undefined so the legacy fallback runs; locate the const { items: resolvedFAQs, requiredLength } = resolveListSectionItems<FAQStruct> call and insert a check (treating empty string or undefined as empty) to set resolvedFAQs to undefined when all mapped items are empty before using it to render FAQ cards.
🤖 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/InsightSection/InsightCardsWrapper.tsx`:
- Around line 168-171: The early return on params.metadata.streamDocument is too
broad and prevents the constant-value path from rebuilding props.slots.CardSlot;
move the streamDocument guard so it only applies inside the linked-source branch
(the code path that reads params.metadata.streamDocument) rather than before
both branches, e.g., keep the constant-value / static-cards branch reachable
when params.metadata.streamDocument is falsy and only return or bail out inside
the linked-source handling where streamDocument is actually required.
In
`@packages/visual-editor/src/components/pageSections/TeamSection/TeamCardsWrapper.tsx`:
- Around line 164-167: The current early return in resolveData
(TeamCardsWrapper.tsx) short-circuits the entire constant-value flow when
params.metadata.streamDocument is falsy; remove or move that guard so only the
linked-entity resolution path checks streamDocument. Specifically, in
resolveData, keep the normal constant/manual card processing (so CardSlot sync
still runs) and wrap only the linked-entity lookup logic in a conditional that
requires streamDocument (e.g., if (streamDocument) { /* resolve linked entities
using streamDocument */ }); ensure functions/blocks referencing streamDocument
(the linked-entity resolver) handle its absence gracefully rather than returning
the whole data early.
In
`@packages/visual-editor/src/components/pageSections/TestimonialSection/TestimonialCardsWrapper.tsx`:
- Around line 139-142: The early return in TestimonialCardsWrapper that checks
params.metadata.streamDocument currently prevents the constant-value branch from
rebuilding CardSlot from data.constantValue; instead, remove the blanket return
and only guard the linked-field resolution logic that depends on streamDocument
(i.e., keep reading data.constantValue and constructing CardSlot regardless),
wrapping only the code that accesses streamDocument in a conditional (check
streamDocument before performing linked-field resolution/update), so CardSlot
creation from data.constantValue still runs when params.metadata.streamDocument
is missing.
---
Duplicate comments:
In
`@packages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsx`:
- Around line 219-236: The mapped FAQ array returned by resolveListSectionItems
(resolvedFAQs) can be a truthy array of empty objects which blocks the
resolveLegacyItems fallback; after calling resolveListSectionItems in the
FAQsSection component, detect when resolvedFAQs is an array where every item has
both question and answer undefined/empty and collapse it to undefined so the
legacy fallback runs; locate the const { items: resolvedFAQs, requiredLength } =
resolveListSectionItems<FAQStruct> call and insert a check (treating empty
string or undefined as empty) to set resolvedFAQs to undefined when all mapped
items are empty before using it to render FAQ cards.
In
`@packages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.tsx`:
- Around line 205-220: The mapped name is being narrowed to a plain string which
drops localized (translatable) objects; in buildMappedItem
(resolvedItemFields.name) assign the value directly as ProductStruct["name"]
instead of forcing a string, and update isValidItem (product.name) to accept
localized-name shapes (e.g., check for non-null/undefined or presence of a text
value rather than typeof === "string") so localized objects are not filtered
out.
🪄 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: 3a8a5217-f6ff-4ddb-9224-637ba3722898
⛔ Files ignored due to path filters (2)
packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] latest version default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] latest version multi-pageset default props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (11)
packages/visual-editor/src/components/pageSections/EventSection/EventCardsWrapper.test.tsxpackages/visual-editor/src/components/pageSections/EventSection/EventCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsxpackages/visual-editor/src/components/pageSections/InsightSection/InsightCardsWrapper.test.tsxpackages/visual-editor/src/components/pageSections/InsightSection/InsightCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/TeamSection/TeamCardsWrapper.test.tsxpackages/visual-editor/src/components/pageSections/TeamSection/TeamCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/TestimonialSection/TestimonialCardsWrapper.test.tsxpackages/visual-editor/src/components/pageSections/TestimonialSection/TestimonialCardsWrapper.tsxpackages/visual-editor/src/editor/ListSourceField.tsx
auto-screenshot-update: true
jwartofsky-yext
changed the title
jwartofsky-yext
requested review from
asanehisa,
benlife5,
briantstephan and
mkilpatrick
asanehisa
reviewed
Apr 28, 2026
benlife5
reviewed
Apr 28, 2026
Contributor
benlife5
left a comment
benlife5
left a comment
There was a problem hiding this comment.
Generally LGTM
To confirm, this functionality is the ability to render a built-in VE field type from a linked entity, so EntityFieldSelector current entity > c_linkedEntities > c_events => renders one event card per event in the first linked location's c_events field?
Which is different functionality than being able to render a list of linked entities:
current entity > c_linkedProducts => renders one product card per linked product entity?
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.test.tsx (1)
28-80: Add a static-value regression test for slot/id synchronization.Given the refactor in static rebuild logic, one test around duplicate/missing ids in
constantValuewould harden this path against regressions.Suggested test shape
+it("rebuilds static cards and re-syncs constantValue ids", async () => { + // seed props with constantValue entries (including duplicate/missing id) + // call resolveData with constantValueEnabled: true + // assert CardSlot length, unique card ids, and data.constantValue id sync +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.test.tsx` around lines 28 - 80, Add a regression test to ProductCardsWrapper that verifies slot/id synchronization when constantValue is used: create a new it block that calls resolveData with constantValueEnabled: true and a constantValue payload containing items with missing or duplicate ids, then assert the returned result.props!.slots!.CardSlot has the expected number of cards and that each card's props.parentData includes a stable, unique id (or that missing ids are generated consistently) to prevent regressions from the static rebuild refactor; reference resolveData, ProductCardsWrapper, and constantValueEnabled/constantValue in the test.packages/visual-editor/src/components/pageSections/InsightSection/InsightCardsWrapper.tsx (1)
162-215: Consider extracting static card-slot rebuild into a shared helper.This block is now effectively duplicated across multiple wrappers with small behavioral drift. Pulling it into a common utility would reduce future regressions and keep id-collision/sync behavior consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/InsightSection/InsightCardsWrapper.tsx` around lines 162 - 215, The card-slot rebuild logic in InsightCardsWrapper (the block that creates inUseIds, iterates data.props.data.constantValue, finds existingCard from data.props.slots.CardSlot, deep-clones, generates newId via crypto.randomUUID, updates nested slot ids, uses defaultInsightCardSlotData, and calls setDeep to update props.slots.CardSlot and props.data.constantValue) is duplicated across wrappers; extract this into a shared utility (e.g., rebuildInsightCardSlots) that accepts the incoming component data, sharedCardProps, and index strategy and returns the newSlots array and updated constantValue entries, then replace the inline block with a call to that helper and reuse it from other wrappers to centralize id-collision handling and syncing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/visual-editor/src/components/pageSections/InsightSection/InsightCardsWrapper.tsx`:
- Around line 162-215: The card-slot rebuild logic in InsightCardsWrapper (the
block that creates inUseIds, iterates data.props.data.constantValue, finds
existingCard from data.props.slots.CardSlot, deep-clones, generates newId via
crypto.randomUUID, updates nested slot ids, uses defaultInsightCardSlotData, and
calls setDeep to update props.slots.CardSlot and props.data.constantValue) is
duplicated across wrappers; extract this into a shared utility (e.g.,
rebuildInsightCardSlots) that accepts the incoming component data,
sharedCardProps, and index strategy and returns the newSlots array and updated
constantValue entries, then replace the inline block with a call to that helper
and reuse it from other wrappers to centralize id-collision handling and syncing
behavior.
In
`@packages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.test.tsx`:
- Around line 28-80: Add a regression test to ProductCardsWrapper that verifies
slot/id synchronization when constantValue is used: create a new it block that
calls resolveData with constantValueEnabled: true and a constantValue payload
containing items with missing or duplicate ids, then assert the returned
result.props!.slots!.CardSlot has the expected number of cards and that each
card's props.parentData includes a stable, unique id (or that missing ids are
generated consistently) to prevent regressions from the static rebuild refactor;
reference resolveData, ProductCardsWrapper, and
constantValueEnabled/constantValue in the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 11d05cd5-b1bb-4aa2-9aac-5b91621d0dae
📒 Files selected for processing (14)
packages/visual-editor/src/components/pageSections/EventSection/EventCardsWrapper.test.tsxpackages/visual-editor/src/components/pageSections/EventSection/EventCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsxpackages/visual-editor/src/components/pageSections/InsightSection/InsightCardsWrapper.test.tsxpackages/visual-editor/src/components/pageSections/InsightSection/InsightCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.test.tsxpackages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/TeamSection/TeamCardsWrapper.test.tsxpackages/visual-editor/src/components/pageSections/TeamSection/TeamCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/TestimonialSection/TestimonialCardsWrapper.test.tsxpackages/visual-editor/src/components/pageSections/TestimonialSection/TestimonialCardsWrapper.tsxpackages/visual-editor/src/docs/components.mdpackages/visual-editor/src/utils/cardSlots/cardWrapperHelpers.tspackages/visual-editor/src/utils/cardSlots/listSectionData.ts
✅ Files skipped from review due to trivial changes (1)
- packages/visual-editor/src/components/pageSections/InsightSection/InsightCardsWrapper.test.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/visual-editor/src/utils/cardSlots/listSectionData.ts
- packages/visual-editor/src/components/pageSections/EventSection/EventCardsWrapper.test.tsx
- packages/visual-editor/src/components/pageSections/TeamSection/TeamCardsWrapper.test.tsx
- packages/visual-editor/src/components/pageSections/TestimonialSection/TestimonialCardsWrapper.test.tsx
- packages/visual-editor/src/components/pageSections/EventSection/EventCardsWrapper.tsx
- packages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsx
auto-screenshot-update: true
Contributor
Author
Yes, that's the new functionality You can see it in action here: 
Also feel free to play around with the dev site |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
packages/visual-editor/src/components/pageSections/TeamSection/TeamCardsWrapper.tsx (1)
117-120:⚠️ Potential issue | 🟠 MajorAvoid rebuilding from
constantValuewhen linked team data can’t be resolved.Here,
!constantValueEnabled && fieldplus a missingstreamDocumentfalls through to the static branch. Because the linked-team branch never updatesdata.props.data.constantValue, that fallback can wipe or stale outCardSlotinstead of preserving the current linked-card state.Also applies to: 159-218
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/TeamSection/TeamCardsWrapper.tsx` around lines 117 - 120, The conditional in TeamCardsWrapper that decides between the linked-team stream branch and the static constantValue branch is too loose and falls back to rebuilding from constantValue when linked data can't be resolved; update the guard used in the block around the existing if (streamDocument && !data.props.data.constantValueEnabled && data.props.data.field) (and the similar logic in the 159-218 region) so that you only take the static/constantValue branch when constantValueEnabled is true OR when data.props.data.constantValue is explicitly present (not null/undefined); otherwise preserve the linked-card state (CardSlot) and avoid overwriting it when streamDocument/link resolution fails. Include references to data.props.data.constantValue, constantValueEnabled, streamDocument, field, and CardSlot when making the change so reviewers can locate the logic.packages/visual-editor/src/components/pageSections/TestimonialSection/TestimonialCardsWrapper.tsx (1)
114-117:⚠️ Potential issue | 🟠 MajorPreserve linked testimonial cards when
streamDocumentis unavailable.This branch structure still falls back to the static rebuild when
constantValueEnabledisfalse,fieldis set, andstreamDocumentis missing. Since the linked-testimonial path above never re-syncsdata.props.data.constantValue, that fallback can rebuildCardSlotfrom stale ids or empty it entirely.Also applies to: 158-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/TestimonialSection/TestimonialCardsWrapper.tsx` around lines 114 - 117, The current conditional lets the component fall back to rebuilding static CardSlot when streamDocument is missing even though a linked-testimonial path exists; update the condition around streamDocument, data.props.data.constantValueEnabled, and data.props.data.field so that if the testimonial is linked (e.g., linked-testimonial path / data.props.data.constantValue indicates linked IDs) we do NOT perform the static rebuild when streamDocument is unavailable — instead keep the existing CardSlot state or render the linked cards from the last-known IDs; specifically, change the if that checks streamDocument && !data.props.data.constantValueEnabled && data.props.data.field (and the analogous logic in the 158-220 block) to first detect the linked-testimonial mode (the flag/shape that identifies linked testimonials) and only run the static rebuild when there is no linked testimonial and no streamDocument, preserving CardSlot contents when linked mode is active.packages/visual-editor/src/components/pageSections/InsightSection/InsightCardsWrapper.tsx (1)
118-121:⚠️ Potential issue | 🟠 MajorKeep linked-field mode out of the static rebuild when
streamDocumentis missing.If
constantValueEnabledisfalseanddata.props.data.fieldis set butstreamDocumentis absent, this now drops into the static branch below. That branch rebuilds fromdata.props.data.constantValue, but the linked-entity path above never re-syncs those ids, so metadata-light renders can show stale cards or clearCardSlotentirely. Returndatain that case instead of running the static rebuild.Also applies to: 162-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/InsightSection/InsightCardsWrapper.tsx` around lines 118 - 121, In InsightCardsWrapper, avoid running the static rebuild when constantValueEnabled is false and a linked field is present but streamDocument is missing: detect the case where streamDocument is falsy while data.props.data.field is set and data.props.data.constantValueEnabled === false, and simply return data early instead of falling through to the static rebuild logic that reads data.props.data.constantValue; apply the same early-return guard to the other occurrence of the same logic block (the branch referenced around lines 162-215) so linked-field mode never triggers the constantValue rebuild when streamDocument is absent.packages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsx (1)
193-196:⚠️ Potential issue | 🟠 MajorLinked FAQ mode should not fall back to the static-card rebuild.
When
constantValueEnabledisfalseandfieldis selected butstreamDocumentis missing, theelsebranch below rebuildsCardSlotfromdata.props.data.constantValue. The linked FAQ path above never syncs those ids, so this can replace the resolved FAQ cards with stale or empty static state instead of preserving the current data.Also applies to: 234-285
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsx` around lines 193 - 196, The current conditional lets the else branch rebuild CardSlot from data.props.data.constantValue when constantValueEnabled is false and data.props.data.field is set but streamDocument is missing, which can overwrite resolved linked FAQ cards with stale static data; update the logic in FAQsSection (the conditional around streamDocument / data.props.data.constantValueEnabled / data.props.data.field and the else that constructs CardSlot from constantValue) so that when a field is selected but streamDocument is absent you do NOT fall back to building from constantValue — instead preserve the existing resolved FAQ card state (e.g., skip the rebuild or return the already-resolved cards) and only use constantValue when no field is selected; ensure the same change is applied to the other affected block (lines ~234-285) that mirrors this behavior.
🤖 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/EventSection/EventCardsWrapper.tsx`:
- Around line 106-109: The current condition lets execution fall through to the
static-card branch when a selected linked field exists but streamDocument is
missing; change the logic in EventCardsWrapper so that if data.props.data.field
is set and data.props.data.constantValueEnabled === false but streamDocument is
falsy, you do not render the static cards — instead early-return (or render a
loading/placeholder) to preserve the linked state; update the same guard used
around the later block (the branch covering lines ~149-201) so both places check
for streamDocument presence before falling back to static rendering and do not
touch data.props.data.constantValue in the linked-event flow.
---
Duplicate comments:
In
`@packages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsx`:
- Around line 193-196: The current conditional lets the else branch rebuild
CardSlot from data.props.data.constantValue when constantValueEnabled is false
and data.props.data.field is set but streamDocument is missing, which can
overwrite resolved linked FAQ cards with stale static data; update the logic in
FAQsSection (the conditional around streamDocument /
data.props.data.constantValueEnabled / data.props.data.field and the else that
constructs CardSlot from constantValue) so that when a field is selected but
streamDocument is absent you do NOT fall back to building from constantValue —
instead preserve the existing resolved FAQ card state (e.g., skip the rebuild or
return the already-resolved cards) and only use constantValue when no field is
selected; ensure the same change is applied to the other affected block (lines
~234-285) that mirrors this behavior.
In
`@packages/visual-editor/src/components/pageSections/InsightSection/InsightCardsWrapper.tsx`:
- Around line 118-121: In InsightCardsWrapper, avoid running the static rebuild
when constantValueEnabled is false and a linked field is present but
streamDocument is missing: detect the case where streamDocument is falsy while
data.props.data.field is set and data.props.data.constantValueEnabled === false,
and simply return data early instead of falling through to the static rebuild
logic that reads data.props.data.constantValue; apply the same early-return
guard to the other occurrence of the same logic block (the branch referenced
around lines 162-215) so linked-field mode never triggers the constantValue
rebuild when streamDocument is absent.
In
`@packages/visual-editor/src/components/pageSections/TeamSection/TeamCardsWrapper.tsx`:
- Around line 117-120: The conditional in TeamCardsWrapper that decides between
the linked-team stream branch and the static constantValue branch is too loose
and falls back to rebuilding from constantValue when linked data can't be
resolved; update the guard used in the block around the existing if
(streamDocument && !data.props.data.constantValueEnabled &&
data.props.data.field) (and the similar logic in the 159-218 region) so that you
only take the static/constantValue branch when constantValueEnabled is true OR
when data.props.data.constantValue is explicitly present (not null/undefined);
otherwise preserve the linked-card state (CardSlot) and avoid overwriting it
when streamDocument/link resolution fails. Include references to
data.props.data.constantValue, constantValueEnabled, streamDocument, field, and
CardSlot when making the change so reviewers can locate the logic.
In
`@packages/visual-editor/src/components/pageSections/TestimonialSection/TestimonialCardsWrapper.tsx`:
- Around line 114-117: The current conditional lets the component fall back to
rebuilding static CardSlot when streamDocument is missing even though a
linked-testimonial path exists; update the condition around streamDocument,
data.props.data.constantValueEnabled, and data.props.data.field so that if the
testimonial is linked (e.g., linked-testimonial path /
data.props.data.constantValue indicates linked IDs) we do NOT perform the static
rebuild when streamDocument is unavailable — instead keep the existing CardSlot
state or render the linked cards from the last-known IDs; specifically, change
the if that checks streamDocument && !data.props.data.constantValueEnabled &&
data.props.data.field (and the analogous logic in the 158-220 block) to first
detect the linked-testimonial mode (the flag/shape that identifies linked
testimonials) and only run the static rebuild when there is no linked
testimonial and no streamDocument, preserving CardSlot contents when linked mode
is active.
🪄 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: 61e853be-fc5c-4d17-a18b-c3d328824904
📒 Files selected for processing (14)
packages/visual-editor/src/components/pageSections/EventSection/EventCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/EventSection/EventSection.test.tsxpackages/visual-editor/src/components/pageSections/FAQsSection/FAQsSection.tsxpackages/visual-editor/src/components/pageSections/InsightSection/InsightCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/InsightSection/InsightSection.test.tsxpackages/visual-editor/src/components/pageSections/ProductSection/ProductCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/ProductSection/ProductSection.test.tsxpackages/visual-editor/src/components/pageSections/TeamSection/TeamCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/TeamSection/TeamSection.test.tsxpackages/visual-editor/src/components/pageSections/TestimonialSection/TestimonialCardsWrapper.tsxpackages/visual-editor/src/components/pageSections/TestimonialSection/TestimonialSection.test.tsxpackages/visual-editor/src/docs/components.mdpackages/visual-editor/src/utils/cardSlots/cardWrapperHelpers.tspackages/visual-editor/src/utils/cardSlots/listSectionData.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/visual-editor/src/utils/cardSlots/listSectionData.ts
asanehisa
reviewed
Apr 29, 2026
| } | ||
| ); | ||
|
|
||
| it("when using a linked section field then the first linked entity's events are resolved", async () => { |
Contributor
There was a problem hiding this comment.
does this need to be its own it() test vs an additional screenshot test using the correct data?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds linked entity field support across Visual Editor section and subcomponent selectors while keeping the existing data model intact.
Main changes:
streamDocumentis missing.Also included:
Test Site: https://dev.yext.com/s/1000168938/yextsites/67108/branches/6758/editor#pageSetId=dev78loc&locale=en&themeId=dev-release&entityId=9125102&xYextDebug=true