chore: merge main into templates branch#1249
Closed
briantstephan wants to merge 12 commits into
Closed
Conversation
Allows EntityFieldSelector and itemSource to treat text fields as satisfying a rich_text field. This allows mapping rich_text fields to ordinary text fields and treats objects with text fields as satisfying the rich_text requirement for itemSource and slottedItemSource. Verified that RichText fields can be mapped to ordinary Text fields. Verified that ordinary Text fields cannot be mapped to RichText. Verified that itemSource/slottedItemSource mapping requirements now accept Text in place of RichText. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Removes the "Api Key" prop from the Static Map Section and Mapbox Static Map components. They now use the key from the environment variable. If the key fails to load, the component will not appear on the live page. Tested using local starter. --------- 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>
long emails before: <img width="686" height="205" alt="Screenshot 2026-06-09 at 1 54 42 PM" src="https://github.com/user-attachments/assets/b09dfe11-168a-4c73-b0e7-d137624f6e0c" /> <img width="395" height="308" alt="Screenshot 2026-06-09 at 1 54 47 PM" src="https://github.com/user-attachments/assets/2990e7d0-9816-477b-9d8d-43aa9fc179e2" /> after: <img width="870" height="278" alt="Screenshot 2026-06-09 at 1 50 41 PM" src="https://github.com/user-attachments/assets/b18134e2-a564-442c-89d5-4537e4298c3c" /> <img width="394" height="368" alt="Screenshot 2026-06-09 at 1 50 34 PM" src="https://github.com/user-attachments/assets/7c6de2cc-b13e-413e-98a2-b5d2f708f869" /> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Slack for context: https://yext.slack.com/archives/C02UVSE7P6W/p1781045532366289?thread_ts=1780506413.231889&cid=C02UVSE7P6W https://github.com/user-attachments/assets/503ce53b-a359-4824-9c33-ae704efcf54d --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
In mobile view, display a "show more" button at the end of the search results instead of pagination. J=WAT-5580 TEST=manual tested in dev mode and platform https://drive.google.com/file/d/1Bw7xFbmvdOncZl37DXCalcTiLl3zod0n/view?usp=sharing --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
For this [vuln](https://yext.atlassian.net/browse/VULN-44969?atlOrigin=eyJpIjoiMWY4NmMxOGU2ZDE3NDgyMjgxYjAyMTYzOTA2NjUwNGYiLCJwIjoiaiJ9), have esbuild be 0.28.1 everywhere. Additionally removed some dependencies that weren't used in packages/visual-editor and moved others only used in dev to devDependencies. And aligned all the nodes to `"node": "^20.6.0 || ^22 || ^24"` (already was done in other package.json files) --------- 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>
J=WAT-5650 TEST=manual --------- 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>
By default, we will still always show the "Include Caret" field, but this gives us the option to hide it for links where it is not relevant, including the navigation links in the shared header. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Contributor
|
Warning Review limit reached
Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (45)
WalkthroughVersion 1.3.3 of Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (1)
packages/visual-editor/src/components/pageSections/StaticMapSection.test.tsx (1)
20-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the layout-editor token override.
These cases cover
_env.YEXT_MAPBOX_API_KEY, but they never exercise the newpreview-frame+_env.YEXT_EDIT_LAYOUT_MODE_MAPBOX_API_KEYbranch inMapboxStaticMapComponent. A small render test for that path would prevent regressions in layout-edit mode. Based on learnings, that iframe check is the intentional way this repo detects layout-editor rendering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/visual-editor/src/components/pageSections/StaticMapSection.test.tsx` around lines 20 - 67, The StaticMapSection tests currently cover only the _env.YEXT_MAPBOX_API_KEY path and miss the layout-editor token override branch in MapboxStaticMapComponent. Add a focused render test that exercises the preview-frame iframe detection together with _env.YEXT_EDIT_LAYOUT_MODE_MAPBOX_API_KEY so the layout-edit mode token override is verified. Use the existing StaticMapSection test setup and the MapboxStaticMapComponent behavior as the target reference for locating the new coverage.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/visual-editor/CHANGELOG.md`:
- Around line 3-12: The 1.3.3 release notes are missing the
StyledLinkField.showIncludeCaretField change, so update the CHANGELOG entry
under the existing “New Features” section to include that option. Use the
StyledLinkField.showIncludeCaretField symbol as the anchor when adding the note,
and keep the wording consistent with the surrounding release bullets.
In `@packages/visual-editor/locales/components/pt/visual-editor.json`:
- Line 94: The Portuguese CTA text for showMoreLocations is inconsistent with
the adjacent action label style because it uses the infinitive instead of the
imperative. Update the showMoreLocations entry in the Portuguese visual-editor
locale so it matches the existing CTA pattern used by nearby labels like “Mostre
mais,” and keep the button copy in the imperative form.
In `@packages/visual-editor/locales/platform/de/visual-editor.json`:
- Around line 508-509: The German locale labels for subtitle alignment sound
awkward and should be made more idiomatic. Update the strings for
subtitleAlignmentDesktop and subtitleAlignmentMobile in the visual-editor German
JSON to use a clearer label style such as “Untertitel-Ausrichtung (Desktop)” and
“Untertitel-Ausrichtung (Mobil)”, keeping the wording consistent with the
surrounding field labels.
In `@packages/visual-editor/locales/platform/en-GB/visual-editor.json`:
- Around line 508-509: The new en-GB subtitle label strings are awkwardly
phrased for UI copy and should be rewritten to sound natural. Update the
localized values for subtitleAlignmentDesktop and subtitleAlignmentMobile in the
visual-editor locale file to use a more idiomatic label pattern such as “Desktop
Subtitle Alignment” and “Mobile Subtitle Alignment” (or the equivalent
parenthesized form), keeping the wording consistent with the rest of the
translation entries.
In `@packages/visual-editor/locales/platform/en/visual-editor.json`:
- Around line 508-509: The locale strings for subtitle alignment read
inconsistently compared with the other desktop/mobile labels. Update the copy
for subtitleAlignmentDesktop and subtitleAlignmentMobile in the visual-editor
locale JSON to follow the existing “Desktop/Mobile …” pattern used by nearby
alignment fields, keeping the wording consistent across the editor.
In `@packages/visual-editor/locales/platform/hr/visual-editor.json`:
- Around line 508-509: The Croatian locale strings for the subtitle alignment
labels are inconsistent because subtitle is translated with two different terms.
Update the localized value in the visual-editor JSON so both
subtitleAlignmentDesktop and subtitleAlignmentMobile use the same Croatian term
as the existing subtitle field, keeping the labels aligned and consistent across
the editor.
In `@packages/visual-editor/locales/platform/pl/visual-editor.json`:
- Around line 508-509: The subtitle-alignment labels are using inconsistent
Polish terminology by switching from the existing podtytuł wording to napisy.
Update the locale entries in the visual-editor JSON for the subtitle alignment
controls so they match the terminology already used by the related subtitle
field strings, keeping the same podtytuł-based phrasing in both the desktop and
mobile labels.
In `@packages/visual-editor/locales/platform/zh/visual-editor.json`:
- Around line 508-509: The subtitle alignment labels in the zh locale are using
a different Chinese term than the existing subtitle wording in this file. Update
the `subtitleAlignmentDesktop` and `subtitleAlignmentMobile` entries in
`visual-editor.json` to use the same term as `showSubtitle` and the nearby
subtitle label, keeping the `Visual Editor` localization consistent across all
subtitle-related strings.
In `@packages/visual-editor/src/components/locator/LocatorWrapper.tsx`:
- Around line 440-441: The Current Location button is being hidden too
aggressively because `showCurrentLocationButton` depends on
`userLocationRetrieved`, which only becomes true after a successful auto-locate
path in `LocatorWrapper`. Update the state/logic around `userLocationRetrieved`
and the `showCurrentLocationButton` condition so the button remains visible when
starting from `initialLocation`, using the configured center, or after an
initial geolocation failure/skipped attempt. Make the manual current-location
action available even when auto-locate did not succeed, and adjust the related
handling in the nearby geolocation flow and button rendering logic to preserve
that visibility.
- Around line 822-824: Guard the visualEditorConfig parsing in LocatorWrapper so
a malformed value cannot throw during render. Update the requireMapOptIn logic
to safely parse streamDocument.__?.visualEditorConfig with a fallback to false
when parsing fails or the field is missing, using a local try/catch or
equivalent defensive helper around the JSON.parse call.
In `@packages/visual-editor/src/components/locator/Map.tsx`:
- Around line 239-248: `LocatorMapPin` is passing a zero-based `result.index`
into `MapPinIcon`, but that component expects `resultIndex` to be 1-based.
Update the `Map.tsx` render path in `LocatorMapPin` so the `resultIndex` prop is
normalized to start at 1 before being forwarded to `MapPinIcon`, and keep the
rest of the icon props unchanged.
In `@packages/visual-editor/src/docs/components.md`:
- Around line 806-809: The StaticMapSection docs currently imply no
configuration is needed now that apiKey is gone, but the component still depends
on _env.YEXT_MAPBOX_API_KEY at runtime. Update the StaticMapSection description
in the documentation to explicitly mention that the Mapbox key must be provided
via the environment, and note the env-based source of that value instead of
omitting the requirement.
In `@packages/visual-editor/src/internal/utils/getFilteredEntityFields.ts`:
- Around line 247-253: getFilteredEntityFields() is appending the same field
multiple times when filter.types includes overlapping compatible types such as
type.string and type.rich_text_v2. Update the compatible-type collection in
getFilteredEntityFields so that updatedFilteredEntitySubFields is deduplicated
before returning, ideally by field name, while preserving the existing
compatibility logic in getCompatibleEntityFieldTypes and the field-matching flow
around typeToFieldNames and filteredEntitySubFields.
In
`@starter/localData/dev-location-stream__en__cbafb9cd1c3e63d9814e236ba9181377.json`:
- Around line 11-12: The Mapbox fixture values are using truthy placeholder
strings, which makes `MapboxStaticMapComponent` treat them as valid tokens
instead of showing the missing-key state. Update the starter data so
`YEXT_EDIT_LAYOUT_MODE_MAPBOX_API_KEY` and `YEXT_MAPBOX_API_KEY` are empty or
omitted until real environment values are injected, keeping the fixture aligned
with the component’s token check.
---
Nitpick comments:
In
`@packages/visual-editor/src/components/pageSections/StaticMapSection.test.tsx`:
- Around line 20-67: The StaticMapSection tests currently cover only the
_env.YEXT_MAPBOX_API_KEY path and miss the layout-editor token override branch
in MapboxStaticMapComponent. Add a focused render test that exercises the
preview-frame iframe detection together with
_env.YEXT_EDIT_LAYOUT_MODE_MAPBOX_API_KEY so the layout-edit mode token override
is verified. Use the existing StaticMapSection test setup and the
MapboxStaticMapComponent behavior as the target reference for locating the new
coverage.
🪄 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: 952245a2-6532-4882-8906-298bcddac61d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (87)
package.jsonpackages/visual-editor/CHANGELOG.mdpackages/visual-editor/THIRD-PARTY-NOTICESpackages/visual-editor/locales/components/cs/visual-editor.jsonpackages/visual-editor/locales/components/da/visual-editor.jsonpackages/visual-editor/locales/components/de/visual-editor.jsonpackages/visual-editor/locales/components/en-GB/visual-editor.jsonpackages/visual-editor/locales/components/en/visual-editor.jsonpackages/visual-editor/locales/components/es/visual-editor.jsonpackages/visual-editor/locales/components/et/visual-editor.jsonpackages/visual-editor/locales/components/fi/visual-editor.jsonpackages/visual-editor/locales/components/fr/visual-editor.jsonpackages/visual-editor/locales/components/hr/visual-editor.jsonpackages/visual-editor/locales/components/hu/visual-editor.jsonpackages/visual-editor/locales/components/it/visual-editor.jsonpackages/visual-editor/locales/components/ja/visual-editor.jsonpackages/visual-editor/locales/components/lt/visual-editor.jsonpackages/visual-editor/locales/components/lv/visual-editor.jsonpackages/visual-editor/locales/components/nb/visual-editor.jsonpackages/visual-editor/locales/components/nl/visual-editor.jsonpackages/visual-editor/locales/components/pl/visual-editor.jsonpackages/visual-editor/locales/components/pt/visual-editor.jsonpackages/visual-editor/locales/components/ro/visual-editor.jsonpackages/visual-editor/locales/components/sk/visual-editor.jsonpackages/visual-editor/locales/components/sv/visual-editor.jsonpackages/visual-editor/locales/components/tr/visual-editor.jsonpackages/visual-editor/locales/components/zh-TW/visual-editor.jsonpackages/visual-editor/locales/components/zh/visual-editor.jsonpackages/visual-editor/locales/platform/cs/visual-editor.jsonpackages/visual-editor/locales/platform/da/visual-editor.jsonpackages/visual-editor/locales/platform/de/visual-editor.jsonpackages/visual-editor/locales/platform/en-GB/visual-editor.jsonpackages/visual-editor/locales/platform/en/visual-editor.jsonpackages/visual-editor/locales/platform/es/visual-editor.jsonpackages/visual-editor/locales/platform/et/visual-editor.jsonpackages/visual-editor/locales/platform/fi/visual-editor.jsonpackages/visual-editor/locales/platform/fr/visual-editor.jsonpackages/visual-editor/locales/platform/hr/visual-editor.jsonpackages/visual-editor/locales/platform/hu/visual-editor.jsonpackages/visual-editor/locales/platform/it/visual-editor.jsonpackages/visual-editor/locales/platform/ja/visual-editor.jsonpackages/visual-editor/locales/platform/lt/visual-editor.jsonpackages/visual-editor/locales/platform/lv/visual-editor.jsonpackages/visual-editor/locales/platform/nb/visual-editor.jsonpackages/visual-editor/locales/platform/nl/visual-editor.jsonpackages/visual-editor/locales/platform/pl/visual-editor.jsonpackages/visual-editor/locales/platform/pt/visual-editor.jsonpackages/visual-editor/locales/platform/ro/visual-editor.jsonpackages/visual-editor/locales/platform/sk/visual-editor.jsonpackages/visual-editor/locales/platform/sv/visual-editor.jsonpackages/visual-editor/locales/platform/tr/visual-editor.jsonpackages/visual-editor/locales/platform/zh-TW/visual-editor.jsonpackages/visual-editor/locales/platform/zh/visual-editor.jsonpackages/visual-editor/package.jsonpackages/visual-editor/src/components/Locator.tsxpackages/visual-editor/src/components/categories/LocatorCategory.tsxpackages/visual-editor/src/components/contentBlocks/Emails.tsxpackages/visual-editor/src/components/contentBlocks/MapboxStaticMap.tsxpackages/visual-editor/src/components/header/HeaderLinks.tsxpackages/visual-editor/src/components/header/PrimaryHeaderSlot.tsxpackages/visual-editor/src/components/header/viewport.tspackages/visual-editor/src/components/index.tspackages/visual-editor/src/components/locator/Filters.tsxpackages/visual-editor/src/components/locator/Locator.test.tsxpackages/visual-editor/src/components/locator/Locator.tsxpackages/visual-editor/src/components/locator/LocatorResultCard.tsxpackages/visual-editor/src/components/locator/LocatorWrapper.tsxpackages/visual-editor/src/components/locator/Map.tsxpackages/visual-editor/src/components/locator/Results.tsxpackages/visual-editor/src/components/migrations/0078_remove_mapbox_api_key_props.tspackages/visual-editor/src/components/migrations/migrationRegistry.tspackages/visual-editor/src/components/pageSections/ProfessionalHeroSection.tsxpackages/visual-editor/src/components/pageSections/StaticMapSection.test.tsxpackages/visual-editor/src/components/pageSections/StaticMapSection.tsxpackages/visual-editor/src/docs/components.mdpackages/visual-editor/src/editor/yextEntityFieldUtils.test.tspackages/visual-editor/src/editor/yextEntityFieldUtils.tspackages/visual-editor/src/fields/EntityFieldSelectorField.test.tsxpackages/visual-editor/src/fields/styledFields/StyledLinkField.tsxpackages/visual-editor/src/hooks/useViewport.tspackages/visual-editor/src/internal/utils/getFilteredEntityFields.test.tspackages/visual-editor/src/internal/utils/getFilteredEntityFields.tspackages/visual-editor/src/utils/migrate.test.tspackages/visual-editor/vite.config.tsstarter/localData/dev-location-stream__en__cbafb9cd1c3e63d9814e236ba9181377.jsonstarter/package.jsonstarter/vite.config.js
💤 Files with no reviewable changes (3)
- packages/visual-editor/src/components/header/viewport.ts
- packages/visual-editor/src/components/Locator.tsx
- packages/visual-editor/THIRD-PARTY-NOTICES
Comment on lines
+3
to
+12
| ##### New Features | ||
|
|
||
| - add additional fields in Professional Hero ([#1234](https://github.com/yext/visual-editor/pull/1234)) ([d03f15f4](https://github.com/yext/visual-editor/commit/d03f15f405f3d71f1a24dbf980ac08dcdc5a7f19)) | ||
| - mapbox uses YEXT_MAPBOX_API_KEY ([#1227](https://github.com/yext/visual-editor/pull/1227)) ([9467295b](https://github.com/yext/visual-editor/commit/9467295beb7a57acbdd2a2cf45786df587c745ef)) | ||
| - text satisfies rich_text selectors ([#1229](https://github.com/yext/visual-editor/pull/1229)) ([18d19723](https://github.com/yext/visual-editor/commit/18d197233b6b5c76b1f1d576b97466194ff6d69b)) | ||
|
|
||
| ##### Bug Fixes | ||
|
|
||
| - remove separate locator results scrollbar for mobile ([#1237](https://github.com/yext/visual-editor/pull/1237)) ([e211f13c](https://github.com/yext/visual-editor/commit/e211f13c5bad860aecc76623ff1f6e816854ef3c)) | ||
| - squished email icon ([#1232](https://github.com/yext/visual-editor/pull/1232)) ([67e03aa2](https://github.com/yext/visual-editor/commit/67e03aa2d04e2410f49d1fe3127ad8ec91652726)) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Please add the StyledLinkField.showIncludeCaretField change to the 1.3.3 notes.
The PR summary calls that option out explicitly, but this release section never mentions it, so the shipped surface area for 1.3.3 is understated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/visual-editor/CHANGELOG.md` around lines 3 - 12, The 1.3.3 release
notes are missing the StyledLinkField.showIncludeCaretField change, so update
the CHANGELOG entry under the existing “New Features” section to include that
option. Use the StyledLinkField.showIncludeCaretField symbol as the anchor when
adding the note, and keep the wording consistent with the surrounding release
bullets.
| "servicesSection": "Seção de serviços", | ||
| "showLess": "Mostre menos", | ||
| "showMore": "Mostre mais", | ||
| "showMoreLocations": "Mostrar mais locais", |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the Portuguese CTA wording consistent.
Line 94 switches from the imperative used by the adjacent action label ("Mostre mais") to the infinitive ("Mostrar mais locais"). For a button/CTA, "Mostre mais locais" would match the existing pattern better.
💬 Proposed copy fix
- "showMoreLocations": "Mostrar mais locais",
+ "showMoreLocations": "Mostre mais locais",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "showMoreLocations": "Mostrar mais locais", | |
| "showMoreLocations": "Mostre mais locais", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/visual-editor/locales/components/pt/visual-editor.json` at line 94,
The Portuguese CTA text for showMoreLocations is inconsistent with the adjacent
action label style because it uses the infinitive instead of the imperative.
Update the showMoreLocations entry in the Portuguese visual-editor locale so it
matches the existing CTA pattern used by nearby labels like “Mostre mais,” and
keep the button copy in the imperative form.
Comment on lines
+508
to
+509
| "subtitleAlignmentDesktop": "Untertitelausrichtung im Desktop", | ||
| "subtitleAlignmentMobile": "Untertitelausrichtung in Mobilgeräten", |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use more idiomatic German for the subtitle-alignment labels.
Line 508 and Line 509 read awkwardly (im Desktop, in Mobilgeräten) and stand out from the surrounding field labels. A label style like Untertitel-Ausrichtung (Desktop) / Untertitel-Ausrichtung (Mobil) would be clearer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/visual-editor/locales/platform/de/visual-editor.json` around lines
508 - 509, The German locale labels for subtitle alignment sound awkward and
should be made more idiomatic. Update the strings for subtitleAlignmentDesktop
and subtitleAlignmentMobile in the visual-editor German JSON to use a clearer
label style such as “Untertitel-Ausrichtung (Desktop)” and
“Untertitel-Ausrichtung (Mobil)”, keeping the wording consistent with the
surrounding field labels.
Comment on lines
+508
to
+509
| "subtitleAlignmentDesktop": "Subtitle Alignment in Desktop", | ||
| "subtitleAlignmentMobile": "Subtitle Alignment in Mobile", |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rephrase the new en-GB subtitle labels to sound natural.
Subtitle Alignment in Desktop / Subtitle Alignment in Mobile is awkward English for UI labels. Desktop Subtitle Alignment and Mobile Subtitle Alignment (or Subtitle Alignment (Desktop/Mobile)) would fit the rest of this file better.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/visual-editor/locales/platform/en-GB/visual-editor.json` around
lines 508 - 509, The new en-GB subtitle label strings are awkwardly phrased for
UI copy and should be rewritten to sound natural. Update the localized values
for subtitleAlignmentDesktop and subtitleAlignmentMobile in the visual-editor
locale file to use a more idiomatic label pattern such as “Desktop Subtitle
Alignment” and “Mobile Subtitle Alignment” (or the equivalent parenthesized
form), keeping the wording consistent with the rest of the translation entries.
Comment on lines
+508
to
+509
| "subtitleAlignmentDesktop": "Subtitle Alignment in Desktop", | ||
| "subtitleAlignmentMobile": "Subtitle Alignment in Mobile", |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Match the existing desktop/mobile label pattern.
These labels read awkwardly ("in Desktop" / "in Mobile") and break the nearby Desktop/Mobile … wording used for other alignment fields, so they’ll look inconsistent in the editor.
Suggested copy
- "subtitleAlignmentDesktop": "Subtitle Alignment in Desktop",
- "subtitleAlignmentMobile": "Subtitle Alignment in Mobile",
+ "subtitleAlignmentDesktop": "Desktop Subtitle Alignment",
+ "subtitleAlignmentMobile": "Mobile Subtitle Alignment",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "subtitleAlignmentDesktop": "Subtitle Alignment in Desktop", | |
| "subtitleAlignmentMobile": "Subtitle Alignment in Mobile", | |
| "subtitleAlignmentDesktop": "Desktop Subtitle Alignment", | |
| "subtitleAlignmentMobile": "Mobile Subtitle Alignment", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/visual-editor/locales/platform/en/visual-editor.json` around lines
508 - 509, The locale strings for subtitle alignment read inconsistently
compared with the other desktop/mobile labels. Update the copy for
subtitleAlignmentDesktop and subtitleAlignmentMobile in the visual-editor locale
JSON to follow the existing “Desktop/Mobile …” pattern used by nearby alignment
fields, keeping the wording consistent across the editor.
Comment on lines
+822
to
+824
| const requireMapOptIn: boolean = streamDocument.__?.visualEditorConfig | ||
| ? JSON.parse(streamDocument.__?.visualEditorConfig)?.requireMapOptIn | ||
| : false; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard visualEditorConfig parsing.
This JSON.parse runs during render with no fallback. A malformed visualEditorConfig string will throw and blank the entire locator instead of falling back to false.
Proposed fix
- const requireMapOptIn: boolean = streamDocument.__?.visualEditorConfig
- ? JSON.parse(streamDocument.__?.visualEditorConfig)?.requireMapOptIn
- : false;
+ const requireMapOptIn = React.useMemo(() => {
+ if (!streamDocument.__?.visualEditorConfig) {
+ return false;
+ }
+
+ try {
+ return (
+ JSON.parse(streamDocument.__.visualEditorConfig)?.requireMapOptIn ??
+ false
+ );
+ } catch (error) {
+ console.warn("Failed to parse visualEditorConfig:", error);
+ return false;
+ }
+ }, [streamDocument.__?.visualEditorConfig]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const requireMapOptIn: boolean = streamDocument.__?.visualEditorConfig | |
| ? JSON.parse(streamDocument.__?.visualEditorConfig)?.requireMapOptIn | |
| : false; | |
| const requireMapOptIn = React.useMemo(() => { | |
| if (!streamDocument.__?.visualEditorConfig) { | |
| return false; | |
| } | |
| try { | |
| return ( | |
| JSON.parse(streamDocument.__.visualEditorConfig)?.requireMapOptIn ?? | |
| false | |
| ); | |
| } catch (error) { | |
| console.warn("Failed to parse visualEditorConfig:", error); | |
| return false; | |
| } | |
| }, [streamDocument.__?.visualEditorConfig]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/visual-editor/src/components/locator/LocatorWrapper.tsx` around
lines 822 - 824, Guard the visualEditorConfig parsing in LocatorWrapper so a
malformed value cannot throw during render. Update the requireMapOptIn logic to
safely parse streamDocument.__?.visualEditorConfig with a fallback to false when
parsing fails or the field is missing, using a local try/catch or equivalent
defensive helper around the JSON.parse call.
Comment on lines
+239
to
+248
| return ( | ||
| <MapPinIcon | ||
| color={entityLocationStyle?.color} | ||
| resultIndex={result.index} | ||
| icon={entityLocationStyle?.customImage?.url ?? entityLocationStyle?.icon} | ||
| iconWidth={ | ||
| entityLocationStyle?.customImage?.width ?? DEFAULT_PIN_ICON_WIDTH | ||
| } | ||
| disableContrastIcon={!!entityLocationStyle?.customImage} | ||
| aspectRatio={entityLocationStyle?.customImage?.aspectRatio} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pass a 1-based resultIndex to MapPinIcon.
LocatorMapPin forwards result.index directly, but this package's contract for MapPinIcon is resultIndex >= 1. That makes the first locator pin invalid unless the value is normalized here.
Based on learnings, resultIndex for MapPinIcon and related components must always be >= 1.
Proposed fix
<MapPinIcon
color={entityLocationStyle?.color}
- resultIndex={result.index}
+ resultIndex={
+ typeof result.index === "number" && result.index >= 0
+ ? result.index + 1
+ : 1
+ }
icon={entityLocationStyle?.customImage?.url ?? entityLocationStyle?.icon}
iconWidth={
entityLocationStyle?.customImage?.width ?? DEFAULT_PIN_ICON_WIDTH
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return ( | |
| <MapPinIcon | |
| color={entityLocationStyle?.color} | |
| resultIndex={result.index} | |
| icon={entityLocationStyle?.customImage?.url ?? entityLocationStyle?.icon} | |
| iconWidth={ | |
| entityLocationStyle?.customImage?.width ?? DEFAULT_PIN_ICON_WIDTH | |
| } | |
| disableContrastIcon={!!entityLocationStyle?.customImage} | |
| aspectRatio={entityLocationStyle?.customImage?.aspectRatio} | |
| return ( | |
| <MapPinIcon | |
| color={entityLocationStyle?.color} | |
| resultIndex={ | |
| typeof result.index === "number" && result.index >= 0 | |
| ? result.index + 1 | |
| : 1 | |
| } | |
| icon={entityLocationStyle?.customImage?.url ?? entityLocationStyle?.icon} | |
| iconWidth={ | |
| entityLocationStyle?.customImage?.width ?? DEFAULT_PIN_ICON_WIDTH | |
| } | |
| disableContrastIcon={!!entityLocationStyle?.customImage} | |
| aspectRatio={entityLocationStyle?.customImage?.aspectRatio} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/visual-editor/src/components/locator/Map.tsx` around lines 239 -
248, `LocatorMapPin` is passing a zero-based `result.index` into `MapPinIcon`,
but that component expects `resultIndex` to be 1-based. Update the `Map.tsx`
render path in `LocatorMapPin` so the `resultIndex` prop is normalized to start
at 1 before being forwarded to `MapPinIcon`, and keep the rest of the icon props
unchanged.
Source: Learnings
Comment on lines
+806
to
809
| The Static Map Section displays a non-interactive map image of a business's location. It uses the entity's address or coordinates to generate the map. | ||
|
|
||
| Available on Location templates. | ||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the env-based key requirement.
The apiKey prop is gone, but this section now reads as if no configuration is required. StaticMapSection still depends on _env.YEXT_MAPBOX_API_KEY at runtime, so the docs should say where that value comes from instead of dropping the requirement entirely.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/visual-editor/src/docs/components.md` around lines 806 - 809, The
StaticMapSection docs currently imply no configuration is needed now that apiKey
is gone, but the component still depends on _env.YEXT_MAPBOX_API_KEY at runtime.
Update the StaticMapSection description in the documentation to explicitly
mention that the Mapbox key must be provided via the environment, and note the
env-based source of that value instead of omitting the requirement.
Comment on lines
247
to
+253
| filter.types.forEach((type) => { | ||
| updatedFilteredEntitySubFields.push( | ||
| ...filteredEntitySubFields.filter((field) => { | ||
| return typeToFieldNames.get(type)?.includes(field.name); | ||
| }) | ||
| ...getCompatibleEntityFieldTypes(type).flatMap((compatibleType) => | ||
| filteredEntitySubFields.filter((field) => | ||
| typeToFieldNames.get(compatibleType)?.includes(field.name) | ||
| ) | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Deduplicate compatible-type matches before returning.
If filter.types includes both type.string and type.rich_text_v2, the same string field is appended twice: once for the direct string match and once via the rich-text compatibility expansion. getFilteredEntityFields() will then return duplicate field names anywhere this helper is used without downstream deduping.
Suggested fix
if (filter?.types) {
const typeToFieldNames = getEntityTypeToFieldNames(filteredEntitySubFields);
-
- const updatedFilteredEntitySubFields: YextSchemaField[] = [];
- filter.types.forEach((type) => {
- updatedFilteredEntitySubFields.push(
- ...getCompatibleEntityFieldTypes(type).flatMap((compatibleType) =>
- filteredEntitySubFields.filter((field) =>
- typeToFieldNames.get(compatibleType)?.includes(field.name)
- )
- )
- );
- });
- filteredEntitySubFields = updatedFilteredEntitySubFields;
+ const matchingFieldNames = new Set(
+ filter.types.flatMap((type) =>
+ getCompatibleEntityFieldTypes(type).flatMap(
+ (compatibleType) => typeToFieldNames.get(compatibleType) ?? []
+ )
+ )
+ );
+ filteredEntitySubFields = filteredEntitySubFields.filter((field) =>
+ matchingFieldNames.has(field.name)
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| filter.types.forEach((type) => { | |
| updatedFilteredEntitySubFields.push( | |
| ...filteredEntitySubFields.filter((field) => { | |
| return typeToFieldNames.get(type)?.includes(field.name); | |
| }) | |
| ...getCompatibleEntityFieldTypes(type).flatMap((compatibleType) => | |
| filteredEntitySubFields.filter((field) => | |
| typeToFieldNames.get(compatibleType)?.includes(field.name) | |
| ) | |
| ) | |
| if (filter?.types) { | |
| const typeToFieldNames = getEntityTypeToFieldNames(filteredEntitySubFields); | |
| const matchingFieldNames = new Set( | |
| filter.types.flatMap((type) => | |
| getCompatibleEntityFieldTypes(type).flatMap( | |
| (compatibleType) => typeToFieldNames.get(compatibleType) ?? [] | |
| ) | |
| ) | |
| ); | |
| filteredEntitySubFields = filteredEntitySubFields.filter((field) => | |
| matchingFieldNames.has(field.name) | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/visual-editor/src/internal/utils/getFilteredEntityFields.ts` around
lines 247 - 253, getFilteredEntityFields() is appending the same field multiple
times when filter.types includes overlapping compatible types such as
type.string and type.rich_text_v2. Update the compatible-type collection in
getFilteredEntityFields so that updatedFilteredEntitySubFields is deduplicated
before returning, ideally by field name, while preserving the existing
compatibility logic in getCompatibleEntityFieldTypes and the field-matching flow
around typeToFieldNames and filteredEntitySubFields.
Comment on lines
+11
to
+12
| "YEXT_EDIT_LAYOUT_MODE_MAPBOX_API_KEY": "REPLACE ME WITH KEY FROM ENV VARS", | ||
| "YEXT_MAPBOX_API_KEY": "REPLACE ME WITH KEY FROM ENV VARS", |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Don't use truthy placeholder values for the Mapbox keys.
MapboxStaticMapComponent only checks whether the token is truthy, so these "REPLACE ME..." strings will be treated as valid keys and the starter will request broken map images instead of showing the missing-key empty state. Leave them empty or omit them until real values are injected.
Suggested fixture change
- "YEXT_EDIT_LAYOUT_MODE_MAPBOX_API_KEY": "REPLACE ME WITH KEY FROM ENV VARS",
- "YEXT_MAPBOX_API_KEY": "REPLACE ME WITH KEY FROM ENV VARS",
+ "YEXT_EDIT_LAYOUT_MODE_MAPBOX_API_KEY": "",
+ "YEXT_MAPBOX_API_KEY": "",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "YEXT_EDIT_LAYOUT_MODE_MAPBOX_API_KEY": "REPLACE ME WITH KEY FROM ENV VARS", | |
| "YEXT_MAPBOX_API_KEY": "REPLACE ME WITH KEY FROM ENV VARS", | |
| "YEXT_EDIT_LAYOUT_MODE_MAPBOX_API_KEY": "", | |
| "YEXT_MAPBOX_API_KEY": "", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@starter/localData/dev-location-stream__en__cbafb9cd1c3e63d9814e236ba9181377.json`
around lines 11 - 12, The Mapbox fixture values are using truthy placeholder
strings, which makes `MapboxStaticMapComponent` treat them as valid tokens
instead of showing the missing-key state. Update the starter data so
`YEXT_EDIT_LAYOUT_MODE_MAPBOX_API_KEY` and `YEXT_MAPBOX_API_KEY` are empty or
omitted until real environment values are injected, keeping the fixture aligned
with the component’s token check.
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. |
Legal RiskThe following dependencies were released under a license that RecommendationReach out to your security team or Semgrep admin to address this issue. In special cases, exceptions may be made for dependencies with violating licenses, however, the general recommendation is to avoid using a dependency under such a license non-standard |
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
4 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 includes the new
showIncludeCaretFieldoption for StyledLinkField.