diff --git a/frontend/src/components/playlist-details/playlist-details.ts b/frontend/src/components/playlist-details/playlist-details.ts index bd0987f..3b81e3b 100644 --- a/frontend/src/components/playlist-details/playlist-details.ts +++ b/frontend/src/components/playlist-details/playlist-details.ts @@ -9,6 +9,9 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js'; import '@awesome.me/webawesome/dist/components/popup/popup.js'; import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js'; import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js'; +import '@lit-labs/virtualizer'; +import type { LitVirtualizer } from '@lit-labs/virtualizer'; +import { flow } from '@lit-labs/virtualizer/layouts/flow.js'; import { GetPlaylistTracks, @@ -17,7 +20,7 @@ import { RemovePhantomTracks, FindDuplicateTracksInPlaylist, } from '@go/playlist/Service'; -import type { playlist, library } from '@go/models'; +import type { playlist } from '@go/models'; import { EventsOn } from '@runtime/runtime'; import { Events } from '../../events'; import { queueStore } from '@store/queue-store'; @@ -31,6 +34,8 @@ import { } from '@utils/context-menu-controller.js'; import type { ContextMenuHost } from '@utils/context-menu-controller.js'; import { FavoritesController } from '@store/controllers/favorites-controller'; +import { notificationStore } from '@store/notification-store'; +import { describeError } from '@utils/describe-error'; import { hasTrackPayload, getDragPayload, @@ -44,7 +49,8 @@ import { } from '@utils/drag-image'; import { libraryStore } from '@store/library-store'; import '@components/playlist-picker/playlist-picker.js'; -import '@components/track-details/track-details.js'; +import { loadTrackDetails } from '@utils/lazy-track-details.js'; +import { tracksByFilePath, tracksForPaths } from '@utils/track-index.js'; import type { TrackDetails } from '@components/track-details/track-details.js'; import type { CoverArtUrls } from '@components/track-details/track-details.js'; import '@components/phantom-resolver/phantom-resolver.js'; @@ -60,6 +66,13 @@ import { } from '@utils/explore-link'; import { designTokens } from '../../styles/tokens.css'; +/** One playlist row: the track and its position in the *playlist*, + * which is not its position in the filtered view. */ +interface VisibleTrack { + track: playlist.Track; + trackIndex: number; +} + @customElement('playlist-details') export class PlaylistDetails extends LitElement @@ -86,6 +99,36 @@ export class PlaylistDetails private tracksChangedCleanup: (() => void) | null = null; private playlistDeletedCleanup: (() => void) | null = null; + /* `_itemSize` matches the fixed 45 px `.track-item` height, measured + * rather than guessed. Without the hint the flow layout's 100 px + * default drives constant scroll-error correction, which reads as + * the list jumping under the pointer. */ + @query('lit-virtualizer') + private virtualizer?: LitVirtualizer; + + /* The row templates live inside the virtualizer, not in this + * component's own template, so a host re-render alone does not + * repaint them: the virtualizer re-renders when its `items` change, + * and `items` is now memoised precisely so it does not change when + * nothing has. Selection and the playing-track highlight are + * therefore pushed to it explicitly, which is what `track-list` has + * always done. Costing ~36 rows, not the whole playlist. */ + private lastActiveTrackPath: string | null = null; + + private flowLayout = flow({ + _itemSize: { width: 100, height: 45 }, + } as Parameters[0]); + + /* The memo behind `getVisibleTracks()`. Its wrappers used to be + * rebuilt on every render, so the array identity changed even when + * nothing had — which is the one thing a virtualizer keys its work + * on (perf.M5). */ + private visibleCache: VisibleTrack[] | null = null; + private visibleCacheKey: { + tracks: playlist.Track[]; + term: string; + } | null = null; + private dragImageEl: HTMLElement | null = null; @query('#context-menu') @@ -137,6 +180,21 @@ export class PlaylistDetails onSelectionChanged(): void { this.requestUpdate(); + this.virtualizer?.requestUpdate(); + } + + protected override updated( + changed: Map, + ): void { + super.updated(changed); + + const currentPath = + this.player.currentTrack?.filePath ?? null; + + if (currentPath !== this.lastActiveTrackPath) { + this.lastActiveTrackPath = currentPath; + this.virtualizer?.requestUpdate(); + } } // ================================================================= @@ -357,9 +415,9 @@ export class PlaylistDetails break; case 'track-details': if (filePaths.length === 1) { - this.openTrackDetails(filePaths[0]!); + void this.openTrackDetails(filePaths[0]!); } else { - this.openBatchTrackDetails(filePaths); + void this.openBatchTrackDetails(filePaths); } break; case 'phantom-locate': @@ -393,6 +451,16 @@ export class PlaylistDetails this.ctxMenu.close(); } + /** The row is still on screen, so the failure is visible; this + * only says why (errors.m7). */ + private reportRemoveFailure(what: string, err: unknown): void { + notificationStore.transient({ + key: 'playlist-remove', + text: `Could not ${what}. ${describeError(err)}`, + detail: String(err), + }); + } + private async removeSelectedTracks() { const trackIDs = this.getSelectedTrackIDs(); @@ -405,10 +473,8 @@ export class PlaylistDetails ); await this.refreshTracks(); } catch (err) { - console.error( - 'Failed to remove tracks:', - err, - ); + console.error('Failed to remove tracks:', err); + this.reportRemoveFailure('remove those tracks', err); } } @@ -432,21 +498,25 @@ export class PlaylistDetails ); await this.refreshTracks(); } catch (err) { - console.error( - 'Failed to remove phantom tracks:', - err, - ); + console.error('Failed to remove phantom tracks:', err); + this.reportRemoveFailure('remove those missing tracks', err); } } - private openTrackDetails(filePath: string) { + private async openTrackDetails(filePath: string) { const tracks = libraryStore.getCachedTracks(); - const track = tracks?.find( - (t) => t.FilePath === filePath, - ); + const track = tracks + ? tracksByFilePath(tracks).get(filePath) + : undefined; if (!track) return; + const ready = await loadTrackDetails( + () => void this.openTrackDetails(filePath), + ); + + if (!ready) return; + const coverArt = track.CoverArtPath ? { coverArtPath: track.CoverArtPath, @@ -462,7 +532,7 @@ export class PlaylistDetails ); } - private openBatchTrackDetails( + private async openBatchTrackDetails( filePaths: string[], ) { const cachedTracks = @@ -470,19 +540,19 @@ export class PlaylistDetails if (!cachedTracks) return; - const tracks = filePaths - .map((fp) => - cachedTracks.find( - (t) => t.FilePath === fp, - ), - ) - .filter( - (t): t is library.Track => - t != null, - ); + const tracks = tracksForPaths( + cachedTracks, + filePaths, + ); if (tracks.length === 0) return; + const ready = await loadTrackDetails( + () => void this.openBatchTrackDetails(filePaths), + ); + + if (!ready) return; + const first = tracks[0]!; const albumNames = new Set(tracks.map((t) => t.Album)); let coverArt: CoverArtUrls | null = null; @@ -595,10 +665,8 @@ export class PlaylistDetails ); await this.refreshTracks(); } catch (err) { - console.error( - 'Failed to remove phantom track:', - err, - ); + console.error('Failed to remove phantom track:', err); + this.reportRemoveFailure('remove that missing track', err); } } @@ -747,38 +815,58 @@ export class PlaylistDetails // Search filtering // ================================================================= - private getVisibleTracks(): { - track: playlist.Track; - trackIndex: number; - }[] { + private getVisibleTracks(): VisibleTrack[] { const term = this.searchCtrl.term.toLowerCase(); - if (!term) { - return this.tracks.map( - (track, trackIndex) => ({ - track, - trackIndex, - }), - ); + // Keyed on the identity of the tracks array and the term, the + // same signal `track-list`'s memoized caches use: the store + // replaces the array when its contents change and shares every + // unchanged member. + if ( + this.visibleCache && + this.visibleCacheKey && + this.visibleCacheKey.tracks === this.tracks && + this.visibleCacheKey.term === term + ) { + return this.visibleCache; } - return this.tracks - .map((track, trackIndex) => ({ + const all = this.tracks.map( + (track, trackIndex) => ({ track, trackIndex, - })) - .filter( - ({ track }) => - track.Title.toLowerCase().includes( - term, - ) || - track.Artist.toLowerCase().includes( - term, - ), - ); + }), + ); + + const visible = term + ? all.filter( + ({ track }) => + track.Title.toLowerCase().includes( + term, + ) || + track.Artist.toLowerCase().includes( + term, + ), + ) + : all; + + this.visibleCache = visible; + this.visibleCacheKey = { tracks: this.tracks, term }; + + return visible; } + /** Stable across renders, because `lit-virtualizer` declares both + * `renderItem` and `keyFunction` as plain `@property()` — a fresh + * arrow function marks them dirty and forces the virtualizer's own + * render pass on every host update (perf.m1). */ + private renderRow = (entry: VisibleTrack) => + this.renderTrackRow(entry); + + private rowKey = (entry: VisibleTrack) => + entry.trackIndex; + // ================================================================= // Styles // ================================================================= @@ -970,6 +1058,18 @@ export class PlaylistDetails gap: 0; } + /* The virtualizer positions its children absolutely, so a row + * shrinks to fit its content and the columns stop lining up + * with the header above them. track-list has always carried the + * same declaration for the same reason. + * + * No backticks in here: this is inside a css tagged template + * literal, and one ends it. */ + .track-item { + width: 100%; + box-sizing: border-box; + } + .track-header { padding: 6px 8px; font-size: 11px; @@ -1262,9 +1362,21 @@ export class PlaylistDetails
Album
Duration
- ${visibleTracks.map( - ({ track, trackIndex }) => { - const isPhantom = track.Phantom; + + `; + } + + /* One row. Extracted from `renderTrackList` so it can be a stable + * bound field rather than a closure the virtualizer sees as new + * every pass. */ + private renderTrackRow({ track, trackIndex }: VisibleTrack) { + const isPhantom = track.Phantom; const active = !isPhantom && this.isActiveTrack(track); @@ -1383,7 +1495,14 @@ export class PlaylistDetails : html`${trackIndex + 1}
${track.CoverArtSmall || track.CoverArtMedium - ? html`` + ? html`` : nothing}
${trackLink(track.Title, track.Album, track.ReleaseGroupMBID, track.RecordingMBID, undefined, track.Artist) || track.FilePath} @@ -1392,9 +1511,6 @@ export class PlaylistDetails ${formatMilliseconds(track.Duration)}`} `; - }, - )} - `; } private renderContextMenu() { diff --git a/frontend/src/components/queue-panel/queue-panel.ts b/frontend/src/components/queue-panel/queue-panel.ts index d60b77c..10e7efa 100644 --- a/frontend/src/components/queue-panel/queue-panel.ts +++ b/frontend/src/components/queue-panel/queue-panel.ts @@ -11,6 +11,7 @@ import '@awesome.me/webawesome/dist/components/popup/popup.js'; import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js'; import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js'; import { QueueController } from '@store/controllers/queue-controller'; +import { confirmAction } from '@components/confirm-dialog/confirm-dialog'; import '@components/playlist-picker/playlist-picker.js'; import type { PlaylistPicker } from '@components/playlist-picker/playlist-picker.js'; import '@lit-labs/virtualizer'; @@ -41,7 +42,8 @@ import { } from '@utils/drag-image'; import { libraryStore } from '@store/library-store'; import type { library } from '@go/models'; -import '@components/track-details/track-details.js'; +import { loadTrackDetails } from '@utils/lazy-track-details.js'; +import { tracksByFilePath } from '@utils/track-index.js'; import type { TrackDetails } from '@components/track-details/track-details.js'; import type { CoverArtUrls } from '@components/track-details/track-details.js'; import { @@ -49,6 +51,9 @@ import { trackLink, exploreLinkStyles, } from '@utils/explore-link'; +/** Above this many tracks, clearing the queue asks first. */ +const CLEAR_CONFIRM_THRESHOLD = 20; + const MIN_WIDTH = 200; const MAX_WIDTH = 500; const DEFAULT_WIDTH = 320; @@ -651,6 +656,30 @@ export class QueuePanel } override updated() { + // Closed, the panel is `width: 0` — which hides it from the eye + // and from nobody else: its Clear and Add buttons still took tab + // stops at x=1440 and were still read out (H-5). `inert` is the + // one attribute that means "not there" to both. + this.inert = !this.open; + + // ...and a panel that is not there does not render a list + // either (perf.m7). `width: 0` and `contain: layout style + // paint` bound the damage but do not stop the work: the + // virtualizer inside still had a real height and a + // `min-width: 300px`, so it measured its visible window on + // every queue change, and `scrollToIndex` below called + // `scrollIntoView()` on a laid-out but invisible element every + // time the current track changed. The list body renders + // `nothing` while closed; these indices reset so opening the + // panel re-syncs the highlight and the scroll position rather + // than inheriting stale ones. + if (!this.open) { + this.lastRenderedIndex = -1; + this.lastScrolledIndex = -1; + + return; + } + // The virtualizer may not exist on first render // (queue empty). Retry hooks here when it appears. this.attachVirtualizerHooks(); @@ -680,7 +709,27 @@ export class QueuePanel } } - private handleClearQueue = () => { + /** + * Clearing stops playback and discards the list, and it is the one + * mutation in this panel with no way back (errors.m3). A queue you + * could rebuild in a second is not worth a prompt; one you spent + * the evening on is. + */ + private handleClearQueue = async () => { + const count = this.queue.tracks.length; + + if (count > CLEAR_CONFIRM_THRESHOLD) { + const ok = await confirmAction({ + title: 'Clear the queue?', + message: `${count} tracks will be removed and playback will stop.`, + impact: 'The queue cannot be brought back.', + confirmLabel: 'Clear queue', + danger: true, + }); + + if (!ok) return; + } + this.queue.clearQueue(); }; @@ -839,9 +888,9 @@ export class QueuePanel break; case 'track-details': if (indices.length === 1) { - this.openTrackDetails(indices[0]!); + void this.openTrackDetails(indices[0]!); } else { - this.openBatchTrackDetails(indices); + void this.openBatchTrackDetails(indices); } break; } @@ -870,7 +919,7 @@ export class QueuePanel this.ctxMenu.close(); } - private openTrackDetails(index: number) { + private async openTrackDetails(index: number) { const queueTrack = this.queue.tracks[index]; @@ -878,13 +927,20 @@ export class QueuePanel const tracks = libraryStore.getCachedTracks(); - const track = tracks?.find( - (t) => - t.FilePath === queueTrack.filePath, - ); + const track = tracks + ? tracksByFilePath(tracks).get( + queueTrack.filePath, + ) + : undefined; if (!track) return; + const ready = await loadTrackDetails( + () => void this.openTrackDetails(index), + ); + + if (!ready) return; + const coverArt = track.CoverArtPath ? { coverArtPath: track.CoverArtPath, @@ -900,7 +956,7 @@ export class QueuePanel ); } - private openBatchTrackDetails( + private async openBatchTrackDetails( indices: number[], ) { const queueTracks = this.queue.tracks; @@ -909,15 +965,11 @@ export class QueuePanel if (!cachedTracks) return; + const byPath = tracksByFilePath(cachedTracks); const tracks = indices .map((i) => queueTracks[i]) .filter((qt) => qt != null) - .map((qt) => - cachedTracks.find( - (t) => - t.FilePath === qt.filePath, - ), - ) + .map((qt) => byPath.get(qt.filePath)) .filter( (t): t is library.Track => t != null, @@ -925,6 +977,12 @@ export class QueuePanel if (tracks.length === 0) return; + const ready = await loadTrackDetails( + () => void this.openBatchTrackDetails(indices), + ); + + if (!ready) return; + const first = tracks[0]!; const albumNames = new Set(tracks.map((t) => t.Album)); let coverArt: CoverArtUrls | null = null; @@ -1456,7 +1514,7 @@ export class QueuePanel