feat: blocks revamp, registry cleanup#157
Open
WINOFFRG wants to merge 4 commits into
Open
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 (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR adds catalog modules, generalizes asset typing to ChangesPlayer architecture refactor
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
16 issues found across 96 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/www/lib/catalogs/utils.ts">
<violation number="1" location="apps/www/lib/catalogs/utils.ts:10">
P2: Missing feature guard for `AbortSignal.any` can crash combined-signal path. Add a fallback when `any` is unavailable.</violation>
<violation number="2" location="apps/www/lib/catalogs/utils.ts:16">
P2: Abort helper discards original abort reason. Re-throw `signal.reason` to preserve timeout/custom cancellation semantics.</violation>
</file>
<file name="apps/www/lib/catalogs/blender-open-films.ts">
<violation number="1" location="apps/www/lib/catalogs/blender-open-films.ts:154">
P2: `assetId` is interpolated into a URL path segment without encoding. Encode it to avoid malformed requests/path traversal via special characters in ids.</violation>
</file>
<file name="apps/www/content/docs/components/player-layout.mdx">
<violation number="1" location="apps/www/content/docs/components/player-layout.mdx:47">
P3: Doc drops `idle`-state detection and accessibility focus mentions that the component still implements. The component reads `idle`/`forceIdle` from `useMediaStore`, sets `data-idle`, `role="region"`, `aria-label`, and `focus-visible` outline classes — none of this is reflected in the new description.</violation>
</file>
<file name="apps/www/registry/default/blocks/audio-player/player.tsx">
<violation number="1" location="apps/www/registry/default/blocks/audio-player/player.tsx:94">
P2: `theme="light"` cannot override a dark parent because the stylesheet also targets `.dark .limeplay`. Consumers in a dark app will still get dark audio-player variables despite passing the new light theme.</violation>
</file>
<file name="apps/www/registry/default/examples/volume-slider-control-horizontal-demo.tsx">
<violation number="1" location="apps/www/registry/default/examples/volume-slider-control-horizontal-demo.tsx:9">
P3: `hit-area` is redundant when `hit-area-x-[12px]` and `hit-area-y-[2px]` are already present — both of those utilities already apply `position: relative` and generate the `::before` pseudo-element. Adds unnecessary CSS weight.</violation>
</file>
<file name="apps/www/registry/default/blocks/audio-player/styles.css">
<violation number="1" location="apps/www/registry/default/blocks/audio-player/styles.css:49">
P1: `--secondary` and `--secondary-foreground` produce near-zero contrast in the dark theme (light gray bg + near-white text, ~1.23:1, far below WCAG AA 4.5:1). In the light theme, secondary is pure black — an outlier vs. `--muted`/`--accent` at `0.94`. The lightness values appear swapped between themes.</violation>
</file>
<file name="apps/www/content/docs/hooks/use-controls-visibility.mdx">
<violation number="1" location="apps/www/content/docs/hooks/use-controls-visibility.mdx:13">
P2: Broken link: `/docs/hooks/use-media` does not exist. The Callout references a documentation page for `mediaFeature` that would resolve to a 404. Either add the missing use-media.mdx or fix the link to point to existing documentation (e.g., the registry file reference or an external API doc).</violation>
</file>
<file name="apps/www/registry/default/blocks/video-player/components/bottom-controls.tsx">
<violation number="1" location="apps/www/registry/default/blocks/video-player/components/bottom-controls.tsx:53">
P2: Playback rate control was dropped from the rendered bottom controls while its component/dependencies remain in the video-player block, making speed selection inaccessible in this block. Re-add `<PlaybackRateControl />` to the control group or remove the unused registry entries/dependencies if the feature is intentionally removed.</violation>
</file>
<file name="apps/www/registry/default/ui/player-layout.tsx">
<violation number="1" location="apps/www/registry/default/ui/player-layout.tsx:71">
P2: ControlsOverlayContainer lost its default fade styling, so documented/default usage renders an invisible overlay. Keep the structural ref support but preserve the gradient/visibility classes for callers that don't pass a custom className.</violation>
</file>
<file name="apps/www/lib/catalogs/player-assets.ts">
<violation number="1" location="apps/www/lib/catalogs/player-assets.ts:74">
P2: Do not fall back from `poster` to `thumbnail` here: `thumbnail` is a VTT track URL in `StreamPreset`, not an image poster. Presets without `poster` will render broken poster images or pass a VTT URL as the video poster.</violation>
</file>
<file name="apps/www/registry/default/lib/utils.ts">
<violation number="1" location="apps/www/registry/default/lib/utils.ts:3">
P2: Add `cnfast` to the `utils` registry item dependencies, or consumers installing registry components will get an unresolved `cnfast` import.</violation>
</file>
<file name="apps/www/registry/default/hooks/use-controls-visibility.ts">
<violation number="1" location="apps/www/registry/default/hooks/use-controls-visibility.ts:45">
P2: Treat `ended` like other non-playing states so controls become visible when playback finishes. Otherwise an idle video can end with replay/timeline controls still hidden until pointer movement.</violation>
</file>
<file name="apps/www/lib/catalogs/apple-music.ts">
<violation number="1" location="apps/www/lib/catalogs/apple-music.ts:154">
P2: Validate derived storefronts before sending them. Numeric/invalid locale regions like `en-001` produce `001`, causing chart requests to use an unsupported storefront instead of `us`.</violation>
</file>
<file name="apps/www/registry/default/blocks/video-player/components/playback-rate-control.tsx">
<violation number="1" location="apps/www/registry/default/blocks/video-player/components/playback-rate-control.tsx:28">
P1: Removed `dark` class from portal-rendered `SelectContent`. Since this component uses `SelectPrimitive.Portal`, the popup is rendered at the end of `document.body` — outside any `.dark` ancestor. Without the `dark` class, the dropdown will render with light-mode colors (`bg-popover`, `text-popover-foreground`) when the user's system is in light mode, creating a visual mismatch against the dark-themed video player UI.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| --popover-foreground: oklch(0.985 0 0); | ||
| --primary: oklch(0.87 0 0); | ||
| --primary-foreground: oklch(0.205 0 0); | ||
| --secondary: oklch(0.7889 0 0); |
There was a problem hiding this comment.
P1: --secondary and --secondary-foreground produce near-zero contrast in the dark theme (light gray bg + near-white text, ~1.23:1, far below WCAG AA 4.5:1). In the light theme, secondary is pure black — an outlier vs. --muted/--accent at 0.94. The lightness values appear swapped between themes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/www/registry/default/blocks/audio-player/styles.css, line 49:
<comment>`--secondary` and `--secondary-foreground` produce near-zero contrast in the dark theme (light gray bg + near-white text, ~1.23:1, far below WCAG AA 4.5:1). In the light theme, secondary is pure black — an outlier vs. `--muted`/`--accent` at `0.94`. The lightness values appear swapped between themes.</comment>
<file context>
@@ -0,0 +1,74 @@
+ --popover-foreground: oklch(0.985 0 0);
+ --primary: oklch(0.87 0 0);
+ --primary-foreground: oklch(0.205 0 0);
+ --secondary: oklch(0.7889 0 0);
+ --secondary-foreground: oklch(0.985 0 0);
+ --muted: oklch(0.269 0 0);
</file context>
| <Select.SelectContent | ||
| align="start" | ||
| className={`dark min-w-28 border border-border bg-background/85 backdrop-blur-lg`} | ||
| className={`min-w-28 border border-border backdrop-blur-lg`} |
There was a problem hiding this comment.
P1: Removed dark class from portal-rendered SelectContent. Since this component uses SelectPrimitive.Portal, the popup is rendered at the end of document.body — outside any .dark ancestor. Without the dark class, the dropdown will render with light-mode colors (bg-popover, text-popover-foreground) when the user's system is in light mode, creating a visual mismatch against the dark-themed video player UI.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/www/registry/default/blocks/video-player/components/playback-rate-control.tsx, line 28:
<comment>Removed `dark` class from portal-rendered `SelectContent`. Since this component uses `SelectPrimitive.Portal`, the popup is rendered at the end of `document.body` — outside any `.dark` ancestor. Without the `dark` class, the dropdown will render with light-mode colors (`bg-popover`, `text-popover-foreground`) when the user's system is in light mode, creating a visual mismatch against the dark-themed video player UI.</comment>
<file context>
@@ -5,22 +5,27 @@ import React from "react"
<Select.SelectContent
align="start"
- className={`dark min-w-28 border border-border bg-background/85 backdrop-blur-lg`}
+ className={`min-w-28 border border-border backdrop-blur-lg`}
side="top"
sideOffset={12}
</file context>
Suggested change
| className={`min-w-28 border border-border backdrop-blur-lg`} | |
| className={`dark min-w-28 border border-border bg-background/85 backdrop-blur-lg`} |
| const timeoutSignal = AbortSignal.timeout(timeoutMs) | ||
| if (!signal) return timeoutSignal | ||
|
|
||
| return AbortSignal.any([signal, timeoutSignal]) |
There was a problem hiding this comment.
P2: Missing feature guard for AbortSignal.any can crash combined-signal path. Add a fallback when any is unavailable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/www/lib/catalogs/utils.ts, line 10:
<comment>Missing feature guard for `AbortSignal.any` can crash combined-signal path. Add a fallback when `any` is unavailable.</comment>
<file context>
@@ -0,0 +1,17 @@
+ const timeoutSignal = AbortSignal.timeout(timeoutMs)
+ if (!signal) return timeoutSignal
+
+ return AbortSignal.any([signal, timeoutSignal])
+}
+
</file context>
| export function throwIfAborted(signal?: AbortSignal): void { | ||
| if (!signal?.aborted) return | ||
|
|
||
| throw new DOMException("Aborted", "AbortError") |
There was a problem hiding this comment.
P2: Abort helper discards original abort reason. Re-throw signal.reason to preserve timeout/custom cancellation semantics.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/www/lib/catalogs/utils.ts, line 16:
<comment>Abort helper discards original abort reason. Re-throw `signal.reason` to preserve timeout/custom cancellation semantics.</comment>
<file context>
@@ -0,0 +1,17 @@
+export function throwIfAborted(signal?: AbortSignal): void {
+ if (!signal?.aborted) return
+
+ throw new DOMException("Aborted", "AbortError")
+}
</file context>
| styles.dark, | ||
| "relative z-50 h-18 w-full border-t border-border bg-background", | ||
| "limeplay relative z-50 h-18 w-full border-t border-border bg-background", | ||
| theme === "dark" && "dark", |
There was a problem hiding this comment.
P2: theme="light" cannot override a dark parent because the stylesheet also targets .dark .limeplay. Consumers in a dark app will still get dark audio-player variables despite passing the new light theme.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/www/registry/default/blocks/audio-player/player.tsx, line 94:
<comment>`theme="light"` cannot override a dark parent because the stylesheet also targets `.dark .limeplay`. Consumers in a dark app will still get dark audio-player variables despite passing the new light theme.</comment>
<file context>
@@ -71,14 +90,16 @@ export const AudioPlayer = React.forwardRef<HTMLDivElement, AudioPlayerProps>(
- styles.dark,
- "relative z-50 h-18 w-full border-t border-border bg-background",
+ "limeplay relative z-50 h-18 w-full border-t border-border bg-background",
+ theme === "dark" && "dark",
className
)}
</file context>
| export function cn(...inputs: ClassValue[]) { | ||
| return twMerge(clsx(inputs)) | ||
| } | ||
| export { cn } from "cnfast" |
There was a problem hiding this comment.
P2: Add cnfast to the utils registry item dependencies, or consumers installing registry components will get an unresolved cnfast import.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/www/registry/default/lib/utils.ts, line 3:
<comment>Add `cnfast` to the `utils` registry item dependencies, or consumers installing registry components will get an unresolved `cnfast` import.</comment>
<file context>
@@ -1,11 +1,6 @@
-export function cn(...inputs: ClassValue[]) {
- return twMerge(clsx(inputs))
-}
+export { cn } from "cnfast"
/**
</file context>
| idle && | ||
| status !== "buffering" && | ||
| status !== "error" && | ||
| status !== "paused" |
There was a problem hiding this comment.
P2: Treat ended like other non-playing states so controls become visible when playback finishes. Otherwise an idle video can end with replay/timeline controls still hidden until pointer movement.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/www/registry/default/hooks/use-controls-visibility.ts, line 45:
<comment>Treat `ended` like other non-playing states so controls become visible when playback finishes. Otherwise an idle video can end with replay/timeline controls still hidden until pointer movement.</comment>
<file context>
@@ -0,0 +1,144 @@
+ idle &&
+ status !== "buffering" &&
+ status !== "error" &&
+ status !== "paused"
+
+ const clearHideTimer = React.useCallback(() => {
</file context>
|
|
||
| return { | ||
| locale, | ||
| storefront: country?.toLowerCase() ?? DEFAULT_APPLE_MUSIC_STOREFRONT, |
There was a problem hiding this comment.
P2: Validate derived storefronts before sending them. Numeric/invalid locale regions like en-001 produce 001, causing chart requests to use an unsupported storefront instead of us.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/www/lib/catalogs/apple-music.ts, line 154:
<comment>Validate derived storefronts before sending them. Numeric/invalid locale regions like `en-001` produce `001`, causing chart requests to use an unsupported storefront instead of `us`.</comment>
<file context>
@@ -0,0 +1,208 @@
+
+ return {
+ locale,
+ storefront: country?.toLowerCase() ?? DEFAULT_APPLE_MUSIC_STOREFRONT,
+ }
+}
</file context>
| ### RootContainer | ||
|
|
||
| Outermost wrapper. Manages idle state detection and focus management for accessibility. | ||
| Outermost wrapper. Provides the player region, aspect ratio, shared refs, and root data attributes for child layout styling. |
There was a problem hiding this comment.
P3: Doc drops idle-state detection and accessibility focus mentions that the component still implements. The component reads idle/forceIdle from useMediaStore, sets data-idle, role="region", aria-label, and focus-visible outline classes — none of this is reflected in the new description.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/www/content/docs/components/player-layout.mdx, line 47:
<comment>Doc drops `idle`-state detection and accessibility focus mentions that the component still implements. The component reads `idle`/`forceIdle` from `useMediaStore`, sets `data-idle`, `role="region"`, `aria-label`, and `focus-visible` outline classes — none of this is reflected in the new description.</comment>
<file context>
@@ -44,7 +44,7 @@ export function MediaPlayer() {
### RootContainer
-Outermost wrapper. Manages idle state detection and focus management for accessibility.
+Outermost wrapper. Provides the player region, aspect ratio, shared refs, and root data attributes for child layout styling.
### PlayerContainer
</file context>
Suggested change
| Outermost wrapper. Provides the player region, aspect ratio, shared refs, and root data attributes for child layout styling. | |
| Outermost wrapper. Provides the player region, aspect ratio, shared refs, root data attributes for child layout styling. Manages idle state detection and accessibility focus management. |
| <VolumeStateControlDemo /> | ||
| <VolumeSlider.Root | ||
| className="focus-area relative h-1 w-16 cursor-crosshair rounded-md -focus-area-x-12 -focus-area-y-2" | ||
| className="hit-area-x-[12px] hit-area-y-[2px] hit-area relative h-1 w-16 cursor-crosshair rounded-md" |
There was a problem hiding this comment.
P3: hit-area is redundant when hit-area-x-[12px] and hit-area-y-[2px] are already present — both of those utilities already apply position: relative and generate the ::before pseudo-element. Adds unnecessary CSS weight.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/www/registry/default/examples/volume-slider-control-horizontal-demo.tsx, line 9:
<comment>`hit-area` is redundant when `hit-area-x-[12px]` and `hit-area-y-[2px]` are already present — both of those utilities already apply `position: relative` and generate the `::before` pseudo-element. Adds unnecessary CSS weight.</comment>
<file context>
@@ -6,7 +6,7 @@ export function VolumeSliderControlHorizontalDemo() {
<VolumeStateControlDemo />
<VolumeSlider.Root
- className="focus-area relative h-1 w-16 cursor-crosshair rounded-md -focus-area-x-12 -focus-area-y-2"
+ className="hit-area-x-[12px] hit-area-y-[2px] hit-area relative h-1 w-16 cursor-crosshair rounded-md"
orientation="horizontal"
>
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/www/components/hero-buttons.tsx (1)
21-30: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a real button for the copy CTA.
Line 21 renders the install action as a clickable
motion.div, so it isn't keyboard-focusable and won't get Enter/Space button semantics.focus-ringstyling doesn't help unless the element can receive focus.♿ Proposed fix
- <motion.div + <motion.button + type="button" className={` group relative flex h-10 cursor-pointer items-center justify-center overflow-hidden rounded-xl bg-foreground/15 font-mono text-foreground/80 focus-ring backdrop-blur-xs md:h-12 `} initial={{ padding: "0px 20px" }} onClick={() => { copyToClipboard(command) }} @@ - </motion.div> + </motion.button>Also applies to: 80-80
🤖 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/components/hero-buttons.tsx` around lines 21 - 30, The copy CTA in hero-buttons is implemented as a clickable motion.div, so it lacks native button semantics and keyboard accessibility. Update the install/copy action in the relevant motion wrapper to use a real button element while preserving the existing styling and motion behavior, and keep the copyToClipboard handler attached so it remains clickable and keyboard-activatable with Enter/Space.apps/www/registry/default/hooks/use-asset.ts (1)
795-887: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftRemove the public
useAsset()convenience hook from this feature module.This path is supposed to expose selector hooks only, but this change keeps extending
useAsset()as a public wrapper API. Please move consumers touseAssetStore(...)/playlist selectors and keep any composition helper internal instead.As per path instructions,
apps/www/registry/default/hooks/**:No convenience hooks should exist — only useXxxStore selectors.🤖 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/hooks/use-asset.ts` around lines 795 - 887, The public useAsset() wrapper in use-asset.ts should be removed because this feature module is meant to expose only selector hooks. Delete the exported useAsset compositional hook, and move any needed composition logic into internal helpers while keeping callers on useAssetStore(...) and the existing playlist selectors like usePlaylist<TItem>(). Ensure no public convenience API remains under this hooks module.Source: Path instructions
🧹 Nitpick comments (2)
apps/www/registry/default/ui/player-layout.tsx (1)
61-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve
asChildsupport on the refactored layout containers.The changed containers are fixed to
<div>, so consumers cannot slot their own wrapper elements. AddasChild?: boolean, selectSlotwhen enabled, and render children through that component. As per coding guidelines,apps/www/registry/default/ui/**/*.{ts,tsx}: “SupportasChildprop via@radix-ui/react-slotin components.” As per path instructions,apps/www/registry/default/ui/**: “Ensure asChild support via@radix-ui/react-slot.”Also applies to: 81-117
🤖 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/player-layout.tsx` around lines 61 - 75, The refactored layout containers are hardcoded to render a div, which removes existing slotting behavior. Update the relevant container components in player-layout.tsx (including ControlsOverlayContainer and the other affected layout wrappers) to accept asChild?: boolean, import and use Slot from `@radix-ui/react-slot` when asChild is true, and otherwise keep rendering a div; make sure children are passed through the selected component so consumers can provide their own wrapper elements.Sources: Coding guidelines, Path instructions
apps/www/registry/default/blocks/video-player/player.tsx (1)
158-159: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse a granular asset selector here.
CurrentAssetMediaonly needs the current poster, butuseAsset()subscribes the whole asset slice and will re-render this media surface on unrelated asset changes. Pull just the poster through the store selector instead. As per coding guidelines,Always use granular per-feature selectors like useXxxStore(s => s.field) for state access—never access entire slices.🤖 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/player.tsx` around lines 158 - 159, The CurrentAssetMedia logic in player.tsx is subscribing to the entire video asset slice via useAsset<VideoPlayerAsset>(), which causes unnecessary re-renders on unrelated asset updates. Replace this with a granular store selector that reads only the current poster field needed by currentPoster, and keep the rest of the component using that single selected value rather than destructuring currentItem from the whole asset.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/content/docs/hooks/use-asset.mdx`:
- Around line 85-93: The later `loadSource` example is inconsistent with the
narrowed `TAsset` contract described in the docs. Update the example near
`loadSource` so it either explicitly extends `TAsset` with the extra metadata
like `title`, or uses only the base `id`/`src`/`config` fields. Keep the
`useAsset` and `loadSource` examples aligned with the `TAsset` description so
the page remains self-consistent.
In `@apps/www/content/docs/hooks/use-controls-visibility.mdx`:
- Around line 6-53: The hook docs are missing required sections between
Installation and Usage/API Reference, so add a Feature Registration section for
the dependencies used by useControlsVisibility, plus explicit Store and Events
sections with State/Actions and event tables; if this hook exposes no store or
events, state that clearly instead of leaving them out. Keep the existing
useControlsVisibility, CONTROLS_FORCE_VISIBLE_ATTRIBUTE, and AutoTypeTable
content, but reorganize the page to match the standard hook-doc structure used
elsewhere.
In `@apps/www/lib/catalogs/blender-open-films.ts`:
- Around line 136-145: `fetchBlenderOpenFilmAssets()` is missing the same
timeout handling used by the other catalog fetch paths, so a stalled request can
hang longer than expected. Update this function to create and use
`createTimeoutSignal()` internally (matching the pattern in the other catalog
loaders) and ensure the timeout signal is passed to the `fetch` call alongside
any caller-provided `signal`. Keep the change localized to
`fetchBlenderOpenFilmAssets` so `useStreamPanelSync.loadPlaylistPreset()`
benefits from the same timeout behavior.
In `@apps/www/registry/collection/registry-blocks.ts`:
- Around line 19-25: The registry mapping for the video-player and audio-player
blocks places player.tsx at components/player.tsx while styles.css is installed
under components/<block>/styles.css, so the relative import in player.tsx
resolves incorrectly. Update the block definitions in registry-blocks.ts so each
block’s player.tsx and styles.css are colocated under the same
components/<block>/ path, or otherwise ensure the stylesheet target matches the
import used by player.tsx.
In `@apps/www/registry/collection/registry-hooks.ts`:
- Around line 39-50: The `use-controls-visibility` registry entry is being
published under the default hooks surface, but this hook is a utility rather
than a feature hook. Move the entry in `registry-hooks.ts` to a non-feature
registry location, or expose it through a compliant store-based API instead of
`apps/www/registry/default/hooks/**`. Keep the registry metadata aligned with
the repo pattern by using a feature-only hook/store symbol such as `useXxxStore`
or `xxxFeature` for anything that remains in the default hooks area.
In `@apps/www/registry/default/blocks/audio-player/player.tsx`:
- Around line 74-75: The `AudioPlayer` theme override is incomplete:
`theme="light"` only affects the local class on the player, but `styles.css`
still applies dark variables through the `.dark .limeplay` selector. Update the
`AudioPlayer`/`player.tsx` theme handling and the related CSS so the `theme`
prop can explicitly force light mode even when rendered inside a dark ancestor,
likely by adding a stronger theme-specific selector or an override class that
neutralizes `.dark .limeplay`.
In
`@apps/www/registry/default/blocks/video-player/components/player-error-screen.tsx`:
- Around line 61-66: The retry button in PlayerErrorScreen only appears when
currentItem exists, so initial loadSource failures can leave users stuck on the
error screen. Update the render condition in player-error-screen.tsx so the
Retry action is shown for source-load failures as well, using the existing
retryStream handler and the retry-from-source state already supported earlier in
the component instead of gating solely on currentItem.
In
`@apps/www/registry/default/blocks/video-player/components/player-root-container.tsx`:
- Around line 31-35: The RootContainer props in PlayerRootContainer are being
overwritten because rootProps are spread before controlsVisibility.rootProps, so
any caller-supplied pointer or focus handlers become no-ops. Update
PlayerRootContainer to compose the handlers from rootProps with
controlsVisibility.rootProps instead of replacing them, using the
RootContainerProps passthrough exposed by PlayerRootContainerProps so both sets
of handlers run.
In
`@apps/www/registry/default/blocks/video-player/components/timeline-slider-control.tsx`:
- Around line 95-119: The live status logic in timeline-slider-control should
not rely on truthiness because liveLatency can be 0 when fully caught up. Update
the two conditional branches around the LiveLatency / “Go to live” UI and the
“LIVE” badge to explicitly check for null or undefined instead of using
`liveLatency && ...`, so `liveLatency === 0` still renders the correct live
indicator.
In `@apps/www/registry/default/examples/picture-in-picture-control-demo.tsx`:
- Line 5: The example is importing Button from the app-level components path
instead of the registry source, which breaks the rule that registry examples
must use internal source imports. Update the import in
picture-in-picture-control-demo to reference the Button from
`@/registry/default/`... so the demo stays aligned with the shipped registry code
and styling; use the existing Button symbol in this example as the locator.
In `@apps/www/registry/default/hooks/use-controls-visibility.ts`:
- Around line 28-137: The issue is that useControlsVisibility is exposed under
the registry feature-hook path as a standalone convenience hook, which violates
the contract that this folder should only contain feature-pattern
hooks/selectors. Update the useControlsVisibility export so it either becomes a
proper feature hook backed by the expected MediaFeature<XxxStore> selector
pattern, or move it out of the registry/default/hooks area entirely; keep the
behavior logic in the same symbols (useControlsVisibility, hideControls,
showControls) but adjust the API to match the feature-hook rules before
publishing it through the registry.
In `@apps/www/registry/default/hooks/use-playback-source.ts`:
- Around line 27-39: `usePlaybackSource()` is being exported as a public
convenience hook, which violates the selector-only hook policy for this hooks
module. Make `usePlaybackSource` module-private by removing its export, and keep
`PlaybackSourceController` as the only public entry point that invokes it
internally. Update the `usePlaybackSource` and `PlaybackSourceController`
symbols together so the file only exposes the intended controller/selector
pattern.
In `@apps/www/registry/default/hooks/use-timeline.ts`:
- Around line 218-230: The live latency calculation in useTimeline is clamping
against the absolute seekRange.end instead of the seek window size, which can
overstate latency when seekRange.start is non-zero. Update the liveLatency clamp
in useTimeline to use seekRangeSize as the upper bound, alongside the existing
seekRange and toFixedNumber logic, so the latency stays bounded to the current
seek window.
---
Outside diff comments:
In `@apps/www/components/hero-buttons.tsx`:
- Around line 21-30: The copy CTA in hero-buttons is implemented as a clickable
motion.div, so it lacks native button semantics and keyboard accessibility.
Update the install/copy action in the relevant motion wrapper to use a real
button element while preserving the existing styling and motion behavior, and
keep the copyToClipboard handler attached so it remains clickable and
keyboard-activatable with Enter/Space.
In `@apps/www/registry/default/hooks/use-asset.ts`:
- Around line 795-887: The public useAsset() wrapper in use-asset.ts should be
removed because this feature module is meant to expose only selector hooks.
Delete the exported useAsset compositional hook, and move any needed composition
logic into internal helpers while keeping callers on useAssetStore(...) and the
existing playlist selectors like usePlaylist<TItem>(). Ensure no public
convenience API remains under this hooks module.
---
Nitpick comments:
In `@apps/www/registry/default/blocks/video-player/player.tsx`:
- Around line 158-159: The CurrentAssetMedia logic in player.tsx is subscribing
to the entire video asset slice via useAsset<VideoPlayerAsset>(), which causes
unnecessary re-renders on unrelated asset updates. Replace this with a granular
store selector that reads only the current poster field needed by currentPoster,
and keep the rest of the component using that single selected value rather than
destructuring currentItem from the whole asset.
In `@apps/www/registry/default/ui/player-layout.tsx`:
- Around line 61-75: The refactored layout containers are hardcoded to render a
div, which removes existing slotting behavior. Update the relevant container
components in player-layout.tsx (including ControlsOverlayContainer and the
other affected layout wrappers) to accept asChild?: boolean, import and use Slot
from `@radix-ui/react-slot` when asChild is true, and otherwise keep rendering a
div; make sure children are passed through the selected component so consumers
can provide their own wrapper elements.
🪄 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: 0bda5579-86c5-4808-ac9b-d7b54b600f5a
⛔ Files ignored due to path filters (6)
README.mdis excluded by none and included by noneapps/www/eslint.config.mjsis excluded by none and included by noneapps/www/next-env.d.tsis excluded by none and included by noneapps/www/package.jsonis excluded by none and included by noneapps/www/scripts/test-registry-install.tsis excluded by none and included by nonebun.lockis excluded by!**/*.lockand included by none
📒 Files selected for processing (90)
apps/www/app/(home)/layout.tsxapps/www/app/(home)/page.tsxapps/www/app/eslint.cssapps/www/app/global.cssapps/www/app/limeplay.cssapps/www/components/blocks/block-page-shell.tsxapps/www/components/blocks/block-showcase.tsxapps/www/components/blocks/block-stream-sync.tsxapps/www/components/blocks/block-toolbar.tsxapps/www/components/component-preview.tsxapps/www/components/hero-buttons.tsxapps/www/components/hero.tsxapps/www/components/immersive-scroll-player.tsxapps/www/components/player-root-demo.tsxapps/www/components/players/audio-player/demo-player.tsxapps/www/components/players/audio-player/demo.tsapps/www/components/players/video-player/player-container.tsxapps/www/components/stream-panel/content-catalog.tsapps/www/components/stream-panel/panel-popover.config.tsapps/www/components/stream-panel/panel-popover.tsxapps/www/components/stream-panel/playlists-overlay.tsxapps/www/components/stream-panel/provider.tsxapps/www/components/stream-panel/use-stream-panel-sync.tsapps/www/content/docs/blocks/audio-player.mdxapps/www/content/docs/blocks/video-player.mdxapps/www/content/docs/components/player-layout.mdxapps/www/content/docs/components/timeline-control.mdxapps/www/content/docs/components/volume-control.mdxapps/www/content/docs/concepts.mdxapps/www/content/docs/hooks/index.mdxapps/www/content/docs/hooks/meta.jsonapps/www/content/docs/hooks/use-asset.mdxapps/www/content/docs/hooks/use-controls-visibility.mdxapps/www/content/docs/hooks/use-playback-source.mdxapps/www/content/docs/usage.mdxapps/www/lib/catalogs/apple-music.tsapps/www/lib/catalogs/blender-open-films.tsapps/www/lib/catalogs/index.tsapps/www/lib/catalogs/player-assets.tsapps/www/lib/catalogs/playlists.tsapps/www/lib/catalogs/types.tsapps/www/lib/catalogs/utils.tsapps/www/lib/stream-presets.tsapps/www/lib/utils.tsapps/www/registry/collection/registry-blocks.tsapps/www/registry/collection/registry-examples.tsapps/www/registry/collection/registry-hooks.tsapps/www/registry/collection/registry-ui.tsapps/www/registry/default/blocks/audio-player/audio-player.module.cssapps/www/registry/default/blocks/audio-player/components/audio-source.tsxapps/www/registry/default/blocks/audio-player/components/fixed-timeline-control.tsxapps/www/registry/default/blocks/audio-player/components/playlist.tsxapps/www/registry/default/blocks/audio-player/components/track-info.tsxapps/www/registry/default/blocks/audio-player/hooks/use-playlist-asset.tsapps/www/registry/default/blocks/audio-player/player.tsxapps/www/registry/default/blocks/audio-player/styles.cssapps/www/registry/default/blocks/video-player/components/asset-metadata-overlay.tsxapps/www/registry/default/blocks/video-player/components/bottom-controls.tsxapps/www/registry/default/blocks/video-player/components/button.tsxapps/www/registry/default/blocks/video-player/components/captions-state-control.tsxapps/www/registry/default/blocks/video-player/components/media-player.tsxapps/www/registry/default/blocks/video-player/components/pip-control.tsxapps/www/registry/default/blocks/video-player/components/playback-rate-control.tsxapps/www/registry/default/blocks/video-player/components/player-error-screen.tsxapps/www/registry/default/blocks/video-player/components/player-root-container.tsxapps/www/registry/default/blocks/video-player/components/playlist-navigation-controls.tsxapps/www/registry/default/blocks/video-player/components/playlist.tsxapps/www/registry/default/blocks/video-player/components/timeline-slider-control.tsxapps/www/registry/default/blocks/video-player/components/top-overlay-container.tsxapps/www/registry/default/blocks/video-player/components/volume-group-control.tsxapps/www/registry/default/blocks/video-player/components/volume-slider-control.tsxapps/www/registry/default/blocks/video-player/page.tsxapps/www/registry/default/blocks/video-player/player.tsxapps/www/registry/default/blocks/video-player/styles.cssapps/www/registry/default/examples/picture-in-picture-control-demo.tsxapps/www/registry/default/examples/timeline-control-demo.tsxapps/www/registry/default/examples/timeline-labels-demo.tsxapps/www/registry/default/examples/volume-slider-control-horizontal-demo.tsxapps/www/registry/default/examples/volume-slider-control-vertical-demo.tsxapps/www/registry/default/hooks/use-asset.tsapps/www/registry/default/hooks/use-controls-visibility.tsapps/www/registry/default/hooks/use-playback-source.tsapps/www/registry/default/hooks/use-timeline.tsapps/www/registry/default/lib/utils.tsapps/www/registry/default/ui/player-layout.tsxapps/www/registry/default/ui/root-container.tsxapps/www/registry/default/ui/seek-controls.tsxapps/www/registry/default/ui/timeline-control.tsxapps/www/registry/default/ui/timeline-labels.tsxapps/www/registry/default/ui/volume-control.tsx
💤 Files with no reviewable changes (9)
- apps/www/registry/default/blocks/video-player/page.tsx
- apps/www/registry/default/blocks/audio-player/audio-player.module.css
- apps/www/registry/default/blocks/video-player/components/media-player.tsx
- apps/www/components/blocks/block-toolbar.tsx
- apps/www/app/limeplay.css
- apps/www/registry/default/blocks/video-player/components/asset-metadata-overlay.tsx
- apps/www/components/stream-panel/content-catalog.ts
- apps/www/lib/stream-presets.ts
- apps/www/registry/collection/registry-ui.ts
Comment on lines
+6
to
+53
| ## Installation | ||
|
|
||
| ```npm | ||
| npx shadcn add @limeplay/use-controls-visibility | ||
| ``` | ||
|
|
||
| <Callout> | ||
| Requires [`mediaFeature`](/docs/hooks/use-media) and | ||
| [`playbackFeature`](/docs/hooks/use-playback) registered in your | ||
| [`createMediaKit`](/docs/components/media-provider). | ||
| </Callout> | ||
|
|
||
| ## Usage | ||
|
|
||
| ```tsx | ||
| import { | ||
| CONTROLS_FORCE_VISIBLE_ATTRIBUTE, | ||
| useControlsVisibility, | ||
| } from "@/hooks/limeplay/use-controls-visibility" | ||
|
|
||
| export function PlayerShell() { | ||
| const controlsVisibility = useControlsVisibility() | ||
|
|
||
| return ( | ||
| <div | ||
| {...controlsVisibility.rootProps} | ||
| className={controlsVisibility.className} | ||
| > | ||
| <video /> | ||
|
|
||
| <div {...{ [CONTROLS_FORCE_VISIBLE_ATTRIBUTE]: "" }}> | ||
| <button>Play</button> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } | ||
| ``` | ||
|
|
||
| `rootProps` should be spread on the player root. Add | ||
| `CONTROLS_FORCE_VISIBLE_ATTRIBUTE` to any controls area that should keep controls | ||
| visible while hovered or scrubbed. | ||
|
|
||
| ## API Reference | ||
|
|
||
| <AutoTypeTable | ||
| path="./registry/default/hooks/use-controls-visibility.ts" | ||
| name="UseControlsVisibilityOptions" | ||
| /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the missing required hook-doc sections.
This page jumps from Installation to Usage/API Reference, so it misses the required Feature Registration section plus the Store and Events tables used across hook docs. If this hook has no store/events, call that out explicitly instead of omitting the sections. As per coding guidelines, "Every hook doc needs: Installation, Feature registration, Store (State + Actions tables), Events table, AutoTypeTable."
🧰 Tools
🪛 LanguageTool
[style] ~44-~44: To form a complete sentence, be sure to include a subject.
Context: .../div> ) } ``` rootProps should be spread on the player root. Add `CONT...
(MISSING_IT_THERE)
🤖 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/content/docs/hooks/use-controls-visibility.mdx` around lines 6 - 53,
The hook docs are missing required sections between Installation and Usage/API
Reference, so add a Feature Registration section for the dependencies used by
useControlsVisibility, plus explicit Store and Events sections with
State/Actions and event tables; if this hook exposes no store or events, state
that clearly instead of leaving them out. Keep the existing
useControlsVisibility, CONTROLS_FORCE_VISIBLE_ATTRIBUTE, and AutoTypeTable
content, but reorganize the page to match the standard hook-doc structure used
elsewhere.
Source: Coding guidelines
Comment on lines
+136
to
+145
| export async function fetchBlenderOpenFilmAssets( | ||
| signal?: AbortSignal | ||
| ): Promise<BlenderOpenFilmAsset[]> { | ||
| const response = await fetch(`${BLENDER_API_BASE_URL}/playlist`, { signal }) | ||
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch Blender playlist: ${response.statusText}`) | ||
| } | ||
|
|
||
| const playlist = BlenderPlaylistResponseSchema.parse(await response.json()) | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wrap the Blender playlist fetch in the same timeout path.
fetchBlenderOpenFilmAssets() is the only catalog request here without createTimeoutSignal(). useStreamPanelSync.loadPlaylistPreset() waits on this from a user action, so a stalled worker can leave the playlist load hanging far longer than the rest of the catalog flows.
Proposed fix
export async function fetchBlenderOpenFilmAssets(
signal?: AbortSignal
): Promise<BlenderOpenFilmAsset[]> {
- const response = await fetch(`${BLENDER_API_BASE_URL}/playlist`, { signal })
+ const combinedSignal = createTimeoutSignal(signal, BLENDER_STREAM_TIMEOUT_MS)
+ const response = await fetch(`${BLENDER_API_BASE_URL}/playlist`, {
+ signal: combinedSignal,
+ })
if (!response.ok) {
throw new Error(`Failed to fetch Blender playlist: ${response.statusText}`)
}📝 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
| export async function fetchBlenderOpenFilmAssets( | |
| signal?: AbortSignal | |
| ): Promise<BlenderOpenFilmAsset[]> { | |
| const response = await fetch(`${BLENDER_API_BASE_URL}/playlist`, { signal }) | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch Blender playlist: ${response.statusText}`) | |
| } | |
| const playlist = BlenderPlaylistResponseSchema.parse(await response.json()) | |
| export async function fetchBlenderOpenFilmAssets( | |
| signal?: AbortSignal | |
| ): Promise<BlenderOpenFilmAsset[]> { | |
| const combinedSignal = createTimeoutSignal(signal, BLENDER_STREAM_TIMEOUT_MS) | |
| const response = await fetch(`${BLENDER_API_BASE_URL}/playlist`, { | |
| signal: combinedSignal, | |
| }) | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch Blender playlist: ${response.statusText}`) | |
| } | |
| const playlist = BlenderPlaylistResponseSchema.parse(await response.json()) |
🤖 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/lib/catalogs/blender-open-films.ts` around lines 136 - 145,
`fetchBlenderOpenFilmAssets()` is missing the same timeout handling used by the
other catalog fetch paths, so a stalled request can hang longer than expected.
Update this function to create and use `createTimeoutSignal()` internally
(matching the pattern in the other catalog loaders) and ensure the timeout
signal is passed to the `fetch` call alongside any caller-provided `signal`.
Keep the change localized to `fetchBlenderOpenFilmAssets` so
`useStreamPanelSync.loadPlaylistPreset()` benefits from the same timeout
behavior.
Comment on lines
+39
to
+50
| { | ||
| files: [ | ||
| { | ||
| path: "hooks/use-controls-visibility.ts", | ||
| target: `${TARGET_BASE_PATH}/use-controls-visibility.ts`, | ||
| type: "registry:hook", | ||
| }, | ||
| ], | ||
| name: "use-controls-visibility", | ||
| registryDependencies: ["use-media", "use-playback", "media-provider"], | ||
| type: "registry:hook", | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Don't publish this as a registry/default/hooks convenience hook.
use-controls-visibility is documented as a utility hook, but apps/www/registry/default/hooks/** is reserved for xxxFeature() / useXxxStore() feature hooks. Keeping it in the hooks registry makes the public surface contradict the repo's own hook pattern; move it to a non-feature location or expose it through a compliant store API instead. As per path instructions, "apps/www/registry/default/hooks/**: These are Limeplay feature hooks using the feature pattern (xxxFeature → MediaFeature)" and "No convenience hooks should exist — only useXxxStore selectors."
🤖 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/collection/registry-hooks.ts` around lines 39 - 50, The
`use-controls-visibility` registry entry is being published under the default
hooks surface, but this hook is a utility rather than a feature hook. Move the
entry in `registry-hooks.ts` to a non-feature registry location, or expose it
through a compliant store-based API instead of
`apps/www/registry/default/hooks/**`. Keep the registry metadata aligned with
the repo pattern by using a feature-only hook/store symbol such as `useXxxStore`
or `xxxFeature` for anything that remains in the default hooks area.
Source: Path instructions
Comment on lines
+95
to
+119
| {liveLatency && player && liveLatency >= LIVE_DELAY_VISIBLE_SEC && ( | ||
| <> | ||
| <LiveLatency className="text-xs font-medium" /> | ||
| <Button | ||
| aria-label="Go to live" | ||
| className=" | ||
| cursor-pointer px-2 | ||
| @md/root:px-2.5 | ||
| @3xl/root:px-3 | ||
| " | ||
| onClick={() => void player.goToLive()} | ||
| size="sm" | ||
| variant="glass" | ||
| > | ||
| <span className="text-xs font-medium text-primary"> | ||
| Go to live | ||
| </span> | ||
| </Button> | ||
| </> | ||
| )} | ||
| {liveLatency && liveLatency < LIVE_DELAY_VISIBLE_SEC && ( | ||
| <div className="flex items-center rounded-full bg-red-600 px-2 py-0.5 text-xs font-semibold tracking-wide"> | ||
| <span className="tracking-widest">LIVE</span> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle liveLatency === 0.
Both branches use liveLatency && ..., so the live indicator disappears when the stream is exactly caught up and liveLatency becomes 0. Check for null/undefined explicitly instead of relying on truthiness.
Suggested fix
- {liveLatency && player && liveLatency >= LIVE_DELAY_VISIBLE_SEC && (
+ {liveLatency != null &&
+ player &&
+ liveLatency >= LIVE_DELAY_VISIBLE_SEC && (
<>
<LiveLatency className="text-xs font-medium" />
<Button
@@
- {liveLatency && liveLatency < LIVE_DELAY_VISIBLE_SEC && (
+ {liveLatency != null && liveLatency < LIVE_DELAY_VISIBLE_SEC && (
<div className="flex items-center rounded-full bg-red-600 px-2 py-0.5 text-xs font-semibold tracking-wide">
<span className="tracking-widest">LIVE</span>
</div>
)}📝 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
| {liveLatency && player && liveLatency >= LIVE_DELAY_VISIBLE_SEC && ( | |
| <> | |
| <LiveLatency className="text-xs font-medium" /> | |
| <Button | |
| aria-label="Go to live" | |
| className=" | |
| cursor-pointer px-2 | |
| @md/root:px-2.5 | |
| @3xl/root:px-3 | |
| " | |
| onClick={() => void player.goToLive()} | |
| size="sm" | |
| variant="glass" | |
| > | |
| <span className="text-xs font-medium text-primary"> | |
| Go to live | |
| </span> | |
| </Button> | |
| </> | |
| )} | |
| {liveLatency && liveLatency < LIVE_DELAY_VISIBLE_SEC && ( | |
| <div className="flex items-center rounded-full bg-red-600 px-2 py-0.5 text-xs font-semibold tracking-wide"> | |
| <span className="tracking-widest">LIVE</span> | |
| </div> | |
| )} | |
| {liveLatency != null && | |
| player && | |
| liveLatency >= LIVE_DELAY_VISIBLE_SEC && ( | |
| <> | |
| <LiveLatency className="text-xs font-medium" /> | |
| <Button | |
| aria-label="Go to live" | |
| className=" | |
| cursor-pointer px-2 | |
| `@md/root`:px-2.5 | |
| `@3xl/root`:px-3 | |
| " | |
| onClick={() => void player.goToLive()} | |
| size="sm" | |
| variant="glass" | |
| > | |
| <span className="text-xs font-medium text-primary"> | |
| Go to live | |
| </span> | |
| </Button> | |
| </> | |
| )} | |
| {liveLatency != null && liveLatency < LIVE_DELAY_VISIBLE_SEC && ( | |
| <div className="flex items-center rounded-full bg-red-600 px-2 py-0.5 text-xs font-semibold tracking-wide"> | |
| <span className="tracking-widest">LIVE</span> | |
| </div> | |
| )} |
🤖 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/timeline-slider-control.tsx`
around lines 95 - 119, The live status logic in timeline-slider-control should
not rely on truthiness because liveLatency can be 0 when fully caught up. Update
the two conditional branches around the LiveLatency / “Go to live” UI and the
“LIVE” badge to explicitly check for null or undefined instead of using
`liveLatency && ...`, so `liveLatency === 0` still renders the correct live
indicator.
| import { PictureInPictureIcon } from "@phosphor-icons/react" | ||
|
|
||
| import { Button } from "@/registry/default/blocks/video-player/components/button" | ||
| import { Button } from "@/components/ui/button" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep registry examples on @/registry/default/... imports.
This switches the demo to an app-level Button, so the example no longer reflects registry source code and can drift from the shipped player control styling. Import the button from @/registry/default/... here instead.
Suggested fix
-import { Button } from "`@/components/ui/button`"
+import { Button } from "`@/registry/default/blocks/video-player/components/button`"As per coding guidelines, "Examples use @/registry/default/... imports (they are internal source code)."
📝 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
| import { Button } from "@/components/ui/button" | |
| import { Button } from "`@/registry/default/blocks/video-player/components/button`" |
🤖 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/examples/picture-in-picture-control-demo.tsx` at
line 5, The example is importing Button from the app-level components path
instead of the registry source, which breaks the rule that registry examples
must use internal source imports. Update the import in
picture-in-picture-control-demo to reference the Button from
`@/registry/default/`... so the demo stays aligned with the shipped registry code
and styling; use the existing Button symbol in this example as the locator.
Source: Coding guidelines
Comment on lines
+28
to
+137
| export function useControlsVisibility({ | ||
| controlsHideDelay = 2000, | ||
| hideCursorOnIdle = true, | ||
| }: UseControlsVisibilityOptions = {}) { | ||
| const debug = useMediaStore((state) => state.debug) | ||
| const forceIdle = useMediaStore((state) => state.forceIdle) | ||
| const idle = useMediaStore((state) => state.idle) | ||
| const setIdle = useMediaStore((state) => state.setIdle) | ||
| const status = usePlaybackStore((state) => state.status) | ||
| const hideTimerRef = React.useRef<null | number>(null) | ||
| const autoHide = controlsHideDelay > 0 | ||
| const controlsHidden = | ||
| !debug && | ||
| !forceIdle && | ||
| idle && | ||
| status !== "buffering" && | ||
| status !== "error" && | ||
| status !== "paused" | ||
|
|
||
| const clearHideTimer = React.useCallback(() => { | ||
| if (hideTimerRef.current === null) return | ||
|
|
||
| window.clearTimeout(hideTimerRef.current) | ||
| hideTimerRef.current = null | ||
| }, []) | ||
|
|
||
| const hideControls = React.useCallback(() => { | ||
| if (forceIdle) return | ||
|
|
||
| clearHideTimer() | ||
|
|
||
| if (autoHide) { | ||
| hideTimerRef.current = window.setTimeout(() => { | ||
| setIdle(true) | ||
| hideTimerRef.current = null | ||
| }, controlsHideDelay) | ||
| return | ||
| } | ||
|
|
||
| setIdle(true) | ||
| }, [autoHide, clearHideTimer, controlsHideDelay, forceIdle, setIdle]) | ||
|
|
||
| const showControls = React.useCallback( | ||
| (options?: { autoHide?: boolean }) => { | ||
| if (forceIdle) return | ||
|
|
||
| clearHideTimer() | ||
| setIdle(false) | ||
|
|
||
| if (options?.autoHide) { | ||
| hideControls() | ||
| } | ||
| }, | ||
| [clearHideTimer, forceIdle, hideControls, setIdle] | ||
| ) | ||
|
|
||
| React.useEffect(() => clearHideTimer, [clearHideTimer]) | ||
|
|
||
| React.useEffect(() => { | ||
| if (forceIdle) { | ||
| clearHideTimer() | ||
| } | ||
| }, [clearHideTimer, forceIdle]) | ||
|
|
||
| const rootProps = React.useMemo<RootInteractionProps>( | ||
| () => ({ | ||
| onBlur: (event) => { | ||
| const relatedTarget = event.relatedTarget | ||
| if ( | ||
| relatedTarget instanceof Node && | ||
| event.currentTarget.contains(relatedTarget) | ||
| ) { | ||
| return | ||
| } | ||
|
|
||
| hideControls() | ||
| }, | ||
| onFocus: () => { | ||
| showControls() | ||
| }, | ||
| onPointerEnter: () => { | ||
| showControls({ autoHide }) | ||
| }, | ||
| onPointerLeave: () => { | ||
| hideControls() | ||
| }, | ||
| onPointerMove: (event) => { | ||
| showControls({ | ||
| autoHide: autoHide && !isKeepVisibleTarget(event.target), | ||
| }) | ||
| }, | ||
| onPointerOver: (event) => { | ||
| if (isKeepVisibleTarget(event.target)) { | ||
| showControls() | ||
| } | ||
| }, | ||
| onPointerUp: (event) => { | ||
| showControls({ | ||
| autoHide: autoHide && !isKeepVisibleTarget(event.target), | ||
| }) | ||
| }, | ||
| }), | ||
| [autoHide, hideControls, showControls] | ||
| ) | ||
|
|
||
| return { | ||
| className: hideCursorOnIdle && controlsHidden ? "cursor-none" : undefined, | ||
| rootProps, | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Align this registry hook with the feature-hook contract.
useControlsVisibility is a standalone convenience hook in registry/default/hooks, but this path is reserved for feature hooks/selectors. Move it outside the registry hook feature path, or model it as a feature with the expected store-selector API before publishing it through the registry. As per path instructions, apps/www/registry/default/hooks/**: “These are Limeplay feature hooks using the feature pattern (xxxFeature → MediaFeature)” and “No convenience hooks should exist — only useXxxStore selectors.”
🤖 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/hooks/use-controls-visibility.ts` around lines 28 -
137, The issue is that useControlsVisibility is exposed under the registry
feature-hook path as a standalone convenience hook, which violates the contract
that this folder should only contain feature-pattern hooks/selectors. Update the
useControlsVisibility export so it either becomes a proper feature hook backed
by the expected MediaFeature<XxxStore> selector pattern, or move it out of the
registry/default/hooks area entirely; keep the behavior logic in the same
symbols (useControlsVisibility, hideControls, showControls) but adjust the API
to match the feature-hook rules before publishing it through the registry.
Source: Path instructions
Comment on lines
+27
to
+39
| export type UsePlaybackSourceOptions<TItem extends TAsset> = | ||
| PlaybackSourceControllerProps<TItem> | ||
|
|
||
| export function PlaybackSourceController<TItem extends TAsset>( | ||
| props: PlaybackSourceControllerProps<TItem> | ||
| ) { | ||
| usePlaybackSource(props) | ||
|
|
||
| return null | ||
| } | ||
|
|
||
| export function usePlaybackSource<TAsset extends Asset>( | ||
| options: UsePlaybackSourceOptions<TAsset> | ||
| export function usePlaybackSource<TItem extends TAsset>( | ||
| options: UsePlaybackSourceOptions<TItem> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Keep usePlaybackSource() internal.
This change cements another public convenience hook in a path that should only expose selector hooks. If PlaybackSourceController remains the public entry point, make usePlaybackSource() module-private and avoid exporting it.
As per path instructions, apps/www/registry/default/hooks/**: No convenience hooks should exist — only useXxxStore selectors.
🤖 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/hooks/use-playback-source.ts` around lines 27 - 39,
`usePlaybackSource()` is being exported as a public convenience hook, which
violates the selector-only hook policy for this hooks module. Make
`usePlaybackSource` module-private by removing its export, and keep
`PlaybackSourceController` as the only public entry point that invokes it
internally. Update the `usePlaybackSource` and `PlaybackSourceController`
symbols together so the file only exposes the intended controller/selector
pattern.
Source: Path instructions
Comment on lines
+218
to
+230
| const seekRangeSize = seekRange.end - seekRange.start | ||
|
|
||
| liveLatency = | ||
| mediaElement.currentTime === 0 | ||
| ? 0 | ||
| : seekRange.end - mediaElement.currentTime | ||
|
|
||
| liveLatency = toFixedNumber(clamp(liveLatency, 0, seekRange.end), 4) | ||
|
|
||
| const seekRangeSize = seekRange.end - seekRange.start | ||
| currentTime = clamp( | ||
| mediaElement.currentTime - seekRange.start, | ||
| 0, | ||
| seekRangeSize | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clamp live latency to the seek-window size.
liveLatency represents distance from the live edge, so clamping it to absolute seekRange.end can overstate latency when seekRange.start is non-zero. Use seekRangeSize as the upper bound.
Proposed fix
- liveLatency = toFixedNumber(clamp(liveLatency, 0, seekRange.end), 4)
+ liveLatency = toFixedNumber(clamp(liveLatency, 0, seekRangeSize), 4)📝 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 seekRangeSize = seekRange.end - seekRange.start | |
| liveLatency = | |
| mediaElement.currentTime === 0 | |
| ? 0 | |
| : seekRange.end - mediaElement.currentTime | |
| liveLatency = toFixedNumber(clamp(liveLatency, 0, seekRange.end), 4) | |
| const seekRangeSize = seekRange.end - seekRange.start | |
| currentTime = clamp( | |
| mediaElement.currentTime - seekRange.start, | |
| 0, | |
| seekRangeSize | |
| ) | |
| const seekRangeSize = seekRange.end - seekRange.start | |
| liveLatency = | |
| mediaElement.currentTime === 0 | |
| ? 0 | |
| : seekRange.end - mediaElement.currentTime | |
| liveLatency = toFixedNumber(clamp(liveLatency, 0, seekRangeSize), 4) | |
| currentTime = clamp( | |
| mediaElement.currentTime - seekRange.start, | |
| 0, | |
| seekRangeSize | |
| ) |
🤖 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/hooks/use-timeline.ts` around lines 218 - 230, The
live latency calculation in useTimeline is clamping against the absolute
seekRange.end instead of the seek window size, which can overstate latency when
seekRange.start is non-zero. Update the liveLatency clamp in useTimeline to use
seekRangeSize as the upper bound, alongside the existing seekRange and
toFixedNumber logic, so the latency stays bounded to the current seek window.
nd override error
rohangupta-zomato
force-pushed
the
feat/blocks-revamp
branch
from
8755cc6 to
a13d468
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/www/registry/default/ui/player-layout.tsx (1)
103-109: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore top-overlay positioning.
TopOverlayContaineronly passes spacing/animation classes. After removing the absolute/top rules from this wrapper, it joins normal flex layout and starts consuming vertical space instead of overlaying the media.🛠️ Proposed fix
- className={cn("pointer-events-auto w-full", className)} + className={cn("pointer-events-auto absolute inset-x-0 top-0 w-full", className)}🤖 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/player-layout.tsx` around lines 103 - 109, Restore the overlay behavior in ControlsTopContainer by putting the top-positioning classes back on the wrapper so it stays absolutely positioned over the media instead of participating in normal layout. Update the className composition in ControlsTopContainer to include the removed absolute/top/inset-style rules along with the existing spacing and animation classes, and keep the rest of the forwarded props unchanged.
🧹 Nitpick comments (1)
apps/www/registry/default/ui/player-layout.tsx (1)
61-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
asChildsupport to these headless wrappers.
ControlsOverlayContainer,ControlsBottomContainer, andControlsTopContainerstill hard-codediv, which breaks the UI-component contract for this path and limits downstream composition. As per coding guidelines and path instructions, "Always supportasChildvia@radix-ui/react-slot" and "Ensure asChild support via@radix-ui/react-slot."🤖 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/player-layout.tsx` around lines 61 - 109, `ControlsOverlayContainer`, `ControlsBottomContainer`, and `ControlsTopContainer` are still hard-coded to render divs, so update these wrappers to follow the headless UI contract by adding `asChild` support using `@radix-ui/react-slot`. Keep the existing component APIs and class merging, but conditionally render a Slot when `asChild` is true and a div otherwise, preserving refs and props in each of the forwardRef components. Use the symbols `ControlsOverlayContainer`, `ControlsBottomContainer`, and `ControlsTopContainer` to apply the change consistently.Sources: Coding guidelines, Path instructions
🤖 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/registry/default/ui/player-layout.tsx`:
- Line 18: Keep the container query root class on PlayerContainer in
player-layout.tsx; the wrapper currently defines the named container needed by
descendant `@3xl/root` and `@5xl/root` variants. Restore or preserve the container
class alongside the existing layout classes so the responsive width/layout rules
in the children continue to apply, and verify the PlayerContainer component
still exposes the named root used by those descendants.
---
Outside diff comments:
In `@apps/www/registry/default/ui/player-layout.tsx`:
- Around line 103-109: Restore the overlay behavior in ControlsTopContainer by
putting the top-positioning classes back on the wrapper so it stays absolutely
positioned over the media instead of participating in normal layout. Update the
className composition in ControlsTopContainer to include the removed
absolute/top/inset-style rules along with the existing spacing and animation
classes, and keep the rest of the forwarded props unchanged.
---
Nitpick comments:
In `@apps/www/registry/default/ui/player-layout.tsx`:
- Around line 61-109: `ControlsOverlayContainer`, `ControlsBottomContainer`, and
`ControlsTopContainer` are still hard-coded to render divs, so update these
wrappers to follow the headless UI contract by adding `asChild` support using
`@radix-ui/react-slot`. Keep the existing component APIs and class merging, but
conditionally render a Slot when `asChild` is true and a div otherwise,
preserving refs and props in each of the forwardRef components. Use the symbols
`ControlsOverlayContainer`, `ControlsBottomContainer`, and
`ControlsTopContainer` to apply the change consistently.
🪄 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: 93f22e39-5e29-4b57-88ac-2b051ffc0e40
⛔ Files ignored due to path filters (5)
README.mdis excluded by none and included by noneapps/www/eslint.config.mjsis excluded by none and included by noneapps/www/package.jsonis excluded by none and included by noneapps/www/scripts/test-registry-install.tsis excluded by none and included by nonebun.lockis excluded by!**/*.lockand included by none
📒 Files selected for processing (92)
apps/www/app/(home)/layout.tsxapps/www/app/(home)/page.tsxapps/www/app/docs/docs.cssapps/www/app/eslint.cssapps/www/app/global.cssapps/www/app/limeplay.cssapps/www/components/blocks/block-page-shell.tsxapps/www/components/blocks/block-showcase.tsxapps/www/components/blocks/block-stream-sync.tsxapps/www/components/blocks/block-toolbar.tsxapps/www/components/component-preview.tsxapps/www/components/hero-buttons.tsxapps/www/components/hero.tsxapps/www/components/immersive-scroll-player.tsxapps/www/components/player-root-demo.tsxapps/www/components/players/audio-player/demo-player.tsxapps/www/components/players/audio-player/demo.tsapps/www/components/players/video-player/player-container.tsxapps/www/components/stream-panel/content-catalog.tsapps/www/components/stream-panel/panel-popover.config.tsapps/www/components/stream-panel/panel-popover.tsxapps/www/components/stream-panel/playlists-overlay.tsxapps/www/components/stream-panel/provider.tsxapps/www/components/stream-panel/use-stream-panel-sync.tsapps/www/content/docs/blocks/audio-player.mdxapps/www/content/docs/blocks/video-player.mdxapps/www/content/docs/components/player-layout.mdxapps/www/content/docs/components/timeline-control.mdxapps/www/content/docs/components/volume-control.mdxapps/www/content/docs/concepts.mdxapps/www/content/docs/hooks/index.mdxapps/www/content/docs/hooks/meta.jsonapps/www/content/docs/hooks/use-asset.mdxapps/www/content/docs/hooks/use-controls-visibility.mdxapps/www/content/docs/hooks/use-playback-source.mdxapps/www/content/docs/usage.mdxapps/www/lib/catalogs/apple-music.tsapps/www/lib/catalogs/blender-open-films.tsapps/www/lib/catalogs/index.tsapps/www/lib/catalogs/player-assets.tsapps/www/lib/catalogs/playlists.tsapps/www/lib/catalogs/types.tsapps/www/lib/catalogs/utils.tsapps/www/lib/stream-presets.tsapps/www/lib/utils.tsapps/www/registry/collection/registry-blocks.tsapps/www/registry/collection/registry-examples.tsapps/www/registry/collection/registry-hooks.tsapps/www/registry/collection/registry-lib.tsapps/www/registry/collection/registry-ui.tsapps/www/registry/default/blocks/audio-player/audio-player.module.cssapps/www/registry/default/blocks/audio-player/components/audio-source.tsxapps/www/registry/default/blocks/audio-player/components/fixed-timeline-control.tsxapps/www/registry/default/blocks/audio-player/components/playlist.tsxapps/www/registry/default/blocks/audio-player/components/track-info.tsxapps/www/registry/default/blocks/audio-player/hooks/use-playlist-asset.tsapps/www/registry/default/blocks/audio-player/player.tsxapps/www/registry/default/blocks/audio-player/styles.cssapps/www/registry/default/blocks/video-player/components/asset-metadata-overlay.tsxapps/www/registry/default/blocks/video-player/components/bottom-controls.tsxapps/www/registry/default/blocks/video-player/components/button.tsxapps/www/registry/default/blocks/video-player/components/captions-state-control.tsxapps/www/registry/default/blocks/video-player/components/media-player.tsxapps/www/registry/default/blocks/video-player/components/pip-control.tsxapps/www/registry/default/blocks/video-player/components/playback-rate-control.tsxapps/www/registry/default/blocks/video-player/components/player-error-screen.tsxapps/www/registry/default/blocks/video-player/components/player-root-container.tsxapps/www/registry/default/blocks/video-player/components/playlist-navigation-controls.tsxapps/www/registry/default/blocks/video-player/components/playlist.tsxapps/www/registry/default/blocks/video-player/components/timeline-slider-control.tsxapps/www/registry/default/blocks/video-player/components/top-overlay-container.tsxapps/www/registry/default/blocks/video-player/components/volume-group-control.tsxapps/www/registry/default/blocks/video-player/components/volume-slider-control.tsxapps/www/registry/default/blocks/video-player/page.tsxapps/www/registry/default/blocks/video-player/player.tsxapps/www/registry/default/blocks/video-player/styles.cssapps/www/registry/default/examples/picture-in-picture-control-demo.tsxapps/www/registry/default/examples/timeline-control-demo.tsxapps/www/registry/default/examples/timeline-labels-demo.tsxapps/www/registry/default/examples/volume-slider-control-horizontal-demo.tsxapps/www/registry/default/examples/volume-slider-control-vertical-demo.tsxapps/www/registry/default/hooks/use-asset.tsapps/www/registry/default/hooks/use-controls-visibility.tsapps/www/registry/default/hooks/use-playback-source.tsapps/www/registry/default/hooks/use-timeline.tsapps/www/registry/default/lib/utils.tsapps/www/registry/default/ui/player-layout.tsxapps/www/registry/default/ui/root-container.tsxapps/www/registry/default/ui/seek-controls.tsxapps/www/registry/default/ui/timeline-control.tsxapps/www/registry/default/ui/timeline-labels.tsxapps/www/registry/default/ui/volume-control.tsx
💤 Files with no reviewable changes (10)
- apps/www/components/blocks/block-toolbar.tsx
- apps/www/lib/stream-presets.ts
- apps/www/app/docs/docs.css
- apps/www/registry/default/blocks/video-player/page.tsx
- apps/www/registry/default/blocks/video-player/components/asset-metadata-overlay.tsx
- apps/www/app/limeplay.css
- apps/www/registry/default/blocks/audio-player/audio-player.module.css
- apps/www/components/stream-panel/content-catalog.ts
- apps/www/registry/default/blocks/video-player/components/media-player.tsx
- apps/www/registry/collection/registry-ui.ts
✅ Files skipped from review due to trivial changes (26)
- apps/www/registry/default/blocks/video-player/components/captions-state-control.tsx
- apps/www/content/docs/concepts.mdx
- apps/www/registry/default/examples/volume-slider-control-vertical-demo.tsx
- apps/www/content/docs/hooks/meta.json
- apps/www/registry/default/examples/timeline-labels-demo.tsx
- apps/www/lib/catalogs/index.ts
- apps/www/content/docs/blocks/audio-player.mdx
- apps/www/content/docs/components/volume-control.mdx
- apps/www/content/docs/hooks/use-asset.mdx
- apps/www/components/stream-panel/panel-popover.config.ts
- apps/www/registry/default/blocks/audio-player/components/playlist.tsx
- apps/www/registry/default/ui/volume-control.tsx
- apps/www/app/(home)/layout.tsx
- apps/www/app/eslint.css
- apps/www/content/docs/blocks/video-player.mdx
- apps/www/content/docs/components/player-layout.mdx
- apps/www/components/component-preview.tsx
- apps/www/content/docs/components/timeline-control.mdx
- apps/www/components/stream-panel/panel-popover.tsx
- apps/www/registry/default/ui/timeline-labels.tsx
- apps/www/components/hero-buttons.tsx
- apps/www/registry/default/blocks/audio-player/hooks/use-playlist-asset.ts
- apps/www/components/stream-panel/playlists-overlay.tsx
- apps/www/registry/default/examples/picture-in-picture-control-demo.tsx
- apps/www/content/docs/hooks/index.mdx
- apps/www/components/blocks/block-page-shell.tsx
🚧 Files skipped from review as they are similar to previous changes (49)
- apps/www/components/players/audio-player/demo.ts
- apps/www/registry/default/examples/timeline-control-demo.tsx
- apps/www/registry/default/blocks/audio-player/components/track-info.tsx
- apps/www/content/docs/hooks/use-playback-source.mdx
- apps/www/registry/default/blocks/video-player/components/pip-control.tsx
- apps/www/registry/default/blocks/audio-player/components/fixed-timeline-control.tsx
- apps/www/components/blocks/block-stream-sync.tsx
- apps/www/registry/default/blocks/video-player/components/playlist.tsx
- apps/www/registry/default/blocks/video-player/components/top-overlay-container.tsx
- apps/www/components/stream-panel/provider.tsx
- apps/www/registry/default/blocks/video-player/components/playlist-navigation-controls.tsx
- apps/www/components/blocks/block-showcase.tsx
- apps/www/components/players/audio-player/demo-player.tsx
- apps/www/registry/default/blocks/video-player/components/volume-group-control.tsx
- apps/www/registry/default/blocks/video-player/components/playback-rate-control.tsx
- apps/www/content/docs/usage.mdx
- apps/www/lib/catalogs/types.ts
- apps/www/registry/collection/registry-hooks.ts
- apps/www/registry/default/lib/utils.ts
- apps/www/registry/collection/registry-examples.ts
- apps/www/lib/catalogs/utils.ts
- apps/www/lib/catalogs/apple-music.ts
- apps/www/components/hero.tsx
- apps/www/app/(home)/page.tsx
- apps/www/registry/default/blocks/video-player/components/volume-slider-control.tsx
- apps/www/registry/default/blocks/video-player/components/player-root-container.tsx
- apps/www/registry/default/blocks/video-player/components/player-error-screen.tsx
- apps/www/lib/utils.ts
- apps/www/registry/default/hooks/use-timeline.ts
- apps/www/registry/default/blocks/video-player/player.tsx
- apps/www/registry/default/examples/volume-slider-control-horizontal-demo.tsx
- apps/www/registry/default/ui/timeline-control.tsx
- apps/www/lib/catalogs/playlists.ts
- apps/www/registry/default/blocks/audio-player/player.tsx
- apps/www/lib/catalogs/blender-open-films.ts
- apps/www/components/immersive-scroll-player.tsx
- apps/www/lib/catalogs/player-assets.ts
- apps/www/registry/default/hooks/use-controls-visibility.ts
- apps/www/registry/default/hooks/use-playback-source.ts
- apps/www/registry/default/ui/seek-controls.tsx
- apps/www/registry/default/blocks/video-player/components/button.tsx
- apps/www/components/stream-panel/use-stream-panel-sync.ts
- apps/www/registry/collection/registry-blocks.ts
- apps/www/registry/default/blocks/video-player/components/timeline-slider-control.tsx
- apps/www/components/players/video-player/player-container.tsx
- apps/www/registry/default/ui/root-container.tsx
- apps/www/registry/default/blocks/audio-player/components/audio-source.tsx
- apps/www/registry/default/blocks/video-player/components/bottom-controls.tsx
- apps/www/registry/default/hooks/use-asset.ts
| <div | ||
| className={cn( | ||
| "@container relative z-20 aspect-(--aspect-ratio) w-full overflow-hidden text-primary", | ||
| "relative z-20 aspect-(--aspect-ratio) w-full overflow-hidden text-primary", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the named container query root on PlayerContainer.
Dropping the container class here disables the @3xl/root and @5xl/root variants used by descendants, so the responsive width/layout rules below this wrapper stop applying.
🛠️ Proposed fix
- "relative z-20 aspect-(--aspect-ratio) w-full overflow-hidden text-primary",
+ "`@container/root` relative z-20 aspect-(--aspect-ratio) w-full overflow-hidden text-primary",📝 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
| "relative z-20 aspect-(--aspect-ratio) w-full overflow-hidden text-primary", | |
| "`@container/root` relative z-20 aspect-(--aspect-ratio) w-full overflow-hidden text-primary", |
🤖 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/player-layout.tsx` at line 18, Keep the
container query root class on PlayerContainer in player-layout.tsx; the wrapper
currently defines the named container needed by descendant `@3xl/root` and
`@5xl/root` variants. Restore or preserve the container class alongside the
existing layout classes so the responsive width/layout rules in the children
continue to apply, and verify the PlayerContainer component still exposes the
named root used by those descendants.
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
use-controls-visibilityto auto-hide playback controls.