Conversation
Contributor
WalkthroughThe PR moves video handling out of the centralized YextField factory and the removed constant-value-fields Video component into a new dedicated VideoField module with a VideoFieldOverride UI. It updates the fields registry to include the new video field and adjusts content block schema usage to define the Sequence Diagram(s)sequenceDiagram
participant Field as VideoFieldOverride
participant Parent as Parent Drawer
participant Store as Editor State
Field->>Parent: send constantValueEditorOpened(messageId)
Parent-->>Field: open drawer, user selects AssetVideo
Parent->>Field: constantValueEditorClosed(messageId, selectedAsset)
alt messageId matches session
Field->>Store: onChange(selectedAsset)
else mismatch
Field-->>Store: ignore
end
Note right of Field: In fake local-dev, "Change" prompts for URL and Field->>Store: onChange(parsed AssetVideo)
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 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 |
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. |
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: 2
🧹 Nitpick comments (1)
packages/visual-editor/src/fields/VideoField.test.tsx (1)
47-71: Cover the new message-driven selection flow too.This suite only exercises rendering and delete. The new behavior in
packages/visual-editor/src/fields/VideoField.tsxis the pending-session/message roundtrip, so one matching/non-matchingconstantValueEditorClosedtest would catch the regressions most likely to break selection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/fields/VideoField.test.tsx` around lines 47 - 71, Add tests to VideoField.test.tsx to exercise the new message-driven selection flow: after rendering via renderField (with and without assetVideo), simulate the pending-session/message roundtrip by dispatching the constantValueEditorClosed message event used by VideoField.tsx (send both a matching session id payload to assert onChange is called with the selected asset and a non-matching session id payload to assert no change), and assert the component reacts correctly (onChange invoked with the expected value for matching session, not invoked for non-matching). Use the existing renderField and assetVideo fixtures and wire the same session id that the component registers so the test mirrors the real message flow.
🤖 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/fields/VideoField.tsx`:
- Around line 59-79: Wrap the local-dev prompt parsing in a safe validation
block: when isFakeStarterLocalDev() is true, capture the prompt result and guard
the URL construction and search param extraction (the new URL(userInput) and
searchParams.get("v")) inside a try/catch and verify that videoId and userInput
are non-empty and valid before calling onChange with the mock AssetVideo object;
if parsing fails or videoId is falsy, simply return without calling onChange to
leave the selection unchanged.
- Around line 136-153: Update the overlay container div in VideoField.tsx (the
div that wraps the Change/Delete Buttons) to include a focus-based visibility
class so keyboard users can see the actions; add focus-within:ve-opacity-100 to
the existing className on that div so focusing either Button (handleSelectVideo
or handleDeleteVideo) will make the overlay visible. Ensure the buttons remain
focusable (no extra change needed unless they've been disabled) so tabbing into
handleSelectVideo/handleDeleteVideo triggers the focus-within rule.
---
Nitpick comments:
In `@packages/visual-editor/src/fields/VideoField.test.tsx`:
- Around line 47-71: Add tests to VideoField.test.tsx to exercise the new
message-driven selection flow: after rendering via renderField (with and without
assetVideo), simulate the pending-session/message roundtrip by dispatching the
constantValueEditorClosed message event used by VideoField.tsx (send both a
matching session id payload to assert onChange is called with the selected asset
and a non-matching session id payload to assert no change), and assert the
component reacts correctly (onChange invoked with the expected value for
matching session, not invoked for non-matching). Use the existing renderField
and assetVideo fixtures and wire the same session id that the component
registers so the test mirrors the real message flow.
🪄 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: c1d53f12-dd6e-4762-a926-eb832063a478
⛔ Files ignored due to path filters (2)
packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] latest version multi-pageset 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 (8)
packages/visual-editor/src/components/contentBlocks/Video.tsxpackages/visual-editor/src/editor/YextField.test.tsxpackages/visual-editor/src/editor/YextField.tsxpackages/visual-editor/src/fields/VideoField.test.tsxpackages/visual-editor/src/fields/VideoField.tsxpackages/visual-editor/src/fields/fields.tspackages/visual-editor/src/fields/index.tspackages/visual-editor/src/internal/puck/constant-value-fields/Video.tsx
💤 Files with no reviewable changes (2)
- packages/visual-editor/src/editor/YextField.test.tsx
- packages/visual-editor/src/internal/puck/constant-value-fields/Video.tsx
auto-screenshot-update: true
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/visual-editor/src/fields/VideoField.tsx (1)
59-67:⚠️ Potential issue | 🟡 MinorGuard local-dev URL parsing before constructing the mock asset.
Line 64 can throw on malformed input, and Line 66 can yield an empty
videoId, producing invalid thumbnail/embed URLs. Bail out on parse failure or missingvbeforeonChange.Suggested fix
if (isFakeStarterLocalDev()) { const userInput = prompt("Enter Video URL:"); if (!userInput) { return; } - const url = new URL(userInput); - const searchParams = new URLSearchParams(url.search); - const videoId = searchParams.get("v") ?? ""; + let url: URL; + try { + url = new URL(userInput); + } catch { + return; + } + const videoId = url.searchParams.get("v"); + if (!videoId) { + return; + } onChange({ name: "Local asset", id: "0",#!/bin/bash # Verify whether local-dev URL parsing is guarded in VideoField. rg -n -C3 'isFakeStarterLocalDev\(\)|new URL\(|try\s*\{|searchParams\.get\("v"\)|url\.searchParams\.get\("v"\)' packages/visual-editor/src/fields/VideoField.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/visual-editor/src/fields/VideoField.tsx` around lines 59 - 67, Guard the local-dev URL parsing in VideoField: wrap the new URL(userInput) / URLSearchParams parsing in a try-catch around the block executed when isFakeStarterLocalDev() is true, and validate that the extracted videoId (from searchParams.get("v")) is non-empty before constructing the mock asset and calling onChange; if parsing fails or videoId is empty, bail out (return) instead of proceeding to create thumbnail/embed URLs.
🤖 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/fields/VideoField.tsx`:
- Around line 59-67: Guard the local-dev URL parsing in VideoField: wrap the new
URL(userInput) / URLSearchParams parsing in a try-catch around the block
executed when isFakeStarterLocalDev() is true, and validate that the extracted
videoId (from searchParams.get("v")) is non-empty before constructing the mock
asset and calling onChange; if parsing fails or videoId is empty, bail out
(return) instead of proceeding to create thumbnail/embed URLs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8aa12248-2bbd-45e3-9963-befd276d09ad
📒 Files selected for processing (1)
packages/visual-editor/src/fields/VideoField.tsx
asanehisa
requested review from
benlife5,
briantstephan,
jwartofsky-yext and
mkilpatrick
benlife5
approved these changes
Apr 28, 2026
mkilpatrick
approved these changes
Apr 28, 2026
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.
Confirmed Video works in fake starter