feat: split thumbnail previews#1760
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (36)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (33)
WalkthroughThis PR implements per-box SPLITS previews end-to-end: data model and utilities, server resolution via expectedPackages and MediaObject fallback, UI plumbing for boxPreviews and scrub settings, shared video-scrub utilities, checkerboard styling, and tests/documentation. ChangesSPLITS Box Previews Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/webui/src/client/lib/SplitPreviewBox.tsx`:
- Around line 48-50: The current JSX in SplitPreviewBox.tsx uses item.previewUrl
as an <img> fallback which is a video URL and causes broken images; change the
logic so the <img> is only rendered when item.thumbnailUrl exists (use
item.thumbnailUrl exclusively for the img src in the video-preview__image) and,
if you need to show a preview when thumbnailUrl is absent, render a <video>
element (or a dedicated video-preview component) using item.previewUrl as its
src/poster instead of passing previewUrl to the <img>. Target the conditional
rendering around item.thumbnailUrl / item.previewUrl in SplitPreviewBox.tsx to
implement this fallback change.
In `@packages/webui/src/client/lib/ui/videoPreviewScrub.ts`:
- Around line 8-15: The loop wrapping uses mismatched units (an extra *1000) and
doesn't guard against negative targetTime; compute the loop window in
milliseconds once using vEl.duration*1000 (call it loopWindowMs = itemDuration>0
? Math.min(vEl.duration*1000, itemDuration) : vEl.duration*1000), then wrap
targetTime into [0, loopWindowMs) with a proper modulus that handles negatives
(e.g. ((targetTime % loopWindowMs) + loopWindowMs) % loopWindowMs) when loop is
true; otherwise clamp targetTime to the [0, itemDuration] range if
itemDuration>0; finally set vEl.currentTime = targetTime / 1000. Use the
existing symbols targetTime, timePosition, seek, loop, itemDuration, and vEl to
locate the change.
In
`@packages/webui/src/client/ui/SegmentStoryboard/StoryboardPartThumbnail/StoryboardPartThumbnail.scss`:
- Line 64: The SCSS include uses empty parentheses which violates the
scss/at-mixin-argumentless-call-parentheses rule; update the include of the
mixin item-type-colors by removing the empty parentheses so the line uses an
argumentless mixin call (replace the `@include` item-type-colors(); usage with the
argumentless form) in StoryboardPartThumbnail.scss.
In
`@packages/webui/src/client/ui/Shelf/DashboardPieceButton/subcomponents/DashboardButtonTagStrip.scss`:
- Line 18: Move the `@import` of SplitButtonLayerBackground so it appears at the
very top of DashboardButtonTagStrip.scss (before any rules or declarations) and
change the import to use the partial format without the .scss extension (i.e.,
import the partial name only), ensuring the import line reads the partial
reference rather than importing the literal filename with extension.
In
`@packages/webui/src/client/ui/Shelf/DashboardPieceButton/usePreviewPopUpSession.ts`:
- Around line 46-50: The effect only updates when previewRequest.contents is
non-empty, leaving a stale popup open when contents become empty; modify the
useEffect that references previewSessionRef, previewRequest and update so that
if previewSessionRef.current exists and previewRequest.contents.length === 0 you
explicitly close/terminate the session (e.g., call
previewSessionRef.current.close() or the session's dispose method), otherwise
call previewSessionRef.current.update(previewRequest.contents) for non-empty
contents; keep the effect tied to [previewRequest] and guard
previewSessionRef.current before invoking methods.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c49e61c7-02a1-4670-b299-edb99d6e5fb2
📒 Files selected for processing (36)
meteor/server/publications/pieceContentStatusUI/__tests__/checkPieceContentStatus.test.tsmeteor/server/publications/pieceContentStatusUI/checkPieceContentStatus.tspackages/corelib/src/dataModel/PieceContentStatus.tspackages/documentation/docs/for-developers/for-blueprint-developers/splits-box-previews.mdpackages/shared-lib/src/package-manager/__tests__/splitBoxMedia.spec.tspackages/shared-lib/src/package-manager/splitBoxMedia.tspackages/webui/src/client/lib/SplitPreviewBox.tsxpackages/webui/src/client/lib/ui/splitPreview.tspackages/webui/src/client/lib/ui/splitsPreviewVideo.tspackages/webui/src/client/lib/ui/videoPreviewScrub.tspackages/webui/src/client/styles/_checkerboard.scsspackages/webui/src/client/styles/main.scsspackages/webui/src/client/styles/rundownView.scsspackages/webui/src/client/styles/shelf/dashboard.scsspackages/webui/src/client/ui/ClockView/CameraScreen/Piece.tsxpackages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.scsspackages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.tsxpackages/webui/src/client/ui/PreviewPopUp/PreviewPopUpContent.tsxpackages/webui/src/client/ui/PreviewPopUp/PreviewPopUpContext.tsxpackages/webui/src/client/ui/PreviewPopUp/Previews/BoxLayoutPreview.tsxpackages/webui/src/client/ui/PreviewPopUp/Previews/VTPreview.tsxpackages/webui/src/client/ui/SegmentContainer/getSplitItems.tsxpackages/webui/src/client/ui/SegmentList/LinePartMainPiece/LinePartMainPiece.tsxpackages/webui/src/client/ui/SegmentStoryboard/StoryboardPartSecondaryPieces/Renderers/SplitsRenderer.tsxpackages/webui/src/client/ui/SegmentStoryboard/StoryboardPartSecondaryPieces/StoryboardSecondaryPiece.tsxpackages/webui/src/client/ui/SegmentStoryboard/StoryboardPartThumbnail/Renderers/SplitsThumbnailRenderer.tsxpackages/webui/src/client/ui/SegmentStoryboard/StoryboardPartThumbnail/StoryboardPartThumbnail.scsspackages/webui/src/client/ui/SegmentStoryboard/StoryboardPartThumbnail/StoryboardPartThumbnailInner.tsxpackages/webui/src/client/ui/Shelf/DashboardPieceButton/DashboardPieceButton.tsxpackages/webui/src/client/ui/Shelf/DashboardPieceButton/subcomponents/DashboardButtonTagStrip.scsspackages/webui/src/client/ui/Shelf/DashboardPieceButton/subcomponents/MediaBox.tsxpackages/webui/src/client/ui/Shelf/DashboardPieceButton/subcomponents/SplitButtonLayerBackground.scsspackages/webui/src/client/ui/Shelf/DashboardPieceButton/subcomponents/SplitButtonLayerBackground.tsxpackages/webui/src/client/ui/Shelf/DashboardPieceButton/useDashboardButtonInteractions.tspackages/webui/src/client/ui/Shelf/DashboardPieceButton/usePreviewPopUpSession.tspackages/webui/src/client/ui/Shelf/DashboardPieceButtonSplitPreview.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/webui/src/client/lib/ui/videoPreviewScrub.ts (1)
12-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit clamping for the non-looping, unlimited-duration case.
When
!loopanditemDuration <= 0,targetTimecan remain negative ifseekis negative, leading tovEl.currentTimebeing assigned a negative value. While most browsers will clamp this to zero, it's cleaner to handle it explicitly.🛡️ Proposed fix to guard against negative currentTime
} else if (itemDuration > 0) { targetTime = Math.max(0, Math.min(targetTime, itemDuration)) + } else { + targetTime = Math.max(0, targetTime) }🤖 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/webui/src/client/lib/ui/videoPreviewScrub.ts` around lines 12 - 14, The code currently only clamps targetTime when itemDuration > 0, so in the non-looping/unlimited-duration case (when loop is false and itemDuration <= 0) a negative seek can leave targetTime negative and be assigned to vEl.currentTime; update the logic in the function handling scrubbing (look for targetTime, itemDuration, loop, seek, and vEl.currentTime) to explicitly clamp targetTime to at least 0 when !loop and itemDuration <= 0 before assigning to vEl.currentTime (e.g., ensure targetTime = Math.max(0, targetTime) in that branch).
🤖 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.
Duplicate comments:
In `@packages/webui/src/client/lib/ui/videoPreviewScrub.ts`:
- Around line 12-14: The code currently only clamps targetTime when itemDuration
> 0, so in the non-looping/unlimited-duration case (when loop is false and
itemDuration <= 0) a negative seek can leave targetTime negative and be assigned
to vEl.currentTime; update the logic in the function handling scrubbing (look
for targetTime, itemDuration, loop, seek, and vEl.currentTime) to explicitly
clamp targetTime to at least 0 when !loop and itemDuration <= 0 before assigning
to vEl.currentTime (e.g., ensure targetTime = Math.max(0, targetTime) in that
branch).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6414e90e-95ac-4cff-9a6b-5c5e93631a68
📒 Files selected for processing (5)
packages/webui/src/client/lib/SplitPreviewBox.tsxpackages/webui/src/client/lib/ui/videoPreviewScrub.tspackages/webui/src/client/ui/SegmentStoryboard/StoryboardPartThumbnail/StoryboardPartThumbnail.scsspackages/webui/src/client/ui/Shelf/DashboardPieceButton/subcomponents/DashboardButtonTagStrip.scsspackages/webui/src/client/ui/Shelf/DashboardPieceButton/usePreviewPopUpSession.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/webui/src/client/ui/Shelf/DashboardPieceButton/usePreviewPopUpSession.ts
- packages/webui/src/client/ui/SegmentStoryboard/StoryboardPartThumbnail/StoryboardPartThumbnail.scss
- packages/webui/src/client/ui/Shelf/DashboardPieceButton/subcomponents/DashboardButtonTagStrip.scss
a6753e2 to
94812f1
Compare
About the Contributor
This pull request is posted on behalf of the BBC
Type of Contribution
This is a: Feature
Current Behavior
SPLITS pieces don't per-box previews/thumbnails.
Hover preview “scrub” behavior for splits is not available (or wasn’t consistently driven by per-box preview URLs).
New Behavior
Core now publishes
status.boxPreviews[]for split pieces (aligned withboxSourceConfigurationorder), enabling per-box thumbnail/preview URLs. WebUI usesboxPreviewsto render SPLITS box media where applicable (notably dashboard shelf buttons and hover popups, including scrub whenpreviewUrlis present).2026-05-27.14-34-32.mp4
Testing
Affected areas
Time Frame
Not urgent, but we would like to get this merged into the in-development release.
Status