feat: media session API and siemap fixes#158
Open
WINOFFRG wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThis PR adds a new ChangesMedia Session Hook and Player Integration
Estimated code review effort: 4 (Complex) | ~60 minutes Playlist previous() Action Refactor
Timeline, Sitemap, and Dev Script Updates
Sequence Diagram(s)sequenceDiagram
participant Player as Audio/Video Player
participant Controller as MediaSessionController
participant Sync as useMediaSessionSync
participant Session as useMediaSession
participant Browser as navigator.mediaSession
Player->>Controller: render with playback/timeline/asset state
Controller->>Controller: compute metadata, position, action handlers
Controller->>Sync: active, metadata, playbackState, position, actions
Sync->>Session: setMetadata / setPlaybackState / setPositionState / setActionHandlers
Session->>Browser: navigator.mediaSession API calls
Browser-->>Session: media key / OS control events
Session-->>Controller: registered handler invoked (play/pause/seek/next/previous)
Controller->>Player: calls mediaApi actions
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/www/app/sitemap.ts (1)
21-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider deduplicating the
docsPages/blocksPagesmapping logic.Both mappings share identical shape (
changeFrequency,lastModified,priority: 0.9,urlfrom${baseUrl}${page.url}), differing only in the source. Extracting a small helper would reduce duplication and keep both branches in sync going forward.♻️ Proposed refactor
+function toSitemapEntries(pages: { url: string }[], baseUrl: string) { + return pages.map((page) => ({ + changeFrequency: "weekly" as const, + lastModified: new Date(), + priority: 0.9, + url: `${baseUrl}${page.url}`, + })) +} + - const docsPages = source.getPages().map((page) => ({ - changeFrequency: "weekly" as const, - lastModified: new Date(), - priority: 0.9, - url: `${baseUrl}${page.url}`, - })) + const docsPages = toSitemapEntries(source.getPages(), baseUrl) - const blocksPages = blocksSource.getPages().map((page) => ({ - changeFrequency: "weekly" as const, - lastModified: new Date(), - priority: 0.9, - url: `${baseUrl}${page.url}`, - })) + const blocksPages = toSitemapEntries(blocksSource.getPages(), baseUrl)🤖 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/app/sitemap.ts` around lines 21 - 33, The docsPages and blocksPages mappings in sitemap generation are duplicated and should be consolidated. Extract the shared page-to-sitemap object निर्माण into a small helper used by both source.getPages() and blocksSource.getPages(), keeping the common fields (changeFrequency, lastModified, priority, url) in one place so Sitemap logic stays consistent and easier to maintain.apps/www/registry/default/blocks/audio-player/components/fixed-timeline-control.tsx (1)
50-62: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winClamp to the rendered time width
HoverTimeswitches toHH:MM:SSonce the duration exceeds an hour, so the current5chbound is too narrow and can let the label clip at the track edges. Use a width based on the longest rendered format instead.🤖 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/audio-player/components/fixed-timeline-control.tsx` around lines 50 - 62, The fixed timeline thumb label is clamped using a hardcoded 5ch width in the TimelineSlider.Thumb style, which is too narrow for the longest HoverTime format. Update the clamp bounds in fixed-timeline-control.tsx to use a width that matches the rendered maximum time label width (including HH:MM:SS) so the thumb text stays fully visible at the track edges.apps/www/app/layout.tsx (1)
78-83: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing
strategy="beforeInteractive"for react-scan.react-scan's documented Next.js App Router setup uses
strategy="beforeInteractive"so it can hook into React before hydration. Without it, this script defaults toafterInteractiveand may miss early renders that react-scan is meant to catch.🔧 Proposed fix
{process.env.NODE_ENV === "development" && ( <Script crossOrigin="anonymous" src="//unpkg.com/react-scan/dist/auto.global.js" + strategy="beforeInteractive" /> )}🤖 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/app/layout.tsx` around lines 78 - 83, The development-only react-scan Script in the app layout is missing the required Next.js loading strategy, so it may run too late to observe early renders. Update the Script usage in the layout component to set strategy="beforeInteractive" on the react-scan import so it initializes before hydration and matches the documented App Router setup.apps/www/registry/default/blocks/video-player/player.tsx (1)
164-164: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTODO left in shipped code.
FallbackPoster/CurrentAssetMediamulti-support fix is flagged as a TODO. Want me to draft theFallbackPostermulti-support implementation, or should this be tracked as a follow-up issue?🤖 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` at line 164, Remove the TODO left in shipped code and replace it with the actual multi-support handling for the poster asset in the video player. Update the `player.tsx` logic around `FallbackPoster` and `CurrentAssetMedia` so the component supports the intended fallback behavior directly instead of deferring it with a comment. Keep the implementation in the same rendering path where the poster/media selection happens, and ensure the final code is clean of placeholder TODOs.
🤖 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/blocks/audio-player/components/media-session-controller.tsx`:
- Around line 97-168: The playlist/metadata helper logic is duplicated between
the audio and video media-session controllers, so extract the shared functions
from media-session-controller into a common module and import them from both
places. Move canMoveToNextTrack, canMoveToPreviousTrack, firstNonEmpty,
getCurrentTimelineTime, hasNextPlaylistItem, hasPreviousPlaylistItem, and
isPlaylistSource into a shared helper near use-media-session or a new shared
lib, then update both controller components to use the shared implementations.
In `@apps/www/registry/default/hooks/use-playlist.ts`:
- Around line 383-411: The previous() handler is always truncating
playlist.history even when the history entry is not the one being consumed,
which can corrupt back-navigation. Update the logic in use-playlist.ts’s
previous method so it only pops history when the chosen previousIndex actually
comes from the last history item path, and preserves history for repeat-one,
shuffle-boundary, or fallback cases where getPreviousIndex() doesn’t advance via
history. Keep the emitPlaylistChange call aligned with the final selected
previousItem/currentItem state.
---
Nitpick comments:
In `@apps/www/app/layout.tsx`:
- Around line 78-83: The development-only react-scan Script in the app layout is
missing the required Next.js loading strategy, so it may run too late to observe
early renders. Update the Script usage in the layout component to set
strategy="beforeInteractive" on the react-scan import so it initializes before
hydration and matches the documented App Router setup.
In `@apps/www/app/sitemap.ts`:
- Around line 21-33: The docsPages and blocksPages mappings in sitemap
generation are duplicated and should be consolidated. Extract the shared
page-to-sitemap object निर्माण into a small helper used by both
source.getPages() and blocksSource.getPages(), keeping the common fields
(changeFrequency, lastModified, priority, url) in one place so Sitemap logic
stays consistent and easier to maintain.
In
`@apps/www/registry/default/blocks/audio-player/components/fixed-timeline-control.tsx`:
- Around line 50-62: The fixed timeline thumb label is clamped using a hardcoded
5ch width in the TimelineSlider.Thumb style, which is too narrow for the longest
HoverTime format. Update the clamp bounds in fixed-timeline-control.tsx to use a
width that matches the rendered maximum time label width (including HH:MM:SS) so
the thumb text stays fully visible at the track edges.
In `@apps/www/registry/default/blocks/video-player/player.tsx`:
- Line 164: Remove the TODO left in shipped code and replace it with the actual
multi-support handling for the poster asset in the video player. Update the
`player.tsx` logic around `FallbackPoster` and `CurrentAssetMedia` so the
component supports the intended fallback behavior directly instead of deferring
it with a comment. Keep the implementation in the same rendering path where the
poster/media selection happens, and ensure the final code is clean of
placeholder TODOs.
🪄 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: 7dca73e7-266a-44d3-9c32-80413a4fd678
📒 Files selected for processing (20)
apps/www/app/(home)/layout.tsxapps/www/app/layout.tsxapps/www/app/sitemap.tsapps/www/content/docs/blocks/audio-player.mdxapps/www/content/docs/blocks/video-player.mdxapps/www/content/docs/hooks/index.mdxapps/www/content/docs/hooks/meta.jsonapps/www/content/docs/hooks/use-media-session.mdxapps/www/registry/collection/registry-blocks.tsapps/www/registry/collection/registry-hooks.tsapps/www/registry/default/blocks/audio-player/components/fixed-timeline-control.tsxapps/www/registry/default/blocks/audio-player/components/media-session-controller.tsxapps/www/registry/default/blocks/audio-player/lib/media-kit.tsapps/www/registry/default/blocks/audio-player/player.tsxapps/www/registry/default/blocks/video-player/components/media-session-controller.tsxapps/www/registry/default/blocks/video-player/lib/media-kit.tsapps/www/registry/default/blocks/video-player/player.tsxapps/www/registry/default/hooks/use-media-session.tsapps/www/registry/default/hooks/use-playlist.tsapps/www/registry/default/ui/timeline-control.tsx
Comment on lines
+97
to
+168
| function canMoveToNextTrack( | ||
| sourceType: AssetSourceType | null, | ||
| playlist: PlaylistStore["playlist"] | ||
| ): boolean { | ||
| return isPlaylistSource(sourceType) && hasNextPlaylistItem(playlist) | ||
| } | ||
|
|
||
| function canMoveToPreviousTrack( | ||
| sourceType: AssetSourceType | null, | ||
| playlist: PlaylistStore["playlist"] | ||
| ): boolean { | ||
| return isPlaylistSource(sourceType) && hasPreviousPlaylistItem(playlist) | ||
| } | ||
|
|
||
| function firstNonEmpty( | ||
| ...values: (null | number | string | undefined)[] | ||
| ): string | undefined { | ||
| for (const value of values) { | ||
| if (typeof value === "number" && Number.isFinite(value)) { | ||
| return String(value) | ||
| } | ||
|
|
||
| if (typeof value !== "string") continue | ||
|
|
||
| const trimmed = value.trim() | ||
| if (trimmed) return trimmed | ||
| } | ||
|
|
||
| return undefined | ||
| } | ||
|
|
||
| function getAudioMediaSessionMetadata( | ||
| asset: AudioPlayerAsset | ||
| ): MediaMetadataInit { | ||
| const metadata = getAudioAssetMetadata(asset) | ||
| const poster = firstNonEmpty(metadata.poster, asset.poster) | ||
|
|
||
| return { | ||
| album: firstNonEmpty(asset.albumName), | ||
| artist: firstNonEmpty(asset.artistName, metadata.subtitle), | ||
| artwork: poster ? [{ sizes: "512x512", src: poster }] : [], | ||
| title: metadata.title, | ||
| } | ||
| } | ||
|
|
||
| function getCurrentTimelineTime({ | ||
| currentTime, | ||
| isLive, | ||
| mediaElement, | ||
| }: { | ||
| currentTime: number | ||
| isLive: boolean | ||
| mediaElement: HTMLMediaElement | null | ||
| }): number { | ||
| return isLive && mediaElement ? mediaElement.currentTime : currentTime | ||
| } | ||
|
|
||
| function hasNextPlaylistItem(playlist: PlaylistStore["playlist"]): boolean { | ||
| if (playlist.repeatMode === "all" && playlist.queue.length > 0) return true | ||
|
|
||
| return playlist.getNextIndex() !== -1 | ||
| } | ||
|
|
||
| function hasPreviousPlaylistItem(playlist: PlaylistStore["playlist"]): boolean { | ||
| if (playlist.repeatMode === "all" && playlist.queue.length > 0) return true | ||
|
|
||
| return playlist.getPreviousIndex() !== -1 | ||
| } | ||
|
|
||
| function isPlaylistSource(sourceType: AssetSourceType | null): boolean { | ||
| return sourceType === AssetSourceType.Playlist | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Extract shared playlist/metadata helpers to avoid duplication with video-player controller.
canMoveToNextTrack, canMoveToPreviousTrack, firstNonEmpty, getCurrentTimelineTime, hasNextPlaylistItem, hasPreviousPlaylistItem, and isPlaylistSource are duplicated verbatim in apps/www/registry/default/blocks/video-player/components/media-session-controller.tsx (lines 108-181). Extracting these into a shared module (e.g., alongside use-media-session.ts or a new shared lib) would prevent divergence as either player evolves.
♻️ Suggested approach
+// e.g. apps/www/registry/default/lib/media-session-controller-utils.ts
+export function isPlaylistSource(sourceType: AssetSourceType | null): boolean { ... }
+export function hasNextPlaylistItem(playlist: PlaylistStore["playlist"]): boolean { ... }
+export function hasPreviousPlaylistItem(playlist: PlaylistStore["playlist"]): boolean { ... }
+export function canMoveToNextTrack(...) { ... }
+export function canMoveToPreviousTrack(...) { ... }
+export function firstNonEmpty(...) { ... }
+export function getCurrentTimelineTime(...) { ... }Then import from both audio-player/components/media-session-controller.tsx and video-player/components/media-session-controller.tsx.
🤖 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/audio-player/components/media-session-controller.tsx`
around lines 97 - 168, The playlist/metadata helper logic is duplicated between
the audio and video media-session controllers, so extract the shared functions
from media-session-controller into a common module and import them from both
places. Move canMoveToNextTrack, canMoveToPreviousTrack, firstNonEmpty,
getCurrentTimelineTime, hasNextPlaylistItem, hasPreviousPlaylistItem, and
isPlaylistSource into a shared helper near use-media-session or a new shared
lib, then update both controller components to use the shared implementations.
Comment on lines
+383
to
+411
| previous: () => { | ||
| const playlist = get().playlist as PlaylistStore["playlist"] | ||
| const previousIndex = playlist.getPreviousIndex() | ||
| if (previousIndex === -1 || previousIndex >= playlist.queue.length) { | ||
| return false | ||
| } | ||
|
|
||
| const previousItem = playlist.queue[previousIndex] as PlaylistItem | ||
| const previousHistory = | ||
| playlist.history.length > 0 | ||
| ? playlist.history.slice(0, -1) | ||
| : playlist.history | ||
|
|
||
| set(({ playlist: p }) => { | ||
| p.currentIndex = previousIndex | ||
| p.currentItem = previousItem | ||
| p.history = previousHistory | ||
| }) | ||
|
|
||
| emitPlaylistChange( | ||
| events.emit, | ||
| previousIndex, | ||
| previousItem, | ||
| playlist.currentIndex, | ||
| playlist.currentItem, | ||
| "previous" | ||
| ) | ||
| return true | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/www/registry/default/hooks/use-playlist.ts"
echo "=== outline ==="
ast-grep outline "$file" --view expanded || true
echo
echo "=== relevant lines around getPreviousIndex and previous ==="
nl -ba "$file" | sed -n '180,230p; 370,430p'Repository: WINOFFRG/limeplay
Length of output: 2784
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/www/registry/default/hooks/use-playlist.ts"
echo "=== getPreviousIndex ==="
nl -ba "$file" | sed -n '180,215p'
echo
echo "=== previous() ==="
nl -ba "$file" | sed -n '383,411p'
echo
echo "=== nearby history helpers ==="
rg -n "history|previousIndex|getPreviousIndex|emitPlaylistChange" "$file"Repository: WINOFFRG/limeplay
Length of output: 220
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/www/registry/default/hooks/use-playlist.ts"
echo "=== getPreviousIndex ==="
sed -n '180,215p' "$file" | cat -n
echo
echo "=== previous() ==="
sed -n '383,411p' "$file" | cat -n
echo
echo "=== history-related helpers ==="
python3 - <<'PY'
from pathlib import Path
text = Path("apps/www/registry/default/hooks/use-playlist.ts").read_text()
for needle in ["history", "getPreviousIndex", "previousHistory", "previous:"]:
print(f"\n-- {needle} --")
for i, line in enumerate(text.splitlines(), 1):
if needle in line:
print(f"{i}: {line}")
PYRepository: WINOFFRG/limeplay
Length of output: 4032
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/www/registry/default/hooks/use-playlist.ts"
echo "=== next() and skipTo() history handling ==="
sed -n '300,360p;520,565p' "$file" | cat -n
echo
echo "=== remove / load handling around history ==="
sed -n '120,150p;250,280p' "$file" | cat -nRepository: WINOFFRG/limeplay
Length of output: 7314
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/www/registry/default/hooks/use-playlist.ts"
echo "=== selectors around hasPrevious / previousItem ==="
sed -n '630,690p' "$file" | cat -n
echo
echo "=== all references to previousItem / hasPrevious ==="
python3 - <<'PY'
from pathlib import Path
text = Path("apps/www/registry/default/hooks/use-playlist.ts").read_text().splitlines()
for i, line in enumerate(text, 1):
if any(k in line for k in ["hasPrevious", "previousItem", "getPreviousIndex()", "previous: playlist.previous"]):
print(f"{i}: {line}")
PYRepository: WINOFFRG/limeplay
Length of output: 3280
Only pop history when previous() actually uses it
apps/www/registry/default/hooks/use-playlist.ts:383-411 — previousHistory always drops the last entry whenever history is non-empty, even when getPreviousIndex() returns currentIndex on repeat-one / shuffle-boundary paths or falls back because the last history item no longer exists in queue. That silently discards a valid back-navigation entry and corrupts later previous() calls.
🤖 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-playlist.ts` around lines 383 - 411, The
previous() handler is always truncating playlist.history even when the history
entry is not the one being consumed, which can corrupt back-navigation. Update
the logic in use-playlist.ts’s previous method so it only pops history when the
chosen previousIndex actually comes from the last history item path, and
preserves history for repeat-one, shuffle-boundary, or fallback cases where
getPreviousIndex() doesn’t advance via history. Keep the emitPlaylistChange call
aligned with the final selected previousItem/currentItem state.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 20 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/registry/default/blocks/audio-player/components/media-session-controller.tsx">
<violation number="1" location="apps/www/registry/default/blocks/audio-player/components/media-session-controller.tsx:136">
P3: Lock-screen/notification metadata can show the album name as the artist when an audio asset has `albumName` but no `artistName`. `metadata.subtitle` is display text that may include album/genre/year, so it is too broad for `MediaMetadata.artist`; consider using artist-specific fields only.</violation>
</file>
<file name="apps/www/app/layout.tsx">
<violation number="1" location="apps/www/app/layout.tsx:79">
P1: react-scan won't capture initial renders or hydration behavior with default `afterInteractive` strategy. The auto-instrumentation script needs to be loaded before React runs to detect re-renders properly. Add `strategy="beforeInteractive"` so it loads before Next.js hydration.</violation>
</file>
<file name="apps/www/registry/default/blocks/video-player/components/media-session-controller.tsx">
<violation number="1" location="apps/www/registry/default/blocks/video-player/components/media-session-controller.tsx:122">
P3: This introduces another copy of media-session helper logic already present in the audio controller, which raises maintenance cost and makes audio/video behavior likely to drift over time. A shared helper/module for these common functions would keep fixes and behavior aligned.</violation>
</file>
<file name="apps/www/registry/default/hooks/use-playlist.ts">
<violation number="1" location="apps/www/registry/default/hooks/use-playlist.ts:391">
P2: The `previous()` method unconditionally pops the last history entry whenever `history` is non-empty, but `getPreviousIndex()` may return an index that wasn't derived from the history stack (e.g., on repeat-one paths, shuffle boundaries, or when the last history item no longer exists in the queue). In those cases, a valid back-navigation entry is silently discarded, corrupting subsequent `previous()` calls. Consider only popping history when the returned `previousIndex` actually matches the last history entry.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| <head> | ||
| <UserJotProvider projectId="cmjs634l4043b15ldylgedgwi" /> | ||
| {process.env.NODE_ENV === "development" && ( | ||
| <Script |
There was a problem hiding this comment.
P1: react-scan won't capture initial renders or hydration behavior with default afterInteractive strategy. The auto-instrumentation script needs to be loaded before React runs to detect re-renders properly. Add strategy="beforeInteractive" so it loads before Next.js hydration.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/www/app/layout.tsx, line 79:
<comment>react-scan won't capture initial renders or hydration behavior with default `afterInteractive` strategy. The auto-instrumentation script needs to be loaded before React runs to detect re-renders properly. Add `strategy="beforeInteractive"` so it loads before Next.js hydration.</comment>
<file context>
@@ -74,6 +75,12 @@ export default function Layout({ children }: { children: ReactNode }) {
<head>
<UserJotProvider projectId="cmjs634l4043b15ldylgedgwi" />
+ {process.env.NODE_ENV === "development" && (
+ <Script
+ crossOrigin="anonymous"
+ src="//unpkg.com/react-scan/dist/auto.global.js"
</file context>
| } | ||
|
|
||
| const previousItem = playlist.queue[previousIndex] as PlaylistItem | ||
| const previousHistory = |
There was a problem hiding this comment.
P2: The previous() method unconditionally pops the last history entry whenever history is non-empty, but getPreviousIndex() may return an index that wasn't derived from the history stack (e.g., on repeat-one paths, shuffle boundaries, or when the last history item no longer exists in the queue). In those cases, a valid back-navigation entry is silently discarded, corrupting subsequent previous() calls. Consider only popping history when the returned previousIndex actually matches the last history entry.
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-playlist.ts, line 391:
<comment>The `previous()` method unconditionally pops the last history entry whenever `history` is non-empty, but `getPreviousIndex()` may return an index that wasn't derived from the history stack (e.g., on repeat-one paths, shuffle boundaries, or when the last history item no longer exists in the queue). In those cases, a valid back-navigation entry is silently discarded, corrupting subsequent `previous()` calls. Consider only popping history when the returned `previousIndex` actually matches the last history entry.</comment>
<file context>
@@ -380,6 +380,35 @@ export function playlistFeature(): MediaFeature<PlaylistStore> {
+ }
+
+ const previousItem = playlist.queue[previousIndex] as PlaylistItem
+ const previousHistory =
+ playlist.history.length > 0
+ ? playlist.history.slice(0, -1)
</file context>
|
|
||
| return { | ||
| album: firstNonEmpty(asset.albumName), | ||
| artist: firstNonEmpty(asset.artistName, metadata.subtitle), |
There was a problem hiding this comment.
P3: Lock-screen/notification metadata can show the album name as the artist when an audio asset has albumName but no artistName. metadata.subtitle is display text that may include album/genre/year, so it is too broad for MediaMetadata.artist; consider using artist-specific fields only.
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/components/media-session-controller.tsx, line 136:
<comment>Lock-screen/notification metadata can show the album name as the artist when an audio asset has `albumName` but no `artistName`. `metadata.subtitle` is display text that may include album/genre/year, so it is too broad for `MediaMetadata.artist`; consider using artist-specific fields only.</comment>
<file context>
@@ -0,0 +1,168 @@
+
+ return {
+ album: firstNonEmpty(asset.albumName),
+ artist: firstNonEmpty(asset.artistName, metadata.subtitle),
+ artwork: poster ? [{ sizes: "512x512", src: poster }] : [],
+ title: metadata.title,
</file context>
Suggested change
| artist: firstNonEmpty(asset.artistName, metadata.subtitle), | |
| artist: firstNonEmpty(asset.artistName), |
| return isPlaylistSource(sourceType) && hasPreviousPlaylistItem(playlist) | ||
| } | ||
|
|
||
| function firstNonEmpty( |
There was a problem hiding this comment.
P3: This introduces another copy of media-session helper logic already present in the audio controller, which raises maintenance cost and makes audio/video behavior likely to drift over time. A shared helper/module for these common functions would keep fixes and behavior aligned.
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/media-session-controller.tsx, line 122:
<comment>This introduces another copy of media-session helper logic already present in the audio controller, which raises maintenance cost and makes audio/video behavior likely to drift over time. A shared helper/module for these common functions would keep fixes and behavior aligned.</comment>
<file context>
@@ -0,0 +1,181 @@
+ return isPlaylistSource(sourceType) && hasPreviousPlaylistItem(playlist)
+}
+
+function firstNonEmpty(
+ ...values: (null | number | string | undefined)[]
+): string | undefined {
</file context>
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
New Features
Documentation
Bug Fixes