Conversation
…NES-1210) The Video Source accordion was showing the video title (e.g. "JESUS") instead of the source type. Now uses the existing videoBlockSourceToLabel helper to display "Jesus Film Library", "YouTube", or "Custom". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
WalkthroughUpdated the Video component to compute the video source label using a dedicated Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
View your CI Pipeline Execution ↗ for commit 4cc99f0
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Video/Video.spec.tsx (1)
82-82: Add coverage for the other source mappings (YouTubeandCustom).Line 82 validates only the internal/library case. Given this fix is source-mapping specific, adding cases for the other
VideoBlockSourcevalues will better protect against regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Video/Video.spec.tsx` at line 82, Add unit tests in Video.spec.tsx to cover the other VideoBlockSource mappings (YouTube and Custom) in addition to the existing internal/library case; update or add test cases that render the component with source set to VideoBlockSource.YouTube and VideoBlockSource.Custom and assert the expected UI text (e.g., the YouTube label and the custom label) using the same testing utilities (getByText/toBeInTheDocument) as the existing test so mapping changes will be caught by CI. Ensure you reference the same render helper/setup used in the file so the new tests target the same component instance and props as the original internal/library assertion.apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Video/Video.tsx (1)
13-13: Consider movingvideoBlockSourceToLabelto a shared video utility location.Line 13 imports from a
VisitorInfo/VisitorJourneysListpath, which tightly couples this editor property panel to another feature area. A shared block/video util module would keep boundaries cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Video/Video.tsx` at line 13, The import of videoBlockSourceToLabel in Video.tsx couples the editor to VisitorJourneysList; extract or move the videoBlockSourceToLabel function into a shared video utility module (e.g., a new shared/utils/video or shared/blocks/video util), update Video.tsx to import videoBlockSourceToLabel from that shared module, and update any other files that currently import it from VisitorInfo/VisitorJourneysList to reference the new shared location so all usages (including the Video component) use the centralized util.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Video/Video.spec.tsx`:
- Line 82: Add unit tests in Video.spec.tsx to cover the other VideoBlockSource
mappings (YouTube and Custom) in addition to the existing internal/library case;
update or add test cases that render the component with source set to
VideoBlockSource.YouTube and VideoBlockSource.Custom and assert the expected UI
text (e.g., the YouTube label and the custom label) using the same testing
utilities (getByText/toBeInTheDocument) as the existing test so mapping changes
will be caught by CI. Ensure you reference the same render helper/setup used in
the file so the new tests target the same component instance and props as the
original internal/library assertion.
In
`@apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Video/Video.tsx`:
- Line 13: The import of videoBlockSourceToLabel in Video.tsx couples the editor
to VisitorJourneysList; extract or move the videoBlockSourceToLabel function
into a shared video utility module (e.g., a new shared/utils/video or
shared/blocks/video util), update Video.tsx to import videoBlockSourceToLabel
from that shared module, and update any other files that currently import it
from VisitorInfo/VisitorJourneysList to reference the new shared location so all
usages (including the Video component) use the centralized util.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 537058ba-39ee-4134-9d0e-ff0956e248d6
📒 Files selected for processing (2)
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Video/Video.spec.tsxapps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/blocks/Video/Video.tsx
Contributor
|
The latest updates on your projects.
|
…plays-video-title-instead-of-source
Ur-imazing
approved these changes
Apr 7, 2026
jianwei1
deleted the
jianweichong/nes-1210-video-source-label-displays-video-title-instead-of-source
branch
tanflem
pushed a commit
that referenced
this pull request
Apr 13, 2026
…NES-1210) (#8968) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
2 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.
Summary
videoBlockSourceToLabelhelper to show "Jesus Film Library", "YouTube", or "Custom" based onblock.sourceTest plan
Closes NES-1210
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes