diff --git a/README.md b/README.md index 99201de..c46d73c 100644 --- a/README.md +++ b/README.md @@ -302,6 +302,10 @@ Then put it behind your existing reverse proxy like everything else - [x] Cast a party's video to a Chromecast (Default Media Receiver), with a play/pause/seek remote on the sender's own screen — see docs/architecture.md +- [x] Fixed the listen-party queue's broken layout (it was reusing an + album page's row grid) and its missing album art (a track almost + never has its own image — now falls back to its album's) — see + docs/architecture.md - [ ] Try a full party end-to-end with two real accounts (still the main gap) - [ ] Try casting to a real Chromecast (only the SDK's own bootstrap is confirmed here — no real device/session) diff --git a/client/src/musicBar/MusicBar.css b/client/src/musicBar/MusicBar.css index 83a3e0c..e8abf82 100644 --- a/client/src/musicBar/MusicBar.css +++ b/client/src/musicBar/MusicBar.css @@ -99,3 +99,54 @@ .musicBarInfo { margin-right: 0; flex-basis: 100%; order: -1; text-align: center; } .musicBarQueue { max-width: none; margin: 0; border-radius: 0; } } + +/* One queue entry - a plain flex row (image, then a shrinking/truncating + info column, then the duration), NOT views/album's shared .trackRow + grid (styles/trackList.css) - that one's 4 fixed columns assume + Album.ts's own index/name/duration/button shape, which crushed this + list's track names into its 28px index column when reused here. See + MusicBar.ts's own comment. */ +.queueRow { + display: flex; + align-items: center; + gap: 10px; + background: var(--panel); + border-radius: 8px; + padding: 8px 10px; +} + +.queueRowArt { + width: 38px; + height: 38px; + object-fit: cover; + border-radius: 6px; + background: var(--bg-header); + flex-shrink: 0; +} + +.queueRowArt.noImage { visibility: hidden; } + +.queueRowInfo { + display: flex; + flex-direction: column; + gap: 2px; + min-width: 0; + flex: 1; +} + +.queueRowName { + font-size: 13px; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +/* Artist/album/"Ajouté par X" condensed onto one secondary line (see + MusicBar.ts) rather than each getting their own column - there's no + room for 3+ columns once a name needs to read in full, unlike + Album.ts's simpler index/name/duration/button row. */ +.queueRowMeta { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} diff --git a/client/src/musicBar/MusicBar.ts b/client/src/musicBar/MusicBar.ts index 3da084a..34fc04e 100644 --- a/client/src/musicBar/MusicBar.ts +++ b/client/src/musicBar/MusicBar.ts @@ -267,13 +267,22 @@ function applyMusicState(music: MusicPublicState): void { playPauseBtn.disabled = !music.current; updateVoteSkipLabel(); + // Its own .queueRow, NOT views/album's shared .trackRow (see + // styles/trackList.css) - that one is a CSS grid whose 4 columns + // (28px index / 1fr name / auto duration / auto button) assume + // Album.ts's own row shape. Reusing it here used to crush the track + // NAME into the 28px column meant for a track number, since this + // queue's rows never had an index at all - a real, visibly broken + // layout bug, not just a style mismatch. See docs/architecture.md. queueList.innerHTML = music.queue.length ? music.queue.map((t) => ` -
- ${escapeHtml(t.itemName)} - ${escapeHtml([t.artistName, t.albumName].filter(Boolean).join(' - '))} - ${t.durationSeconds ? formatDuration(t.durationSeconds) : ''} - Ajouté par ${escapeHtml(t.addedByUserName)} +
+ +
+ ${escapeHtml(t.itemName)} + ${escapeHtml([t.artistName, t.albumName, `Ajouté par ${t.addedByUserName}`].filter(Boolean).join(' · '))} +
+ ${t.durationSeconds ? formatDuration(t.durationSeconds) : ''}
`).join('') : '

Rien en attente.

'; diff --git a/client/src/views/home/Home.css b/client/src/views/home/Home.css index d6209c9..2b77ab6 100644 --- a/client/src/views/home/Home.css +++ b/client/src/views/home/Home.css @@ -30,6 +30,14 @@ flex-shrink: 0; } +/* A music party's own cover art is square (an album's, not a movie/show + poster) - the portrait 48x72 box above was forcing a hard crop on it. + Video parties keep the portrait box unchanged. */ +.partyCard img.partyCardArt--square { + width: 56px; + height: 56px; +} + .partyCard img.noImage { visibility: hidden; } .partyCardInfo { diff --git a/client/src/views/home/Home.ts b/client/src/views/home/Home.ts index 9086edf..748a256 100644 --- a/client/src/views/home/Home.ts +++ b/client/src/views/home/Home.ts @@ -54,7 +54,7 @@ function renderParties(root: HTMLElement, parties: LobbyPartyDto[]): void { section.hidden = false; list.innerHTML = parties.map((p) => `
- +
${escapeHtml(p.itemName || '')} diff --git a/docs/architecture.md b/docs/architecture.md index 4a93582..055a438 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -2520,6 +2520,63 @@ output cleanly are all unconfirmed, the same way this project already flags its other can't-verify-without-the-real-thing assumptions. Worth trying with a real device once deployed. +## Fixing the listen-party queue's layout and its missing album art + +Reported directly: the music bar's "File d'attente" popup looked broken, +and the listen-party card on the home page never showed an image. A +mockup was put together first (before touching any code) to confirm the +direction, since the queue side was a genuine UI rework, not just a +one-line fix. + +**The layout bug**: `MusicBar.ts`'s queue rows were reusing +`styles/trackList.css`'s shared `.trackRow` - a CSS grid with 4 fixed +columns (`28px 1fr auto auto`) sized for `views/album`'s own row shape +(track number, name, duration, an "Ajouter"/"Écouter ensemble" button). +The queue's rows never had a track number at all, and rendered +completely different content in that same slot - the track's own NAME +was the first child, so it landed in the 28px column meant for a +one-or-two-digit index, crushed down to its first couple of characters, +while the artist/album text spilled into the wide `1fr` column instead +meant for the name. Fixed with a dedicated `.queueRow` (flex, not grid - +image, then a truncating info column, then the duration), rather than +trying to force this list's different shape through Album's grid with +more special-casing. + +**The missing images**: confirmed directly against the real Emby server +(three sampled tracks, `Fields=AlbumId,ImageTags`) that an individual +Audio item essentially never carries its own Primary image - every one +tested answered `500`, not even a clean `404` - while the same tracks' +parent albums mostly did (`200 image/jpeg` for two of three; the third +album genuinely has no artwork uploaded at all, a real data gap, not a +bug). Every place that built an image URL from a raw `EmbyItem` +independently reconstructed `/api/catalog/image/${item.Id}` by hand +(`CatalogService.toItemDto`, and three separate spots in `index.ts`'s +party-creation routes that skip `toItemDto` entirely) - all now funnel +through one new `CatalogService.imageUrlFor(item)`, which points a music +track at its `AlbumId` instead of its own `Id` when one's known. +`EmbyItem` gained the `AlbumId` field, and every `Fields` param that +lists Audio items among its results (`getAlbumTracks`, +`getPlaylistItemsRaw`, and defensively `getRecentItems`/ +`getLibraryItems`) now requests it - a single-item detail fetch +(`getRawItem`, used by party creation) already includes it with no +`Fields` param needed, confirmed against the real server. + +While fixing the home-page card's now-working image, its box turned out +to be the wrong shape for music too: `.partyCard img` was a fixed +48×72 portrait box - correct for a movie/show poster, but a hard crop +for an album's square cover. A video party's card is untouched; a +music party's now gets a 56×56 square box instead +(`.partyCardArt--square`, toggled from `LobbyPartyDto.mediaType` in +`Home.ts`). + +Verified: full `typecheck`/`test`/`build` on both projects, including +new unit tests for `imageUrlFor` (a movie/episode unaffected, a track +with an `AlbumId` falls back to it, a track with none still falls back +to its own id rather than building a broken URL). No server is needed +to see the queue/card layout itself, so those were checked visually +against a mockup built from this app's own real theme tokens/typefaces +rather than a live instance. + ## Known limitations - Playback always goes through `hls.js` against whatever Emby's diff --git a/server/src/index.ts b/server/src/index.ts index 8a4ab59..bae490f 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -8,7 +8,7 @@ import { Server as SocketIoServer } from 'socket.io'; import { EMBY_SERVER_URL, PORT } from './config'; import { Logger } from './Logger'; import { AuthService, SESSION_COOKIE, SESSION_TTL_MS } from './services/AuthService'; -import { CatalogService, CatalogFetchError, splitPlaylistByMediaType } from './services/CatalogService'; +import { CatalogService, CatalogFetchError, imageUrlFor, splitPlaylistByMediaType } from './services/CatalogService'; import { PartyService } from './services/PartyService'; import { MusicQueueService } from './services/MusicQueueService'; import { TranscodeService } from './services/TranscodeService'; @@ -323,7 +323,7 @@ app.post('/api/parties', requireSession, asyncRoute(async (req, res) => { session.userName, itemId, queue ? queue[0]!.itemName : item.Name, - `/api/catalog/image/${itemId}`, + imageUrlFor(item), queue, mediaType ); @@ -335,7 +335,7 @@ app.post('/api/parties', requireSession, asyncRoute(async (req, res) => { itemName: item.Name, artistName: item.AlbumArtist || item.Artists?.[0] || null, albumName: item.Album || null, - itemImageUrl: `/api/catalog/image/${itemId}`, + itemImageUrl: imageUrlFor(item), durationTicks: item.RunTimeTicks || null }, session.userId, session.userName); } @@ -375,7 +375,7 @@ async function createPartyFromPlaylist(res: Response, session: EmbySession, play const first = items[0]!; const party = PartyService.create( session.userId, session.userName, - first.Id, first.Name, `/api/catalog/image/${first.Id}`, + first.Id, first.Name, imageUrlFor(first), null, mediaType ); PartyService.setMemberEmbyToken(party, session.userId, session.embyToken); @@ -388,7 +388,7 @@ async function createPartyFromPlaylist(res: Response, session: EmbySession, play itemName: track.Name, artistName: track.AlbumArtist || track.Artists?.[0] || null, albumName: track.Album || null, - itemImageUrl: `/api/catalog/image/${track.Id}`, + itemImageUrl: imageUrlFor(track), durationTicks: track.RunTimeTicks || null }; if (i === 0) { diff --git a/server/src/services/CatalogService.ts b/server/src/services/CatalogService.ts index cd8c34b..6e67523 100644 --- a/server/src/services/CatalogService.ts +++ b/server/src/services/CatalogService.ts @@ -48,6 +48,26 @@ const IMAGE_SUBTITLE_CODECS = new Set(['hdmv_pgs_subtitle', 'dvd_subtitle', 'dvb * unit-tested directly (see catalogService.test.ts) without needing to * exercise the actual /api/parties route that calls it (index.ts). */ +/** + * The image URL for a raw Emby item - every place that builds one from an + * `EmbyItem` (toItemDto below, and the party-creation routes in index.ts + * that hand-build one straight from a freshly-fetched item without going + * through a DTO) funnels through here. Almost always just `item.Id` + * itself, EXCEPT a music track: confirmed directly against a real server + * that an individual Audio item essentially never carries its own Primary + * image (every track sampled answered with a 500, not even a clean 404), + * while its parent album usually does - so a track's own id would just + * mean a permanently broken/missing thumbnail everywhere one is shown + * (the listen-party queue, the home page's live-party card) instead of + * the same cover art every other client shows next to it. Callers must + * request `AlbumId` in their own `Fields` param for this to have + * anything to fall back to - see getAlbumTracks/getPlaylistItemsRaw. + */ +export function imageUrlFor(item: EmbyItem): string { + const imageItemId = item.Type === 'Audio' && item.AlbumId ? item.AlbumId : item.Id; + return `/api/catalog/image/${imageItemId}`; +} + export function splitPlaylistByMediaType(items: EmbyItem[]): { mediaType: MediaType; items: EmbyItem[] } { if (!items.length) return { mediaType: 'video', items: [] }; const isMusic = items[0]!.Type === 'Audio'; @@ -129,7 +149,7 @@ class CatalogServiceClass { /** Home page carousel: Emby's own "recently added" endpoint, one row per library. */ async getRecentItems(session: EmbySession, libraryId: string): Promise { - const params = new URLSearchParams({ ParentId: libraryId, Limit: '16', Fields: 'ProductionYear' }); + const params = new URLSearchParams({ ParentId: libraryId, Limit: '16', Fields: 'ProductionYear,AlbumId' }); const data = await this.fetchJson(`/Users/${session.userId}/Items/Latest?${params}`, session, 'Impossible de récupérer les ajouts récents.'); // Unlike most Items endpoints, Latest returns a plain array, not {Items:[...]}. const items = Array.isArray(data) ? data : (data.Items || []); @@ -164,7 +184,7 @@ class CatalogServiceClass { * getPlaylistItems below for the DTO-shaped equivalent the client's * own playlist page reads instead. */ async getPlaylistItemsRaw(session: EmbySession, playlistId: string): Promise { - const params = new URLSearchParams({ UserId: session.userId, Fields: 'Overview,ProductionYear,RunTimeTicks,IndexNumber,Album,AlbumArtist' }); + const params = new URLSearchParams({ UserId: session.userId, Fields: 'Overview,ProductionYear,RunTimeTicks,IndexNumber,Album,AlbumArtist,AlbumId' }); const data = await this.fetchJson(`/Playlists/${playlistId}/Items?${params}`, session, 'Impossible de récupérer le contenu de cette playlist.'); return data.Items || []; } @@ -203,7 +223,7 @@ class CatalogServiceClass { UserId: session.userId, IncludeItemTypes: 'Audio', SortBy: 'IndexNumber', - Fields: 'RunTimeTicks,IndexNumber,Album,AlbumArtist' + Fields: 'RunTimeTicks,IndexNumber,Album,AlbumArtist,AlbumId' }); const data = await this.fetchJson(`/Users/${session.userId}/Items?${params}`, session, 'Impossible de récupérer les morceaux.'); return (data.Items || []).map((t) => this.toItemDto(t, session.serverId)); @@ -214,7 +234,7 @@ class CatalogServiceClass { ParentId: libraryId, Recursive: 'false', // top-level: Movies as movies, TV as Series tiles (not every episode) SortBy: 'SortName', - Fields: 'Overview,ProductionYear', + Fields: 'Overview,ProductionYear,AlbumId', ImageTypeLimit: '1', Limit: '300' }); @@ -410,7 +430,7 @@ class CatalogServiceClass { year: item.ProductionYear, type: item.Type, canWatchParty: WATCH_PARTY_TYPES.has(item.Type || ''), - imageUrl: `/api/catalog/image/${item.Id}`, + imageUrl: imageUrlFor(item), embyUrl: `${EMBY_SERVER_URL}/web/index.html#!/item?id=${item.Id}&serverId=${encodeURIComponent(serverId || '')}`, // Track-specific (null for anything else - harmless, every consumer // that cares about these only reads them for an Audio item). diff --git a/server/src/types.ts b/server/src/types.ts index 85efc80..fa44aa2 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -163,6 +163,12 @@ export interface EmbyItem { Album?: string; AlbumArtist?: string; Artists?: string[]; + /** The parent album's own id - only meaningful for Type: 'Audio'. See + * CatalogService.imageUrlFor: confirmed against a real server that an + * individual track essentially never has its own Primary image (every + * one sampled 500'd), only its album does - this is how an image URL + * falls back to the album's art instead. */ + AlbumId?: string; } /** This app's own DTO shape - what toItemDto() in CatalogService produces, diff --git a/server/test/catalogService.test.ts b/server/test/catalogService.test.ts index a3e19dc..2a14a07 100644 --- a/server/test/catalogService.test.ts +++ b/server/test/catalogService.test.ts @@ -4,7 +4,7 @@ import { describe, it } from 'node:test'; import assert from 'node:assert/strict'; import type { Request, Response } from 'express'; -import { CatalogService, splitPlaylistByMediaType } from '../src/services/CatalogService'; +import { CatalogService, imageUrlFor, splitPlaylistByMediaType } from '../src/services/CatalogService'; import { EmbyClient } from '../src/emby/EmbyClient'; import type { EmbyItem, EmbySession } from '../src/types'; @@ -223,3 +223,17 @@ describe('splitPlaylistByMediaType', () => { assert.equal(items.length, 0); }); }); + +describe('imageUrlFor', () => { + it('a movie/episode points at its own id, same as ever', () => { + assert.equal(imageUrlFor({ Id: 'm1', Name: 'Un Film', Type: 'Movie' }), '/api/catalog/image/m1'); + }); + + it('a music track falls back to its ALBUM\'s id - confirmed against a real server that a track essentially never has its own Primary image', () => { + assert.equal(imageUrlFor({ Id: 't1', Name: 'Track', Type: 'Audio', AlbumId: 'al1' }), '/api/catalog/image/al1'); + }); + + it('a track with no AlbumId at all (nothing to fall back to) still points at its own id, not a broken URL', () => { + assert.equal(imageUrlFor({ Id: 't1', Name: 'Track', Type: 'Audio' }), '/api/catalog/image/t1'); + }); +});