Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds a timer-based controls auto-hide system and optional cursor hiding to ChangesVideo Player Controls, Metadata Overlay, and Stream Preset Refactor
Sequence Diagram(s)sequenceDiagram
participant User
participant VideoPlayerContainer
participant VideoPlayer
participant RootContainer
participant AssetMetadataOverlay
User->>VideoPlayerContainer: renders on device
VideoPlayerContainer->>VideoPlayerContainer: useVideoPlayerControlsVisibility(disabled, isMobile)
VideoPlayerContainer->>VideoPlayer: spread controlsVisibility {controlsHideDelay, hideCursorOnIdle}
VideoPlayer->>RootContainer: pass controlsHideDelay, hideCursorOnIdle
RootContainer->>RootContainer: pointer move → showControls()
RootContainer->>RootContainer: pointer idle → hideControls(autoHide) → schedules timer
RootContainer->>RootContainer: timer fires → setIdle(true), controlsHidden=true
RootContainer->>RootContainer: hideCursorOnIdle=true → apply cursor-none
RootContainer->>AssetMetadataOverlay: renders inside controls overlay
AssetMetadataOverlay->>AssetMetadataOverlay: useAsset() → extract title/description → render gradient overlay
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/www/registry/default/blocks/video-player/components/playlist.tsx (1)
76-78: 💤 Low valueDouble type assertion masks underlying type mismatch.
The
as unknown as ComponentProps<...>pattern bypasses TypeScript's type checking. SinceDropdownMenuContentalready acceptscollisionBoundaryandcollisionPaddingprops, consider typingdropdownCollisionPropsexplicitly or investigating why the types don't align:♻️ Suggested fix
- const dropdownCollisionProps = { + const dropdownCollisionProps: Pick< + ComponentProps<typeof DropdownMenuContent>, + "collisionBoundary" | "collisionPadding" + > = { collisionBoundary: containerRef ?? undefined, collisionPadding: 12, }Then spread without the cast:
- {...(dropdownCollisionProps as unknown as ComponentProps< - typeof DropdownMenuContent - >)} + {...dropdownCollisionProps}🤖 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 `@apps/www/registry/default/blocks/video-player/components/playlist.tsx` around lines 76 - 78, The double type assertion using `as unknown as ComponentProps<typeof DropdownMenuContent>` bypasses TypeScript type checking for the `dropdownCollisionProps` object spread into the DropdownMenuContent component. Instead of using this double cast, explicitly type `dropdownCollisionProps` to match the props that DropdownMenuContent expects (such as collisionBoundary and collisionPadding), or ensure its source type definition aligns with ComponentProps<typeof DropdownMenuContent>. Once the types are properly aligned, remove the double assertion and spread the object directly without the cast.apps/www/registry/default/ui/root-container.tsx (1)
199-206: ⚡ Quick winCompose consumer handlers before internal handlers in event composition.
Running
internalHandlerfirst prevents consumers from influencing internal behavior viaevent.defaultPrevented, which weakens composability for this shared UI primitive.Suggested refactor
function composeEventHandlers<E extends React.SyntheticEvent>( consumerHandler: ((event: E) => void) | undefined, internalHandler: (event: E) => void ) { return (event: E) => { - internalHandler(event) consumerHandler?.(event) + if (!event.defaultPrevented) { + internalHandler(event) + } } }As per coding guidelines, “Never block event propagation — compose events.”
🤖 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 `@apps/www/registry/default/ui/root-container.tsx` around lines 199 - 206, In the composeEventHandlers function, reverse the order of handler execution so that consumerHandler is called before internalHandler. This allows consumers to set event.defaultPrevented and have that influence internal behavior, improving composability of this shared UI primitive. Change the return statement to call consumerHandler?.(event) first, followed by internalHandler(event).Source: Coding guidelines
🤖 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 `@apps/www/components/stream-panel/content-catalog.ts`:
- Line 45: The BlenderOpenFilmImages interface at line 45 now includes the
thumbnail field, but the BlenderOpenFilmImagesSchema Zod schema definition has
not been updated to match. Add the thumbnail field definition to
BlenderOpenFilmImagesSchema to ensure the schema properly validates and
preserves the thumbnail field when parsing Blender stream payloads, keeping the
schema and interface in sync.
In `@apps/www/lib/stream-presets.ts`:
- Line 42: The refactor in stream-presets.ts replaced the `name` property with a
required `title` property on line 42, but downstream code in
apps/www/components/stream-panel/presets-overlay.tsx still references the old
`preset.name` property, causing type errors and rendering issues. Update all
references to `preset.name` in the presets-overlay.tsx component to use
`preset.title` instead to align with the refactored contract.
In `@apps/www/registry/default/blocks/video-player/components/playlist.tsx`:
- Around line 128-133: The year display in the playlist component lacks
null-safety, causing inconsistency between the title attribute and rendered
content. When asset.year is undefined or null, String(asset.year) produces
"undefined" or "null" in the title attribute while {asset.year} renders nothing.
To fix this, add a null/undefined check and use a consistent fallback value
(such as an empty string or a placeholder) for both the title attribute and the
rendered content in the span element. This ensures the behavior is consistent
whether the year data is present or missing.
In `@apps/www/registry/default/ui/root-container.tsx`:
- Around line 152-157: The onBlur handler in the root-container is hiding
controls on every blur event, including when focus moves between child elements
within the container. To fix this, modify the onBlur handler to only call
hideControls() when focus actually exits the root container entirely. Check the
blur event's relatedTarget property to determine if focus is moving to an
element outside the container, and only hide controls if the relatedTarget is
null or not a descendant of the root container. Keep the showControls() call in
onFocus unchanged.
---
Nitpick comments:
In `@apps/www/registry/default/blocks/video-player/components/playlist.tsx`:
- Around line 76-78: The double type assertion using `as unknown as
ComponentProps<typeof DropdownMenuContent>` bypasses TypeScript type checking
for the `dropdownCollisionProps` object spread into the DropdownMenuContent
component. Instead of using this double cast, explicitly type
`dropdownCollisionProps` to match the props that DropdownMenuContent expects
(such as collisionBoundary and collisionPadding), or ensure its source type
definition aligns with ComponentProps<typeof DropdownMenuContent>. Once the
types are properly aligned, remove the double assertion and spread the object
directly without the cast.
In `@apps/www/registry/default/ui/root-container.tsx`:
- Around line 199-206: In the composeEventHandlers function, reverse the order
of handler execution so that consumerHandler is called before internalHandler.
This allows consumers to set event.defaultPrevented and have that influence
internal behavior, improving composability of this shared UI primitive. Change
the return statement to call consumerHandler?.(event) first, followed by
internalHandler(event).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 787fc539-31bb-4254-bca0-97cadc71a979
📒 Files selected for processing (9)
apps/www/components/blocks/preview-pane.tsxapps/www/components/players/video-player/player-container.tsxapps/www/components/stream-panel/content-catalog.tsapps/www/components/stream-panel/use-stream-panel-sync.tsapps/www/lib/stream-presets.tsapps/www/registry/default/blocks/video-player/components/asset-metadata-overlay.tsxapps/www/registry/default/blocks/video-player/components/media-player.tsxapps/www/registry/default/blocks/video-player/components/playlist.tsxapps/www/registry/default/ui/root-container.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 9 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
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
1 participant
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 by CodeRabbit
Release Notes