diff --git a/.planning/plans/active/019-android-touch-model.md b/.planning/plans/completed/019-android-touch-model.md similarity index 76% rename from .planning/plans/active/019-android-touch-model.md rename to .planning/plans/completed/019-android-touch-model.md index 784271a..969f8e4 100644 --- a/.planning/plans/active/019-android-touch-model.md +++ b/.planning/plans/completed/019-android-touch-model.md @@ -5,7 +5,7 @@ **Relates:** #67 (inline links into the menu), #71 ("More" nav), #54 (native feel), #5/#8 (selection, drag to queue — the desktop semantics being diverged from) -**Status:** in flight. +**Status:** shipped (phases 1-4). Its one deliberate remainder is #200. #73 puts #60 first in Phase 4 because it is "the presentation every other item needs", and this is the next one. The Direction on #63 asks @@ -229,12 +229,58 @@ finding above and a reveal-and-snap affordance. **Shipped**; what the device said about it is the section below. **Phase 3 — the other three surfaces**, which is mostly wiring, since -they already share the controller. +they already share the controller. **Shipped**, and it was not entirely +wiring — see below. **Phase 4 — what this leaves behind.** The inline `explore-link`s in a row are a single-click target inside a row whose single tap now plays; that conflict is #67's, and this plan should not pre-empt its answer -beyond making tap-to-play win on touch. +beyond making tap-to-play win on touch. **Shipped.** + +## Phase 3 was not symmetric, in two places + +**A tap on a queue row plays that position**, not the list. Copying +`track-list`'s tap — which sets the queue to the list the row is in — +would rebuild the queue *from* the queue, discarding its source, its +shuffle order and everything a user had inserted by hand. It reads as a +no-op and is not one. + +**The queue panel has no swipe, deliberately.** A right swipe means +*add to the queue* everywhere else it exists, and a queue row is +already in the queue; the only thing it could mean there is *remove*, +which is the same gesture with the opposite effect one screen away — +the fault `utils/icon-language.ts` exists to have fixed for glyphs. +Removing a queue row is on the row itself (the ×), on its bottom sheet +since #60, and on the selection bar this phase gave it. The assertion +is that its rows do **not** carry `data-swipe`, so a swipe there cannot +silently become a second meaning for the app's one horizontal gesture. + +And the affordance became `utils/swipe-to-queue.ts` rather than being +copied twice. Three lists want it; three copies of "how far is far +enough" is three chances for them to disagree, which is what +`utils/library-status.ts` and `utils/ownership.ts` each exist to have +stopped happening. The shared stylesheet is keyed on `[data-swipe]` +rather than on a class name, because the three lists call their rows +two different things and the `touch-action` half of the device fix has +to reach all of them. + +## Phase 4 was already true, which is why it is asserted + +A claimed tap has its click swallowed at document capture, so an +`explore-link` inside the row never sees one and tap-to-play wins with +no rule of its own. Nothing in the suite would have failed if that +stopped covering the link, and the symptom — tapping a track's *title* +navigating to its album instead of playing it — is one a phone user +meets constantly and a mouse user never does. + +**Its test was vacuous when written**, in the way this file keeps +finding: the tap helper dispatched `pointerdown` and `pointerup` and no +`click`, so there was nothing to swallow and the assertion held on any +build. It sends the trailing click now, which also strengthened phase +1's "a tap plays and does not also select". The fixture needed an MBID +for the same reason — without one the link asks the backend for a local +album first and gives up when nothing answers, so "it did not navigate" +was true of a working build and a broken one alike. --- @@ -329,14 +375,34 @@ on the fixed build, six clean. ## Open questions 1. **Does selection mode have an escape other than the bar's own - close?** Back is the platform's answer and the shell already owns - the history stack (#6/#55). Pushing an entry for a *mode* rather - than a place is the same argument #55 settled for the overlaid - queue, and it should probably be settled the same way — but the - queue is a screen and a selection mode is not, so it wants its own - paragraph rather than an assumption. + close?** *Settled: Escape, here; back, not here.* + + Escape leaves the mode, from `selection-bar` rather than from each + of the four hosts — that element exists only while the mode does, so + it is the one place a dismissal can be attached and detached with + the thing it dismisses. It is the same documented exception the + overlaid queue's Escape is: **a dismissal, not a shortcut**, so it + is not a panel-scoped binding. + + The back gesture is the half that is *not* done, and deliberately. + The obvious version — `selection-bar` pushing a history entry — is + precisely the fault `navStack` was deleted for: the shell owns the + stack (#6/#55) and is the only thing that calls `pushState`, so that + two stacks cannot disagree about what one press means. Four lists + each reaching for `history` is four stacks. It is also wrong on its + own terms, since a mode is per-component and a user who enters one, + navigates away and returns has an entry for a mode that no longer + exists. #55 settled the shape for a *place*; a mode is not one, + which is why it could not simply inherit that answer. + + What it wants is one shell-owned register of dismissible surfaces, + which would retro-fit the queue overlay, the dialogs and this alike + rather than adding a fourth private answer. **#200.** + 2. **Does a tap on a row's favourite icon still toggle it in normal - mode?** It is inside the row and the row now plays. It has to keep - working — it is a 44px target since #56 — so the gesture layer needs - the same "a control inside the row wins" rule the keyboard service - has for a focused control that owns a key. + mode?** *Settled in phase 1: yes.* A control inside the row keeps + its own tap — the gesture is simply not claimed there, so the click + behind it falls through untouched. It is the same rule the shortcut + service has for a focused control that owns a key, and it is what + keeps the 44px favourite target (#56) from becoming a 44px play + target. The queue row's × is the second instance of it. diff --git a/CLAUDE.md b/CLAUDE.md index a917b06..97eb7a5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1506,6 +1506,56 @@ rest of the press (a scroll that curves is still a scroll), and a gesture that is not *strictly* more horizontal than vertical is the scroller's. +**And what a finger *means* on a row is the inversion of what a mouse +means, decided per event** (#63). A click selects and a double-click +plays; a tap **plays** and a hold enters **selection mode**, in which a +tap toggles. The predicate is `pointerType`, never a viewport width and +never a platform flag — #64's rule, and with #64's warning: keyed on a +width, an Android tablet over 600px gets desktop semantics on a +touchscreen, a touchscreen laptop cannot be described at all, and a +narrow desktop window gets phone semantics with a mouse. + +There is deliberately **no double-tap**, which #63 asked for. The first +tap of one is indistinguishable from a single tap until the interval +expires, so tapping would have to wait `DOUBLE_CLICK_GRACE_MS` before +acting — 250ms on top of a measured ~100ms play, 3.5x the app's primary +interaction, to reach a menu a hold already reaches. So the menu and +the action bar are the same surface: `components/selection-bar/` is +presentational (a count and a list of actions, no store, no selection), +`SelectionController` carries the mode for all four selecting surfaces, +and #60's bottom sheet is the overflow behind "More" — so +`contextMenuStyles`, `MenuKeyboard` and `menu-surface` are reused +rather than reimplemented. + +Three things about it are load-bearing. **A control inside a row keeps +its own tap**: the gesture is simply not claimed there, so the click +behind it falls through, which is what stops the 44px favourite target +(#56) becoming a 44px play target — the queue row's × is the second +instance. **A swipe right queues**, and its affordance is +`utils/swipe-to-queue.ts` once rather than in each of the three lists +that draw it: the row does not move, its *children* do (a row here is +`contain: strict` with `overflow: hidden`, so a pane held at the row's +original position while the row translates is at a negative offset +inside a clipping box and is not painted), the travel is written to the +row's own style rather than rendered, and the threshold is a fraction +of the row because the row is 424x52 on the reference device. **The +queue panel takes the tap and the hold and refuses the swipe**, because +a right swipe means *add to the queue* everywhere it exists and a queue +row is already in it — the only thing it could mean there is *remove*, +which is the same gesture with the opposite effect one screen away. +A tap there plays that *position*, too: setting the queue to the queue +reads as a no-op and discards its source, its shuffle order and +anything inserted by hand. + +Escape leaves the mode, from `selection-bar` rather than from each +host, since that element exists only while the mode does — the same +exception the overlaid queue's Escape is, *a dismissal, not a +shortcut*. The platform's back gesture deliberately does **not** reach +it: the shell owns the history stack and four lists each reaching for +`history` is four stacks, which is the fault `navStack` was deleted +for. That wants one shell-owned register of dismissible surfaces, which +is #200. + **A control revealed by `:hover` is gated on the device having hover, and which way round depends on whether it is the only route to its action.** The gate itself is not optional: a touch long-press diff --git a/frontend/src/components/playlist-details/playlist-details.ts b/frontend/src/components/playlist-details/playlist-details.ts index 80c222c..e45e499 100644 --- a/frontend/src/components/playlist-details/playlist-details.ts +++ b/frontend/src/components/playlist-details/playlist-details.ts @@ -38,6 +38,10 @@ import { } from '@utils/context-menu-controller.js'; import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js'; import { focusRovingRow, nextRovingIndex } from '@utils/roving-rows'; +import type { GestureEvent } from '@utils/touch-gestures'; +import { SwipeToQueue, swipeRevealStyles } from '@utils/swipe-to-queue'; +import '@components/selection-bar/selection-bar'; +import type { SelectionAction } from '@components/selection-bar/selection-bar'; import { FavoritesController } from '@store/controllers/favorites-controller'; import { notificationStore } from '@store/notification-store'; import { describeError } from '@utils/describe-error'; @@ -71,11 +75,14 @@ import { exploreLinkStyles, } from '@utils/explore-link'; import { designTokens } from '../../styles/tokens.css'; +import { srOnly } from '../../styles/sr-only.css'; import { backButton } from '../../styles/back-button.css'; import { list } from '@utils/binding'; import { + ICON_PLAY, ICON_PLAYLIST, ICON_QUEUE, + ICON_REMOVE, } from '@utils/icon-language'; /** One playlist row: the track and its position in the *playlist*, @@ -422,6 +429,127 @@ export class PlaylistDetails queueStore.setQueue(filePaths, trackIndex, false, { type: 'playlist', id: this.playlistId, label: this.playlistName }); } + // ================================================================= + // A finger on a playlist row (plan 019 phase 3, #63) + // ================================================================= + + /** The row an announced gesture is on, with its track. */ + private rowFromGesture( + e: Event, + ): { index: number; track: playlist.Track } | null { + const row = (e.target as HTMLElement).closest( + '.track-item', + ) as HTMLElement | null; + + if (!row) return null; + + const index = Number(row.dataset.index); + const track = this.tracks[index]; + + if (Number.isNaN(index) || !track) return null; + + return { index, track }; + } + + /** + * A tap plays the playlist from that row. + * + * The same thing a double-click does, which is the rule the whole + * app follows: activating one row plays the list the row is in, + * from that row, rather than a queue of one that stops when the + * song ends. + */ + private onRowTap = (e: GestureEvent) => { + const hit = this.rowFromGesture(e); + + if (!hit) return; + + if (this.selection.selectionMode) { + e.preventDefault(); + this.focusedIndex = hit.index; + this.selection.toggleInMode(String(hit.index), hit.index); + this.virtualizer?.requestUpdate(); + + return; + } + + // A missing file has nothing to play, so the tap is left + // unclaimed and falls through to the click that selects it -- + // which is what a mouse does here and the only useful thing a + // phantom row can answer. + if (hit.track.Phantom) return; + + e.preventDefault(); + this.focusedIndex = hit.index; + this.handleTrackDblClick(hit.index); + }; + + private onRowLongPress = (e: GestureEvent) => { + const hit = this.rowFromGesture(e); + + if (!hit) return; + + e.preventDefault(); + this.focusedIndex = hit.index; + this.selection.enterSelectionMode(String(hit.index), hit.index); + this.virtualizer?.requestUpdate(); + }; + + /** + * Swipe a row right to queue it. + * + * `track-list`'s rule, one list over: one row is a position and + * several rows are an explicit choice, and a swipe never changes + * the selection it reads. + */ + private swipe = new SwipeToQueue(this, { + resolve: (e) => { + const hit = this.rowFromGesture(e); + + // A phantom has no file to queue, so there is nothing for + // the reveal to promise. + if (!hit || hit.track.Phantom) return null; + + const selected = this.selection.getSelectedIndices(); + const many = + selected.length > 1 && selected.includes(hit.index); + const filePaths = many + ? this.getSelectedFilePaths() + : [hit.track.FilePath]; + + return { index: hit.index, filePaths, label: hit.track.Title }; + }, + repaint: () => this.virtualizer?.requestUpdate(), + }); + + /** The three worth a thumb; the sheet behind "More" is the rest. */ + private static readonly SELECTION_ACTIONS: SelectionAction[] = [ + { id: 'play', label: 'Play', icon: ICON_PLAY }, + { id: 'add-to-queue', label: 'Add to queue', icon: ICON_QUEUE }, + { id: 'remove', label: 'Remove', icon: ICON_REMOVE, danger: true }, + ]; + + private renderSelectionBar() { + if (!this.selection.selectionMode) return nothing; + + return html` + ) => + this.onContextMenuAction(e.detail.id)} + @selection-more=${(e: CustomEvent<{ x: number; y: number }>) => + this.ctxMenu.openAt(e.detail.x, e.detail.y)} + > + `; + } + + private onSelectionExit = () => { + this.selection.exitSelectionMode(); + this.virtualizer?.requestUpdate(); + }; + private handleTrackContextMenu( e: MouseEvent, trackIndex: number, @@ -955,9 +1083,11 @@ export class PlaylistDetails static override styles = [ designTokens, + srOnly, backButton, contextMenuStyles, exploreLinkStyles, + swipeRevealStyles, css` :host { display: flex; @@ -1136,6 +1266,9 @@ export class PlaylistDetails .track-item { width: 100%; box-sizing: border-box; + /* The swipe reveal is absolute inside the row. */ + position: relative; + overflow: hidden; } .track-header { @@ -1433,6 +1566,9 @@ export class PlaylistDetails
Album
Duration
+
+ ${this.swipe.announcement} +
+ ${this.renderSelectionBar()} `; } @@ -1464,6 +1606,7 @@ export class PlaylistDetails active ? 'active' : '', selected ? 'selected' : '', isPhantom ? 'phantom' : '', + this.swipe.isSwiping(trackIndex) ? 'swiping' : '', ] .filter(Boolean) .join(' '); @@ -1474,6 +1617,7 @@ export class PlaylistDetails role="option" aria-selected=${selected} data-index=${trackIndex} + data-swipe tabindex=${trackIndex === this.focusedIndex ? 0 : -1} @keydown=${(e: KeyboardEvent) => this.onRowKeydown(e, trackIndex)} @@ -1520,6 +1664,7 @@ export class PlaylistDetails ? nothing : this.onTrackDragEnd} > + ${this.swipe.renderReveal(trackIndex)} ${isPhantom ? html`
{ + const idx = this.resolveTrackIndexFromEvent(e); + + if (idx === null) return; + + // A control inside the row owns its own tap -- the same rule + // the shortcut service has for a focused control that owns a + // key. The remove button is the one here. + if ((e.target as HTMLElement).closest('.remove-button')) return; + + e.preventDefault(); + + // The roving tab stop follows the finger, or Tab returns to + // wherever the arrows last were rather than to the row that was + // just touched. + this.focusedIndex = idx; + + if (this.selection.selectionMode) { + this.selection.toggleInMode(String(idx), idx); + this.virtualizer?.requestUpdate(); + + return; + } + + this.selection.clear(); + this.queue.playAtIndex(idx); + }; + + private onRowLongPress = (e: GestureEvent) => { + const idx = this.resolveTrackIndexFromEvent(e); + + if (idx === null) return; + + e.preventDefault(); + this.focusedIndex = idx; + this.selection.enterSelectionMode(String(idx), idx); + this.virtualizer?.requestUpdate(); + }; + + /** + * The two worth a thumb, and "More" for the rest. + * + * Remove is here rather than left to the overflow because it is + * what a selection in a *queue* is most often made for, and it is + * the action the row's own × offers one row at a time. + */ + private static readonly SELECTION_ACTIONS: SelectionAction[] = [ + { id: 'play', label: 'Play', icon: ICON_PLAY }, + { id: 'remove', label: 'Remove', icon: ICON_REMOVE, danger: true }, + ]; + + private renderSelectionBar() { + if (!this.selection.selectionMode) return nothing; + + return html` + ) => + this.onContextMenuAction(e.detail.id)} + @selection-more=${(e: CustomEvent<{ x: number; y: number }>) => + this.ctxMenu.openAt(e.detail.x, e.detail.y)} + > + `; + } + + private onSelectionExit = () => { + this.selection.exitSelectionMode(); + this.virtualizer?.requestUpdate(); + }; + private handleTrackContextMenu( e: MouseEvent, index: number, @@ -2157,6 +2252,7 @@ export class QueuePanel > `}
+ ${this.renderSelectionBar()} { + if (e.key !== 'Escape' || this.count <= 0) return; + + e.preventDefault(); + e.stopPropagation(); + this.emit('selection-exit'); + }; + + override connectedCallback() { + super.connectedCallback(); + document.addEventListener('keydown', this.onKeydown, true); + } + + override disconnectedCallback() { + super.disconnectedCallback(); + document.removeEventListener('keydown', this.onKeydown, true); + } + override render() { if (this.count <= 0) return nothing; diff --git a/frontend/src/components/smart-playlist-details/smart-playlist-details.ts b/frontend/src/components/smart-playlist-details/smart-playlist-details.ts index a97f428..edde4c7 100644 --- a/frontend/src/components/smart-playlist-details/smart-playlist-details.ts +++ b/frontend/src/components/smart-playlist-details/smart-playlist-details.ts @@ -28,6 +28,10 @@ import { } from '@utils/context-menu-controller.js'; import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js'; import { focusRovingRow, nextRovingIndex } from '@utils/roving-rows'; +import type { GestureEvent } from '@utils/touch-gestures'; +import { SwipeToQueue, swipeRevealStyles } from '@utils/swipe-to-queue'; +import '@components/selection-bar/selection-bar'; +import type { SelectionAction } from '@components/selection-bar/selection-bar'; import { FavoritesController } from '@store/controllers/favorites-controller'; import { setDragPayload, @@ -63,8 +67,11 @@ import { import '@components/smart-playlist-editor/smart-playlist-editor.js'; import { designTokens } from '../../styles/tokens.css'; import { backButton } from '../../styles/back-button.css'; +import { srOnly } from '../../styles/sr-only.css'; import { list } from '@utils/binding'; import { + ICON_PLAY, + ICON_PLAY_NEXT, ICON_PLAYLIST, ICON_QUEUE, ICON_SMART_PLAYLIST, @@ -243,9 +250,11 @@ export class SmartPlaylistDetails static override styles = [ designTokens, + srOnly, backButton, contextMenuStyles, exploreLinkStyles, + swipeRevealStyles, css` :host { display: flex; @@ -444,6 +453,9 @@ export class SmartPlaylistDetails .track-item { width: 100%; box-sizing: border-box; + /* The swipe reveal is absolute inside the row. */ + position: relative; + overflow: hidden; } .track-header { @@ -920,6 +932,110 @@ export class SmartPlaylistDetails // Context menu actions // ================================================================= + // ================================================================= + // A finger on a smart playlist row (plan 019 phase 3, #63) + // ================================================================= + + /** The row an announced gesture is on, with its track. */ + private rowFromGesture( + e: Event, + ): { index: number; track: playlist.Track } | null { + const row = (e.target as HTMLElement).closest( + '.track-item', + ) as HTMLElement | null; + + if (!row) return null; + + const index = Number(row.dataset.index); + const track = this.tracks[index]; + + if (Number.isNaN(index) || !track) return null; + + return { index, track }; + } + + /** A tap plays the playlist from that row -- `playlist-details`' + * rule, and the app's: activating a row plays the list it is in. */ + private onRowTap = (e: GestureEvent) => { + const hit = this.rowFromGesture(e); + + if (!hit) return; + + if (this.selection.selectionMode) { + e.preventDefault(); + this.focusedIndex = hit.index; + this.selection.toggleInMode(String(hit.index), hit.index); + this.virtualizer?.requestUpdate(); + + return; + } + + // A missing file has nothing to play, so the tap falls through + // to the click that selects it. + if (hit.track.Phantom) return; + + e.preventDefault(); + this.focusedIndex = hit.index; + this.handleTrackDblClick(hit.index); + }; + + private onRowLongPress = (e: GestureEvent) => { + const hit = this.rowFromGesture(e); + + if (!hit) return; + + e.preventDefault(); + this.focusedIndex = hit.index; + this.selection.enterSelectionMode(String(hit.index), hit.index); + this.virtualizer?.requestUpdate(); + }; + + private swipe = new SwipeToQueue(this, { + resolve: (e) => { + const hit = this.rowFromGesture(e); + + if (!hit || hit.track.Phantom) return null; + + const selected = this.selection.getSelectedIndices(); + const many = + selected.length > 1 && selected.includes(hit.index); + const filePaths = many + ? this.getSelectedFilePaths() + : [hit.track.FilePath]; + + return { index: hit.index, filePaths, label: hit.track.Title }; + }, + repaint: () => this.virtualizer?.requestUpdate(), + }); + + /** The three worth a thumb; the sheet behind "More" is the rest. */ + private static readonly SELECTION_ACTIONS: SelectionAction[] = [ + { id: 'play', label: 'Play', icon: ICON_PLAY }, + { id: 'add-to-queue', label: 'Add to queue', icon: ICON_QUEUE }, + { id: 'play-next', label: 'Play next', icon: ICON_PLAY_NEXT }, + ]; + + private renderSelectionBar() { + if (!this.selection.selectionMode) return nothing; + + return html` + ) => + this.onContextMenuAction(e.detail.id)} + @selection-more=${(e: CustomEvent<{ x: number; y: number }>) => + this.ctxMenu.openAt(e.detail.x, e.detail.y)} + > + `; + } + + private onSelectionExit = () => { + this.selection.exitSelectionMode(); + this.virtualizer?.requestUpdate(); + }; + private onContextMenuAction(action: string) { const filePaths = this.getSelectedFilePaths(); @@ -1334,6 +1450,9 @@ export class SmartPlaylistDetails
Album
Duration
+
+ ${this.swipe.announcement} +
+ ${this.renderSelectionBar()} `; } @@ -1364,6 +1489,7 @@ export class SmartPlaylistDetails active ? 'active' : '', selected ? 'selected' : '', isPhantom ? 'phantom' : '', + this.swipe.isSwiping(trackIndex) ? 'swiping' : '', ] .filter(Boolean) .join(' '); @@ -1374,6 +1500,7 @@ export class SmartPlaylistDetails role="option" aria-selected=${selected} data-index=${trackIndex} + data-swipe tabindex=${trackIndex === this.focusedIndex ? 0 : -1} @keydown=${(e: KeyboardEvent) => this.onRowKeydown(e, trackIndex)} @@ -1408,6 +1535,7 @@ export class SmartPlaylistDetails ? nothing : this.onTrackDragEnd} > + ${this.swipe.renderReveal(trackIndex)} ${isPhantom ? html`
{ + const hit = this.resolveTrackFromEvent(e); - /** How far along the row a swipe has to reach to mean it. */ - private static readonly SWIPE_COMMIT_FRACTION = 0.3; + if (!hit) return null; - /** … and a floor, for a narrow list embedded in a detail page. */ - private static readonly SWIPE_COMMIT_MIN_PX = 72; - - /** How long the reveal holds its confirmation before snapping. */ - private static readonly SWIPE_CONFIRM_MS = 550; - - /** The snap itself, which the stylesheet also states. */ - private static readonly SWIPE_SETTLE_MS = 180; - - /** Which row is being swiped, and therefore which draws a reveal. */ - @state() private swipeIndex: number | null = null; - - /** Past the commit threshold: the reveal says so, in words. */ - @state() private swipeArmed = false; - - /** Committed, and holding the confirmation. */ - @state() private swipeDone = false; - - /** What the gesture did, for anyone not watching the row. */ - @state() private swipeAnnouncement = ''; - - private swipeRow: HTMLElement | null = null; - private swipeKeys: string[] = []; - private swipeCommitPx = 0; - private swipeSettleTimer = 0; + return { + index: hit.index, + filePaths: this.swipeTargetKeys(hit.track.FilePath), + label: hit.track.TrackName, + }; + }, + repaint: () => this.virtualizer?.requestUpdate(), + }); private handleSelectAll = (): void => { this.selection.selectAll(); @@ -1047,7 +1040,7 @@ export class TrackList this.requestUpdate(); }; - static override styles = [designTokens, srOnly, contextMenuStyles, exploreLinkStyles, css` + static override styles = [designTokens, srOnly, contextMenuStyles, exploreLinkStyles, swipeRevealStyles, css` :host { display: flex; flex-direction: column; @@ -1166,18 +1159,6 @@ export class TrackList height: 33px; box-sizing: border-box; contain: strict; - /* Swipe right to queue (plan 019 phase 2, #63). Half of what - makes the gesture reach us on the device: auto lets Chrome - 113's WebView commit to a horizontal pan on the first move - past slop, and the pointer stream is cancelled before any - threshold can be crossed. The other half is the non-passive - preventDefault in utils/touch-gestures.ts, and neither works - alone -- both were measured three ways on the phone. - Never none: that takes the list's own vertical scrolling with - it. The cost is that a finger starting on a row can no longer - pan the shell sideways in the 600-899 band, where the shell - can still overflow; anywhere else on the page still can. */ - touch-action: pan-y; } /* A phone row is two lines, and this height must equal @@ -1256,61 +1237,6 @@ export class TrackList background-color: var(--yj-selection-bg, rgba(100, 160, 255, 0.15)); } - /* The reveal behind a swiped row (plan 019 phase 2, #63). - - The row itself does not move -- its *cells* do. Moving the row - and counter-translating the pane inside it is the obvious - arrangement and does not work here: .track-row is contain: - strict with overflow: hidden, so a pane held at the row's - original position is a pane at a negative offset inside a - clipping box, and it is simply not painted. Sliding the cells - instead leaves the pane where it was drawn, clips the cells off - the right edge, and needs no wrapper element in a row that is - already a grid. - - It is not only a colour (WCAG 1.4.1, the rule the playing-row - marker is here for): the pane carries an icon and words, the - words change at the commit threshold, and the outcome is - announced in the list's live region. */ - .swipe-reveal { - position: absolute; - left: 0; - top: 0; - bottom: 0; - width: var(--yj-swipe-dx, 0px); - box-sizing: border-box; - display: flex; - align-items: center; - gap: 0.4em; - padding-left: 8px; - overflow: hidden; - white-space: nowrap; - pointer-events: none; - font-size: var(--yj-text-xs); - background-color: var(--yj-bg-elevated, #343a40); - color: var(--yj-text-secondary, #b3b3b3); - } - - .swipe-reveal.armed { - background-color: var(--yj-success, #2f9e44); - color: var(--yj-success-fg, #fff); - } - - .track-row.swiping > :not(.swipe-reveal) { - transform: translateX(var(--yj-swipe-dx, 0px)); - } - - .track-row.settling > * { - transition: - transform 160ms ease-out, - width 160ms ease-out; - } - - @media (prefers-reduced-motion: reduce) { - .track-row.settling > * { - transition: none; - } - } .cell { overflow: hidden; @@ -1410,9 +1336,9 @@ export class TrackList virt.removeEventListener('contextmenu', this.onDelegatedContextMenu); virt.removeEventListener('yj-tap', this.onRowTap); virt.removeEventListener('yj-long-press', this.onRowLongPress); - virt.removeEventListener('yj-swipe-start', this.onRowSwipeStart); - virt.removeEventListener('yj-swipe-move', this.onRowSwipeMove); - virt.removeEventListener('yj-swipe-end', this.onRowSwipeEnd); + virt.removeEventListener('yj-swipe-start', this.swipe.onSwipeStart); + virt.removeEventListener('yj-swipe-move', this.swipe.onSwipeMove); + virt.removeEventListener('yj-swipe-end', this.swipe.onSwipeEnd); virt.removeEventListener('dragstart', this.onDelegatedDragStart); virt.removeEventListener('dragend', this.onTrackDragEnd); } @@ -1559,9 +1485,9 @@ export class TrackList // through the same path a real click takes (plan 019). virt.addEventListener('yj-tap', this.onRowTap); virt.addEventListener('yj-long-press', this.onRowLongPress); - virt.addEventListener('yj-swipe-start', this.onRowSwipeStart); - virt.addEventListener('yj-swipe-move', this.onRowSwipeMove); - virt.addEventListener('yj-swipe-end', this.onRowSwipeEnd); + virt.addEventListener('yj-swipe-start', this.swipe.onSwipeStart); + virt.addEventListener('yj-swipe-move', this.swipe.onSwipeMove); + virt.addEventListener('yj-swipe-end', this.swipe.onSwipeEnd); this.delegationAttached = true; } @@ -1881,10 +1807,6 @@ export class TrackList this.virtualizer?.requestUpdate(); }; - // ================================================================= - // Swipe right to queue (plan 019 phase 2, #63) - // ================================================================= - /** * What a swipe on this row would queue. * @@ -1909,165 +1831,6 @@ export class TrackList return [filePath]; } - private onRowSwipeStart = (e: SwipeEvent) => { - // Rightward only. Nothing is bound to a leftward swipe, and - // claiming one would take a gesture away to do nothing with it. - if (e.detail.dx <= 0) return; - - const hit = this.resolveTrackFromEvent(e); - - if (!hit) return; - - const row = (e.target as HTMLElement).closest( - '.track-row', - ) as HTMLElement | null; - - if (!row) return; - - e.preventDefault(); - - this.swipeRow = row; - this.swipeKeys = this.swipeTargetKeys(hit.track.FilePath); - // A fraction of the row, with a floor: the row is 424x52 on the - // reference device, so a threshold in bare pixels is a fraction - // of a row height on one screen and a third of the width on - // another. - this.swipeCommitPx = Math.max( - TrackList.SWIPE_COMMIT_MIN_PX, - row.getBoundingClientRect().width * - TrackList.SWIPE_COMMIT_FRACTION, - ); - this.swipeArmed = false; - this.swipeDone = false; - this.swipeIndex = hit.index; - this.virtualizer?.requestUpdate(); - this.setSwipeOffset(0); - }; - - private onRowSwipeMove = (e: SwipeEvent) => { - if (this.swipeIndex === null) return; - - const dx = Math.min( - Math.max(e.detail.dx, 0), - this.swipeCommitPx * 2, - ); - const armed = dx >= this.swipeCommitPx; - - // Crossing the threshold is the only thing here that renders. - // The offset itself is written straight to the row's style, or - // a virtualized list would re-render every visible row for - // every frame of one finger's travel. - if (armed !== this.swipeArmed) { - this.swipeArmed = armed; - this.virtualizer?.requestUpdate(); - } - - this.setSwipeOffset(dx); - }; - - private onRowSwipeEnd = (e: SwipeEvent) => { - if (this.swipeIndex === null) return; - - const commit = - !e.detail.canceled && e.detail.dx >= this.swipeCommitPx; - - if (!commit) { - this.settleSwipe(0); - - return; - } - - queueStore.addTracksToQueue(this.swipeKeys); - - const count = this.swipeKeys.length; - const only = - count === 1 - ? tracksByFilePath(this.tracks).get(this.swipeKeys[0]!) - : undefined; - - // The reveal is the only thing on screen that says this - // happened -- the queue panel may well be closed -- so it holds - // its confirmation for a moment rather than snapping back the - // instant the finger lifts. The live region is the same - // sentence for anyone not watching it. - this.swipeDone = true; - this.swipeAnnouncement = - count === 1 - ? `Added ${only?.TrackName ?? 'the track'} to the queue.` - : `Added ${count} tracks to the queue.`; - this.virtualizer?.requestUpdate(); - this.settleSwipe(TrackList.SWIPE_CONFIRM_MS); - }; - - /** Write the travel to the row itself, with no render. */ - private setSwipeOffset(dx: number) { - this.swipeRow?.style.setProperty('--yj-swipe-dx', `${dx}px`); - } - - /** - * Put the row back, after `delay`, and forget the swipe. - * - * The row element is held rather than looked up again: a - * virtualizer recycles its rows, and by the time this runs the - * element may be drawing a different track. Clearing the property - * off whatever it holds now is right either way, since - * `swipeIndex` is what decides who draws the reveal. - */ - private settleSwipe(delay: number) { - const row = this.swipeRow; - - window.clearTimeout(this.swipeSettleTimer); - - this.swipeSettleTimer = window.setTimeout(() => { - row?.classList.add('settling'); - this.setSwipeOffset(0); - - this.swipeSettleTimer = window.setTimeout(() => { - row?.classList.remove('settling'); - row?.style.removeProperty('--yj-swipe-dx'); - this.swipeRow = null; - this.swipeIndex = null; - this.swipeArmed = false; - this.swipeDone = false; - this.virtualizer?.requestUpdate(); - }, TrackList.SWIPE_SETTLE_MS); - }, delay); - } - - /** - * What is revealed behind the row, in three states. - * - * One glyph throughout, and the words carry the state. A tick - * would read better for the last of them and is `ICON_IN_LIBRARY` - * -- it means *you own this* -- and `icon-language.ts` exists - * because `plus` came to mean four things that way. - */ - private renderSwipeReveal() { - const count = this.swipeKeys.length; - const what = - count === 1 ? 'to queue' : `${count} tracks to queue`; - - return html` - - `; - } - private onDelegatedDragStart = (e: DragEvent) => { const hit = this.resolveTrackFromEvent(e); @@ -2489,7 +2252,7 @@ export class TrackList 'track-row': true, active, selected, - swiping: this.swipeIndex === index, + swiping: this.swipe.isSwiping(index), })} role="row" aria-rowindex=${index + 1} @@ -2499,9 +2262,10 @@ export class TrackList draggable="true" data-index=${index} data-testid="track-row" + data-swipe data-file-path=${track.FilePath} > - ${this.swipeIndex === index ? this.renderSwipeReveal() : nothing} + ${this.swipe.renderReveal(index)}
- ${this.swipeAnnouncement} + ${this.swipe.announcement}
${this.tracks.length === 0 ? this.renderPlaceholder() diff --git a/frontend/src/utils/swipe-to-queue.ts b/frontend/src/utils/swipe-to-queue.ts new file mode 100644 index 0000000..1325606 --- /dev/null +++ b/frontend/src/utils/swipe-to-queue.ts @@ -0,0 +1,362 @@ +import { css, html, nothing } from 'lit'; +import type { ReactiveController, ReactiveControllerHost } from 'lit'; +import { classMap } from 'lit/directives/class-map.js'; + +import { queueStore } from '@store/queue-store'; +import { ICON_QUEUE } from '@utils/icon-language'; +import type { SwipeEvent } from '@utils/touch-gestures'; + +/** + * Swipe a row right to add it to the queue (plan 019, #63). + * + * This is the *affordance* and the arithmetic, written once, because + * three lists want it: `track-list` and both playlist detail views. + * It was `track-list`'s own for one phase and is here rather than + * copied twice, on the rule the rest of this app is built on — three + * copies of "how far is far enough" is three chances for them to + * disagree, which is what `utils/library-status.ts` and + * `utils/ownership.ts` each exist to have stopped happening. + * + * The host keeps three things: what a row *is*, what a swipe on it + * would queue, and what to call it afterwards. Everything else — + * the threshold, the reveal, the settle, the announcement, the + * repaint — is here. + * + * **The queue panel deliberately does not use it.** A right swipe means + * *add to the queue* everywhere it exists, and a queue row is already + * in the queue; the only thing it could sensibly mean there is + * *remove*, which is the same gesture with the opposite effect one + * screen away. Removing a queue row is on its own row (the ×), on its + * bottom sheet since #60, and on the selection bar #63 gave it. + * + * Five things about it are load-bearing. + * + * **Both halves of the device fix are here or next door.** + * `swipeRevealStyles` carries `touch-action: pan-y` on `[data-swipe]`, + * and `utils/touch-gestures.ts` carries the non-passive + * `preventDefault`. Chrome 113's WebView cancels the pointer stream + * ~16px into any drag whatever `touch-action` says, and with the + * `preventDefault` alone but `touch-action` at `auto` the gesture dies + * after one move. Neither works without the other and **both are + * correct in Chromium either way**, which is why the component tier + * asserts the stylesheet rather than the rendering. + * + * **The row does not move; its cells do.** A row here is + * `contain: strict` with `overflow: hidden`, so translating the row + * and counter-translating a pane inside it puts that pane at a + * negative offset inside a clipping box, where it is simply not + * painted. Sliding the children instead leaves the pane where it was + * drawn, clips the cells off the right edge, and needs no wrapper + * element in a row that is already a grid. + * + * **The travel is written to the row's own style, never rendered.** + * One render when the gesture starts, one when it crosses the + * threshold, one when it ends — a virtualizer re-rendering every + * visible row per frame of one finger's travel is exactly what audit + * `perf.m1` is about. + * + * **The threshold is a fraction of the row**, with a floor. The row is + * 424x52 on the reference device, so a threshold in bare pixels is a + * fraction of a row height on one screen and a third of the width on + * the next. + * + * **It is not only a colour** (WCAG 1.4.1, the rule the playing-row + * marker exists for). The pane carries the queue icon and words, the + * words change at the threshold, and the outcome goes to a live + * region — one glyph throughout, because a tick is `ICON_IN_LIBRARY` + * and means *you own this*. + */ + +/** How far along the row a swipe has to reach to mean it. */ +export const SWIPE_COMMIT_FRACTION = 0.3; + +/** … and a floor, for a narrow list embedded in a detail page. */ +export const SWIPE_COMMIT_MIN_PX = 72; + +/** How long the reveal holds its confirmation before snapping back. */ +const CONFIRM_MS = 550; + +/** The snap itself. `swipeRevealStyles` states the same number. */ +const SETTLE_MS = 180; + +/** What a swipe on one row would do, as the host understands it. */ +export interface SwipeTarget { + /** Which row draws the reveal. */ + index: number; + + /** The file paths a commit queues, in the order they are shown. */ + filePaths: string[]; + + /** What to call a single track when saying it was added. */ + label: string; +} + +export interface SwipeToQueueOptions { + /** + * The row the gesture is on, or null for anything that is not a + * swipeable row — a header, a gap, a track with no file. + */ + resolve(e: SwipeEvent): SwipeTarget | null; + + /** + * Repaint the rows. A `` renders through the + * `virtualize` directive and reacts to its *own* properties, so a + * host update alone leaves the rows exactly as they were. + */ + repaint(): void; +} + +export class SwipeToQueue implements ReactiveController { + private host: ReactiveControllerHost; + private opts: SwipeToQueueOptions; + + /** Which row is being swiped, and therefore draws a reveal. */ + private index: number | null = null; + + /** Past the commit threshold: the reveal says so, in words. */ + private armed = false; + + /** Committed, and holding its confirmation. */ + private done = false; + + private row: HTMLElement | null = null; + private keys: string[] = []; + private commitPx = 0; + private settleTimer = 0; + + /** What the gesture did, for anyone not watching the row. */ + announcement = ''; + + constructor(host: ReactiveControllerHost, opts: SwipeToQueueOptions) { + this.host = host; + this.opts = opts; + host.addController(this); + } + + hostConnected(): void { + // No-op; state is component-local. + } + + hostDisconnected(): void { + window.clearTimeout(this.settleTimer); + this.forget(); + } + + /** Whether this row is the one under the finger. */ + isSwiping(index: number): boolean { + return this.index === index; + } + + onSwipeStart = (e: SwipeEvent): void => { + // Rightward only. Nothing is bound to a leftward swipe, and + // claiming one would take a gesture away to do nothing with it. + if (e.detail.dx <= 0) return; + + const target = this.opts.resolve(e); + + if (!target || target.filePaths.length === 0) return; + + const row = (e.target as HTMLElement).closest( + '[data-swipe]', + ) as HTMLElement | null; + + if (!row) return; + + e.preventDefault(); + + this.row = row; + this.keys = target.filePaths; + this.trackLabel = target.label; + this.commitPx = Math.max( + SWIPE_COMMIT_MIN_PX, + row.getBoundingClientRect().width * SWIPE_COMMIT_FRACTION, + ); + this.armed = false; + this.done = false; + this.index = target.index; + this.host.requestUpdate(); + this.opts.repaint(); + this.offset(0); + }; + + onSwipeMove = (e: SwipeEvent): void => { + if (this.index === null) return; + + const dx = Math.min(Math.max(e.detail.dx, 0), this.commitPx * 2); + const armed = dx >= this.commitPx; + + if (armed !== this.armed) { + this.armed = armed; + this.host.requestUpdate(); + this.opts.repaint(); + } + + this.offset(dx); + }; + + onSwipeEnd = (e: SwipeEvent): void => { + if (this.index === null) return; + + if (e.detail.canceled || e.detail.dx < this.commitPx) { + this.settle(0); + + return; + } + + queueStore.addTracksToQueue(this.keys); + + const count = this.keys.length; + + // The reveal is the only thing on screen that says this + // happened -- the queue panel may well be closed -- so it holds + // its confirmation for a moment rather than vanishing the + // instant the finger lifts. + this.done = true; + this.announcement = + count === 1 + ? `Added ${this.label()} to the queue.` + : `Added ${count} tracks to the queue.`; + this.host.requestUpdate(); + this.opts.repaint(); + this.settle(CONFIRM_MS); + }; + + /** What is revealed behind the row, in three states. */ + renderReveal(index: number) { + if (this.index !== index) return nothing; + + const count = this.keys.length; + const what = count === 1 ? 'to queue' : `${count} tracks to queue`; + const words = this.done + ? 'Added' + : this.armed + ? 'Release to add' + : `Add ${what}`; + + return html` + + `; + } + + /** + * What to call a single track, taken when the gesture starts. + * + * Held rather than looked up at the end, because a swipe outlives + * a refetch: the store replaces its array when a play count + * changes, which is once a song. + */ + private trackLabel = ''; + + private label(): string { + return this.trackLabel === '' ? 'the track' : this.trackLabel; + } + + /** Write the travel to the row itself, with no render. */ + private offset(dx: number): void { + this.row?.style.setProperty('--yj-swipe-dx', `${dx}px`); + } + + /** + * Put the row back, after `delay`, and forget the swipe. + * + * The row element is held rather than looked up again: a + * virtualizer recycles its rows, and by the time this runs the + * element may be drawing a different track. Clearing the property + * off whatever it holds now is right either way, since `index` is + * what decides who draws the reveal. + */ + private settle(delay: number): void { + const row = this.row; + + window.clearTimeout(this.settleTimer); + + this.settleTimer = window.setTimeout(() => { + row?.classList.add('settling'); + this.offset(0); + + this.settleTimer = window.setTimeout(() => { + row?.classList.remove('settling'); + row?.style.removeProperty('--yj-swipe-dx'); + this.forget(); + this.host.requestUpdate(); + this.opts.repaint(); + }, SETTLE_MS); + }, delay); + } + + private forget(): void { + this.row = null; + this.index = null; + this.armed = false; + this.done = false; + } +} + +/** + * The reveal, and the `touch-action` half of what makes the gesture + * reach us on the device. + * + * Keyed on `[data-swipe]` rather than on a class name, so one + * stylesheet serves three lists whose rows are called three different + * things. + */ +export const swipeRevealStyles = css` + /* Half of what makes the gesture reach us on Chrome 113's WebView: + auto lets it commit to a horizontal pan on the first move past + slop, and the pointer stream is cancelled before any threshold + can be crossed. The other half is the non-passive preventDefault + in utils/touch-gestures.ts, and neither works alone -- both were + measured three ways on the phone. Never none: that takes the + list's own vertical scrolling with it. */ + [data-swipe] { + touch-action: pan-y; + } + + .swipe-reveal { + position: absolute; + left: 0; + top: 0; + bottom: 0; + width: var(--yj-swipe-dx, 0px); + box-sizing: border-box; + display: flex; + align-items: center; + gap: 0.4em; + padding-left: 8px; + overflow: hidden; + white-space: nowrap; + pointer-events: none; + font-size: var(--yj-text-xs); + background-color: var(--yj-bg-elevated, #343a40); + color: var(--yj-text-secondary, #b3b3b3); + } + + .swipe-reveal.armed { + background-color: var(--yj-success, #2f9e44); + color: var(--yj-success-fg, #fff); + } + + /* The children move, not the row -- see the header. */ + [data-swipe].swiping > :not(.swipe-reveal) { + transform: translateX(var(--yj-swipe-dx, 0px)); + } + + [data-swipe].settling > * { + transition: + transform 160ms ease-out, + width 160ms ease-out; + } + + @media (prefers-reduced-motion: reduce) { + [data-swipe].settling > * { + transition: none; + } + } +`; diff --git a/frontend/test/components/touch-selection.test.ts b/frontend/test/components/touch-selection.test.ts index 29351df..a4fec7b 100644 --- a/frontend/test/components/touch-selection.test.ts +++ b/frontend/test/components/touch-selection.test.ts @@ -26,6 +26,9 @@ import { fixture, shadow, shadowAll } from '@test/support/render'; import { installTouchGestures, LONG_PRESS_MS } from '@utils/touch-gestures'; const HELD = LONG_PRESS_MS + 120; + +/** Comfortably past `explore-link`'s own DOUBLE_CLICK_GRACE_MS of 250. */ +const EXPLORE_LINK_GRACE = 400; const BRIEF = Math.round(LONG_PRESS_MS / 4); const wait = (ms: number) => new Promise((r) => setTimeout(r, ms)); @@ -39,6 +42,12 @@ function track(n: number) { Album: 'An Album', Duration: 100 + n, ID: n, + // Tagged, so the title's `explore-link` can actually navigate. + // Without an MBID it asks the backend for a local album first and + // gives up when nothing answers -- which makes "the tap did not + // navigate" true of every build, working or not. + ReleaseGroupMBID: 'e8f4b1d2-0000-4000-8000-00000000000' + n, + RecordingMBID: 'a1b2c3d4-0000-4000-8000-00000000000' + n, }; } @@ -60,11 +69,24 @@ function press(el: EventTarget, type: string, init: PointerEventInit = {}) { ); } -/** A whole finger tap: down, a moment, up. */ +/** + * A whole finger tap: down, a moment, up, and **the click a browser + * fires afterwards**. + * + * That last event is not decoration. A tap the component claims has + * its click swallowed at document capture, and the click is the only + * thing that would otherwise select the row, follow the `explore-link` + * in its title, or press whatever the finger landed on. A helper that + * stops at `pointerup` asserts none of that and passes on a build with + * the swallow deleted. + */ async function tap(el: EventTarget) { press(el, 'pointerdown'); await wait(BRIEF); press(el, 'pointerup'); + el.dispatchEvent( + new MouseEvent('click', { bubbles: true, composed: true, cancelable: true }), + ); await wait(0); } @@ -277,3 +299,98 @@ describe('', () => { } }); }); + +/** + * What the gestures leave behind (plan 019 phase 4, #63). + * + * Every track, album and artist name in a row is an `explore-link`, + * which navigates on a genuine single click — and a row's single + * *tap* now plays. That conflict is #67's to answer properly; what + * this plan committed to is the narrower half of it, that **tap-to-play + * wins on touch**, and it falls out of phase 1's design rather than + * needing a rule: a claimed tap has its click swallowed at document + * capture, so the link's own handler never runs. + * + * It falls out, which is exactly why it is asserted. Nothing else in + * the suite would fail if the swallow stopped covering the link, and + * the symptom — a tap on a track's title navigating to its album + * instead of playing it — is one a phone user meets constantly and a + * mouse user never does. + */ +describe('a tap on a name inside a row', () => { + beforeEach(() => { + resetHarness(); + stub('library.Library.GetTracks', TRACKS); + stub('library.Library.GetAllLibrariesWithTrackCounts', []); + stub('config.Config.GetShortcuts', {}); + stub('queue.Queue.SetQueue', null); + uninstall = installTouchGestures(); + }); + + afterEach(() => { + uninstall?.(); + uninstall = null; + vi.restoreAllMocks(); + }); + + /** Where the row's title is rendered, which is a link. */ + function titleLink(el: HTMLElement, row: number): HTMLElement { + const link = rows(el)[row]?.querySelector('.explore-link'); + + expect(link, 'the row renders its title as a link').toBeTruthy(); + + return link as HTMLElement; + } + + it('plays the row rather than navigating', async () => { + const el = await mountList(); + const navigations: Event[] = []; + + document.addEventListener('navigate', (e) => navigations.push(e)); + + await tap(titleLink(el, 1)); + await flush(); + // `explore-link` holds a navigation for DOUBLE_CLICK_GRACE_MS, so + // asserting sooner passes on a build that is about to navigate. + await wait(EXPLORE_LINK_GRACE); + + expect(calls('queue.Queue.SetQueue').length, 'the row played').toBe(1); + expect(navigations, 'and nothing navigated').toHaveLength(0); + }); + + it('toggles the row while selection mode is on', async () => { + const el = await mountList(); + const navigations: Event[] = []; + + document.addEventListener('navigate', (e) => navigations.push(e)); + + await hold(rows(el)[0]!); + await el.updateComplete; + await tap(titleLink(el, 2)); + await el.updateComplete; + await wait(EXPLORE_LINK_GRACE); + + expect( + (shadow(el, 'selection-bar') as unknown as { count: number }).count, + ).toBe(2); + expect(navigations).toHaveLength(0); + }); + + it('leaves the mode on Escape', async () => { + // A mode changes what a tap means, so it needs an exit that is not + // "find the x". It is a dismissal rather than a shortcut, which is + // why it is not a panel-scoped binding. + const el = await mountList(); + + await hold(rows(el)[0]!); + await el.updateComplete; + + document.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }), + ); + await el.updateComplete; + + expect(shadow(el, 'selection-bar')).toBeFalsy(); + expect(rows(el)[0]?.getAttribute('aria-selected')).toBe('false'); + }); +}); diff --git a/frontend/test/components/touch-surfaces.test.ts b/frontend/test/components/touch-surfaces.test.ts new file mode 100644 index 0000000..43bddf0 --- /dev/null +++ b/frontend/test/components/touch-surfaces.test.ts @@ -0,0 +1,333 @@ +/** + * The other three selecting surfaces (plan 019 phase 3, #63). + * + * `track-list` got the gestures in phases 1 and 2; the queue panel and + * both playlist detail views are the rest, and phase 3 was "mostly + * wiring" only in the sense that they already share + * `SelectionController`. Two of them are not symmetric with the track + * list at all, and those two asymmetries are what this file is for: + * + * **A tap on a queue row plays that position in the queue.** Copying + * `track-list`'s tap — which sets the queue to the list the row is in — + * would rebuild the queue from the queue, discarding its source, its + * shuffle order and everything inserted by hand along the way. It is + * not the no-op it reads as. + * + * **The queue panel has no swipe, deliberately.** A right swipe means + * *add to the queue* everywhere it exists, and a queue row is already + * in the queue; the only thing it could mean there is *remove*, which + * is the same gesture with the opposite effect one screen away. So the + * assertion is that the rows do not opt in — a swipe there must not + * silently become a second meaning for the app's one horizontal + * gesture. + * + * The playlist views are the symmetric half, and they are here because + * they bind their gestures **per template** rather than through the + * `firstUpdated` delegation the two virtualized lists use, so "the + * handler is attached at all" is a different question in each. + */ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import '@components/queue-panel/queue-panel'; +import '@components/playlist-details/playlist-details'; + +import { Events } from '../../src/events'; +import { calls, emit, flush, resetHarness, stub } from '@test/support/harness'; +import { fixture, shadow, shadowAll } from '@test/support/render'; +import { installTouchGestures, LONG_PRESS_MS } from '@utils/touch-gestures'; +import type { QueueTrack } from '@store/queue-store'; + +const HELD = LONG_PRESS_MS + 120; +const BRIEF = Math.round(LONG_PRESS_MS / 4); +const wait = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +let uninstall: (() => void) | null = null; + +afterEach(() => { + // The layer is one document listener set, so a suite that leaves it + // installed makes the next file's gestures fire twice. + uninstall?.(); + uninstall = null; +}); + +function queueTrack(n: number): QueueTrack { + return { + id: n, + audioFileId: n, + filePath: `/music/${n}.mp3`, + position: n, + title: `Track ${n}`, + artist: 'Artist', + album: 'Album', + coverArtPath: '', + artistMbid: '', + releaseGroupMbid: '', + recordingMbid: '', + }; +} + +const QUEUE = [1, 2, 3, 4].map(queueTrack); + +function playlistTrack(n: number) { + return { + ID: n, + FilePath: `/music/${n}.mp3`, + Title: `Track ${n}`, + Artist: 'Artist', + Album: 'Album', + Duration: 100 + n, + Position: n, + Phantom: false, + }; +} + +const PLAYLIST_TRACKS = [1, 2, 3, 4].map(playlistTrack); + +function press(el: EventTarget, type: string, init: PointerEventInit = {}) { + el.dispatchEvent( + new PointerEvent(type, { + bubbles: true, + composed: true, + cancelable: true, + pointerType: 'touch', + isPrimary: true, + clientX: 40, + clientY: 60, + ...init, + }), + ); +} + +async function tap(el: EventTarget) { + press(el, 'pointerdown'); + await wait(BRIEF); + press(el, 'pointerup'); + await wait(0); +} + +async function hold(el: EventTarget) { + press(el, 'pointerdown'); + await wait(HELD); + press(el, 'pointerup'); + await wait(0); +} + +/** Drag a row sideways by `dx` and lift, as one finger. */ +async function swipe(el: EventTarget, dx: number) { + const at = (x: number) => + new Touch({ + identifier: 1, + target: el as Element, + clientX: x, + clientY: 100, + }); + const send = (type: string, points: Touch[]) => + el.dispatchEvent( + new TouchEvent(type, { + bubbles: true, + composed: true, + cancelable: true, + touches: points, + changedTouches: points.length > 0 ? points : [at(0)], + }), + ); + + send('touchstart', [at(0)]); + + for (const step of [0.25, 0.5, 0.75, 1]) { + send('touchmove', [at(dx * step)]); + await Promise.resolve(); + } + + send('touchend', []); + await wait(0); +} + +/** Past any commit threshold the row could compute. */ +const FAR = 400; + +describe('a finger on a queue row', () => { + beforeEach(() => { + resetHarness(); + stub('config.Config.GetShortcuts', {}); + stub('queue.Queue.PlayIndex', null); + stub('queue.Queue.SetQueue', null); + stub('queue.Queue.AddTracks', null); + uninstall = installTouchGestures(); + }); + + async function panel() { + const el = await fixture('queue-panel', { open: true }); + + emit(Events.QueueChanged, { + tracks: QUEUE, + currentIndex: 0, + shuffleMode: false, + repeatMode: 'off', + sourcePlaylistId: 0, + }); + await flush(); + await el.updateComplete; + await new Promise((r) => { + requestAnimationFrame(() => r(null)); + }); + + return el; + } + + const rows = (el: HTMLElement) => shadowAll(el, '.track-item'); + + it('plays that position rather than rebuilding the queue', async () => { + const el = await panel(); + + await tap(rows(el)[2]!); + await flush(); + + expect(calls('queue.Queue.PlayIndex')[0]?.args[0]).toBe(2); + + // The asymmetry with `track-list`: setting the queue here would + // discard its source, its shuffle order and anything inserted by + // hand, which is not the no-op it reads as. + expect(calls('queue.Queue.SetQueue').length).toBe(0); + }); + + it('enters selection mode on a hold, with that row selected', async () => { + const el = await panel(); + + await hold(rows(el)[1]!); + await el.updateComplete; + + const bar = shadow(el, 'selection-bar'); + + expect(bar, 'the action bar appears').toBeTruthy(); + expect((bar as unknown as { count: number }).count).toBe(1); + expect(calls('queue.Queue.PlayIndex').length, 'and nothing played').toBe(0); + }); + + it('toggles rows while the mode is on, instead of playing them', async () => { + const el = await panel(); + + await hold(rows(el)[0]!); + await el.updateComplete; + await tap(rows(el)[2]!); + await el.updateComplete; + + expect(calls('queue.Queue.PlayIndex').length).toBe(0); + expect( + (shadow(el, 'selection-bar') as unknown as { count: number }).count, + ).toBe(2); + }); + + it('has no swipe, which is a decision and not an omission', async () => { + const el = await panel(); + + expect( + rows(el)[1]?.hasAttribute('data-swipe'), + 'the row does not opt into the shared rule', + ).toBe(false); + + await swipe(rows(el)[1]!, FAR); + await flush(); + + // A right swipe means "add to the queue" everywhere it exists. + // The only thing it could mean on a queue row is "remove", which + // is the same gesture with the opposite effect one screen away. + expect(calls('queue.Queue.AddTracks').length).toBe(0); + expect(calls('queue.Queue.RemoveTracks').length).toBe(0); + }); +}); + +describe('a finger on a playlist row', () => { + beforeEach(() => { + resetHarness(); + stub('config.Config.GetShortcuts', {}); + stub('playlist.Service.GetPlaylist', { + ID: 1, + Name: 'A Playlist', + TrackCount: PLAYLIST_TRACKS.length, + }); + stub('playlist.Service.GetPlaylistTracks', PLAYLIST_TRACKS); + stub('queue.Queue.SetQueue', null); + stub('queue.Queue.AddTracks', null); + uninstall = installTouchGestures(); + }); + + async function details() { + const el = await fixture('playlist-details', { + playlistId: 1, + playlistName: 'A Playlist', + }); + + await flush(); + await el.updateComplete; + await wait(60); + await el.updateComplete; + + return el; + } + + const rows = (el: HTMLElement) => shadowAll(el, '.track-item'); + + it('plays the playlist from the row it taps', async () => { + const el = await details(); + + expect(rows(el).length, 'the list rendered rows').toBeGreaterThan(2); + + await tap(rows(el)[2]!); + await flush(); + + const queued = calls('queue.Queue.SetQueue'); + + // The app's rule: activating one row plays the list that row is + // in, from that row -- not a queue of one that stops when the song + // ends. + expect(queued.length).toBe(1); + expect(queued[0]?.args[1]).toBe(2); + expect((queued[0]?.args[0] as string[]).length).toBe( + PLAYLIST_TRACKS.length, + ); + }); + + it('enters selection mode on a hold', async () => { + const el = await details(); + + await hold(rows(el)[1]!); + await el.updateComplete; + + expect( + (shadow(el, 'selection-bar') as unknown as { count: number } | null) + ?.count, + ).toBe(1); + expect(calls('queue.Queue.SetQueue').length, 'nothing played').toBe(0); + }); + + it('queues the row a swipe crosses', async () => { + const el = await details(); + + await swipe(rows(el)[1]!, FAR); + await flush(); + + expect(calls('queue.Queue.AddTracks')[0]?.args[0]).toEqual([ + '/music/2.mp3', + ]); + }); + + it('opts its rows into the shared touch-action rule', async () => { + // Half of what makes the gesture reach us on the device, and + // invisible in this browser either way -- the other half is the + // non-passive preventDefault in `utils/touch-gestures.ts`. + const el = await details(); + + expect(rows(el)[0]?.hasAttribute('data-swipe')).toBe(true); + + const css = ( + customElements.get('playlist-details') as unknown as { + styles: { cssText: string }[]; + } + ).styles + .map((s) => s.cssText) + .join('\n'); + + expect(css).toContain('touch-action: pan-y'); + }); +}); diff --git a/frontend/test/components/touch-swipe.test.ts b/frontend/test/components/touch-swipe.test.ts index 16de742..07f9595 100644 --- a/frontend/test/components/touch-swipe.test.ts +++ b/frontend/test/components/touch-swipe.test.ts @@ -266,12 +266,20 @@ describe('a finger swiped right across a track row', () => { expect(rows(el)[0]?.getAttribute('aria-selected')).toBe('false'); }); - it('declares pan-y on the row, which is half of what makes it work', () => { - // The other half is the module's non-passive `preventDefault`. - // Neither works alone on Chrome 113 and both are irrelevant here, - // so this reads the stylesheet rather than the rendering — the - // regression is someone tidying the declaration away, and nothing - // in this browser looks different when they do. + it('declares pan-y on the row, which is half of what makes it work', async () => { + // The other half is the gesture module's non-passive + // `preventDefault`. Neither works alone on Chrome 113 and both are + // irrelevant here, so this reads the stylesheet and the attribute + // rather than the rendering — the regression is someone tidying + // one of them away, and nothing in this browser looks different + // when they do. + const el = await mountList(); + + expect( + rows(el)[0]?.hasAttribute('data-swipe'), + 'the row opts into the shared rule', + ).toBe(true); + const sheets = ( customElements.get('track-list') as unknown as { styles: { cssText: string }[]; @@ -280,9 +288,9 @@ describe('a finger swiped right across a track row', () => { const css = sheets.map((s) => s.cssText).join('\n'); const rule = css .split('}') - .find((block) => /\.track-row\s*\{/.test(block)); + .find((block) => /\[data-swipe\]\s*\{/.test(block)); - expect(rule, 'the row rule is still there to read').toBeTruthy(); + expect(rule, 'the shared rule is in this component').toBeTruthy(); expect(rule).toContain('touch-action: pan-y'); expect(css, 'never none: it takes the scrolling too').not.toContain( 'touch-action: none',