feat: locator static content toggles #1040
Conversation
WalkthroughThe pull request adds support for static/constant values in the Locator component's result card display. New optional properties ( Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/visual-editor/src/components/Locator.tsx`:
- Around line 600-653: The resolveFields function is currently calling
setDeep(...) but not using its return value, so updates are lost; change each
call inside resolveFields to assign the returned object back into updatedFields
(e.g., updatedFields = setDeep(updatedFields,
"resultCard.objectFields.primaryHeading.objectFields.field.visible",
!staticEnabled)) for all setDeep invocations (primaryHeading, secondaryHeading,
tertiaryHeading, image and their constantValue equivalents) so the immutable
updates are preserved before returning updatedFields.
In `@packages/visual-editor/src/components/LocatorResultCard.tsx`:
- Around line 1185-1211: resolveText currently treats an empty string from
resolveComponentData as "no value" and falls back to parseStringFromLocation,
which breaks the intent of config.constantValueEnabled; change the logic in
resolveText so that when config.constantValueEnabled is true you return the
resolvedConstantValue as-is (including an empty string) instead of falling
through to entity data—use config.constantValueEnabled and config.constantValue
with resolveComponentData to decide to return resolvedConstantValue (even if
empty), otherwise call parseStringFromLocation; keep the final fallback behavior
(fallback param) if resolvedConstantValue is undefined.
- Around line 269-278: The image default in LocatorResultCard (the "image"
constantValue) uses a plain AssetImageType instead of the localized shape;
update the constantValue to match the localized form used by headings and tests
by wrapping the asset in an "en" key and adding hasLocalizedValue: "true" (i.e.,
set constantValue to { en: { url: "", height: 0, width: 0 }, hasLocalizedValue:
"true" }) so code expecting LocalizedAssetImage/TranslatableAssetImage will
receive a consistent shape.
🧹 Nitpick comments (3)
packages/visual-editor/src/components/Locator.test.tsx (1)
1037-1039: Weak assertion —getByTextalready throws if the element is missing.
page.getByText("Custom Heading")will throw if the element doesn't exist, making.toBeTruthy()a no-op. Consider usingtoBeVisible()or wrapping inwaitForfor a more meaningful check, especially since this runs as an interaction callback and the text comes from a static constant value that may be rendered asynchronously.packages/visual-editor/src/components/Locator.tsx (1)
925-937:handleDragis not memoized, causingmapPropsto recompute every render.
handleDrag(line 744) is a plain function declaration that gets a new identity on every render. Since it's listed in theuseMemodependency array,mapPropswill be recomputed on every render, negating the memoization benefit.This predates this PR's changes, but the refactoring introduces
mapPropsas a newuseMemo— worth wrappinghandleDraginuseCallbackto get the intended benefit.Wrap handleDrag in useCallback
- const handleDrag: OnDragHandler = (center: LngLat, bounds: LngLatBounds) => { - setMapCenter(center); - setMapBounds(bounds); - setShowSearchAreaButton(true); - }; + const handleDrag: OnDragHandler = React.useCallback( + (center: LngLat, bounds: LngLatBounds) => { + setMapCenter(center); + setMapBounds(bounds); + setShowSearchAreaButton(true); + }, + [] + );packages/visual-editor/src/components/LocatorResultCard.tsx (1)
309-329: Repetitive toggle + constant-value field pattern across four sections.The
constantValueEnabled/constantValuefield pair is duplicated nearly identically for primary, secondary, tertiary headings and image. Consider extracting a helper (e.g.,createConstantValueToggleFields(label, fieldTypeFilter, getOptions)) to reduce boilerplate.Also applies to: 353-373, 402-422, 661-675
| image: { | ||
| field: { selection: { value: "headshot" } }, | ||
| constantValue: { | ||
| url: "", | ||
| height: 0, | ||
| width: 0, | ||
| }, | ||
| constantValueEnabled: false, | ||
| liveVisibility: false, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the relevant files
fd "LocatorResultCard.tsx" -x wc -l {}Repository: yext/visual-editor
Length of output: 128
🏁 Script executed:
# Also check for the test file
fd "Locator.test.tsx" -x wc -l {}Repository: yext/visual-editor
Length of output: 123
🏁 Script executed:
# Check directory structure to understand the project layout
git ls-files packages/visual-editor/src/components/ | head -20Repository: yext/visual-editor
Length of output: 1203
🏁 Script executed:
# Read the LocatorResultCard.tsx file focusing on the relevant sections
sed -n '210,230p' packages/visual-editor/src/components/LocatorResultCard.tsxRepository: yext/visual-editor
Length of output: 739
🏁 Script executed:
# Read the image default section
sed -n '265,280p' packages/visual-editor/src/components/LocatorResultCard.tsxRepository: yext/visual-editor
Length of output: 339
🏁 Script executed:
# Read the test file to check test data
sed -n '165,175p' packages/visual-editor/src/components/Locator.test.tsxRepository: yext/visual-editor
Length of output: 339
🏁 Script executed:
# Find the type definitions for TranslatableAssetImage and AssetImageType
rg "TranslatableAssetImage|AssetImageType" --type ts --type tsx -A 3 packages/visual-editor/Repository: yext/visual-editor
Length of output: 89
🏁 Script executed:
# Also check for LocalizedAssetImage type definition
rg "LocalizedAssetImage" --type ts --type tsx -A 3 packages/visual-editor/Repository: yext/visual-editor
Length of output: 89
🏁 Script executed:
# Find the type definitions for TranslatableAssetImage and related types
rg "TranslatableAssetImage|AssetImageType" -A 3 packages/visual-editor/Repository: yext/visual-editor
Length of output: 34246
🏁 Script executed:
# Also check for LocalizedAssetImage type definition
rg "LocalizedAssetImage" -A 3 packages/visual-editor/Repository: yext/visual-editor
Length of output: 1121
🏁 Script executed:
# Check the TranslatableString type definition to understand the pattern
rg "TranslatableString" -B 2 -A 3 packages/visual-editor/src/types/Repository: yext/visual-editor
Length of output: 6878
🏁 Script executed:
# Let's also check how these types are used in resolveComponentData
sed -n '1,80p' packages/visual-editor/src/utils/resolveComponentData.tsxRepository: yext/visual-editor
Length of output: 2717
Image default constantValue uses non-localized shape, inconsistent with heading defaults.
Heading defaults (lines 213, 219, 226) use { en: "", hasLocalizedValue: "true" } (localized), while the image default uses { url: "", height: 0, width: 0 } (plain AssetImageType). Both are valid per TranslatableAssetImage = AssetImageType | LocalizedAssetImage, but this inconsistency may cause issues if code elsewhere assumes the localized shape. The test data (Locator.test.tsx ~168) uses the localized form { en: { url: "", height: 0, width: 0 }, hasLocalizedValue: "true" }, which differs from this default.
🤖 Prompt for AI Agents
In `@packages/visual-editor/src/components/LocatorResultCard.tsx` around lines 269
- 278, The image default in LocatorResultCard (the "image" constantValue) uses a
plain AssetImageType instead of the localized shape; update the constantValue to
match the localized form used by headings and tests by wrapping the asset in an
"en" key and adding hasLocalizedValue: "true" (i.e., set constantValue to { en:
{ url: "", height: 0, width: 0 }, hasLocalizedValue: "true" }) so code expecting
LocalizedAssetImage/TranslatableAssetImage will receive a consistent shape.
| /** | ||
| * Resolves the text for a text element based on the constant value, field value, and fallback. | ||
| */ | ||
| const resolveText = (params: { | ||
| config: { | ||
| constantValue?: TranslatableString; | ||
| constantValueEnabled?: boolean; | ||
| }; | ||
| location: Location; | ||
| language: string; | ||
| fieldId: string | undefined; | ||
| fallback?: string; | ||
| }): string | undefined => { | ||
| const { config, location, language, fieldId, fallback } = params; | ||
|
|
||
| const resolvedConstantValue = | ||
| config.constantValueEnabled && config.constantValue | ||
| ? resolveComponentData(config.constantValue, language, location) | ||
| : undefined; | ||
| const resolvedText = | ||
| typeof resolvedConstantValue === "string" && | ||
| resolvedConstantValue.trim() !== "" | ||
| ? resolvedConstantValue | ||
| : parseStringFromLocation(location, fieldId); | ||
|
|
||
| return resolvedText ?? fallback; | ||
| }; |
There was a problem hiding this comment.
resolveText silently falls back to entity field when static value is enabled but empty.
When constantValueEnabled is true but the user hasn't entered a value yet (empty string), the function falls through to the entity field value (line 1208). This means the toggle state doesn't truly control the data source — enabling "static" with a blank input renders entity data instead, which could confuse editors into thinking the toggle has no effect.
Consider whether this is the intended UX or if an empty string should be preserved when static mode is explicitly enabled (perhaps rendering nothing or a placeholder).
🤖 Prompt for AI Agents
In `@packages/visual-editor/src/components/LocatorResultCard.tsx` around lines
1185 - 1211, resolveText currently treats an empty string from
resolveComponentData as "no value" and falls back to parseStringFromLocation,
which breaks the intent of config.constantValueEnabled; change the logic in
resolveText so that when config.constantValueEnabled is true you return the
resolvedConstantValue as-is (including an empty string) instead of falling
through to entity data—use config.constantValueEnabled and config.constantValue
with resolveComponentData to decide to return resolvedConstantValue (even if
empty), otherwise call parseStringFromLocation; keep the final fallback behavior
(fallback param) if resolvedConstantValue is undefined.
2 participants
Added toggles to Primary/Secondary/Tertiary heading fields, and Image field, which control whether to use static values for those fields, or to pull them from the entity.
Additionally, fixes a small bug in the Locator where updating the map style would not propagate immediately.
J=WAT-5364
vle_static_content_toggles.mov