diff --git a/.planning/plans/active/019-android-touch-model.md b/.planning/plans/completed/019-android-touch-model.md similarity index 53% rename from .planning/plans/active/019-android-touch-model.md rename to .planning/plans/completed/019-android-touch-model.md index b1dd0e3..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 @@ -225,29 +225,184 @@ still works. `SelectionController` gains a mode. `track-list` acts on tap and enters the mode on long press. The action bar. **Phase 2 — swipe right to queue**, with the `touch-action: pan-y` -finding above and a reveal-and-snap affordance. +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. --- +## What phase 2 measured, which was not what phase 2 predicted + +The `touch-action` finding above is **half** of the answer, and +shipping only that half would have been the exact failure it warns +about. Driving a real finger with `adb shell input swipe` across a +track row, three values, all three on the device: + +``` +touch-action: auto pointerdown, 1 move, pointercancel +touch-action: pan-y pointerdown, 2 moves, pointercancel +touch-action: none pointerdown, 2 moves, pointercancel +``` + +`touchmove` kept firing in all three. So **Chrome 113's WebView +cancels the pointer stream ~16px into any drag whatever `touch-action` +says**, and a swipe recognised from `pointermove` — which is what the +rest of this module is built on — is a swipe that dies 16px in. + +The other half is a **non-passive `touchmove` calling +`preventDefault()`**: with it, the same swipe ran to 12 moves and a +`pointerup` at full travel. And both halves are required, which was +measured rather than assumed — with the `preventDefault` in place and +`touch-action` back at `auto`, the gesture died after **one** move. +The reading is that `auto` lets the browser commit to a horizontal pan +on the first move past slop, before any threshold of ours can have +been crossed, while `pan-y` leaves it undecided long enough for the +second move to claim it. + +`none` is the one value to avoid: the list stopped scrolling at all. +With the shipped pair, a vertical drag still scrolls the virtualizer +81px on the same run that a horizontal one survives. + +**`draggable="true"` is not a competitor**, which is the other thing +the device was asked. No `dragstart` fires from a touch drag on this +WebView at all, so the drag-to-playlist attribute on every row needs no +pointer-type gate. + +### And it found a phase 1 defect that no tier can see + +The native `contextmenu` arrives in **either** order, and phase 1 only +handled one of them. `nativeSeen` covers the browser's menu arriving +*during* the hold. The reverse — our 500ms timer firing first, a +component claiming it, and Chrome delivering its own `contextmenu` +50–70ms *later* — was suppressed by nothing, so the context menu +opened on top of the selection bar. Measured over four holds: + +``` +hold 1 yj-long-press, then contextmenu isTrusted=true menu open +hold 2 yj-long-press clean +hold 3 yj-long-press, then contextmenu isTrusted=true menu open +hold 4 yj-long-press clean +``` + +Two in four, on the one surface #63 exists to have changed, and +invisible to both browser tiers because neither synthesises a +`contextmenu` from a dispatched press. A press that has produced its +outcome now suppresses a late one whichever branch it took; six holds +on the fixed build, six clean. + +### The rules phase 2 settled + +- **A swipe is not a selection.** It queues the row it was made on, + unless that row is one of several *explicitly* selected — the same + rule the context menu answers with, because a bar reading "40 + selected" beside a gesture that quietly queues one of them is two + answers to one question. It never changes the selection, which is + where it differs from a right-click. +- **Rightward only.** Nothing is bound to a leftward swipe and + claiming one would take a gesture away to do nothing with it. +- **The commit threshold is a fraction of the row** (0.3, floor 72px), + because the row is 424x52 on this device and a bare pixel count is a + fraction of a row height on one screen and a third of the width on + the next. +- **The affordance 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 ("Add to queue" → "Release + to add" → "Added"), and the outcome is announced in a live region. +- **The row does not move; its cells do.** `.track-row` is + `contain: strict` with `overflow: hidden`, so a pane held at the + row's original position while the row translates is a pane at a + negative offset inside a clipping box and is simply not painted. + Sliding the cells needs no wrapper element in a row that is already + a grid. +- **The travel is written to the row's own style, not rendered.** One + render at the start, one at the threshold, one at the end; a + virtualizer re-rendering every visible row per frame of one finger's + travel is the thing `perf.m1` is about. + ## 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 d50d04e..97eb7a5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1449,18 +1449,112 @@ never opens). **The sweep found two of the fourteen**; twelve were converted by hand. **And a menu opens from a finger, through the event it already has.** -`utils/long-press.ts` is one document-capture listener installed once -from `index.ts`: a touch that holds still for 500 ms dispatches a -synthetic `contextmenu` at the touch point, so all six components that -bind one — delegated on a virtualizer, per row, per card — gained the -gesture without changing. The target is `composedPath()[0]` rather than +`utils/touch-gestures.ts` is one document-capture listener installed +once from `index.ts` — `utils/long-press.ts` until #63 replaced it, +rather than adding a second listener claiming the same 500 ms hold. It +**announces** rather than acts: `yj-tap`, `yj-long-press` and +`yj-swipe-start` are composed and cancelable, and a component claims +one with `preventDefault()`. That is what let #63 reassign the hold +without touching one of the fourteen context menus: an *unclaimed* +`yj-long-press` still becomes a synthetic `contextmenu`, so all six +components that bind one — delegated on a virtualizer, per row, per +card — behave exactly as they did, and only the lists that opt in get +selection mode. The target is `composedPath()[0]` rather than `elementFromPoint`, which stops at the outermost shadow host and so -reaches a delegated listener and no per-row one; a browser that fires -its own long-press `contextmenu` (Chromium does, WebKit and the WebView -vary) wins, ours being told from theirs by **identity** rather than -`isTrusted`, since no test can dispatch a trusted event; and the click -that ends the gesture is swallowed, keyed on the gesture rather than on -a time window so the first tap on the menu it opened is not eaten too. +reaches a delegated listener and no per-row one; and the click that +ends a *claimed* gesture is swallowed, keyed on the gesture rather than +on a time window so the first tap on the menu it opened is not eaten +too. + +Three things about it are load-bearing, and all three were found on the +device rather than in a tier. + +**A browser that fires its own long-press `contextmenu` is a trigger, +not a competitor.** Chromium does, WebKit and the WebView vary. The old +rule was to stand down when a trusted one arrived, which was right +while both paths ended in a context menu and is wrong the moment a hold +can mean something else — standing down silently does the *old* thing. +So the gesture is announced from the native event, and only a component +that claims it suppresses that event. Ours and the browser's are told +apart by **identity** rather than `isTrusted`, since no test can +dispatch a trusted event. + +**That arrives in either order, and both have to be handled.** The +native `contextmenu` mid-hold is one case; the other is our own 500 ms +timer firing first and Chrome delivering its menu **50–70 ms later**, +which nothing suppressed — measured over four holds on the reference +phone, two took that order, so the context menu opened over the +selection bar intermittently, on the one surface #63 changed. A press +that has produced its outcome therefore suppresses a late +`contextmenu` whichever branch it took. + +**A horizontal swipe runs on touch events, and needs two things that +look like one.** Chrome 113's WebView cancels the *pointer* stream +~16 px into any drag — measured at `auto`, `pan-y` and `none` alike, +one or two `pointermove`s and then `pointercancel`, while `touchmove` +kept firing throughout. So the recogniser is `touchmove`, the surface +declares **`touch-action: pan-y`** *and* a claimed swipe calls +**`preventDefault()`** on a non-passive listener. Neither works alone: +with the `preventDefault` in place but `touch-action` back at `auto` +the gesture died after one move, because `auto` lets the browser commit +to a horizontal pan before any threshold can be crossed. `none` is the +value to avoid — it takes the list's own vertical scrolling with it. +**Both are correct in Chromium either way**, which is why this is +written down rather than tested. The tie breaks toward scrolling, in +that order: vertical drift past the tolerance vetoes the swipe for the +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 diff --git a/e2e/specs/touch-gestures.spec.ts b/e2e/specs/touch-gestures.spec.ts index 523f665..6cf1783 100644 --- a/e2e/specs/touch-gestures.spec.ts +++ b/e2e/specs/touch-gestures.spec.ts @@ -1,4 +1,4 @@ -import { test, expect } from '../support/fixtures.js'; +import { test, expect, callBinding } from '../support/fixtures.js'; /** * The touch gestures against the real app (plan 019, #63; long-press @@ -192,3 +192,120 @@ test.describe('a hold anywhere else still opens the menu', () => { expect((await panel(app, 'cover-grid'))?.items).toBeGreaterThan(0); }); }); + +/** + * Swipe right on a track row to queue it (plan 019 phase 2, #63). + * + * The component tier has the rule this obeys — one row is a position, + * several are a choice — against a queue that is a fake. What is only + * true here is that the gesture reaches the *real* queue: `AddTracks` + * is a Go method, the queue is persisted, and "the row was added" + * is a question only the backend can answer. + * + * **It is Chromium-only, and that is a property of the browser rather + * than a gap.** The gesture runs on touch events, because Chrome 113's + * WebView cancels the pointer stream ~16px into any drag whatever + * `touch-action` says. Desktop WebKit implements no `TouchEvent` + * constructor at all — touch events are a mobile-Safari surface — so + * the events this needs cannot be built there. Skipping loudly is + * better than a spec that quietly asserts nothing on half the matrix, + * which is what `layout-overflow.spec.ts` and `back-navigation.spec.ts` + * were each doing when they were green on a broken build. + */ +test.describe('a swipe right on a track row queues it', () => { + test.beforeEach(async ({ app, browserName }) => { + test.skip( + browserName !== 'chromium', + 'desktop WebKit has no TouchEvent constructor to build the gesture from', + ); + + await app.setViewportSize(PHONE); + await app.getByTestId('tab-tracks').click(); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'tracks', + ); + }); + + test.afterEach(async ({ app }) => { + await app.setViewportSize({ width: 1440, height: 900 }); + }); + + /** + * Drag the first row sideways by a fraction of its own width and + * lift. `fraction` is against the row, because the commit threshold + * is — a number of pixels here would be a second declaration of it, + * right on one viewport and wrong on the next. + */ + const swipeFirstRow = (page: Page, fraction: number) => + page.evaluate((f) => { + const row = document + .querySelector('track-list') + ?.shadowRoot?.querySelector('.track-row'); + + if (!row) throw new Error('no track row to swipe'); + + const box = row.getBoundingClientRect(); + const y = box.top + box.height / 2; + const at = (x: number) => + new Touch({ + identifier: 1, + target: row, + clientX: box.left + x, + clientY: y, + }); + const send = (type: string, points: Touch[]) => + row.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(box.width * f * step)]); + } + + send('touchend', []); + }, fraction); + + /** How many tracks the backend says are in the queue. */ + const queueLength = async (page: Page) => { + const state = await callBinding<{ tracks: unknown[] }>( + page, + 'queue.Queue.GetState', + ); + + return state.tracks?.length ?? 0; + }; + + test('adds exactly one track to the real queue', async ({ app }) => { + const before = await queueLength(app); + + await swipeFirstRow(app, 0.6); + + await expect.poll(() => queueLength(app)).toBe(before + 1); + + // Queued, not played: a swipe is not a tap, and the difference is + // what is on screen afterwards. + expect( + await app.getByTestId('main-content').getAttribute('data-active-view'), + ).toBe('tracks'); + }); + + test('does nothing when the finger did not get far enough', async ({ + app, + }) => { + const before = await queueLength(app); + + await swipeFirstRow(app, 0.1); + await app.waitForTimeout(400); + + expect(await queueLength(app)).toBe(before); + }); +}); 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); + + if (!hit) return null; + + return { + index: hit.index, + filePaths: this.swipeTargetKeys(hit.track.FilePath), + label: hit.track.TrackName, + }; + }, + repaint: () => this.virtualizer?.requestUpdate(), + }); + private handleSelectAll = (): void => { this.selection.selectAll(); }; @@ -1016,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; @@ -1213,6 +1237,7 @@ export class TrackList background-color: var(--yj-selection-bg, rgba(100, 160, 255, 0.15)); } + .cell { overflow: hidden; text-overflow: ellipsis; @@ -1311,6 +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.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); } @@ -1457,6 +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.swipe.onSwipeStart); + virt.addEventListener('yj-swipe-move', this.swipe.onSwipeMove); + virt.addEventListener('yj-swipe-end', this.swipe.onSwipeEnd); this.delegationAttached = true; } @@ -1776,6 +1807,30 @@ export class TrackList this.virtualizer?.requestUpdate(); }; + /** + * What a swipe on this row would queue. + * + * The same rule the context menu answers with, and it has to be: + * **one row is a position, several rows are an explicit choice.** + * A finger that swipes a row which is part of a selection of forty + * has not un-made that selection, and queueing the one row it + * touched would quietly contradict the bar above saying forty are + * selected. A swipe on a row *outside* the selection is a statement + * about that row, exactly as a right-click on one is -- and unlike + * a right-click it does not move the selection, because a swipe is + * not a way of selecting anything. + */ + private swipeTargetKeys(filePath: string): string[] { + if ( + this.selection.selectionCount > 1 && + this.selection.isSelected(filePath) + ) { + return this.selection.getSelectedKeysOrdered(); + } + + return [filePath]; + } + private onDelegatedDragStart = (e: DragEvent) => { const hit = this.resolveTrackFromEvent(e); @@ -2197,6 +2252,7 @@ export class TrackList 'track-row': true, active, selected, + swiping: this.swipe.isSwiping(index), })} role="row" aria-rowindex=${index + 1} @@ -2206,8 +2262,10 @@ export class TrackList draggable="true" data-index=${index} data-testid="track-row" + data-swipe data-file-path=${track.FilePath} > + ${this.swipe.renderReveal(index)}
${this.liveStatus(visibleTracks.length)}
+
+ ${this.swipe.announcement} +
${this.tracks.length === 0 ? this.renderPlaceholder() : html` 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/src/utils/touch-gestures.ts b/frontend/src/utils/touch-gestures.ts index f9e554a..885b604 100644 --- a/frontend/src/utils/touch-gestures.ts +++ b/frontend/src/utils/touch-gestures.ts @@ -14,6 +14,13 @@ * * `yj-tap` a short press that did not drift * `yj-long-press` a press that held still for LONG_PRESS_MS + * `yj-swipe-start` a press that has travelled decisively sideways + * + * A claimed swipe is then followed by `yj-swipe-move` and one + * `yj-swipe-end`, which is guaranteed: a swipe that the browser or a + * second finger takes away still ends, with `canceled` set, so the + * affordance a component put on screen always has something to snap + * back from. * * A component that wants the gesture handles it and calls * `preventDefault()`. Nothing else changes. That shape is what lets @@ -80,6 +87,62 @@ * **The click swallow is keyed on the gesture**, cleared by the next * `pointerdown` rather than by a time window, so the first tap on a * sheet that just opened is not eaten too. + * + * ## The swipe runs on touch events, and that is not a style choice + * + * Everything above is Pointer Events. The swipe is not, and the reason + * is measured on the reference device rather than reasoned about: + * **Chrome 113's Android WebView cancels the pointer stream ~16px into + * any drag, whatever `touch-action` says.** Three values were tried on + * a track row, driving a real finger with `adb shell input swipe`: + * + * ``` + * touch-action: auto pointerdown, 1 move, pointercancel + * touch-action: pan-y pointerdown, 2 moves, pointercancel + * touch-action: none pointerdown, 2 moves, pointercancel + * ``` + * + * `touchmove` kept firing throughout all three. So a swipe recognised + * from `pointermove` is a swipe that dies 16px in — plan 019 predicted + * the class of failure ("works in Chromium and not on the phone") and + * named `touch-action: pan-y` as the fix; it is half of it. + * + * The other half is that **a non-passive `touchmove` that calls + * `preventDefault()` is what keeps the gesture ours**. With it, the + * same swipe ran to 12 moves and a `pointerup` at full travel. + * + * Both halves are required, and that was measured too: with the + * `preventDefault` in place but `touch-action` back at `auto`, the + * gesture died after **one** move. The reading is that `auto` lets the + * browser commit to a horizontal pan on the first move past slop — + * before any threshold of ours can have been crossed — while `pan-y` + * leaves it undecided long enough for the second move to claim it. + * + * So a surface that wants a horizontal swipe declares + * `touch-action: pan-y` (`track-list`'s `.track-row` does) *and* gets + * this module's `preventDefault`. Neither alone works on the device, + * and **both work in Chromium either way**, which is exactly why this + * paragraph exists rather than a test. + * + * `touch-action: none` is the one value to avoid: it also takes the + * list's vertical scrolling away, which was measured as a list that + * would not move. + * + * Two consequences of the touch listener worth knowing. + * + * **It is non-passive, which costs the compositor's scroll fast path** + * for the first touchmoves of every scroll, until the browser starts + * scrolling and stops waiting on us. That is the standard price of a + * horizontal gesture in a scroller and it is paid once per gesture, + * not per frame; a vertical drag on the device still scrolls the + * virtualizer 81px on the same measurement that the horizontal one + * survives. + * + * **The tie breaks toward scrolling**, deliberately and in that order: + * vertical drift past the tolerance vetoes the swipe outright, and a + * gesture that is not *strictly* more horizontal than vertical is the + * scroller's. A list that will not scroll is unusable; a swipe that + * needs a second try is not. */ /** How long a press must hold still to mean "long press". */ @@ -92,6 +155,18 @@ export const LONG_PRESS_MS = 500; */ export const MOVE_TOLERANCE_PX = 10; +/** + * How far a press must travel sideways before it is a swipe. + * + * It has a ceiling the other constants do not: the browser's own + * decision is made a little past this, so a threshold much higher is a + * gesture the device never delivers. Measured, the second `touchmove` + * of an `adb input swipe` lands at ~19px and the pointer stream dies + * just after it, so 12 is inside that window with room for a slower + * finger. + */ +export const SWIPE_START_PX = 12; + /** Detail carried by both gesture events. */ export interface GestureDetail { /** Where the finger was, in client coordinates — a menu opens here. */ @@ -99,12 +174,30 @@ export interface GestureDetail { y: number; } +/** Detail carried by the three swipe events. */ +export interface SwipeDetail { + /** Travel from where the finger landed. Signed: right is positive. */ + dx: number; + dy: number; + /** + * The gesture was taken away rather than finished — a second + * finger, a `touchcancel`, a scroll underneath. Only ever true on + * `yj-swipe-end`, and it is the difference between "do the thing" + * and "put the row back". + */ + canceled: boolean; +} + export type GestureEvent = CustomEvent; +export type SwipeEvent = CustomEvent; declare global { interface HTMLElementEventMap { 'yj-tap': GestureEvent; 'yj-long-press': GestureEvent; + 'yj-swipe-start': SwipeEvent; + 'yj-swipe-move': SwipeEvent; + 'yj-swipe-end': SwipeEvent; } } @@ -139,10 +232,44 @@ export function installTouchGestures(): () => void { * anything. */ let swallowClick = false; - /** We dispatched a `contextmenu`, so a trusted one arriving now is - * a duplicate. */ + /** + * This press has already produced its outcome, so a trusted + * `contextmenu` arriving now is a duplicate of it. + * + * It covers **both** outcomes, and that is a fix rather than a + * tidy-up. `nativeSeen` handles the browser's menu arriving + * *during* the hold; the reverse order was never handled, and it + * happens: measured on the reference device over four holds, two + * of them fired our 500ms timer and then delivered a trusted + * `contextmenu` 50-70ms later, which nothing suppressed — so the + * context menu opened on top of the selection bar, intermittently, + * on exactly the surface #63 exists to have changed. Neither the + * component tier nor the e2e tier can see it: no browser they run + * in synthesises a `contextmenu` from a dispatched press at all. + */ let justFired = false; + // --- the swipe, which runs on touch events; see the header ------ + + /** Where the finger landed, and what it landed on. */ + let swipeTarget: EventTarget | null = null; + let swipeOriginX = 0; + let swipeOriginY = 0; + + /** The last travel, kept so a `touchcancel` — which carries no + * coordinates for a touch that is already gone — can still say how + * far the row had moved. */ + let lastDx = 0; + let lastDy = 0; + + /** A component claimed the swipe: it is ours until the finger + * lifts, and every `touchmove` is prevented. */ + let swiping = false; + + /** This press can no longer become a swipe — it went vertical, a + * second finger arrived, or nobody claimed it. */ + let swipeVetoed = false; + const cancel = (): void => { if (timer !== null) clearTimeout(timer); @@ -202,6 +329,12 @@ export function installTouchGestures(): () => void { // selection mode or a card grid let it fall through to a menu. swallowClick = true; + // The press is answered, so a trusted `contextmenu` for it is + // late rather than new. `fireContextMenu` sets this too; it is + // set here as well so the *claimed* branch is covered, which + // is the branch that was showing a menu over the bar. + justFired = true; + // An unclaimed long press is what it has always been. This is // the whole reason the fourteen context menus need no change. if (!announce('yj-long-press', el)) fireContextMenu(el); @@ -223,6 +356,145 @@ export function installTouchGestures(): () => void { timer = setTimeout(onLongPress, LONG_PRESS_MS); }; + /** + * Announce a swipe on the element the finger landed on. + * Returns whether a component claimed it (only `start` asks). + */ + const announceSwipe = ( + name: 'yj-swipe-start' | 'yj-swipe-move' | 'yj-swipe-end', + el: EventTarget, + canceled = false, + ): boolean => { + const event: SwipeEvent = new CustomEvent(name, { + bubbles: true, + cancelable: name === 'yj-swipe-start', + composed: true, + detail: { dx: lastDx, dy: lastDy, canceled }, + }); + + ours.add(event); + el.dispatchEvent(event); + + return event.defaultPrevented; + }; + + /** + * End a claimed swipe, once. + * + * Every exit from a swipe comes through here so that `yj-swipe-end` + * is guaranteed: a component that has put a reveal on screen and a + * row half off its own left edge has no other way to learn the + * gesture is over. + */ + const endSwipe = (canceled: boolean): void => { + const el = swipeTarget; + + swipeTarget = null; + + if (!swiping) return; + + swiping = false; + + if (!el) return; + + // The gesture happened, so the click that ends it is not a + // click on the row it ended over. + swallowClick = true; + announceSwipe('yj-swipe-end', el, canceled); + }; + + const onTouchStart = (e: TouchEvent): void => { + endSwipe(true); + + lastDx = 0; + lastDy = 0; + + // A second finger is a pinch or a scroll, never one of ours. + swipeVetoed = e.touches.length !== 1; + + if (swipeVetoed) return; + + const touch = e.touches[0]; + + if (!touch) return; + + swipeOriginX = touch.clientX; + swipeOriginY = touch.clientY; + // `composedPath()[0]` for the reason the press path uses it: a + // list delegates inside its own shadow root. + swipeTarget = e.composedPath()[0] ?? e.target; + }; + + const onTouchMove = (e: TouchEvent): void => { + if (swipeVetoed || !swipeTarget) return; + + if (e.touches.length !== 1) { + endSwipe(true); + swipeVetoed = true; + + return; + } + + const touch = e.touches[0]; + + if (!touch) return; + + lastDx = touch.clientX - swipeOriginX; + lastDy = touch.clientY - swipeOriginY; + + if (swiping) { + // This is what keeps the stream alive on the device. It is + // only ever reached for a *claimed* swipe, so nothing that + // scrolls is ever prevented. + e.preventDefault(); + announceSwipe('yj-swipe-move', swipeTarget); + + return; + } + + // Vertical first: past the tolerance the list has it, and a + // gesture that is exactly diagonal is the list's too. + if ( + Math.abs(lastDy) > MOVE_TOLERANCE_PX && + Math.abs(lastDy) >= Math.abs(lastDx) + ) { + swipeVetoed = true; + swipeTarget = null; + + return; + } + + if ( + Math.abs(lastDx) < SWIPE_START_PX || + Math.abs(lastDx) <= Math.abs(lastDy) + ) { + return; + } + + if (!announceSwipe('yj-swipe-start', swipeTarget)) { + // Nobody wants it. Leave the gesture to the browser rather + // than holding it open for the rest of the press. + swipeVetoed = true; + swipeTarget = null; + + return; + } + + swiping = true; + + // It is not a tap and it is not a hold. + cancel(); + e.preventDefault(); + }; + + const onTouchEnd = (): void => { + endSwipe(false); + }; + + const onTouchCancel = (): void => { + endSwipe(true); + }; + const onPointerMove = (e: PointerEvent): void => { if (timer === null) return; @@ -301,25 +573,45 @@ export function installTouchGestures(): () => void { // before anything that would act on the event. const opts = { capture: true } as const; + // Non-passive, because `onTouchMove` has to be able to prevent the + // default for a claimed swipe -- see the header. The other three + // are passive: they only read. + const blocking = { capture: true, passive: false } as const; + const listening = { capture: true, passive: true } as const; + + /** A surface moved under the finger: neither gesture survives it. */ + const abort = (): void => { + endSwipe(true); + cancel(); + }; + document.addEventListener('pointerdown', onPointerDown, opts); document.addEventListener('pointermove', onPointerMove, opts); document.addEventListener('pointerup', onPointerUp, opts); document.addEventListener('pointercancel', cancel, opts); document.addEventListener('contextmenu', onContextMenu, opts); document.addEventListener('click', onClick, opts); + document.addEventListener('touchstart', onTouchStart, listening); + document.addEventListener('touchmove', onTouchMove, blocking); + document.addEventListener('touchend', onTouchEnd, listening); + document.addEventListener('touchcancel', onTouchCancel, listening); // A scroll started by something other than the finger (momentum, a // programmatic reveal) still means the press was not a press. - document.addEventListener('scroll', cancel, { capture: true, passive: true }); + document.addEventListener('scroll', abort, listening); uninstall = () => { - cancel(); + abort(); document.removeEventListener('pointerdown', onPointerDown, opts); document.removeEventListener('pointermove', onPointerMove, opts); document.removeEventListener('pointerup', onPointerUp, opts); document.removeEventListener('pointercancel', cancel, opts); document.removeEventListener('contextmenu', onContextMenu, opts); document.removeEventListener('click', onClick, opts); - document.removeEventListener('scroll', cancel, opts); + document.removeEventListener('touchstart', onTouchStart, opts); + document.removeEventListener('touchmove', onTouchMove, opts); + document.removeEventListener('touchend', onTouchEnd, opts); + document.removeEventListener('touchcancel', onTouchCancel, opts); + document.removeEventListener('scroll', abort, opts); uninstall = null; }; diff --git a/frontend/test/components/touch-gestures.test.ts b/frontend/test/components/touch-gestures.test.ts index 6e483dc..50d0fdb 100644 --- a/frontend/test/components/touch-gestures.test.ts +++ b/frontend/test/components/touch-gestures.test.ts @@ -424,6 +424,24 @@ describe("the browser's own long press", () => { expect(menus).toHaveLength(0); }); + it('suppresses a menu that arrives after the hold was claimed', async () => { + // The order the device actually produces, and the one that was + // missing: our 500ms timer fires first and a component claims it, + // then Chrome delivers its own `contextmenu` 50-70ms later. + // Measured over four holds on the reference phone, two took this + // order -- so the context menu opened over the selection bar, + // intermittently, on the one surface #63 changed. + const menus = recordMenus(inner); + + inner.addEventListener('yj-long-press', (e) => e.preventDefault()); + + press(inner, 'pointerdown'); + await wait(HELD); + browserContextMenu(inner); + + expect(menus, 'the menu is late, not new').toHaveLength(0); + }); + it('still opens exactly one menu when nobody claims it', async () => { // The old behaviour, reached by asking instead of assuming. This // is what leaves the card grids, Explore and the playlist rows @@ -438,3 +456,226 @@ describe("the browser's own long press", () => { expect(menus).toHaveLength(1); }); }); + +/** + * The swipe half (plan 019 phase 2, #63). + * + * It runs on touch events rather than pointer events, and that is the + * one thing about it a browser tier cannot check. Measured on the + * reference device: Chrome 113's WebView cancels the *pointer* stream + * ~16px into any drag whatever `touch-action` says, while `touchmove` + * keeps firing — so what these assert is the shape that survives it, + * not that it survives. + * + * What they can hold is everything else: the axis rule, that the tie + * breaks toward the scroller, that an unclaimed swipe is left entirely + * alone, that a claimed one prevents the default (which is the half of + * the device fix that lives in code), and that an end always arrives. + */ +describe('a finger dragged sideways', () => { + beforeEach(() => { + uninstall = installTouchGestures(); + ({ host, inner } = mountRow()); + }); + + afterEach(() => { + uninstall?.(); + uninstall = null; + host.remove(); + }); + + /** One finger, at an offset from where it landed. */ + function touch(el: EventTarget, type: string, dx = 0, dy = 0): TouchEvent { + const point = new Touch({ + identifier: 1, + target: el as EventTarget as Element, + clientX: 40 + dx, + clientY: 60 + dy, + }); + const event = new TouchEvent(type, { + bubbles: true, + composed: true, + cancelable: true, + touches: type === 'touchend' || type === 'touchcancel' ? [] : [point], + changedTouches: [point], + }); + + el.dispatchEvent(event); + + return event; + } + + /** Record the swipe events a component would bind. */ + function recordSwipes( + el: EventTarget, + opts: { claim?: boolean } = {}, + ): { type: string; dx: number; canceled: boolean }[] { + const seen: { type: string; dx: number; canceled: boolean }[] = []; + + for (const name of ['yj-swipe-start', 'yj-swipe-move', 'yj-swipe-end']) { + el.addEventListener(name, (e) => { + const detail = (e as CustomEvent<{ dx: number; canceled: boolean }>) + .detail; + + if (name === 'yj-swipe-start' && opts.claim !== false) { + e.preventDefault(); + } + + seen.push({ type: name, dx: detail.dx, canceled: detail.canceled }); + }); + } + + return seen; + } + + it('announces a swipe once it has travelled decisively sideways', () => { + const seen = recordSwipes(inner); + + touch(inner, 'touchstart'); + touch(inner, 'touchmove', 4); + expect(seen, 'a wobble is not a swipe').toHaveLength(0); + + touch(inner, 'touchmove', 30); + touch(inner, 'touchmove', 60); + touch(inner, 'touchend'); + + expect(seen.map((s) => s.type)).toEqual([ + 'yj-swipe-start', + 'yj-swipe-move', + 'yj-swipe-end', + ]); + expect(seen.at(-1)?.dx).toBe(60); + expect(seen.at(-1)?.canceled).toBe(false); + }); + + it('prevents the default only for a claimed swipe', () => { + // This is the half of the device fix that lives in code: a + // non-passive `touchmove` calling `preventDefault` is what keeps + // the gesture ours on Chrome 113. Preventing anything else would + // be taking the list's scrolling away. + recordSwipes(inner); + + touch(inner, 'touchstart'); + + const early = touch(inner, 'touchmove', 4); + + expect(early.defaultPrevented, 'a wobble scrolls').toBe(false); + + const claimed = touch(inner, 'touchmove', 30); + const after = touch(inner, 'touchmove', 60); + + expect(claimed.defaultPrevented).toBe(true); + expect(after.defaultPrevented).toBe(true); + }); + + it('leaves an unclaimed swipe entirely alone', () => { + const seen = recordSwipes(inner, { claim: false }); + + touch(inner, 'touchstart'); + touch(inner, 'touchmove', 30); + + const later = touch(inner, 'touchmove', 60); + + touch(inner, 'touchend'); + + // Asked once, refused, and then not asked again for the rest of + // the press -- and nothing prevented, so the browser still owns it. + expect(seen.map((s) => s.type)).toEqual(['yj-swipe-start']); + expect(later.defaultPrevented).toBe(false); + }); + + it('gives a drag that went vertical to the scroller, and keeps it', () => { + // The veto is a *latch*, and that is the whole of it: a scroll + // that curves — which is what a thumb does — would otherwise + // become a swipe halfway down the list, snatching the list out + // from under itself. Without the latch the second move here is + // decisively horizontal and would claim the gesture. + const seen = recordSwipes(inner); + + touch(inner, 'touchstart'); + touch(inner, 'touchmove', 4, 30); + touch(inner, 'touchmove', 80, 35); + touch(inner, 'touchend'); + + expect(seen, 'the list has it').toHaveLength(0); + }); + + it('gives the scroller the tie as well', () => { + // Exactly diagonal is not "decisively sideways". A list that will + // not scroll is unusable; a swipe that needs a second try is not. + const seen = recordSwipes(inner); + + touch(inner, 'touchstart'); + touch(inner, 'touchmove', 60, 60); + touch(inner, 'touchend'); + + expect(seen).toHaveLength(0); + }); + + it('is not a tap and not a hold once it is a swipe', async () => { + const menus = recordMenus(inner); + const taps: Event[] = []; + + inner.addEventListener('yj-tap', (e) => taps.push(e)); + recordSwipes(inner); + + press(inner, 'pointerdown'); + touch(inner, 'touchstart'); + touch(inner, 'touchmove', 40); + await wait(HELD); + touch(inner, 'touchend'); + press(inner, 'pointerup'); + + expect(menus, 'the hold did not become a menu').toHaveLength(0); + expect(taps, 'the lift did not become a tap').toHaveLength(0); + }); + + it('always ends, even when the gesture is taken away', () => { + // A component that has a row half off its own left edge has no + // other way to learn the finger is gone -- so `touchcancel` is an + // end with `canceled` set, not a silence. + const seen = recordSwipes(inner); + + touch(inner, 'touchstart'); + touch(inner, 'touchmove', 40); + touch(inner, 'touchcancel'); + + expect(seen.at(-1)?.type).toBe('yj-swipe-end'); + expect(seen.at(-1)?.canceled).toBe(true); + expect(seen.at(-1)?.dx).toBe(40); + }); + + it('is never one gesture when there are two fingers', () => { + const seen = recordSwipes(inner); + const two = new TouchEvent('touchstart', { + bubbles: true, + composed: true, + cancelable: true, + touches: [ + new Touch({ identifier: 1, target: inner, clientX: 40, clientY: 60 }), + new Touch({ identifier: 2, target: inner, clientX: 90, clientY: 60 }), + ], + }); + + inner.dispatchEvent(two); + touch(inner, 'touchmove', 60); + + expect(seen, 'a pinch is not a swipe').toHaveLength(0); + }); + + it('swallows the click a claimed swipe ends on', () => { + const clicks: Event[] = []; + + recordSwipes(inner); + inner.addEventListener('click', (e) => clicks.push(e)); + + touch(inner, 'touchstart'); + touch(inner, 'touchmove', 40); + touch(inner, 'touchend'); + inner.dispatchEvent( + new MouseEvent('click', { bubbles: true, composed: true }), + ); + + expect(clicks, 'the row was not also clicked').toHaveLength(0); + }); +}); 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 new file mode 100644 index 0000000..07f9595 --- /dev/null +++ b/frontend/test/components/touch-swipe.test.ts @@ -0,0 +1,299 @@ +/** + * Swipe right on a track row to queue it (plan 019 phase 2, #63). + * + * `touch-gestures.test.ts` holds the recogniser — the axis rule, the + * claim, the guaranteed end. What is here is what the *list* does with + * it, and the two things that are only true of a list: + * + * **A swipe is not a selection.** It acts on the row it was made on, + * unless that row is one of several the user has explicitly chosen, in + * which case it acts on all of them — the same rule the context menu + * answers with, because a bar saying "40 selected" and a gesture that + * quietly queues one of them is two answers to the same question. + * + * **A short swipe is a no-op**, and that is the only thing standing + * between "add to queue" and a scroll that drifted sideways. The + * threshold is a fraction of the row, so it is measured from the row + * here rather than written down twice. + * + * What this tier cannot see is the device, and the reason is in the + * module's own header: Chrome 113's WebView cancels the pointer stream + * ~16px into any drag whatever `touch-action` says, so the gesture + * runs on touch events and needs `touch-action: pan-y` *and* a + * non-passive `preventDefault`. Both are correct in Chromium either + * way. The stylesheet half is asserted below for the same reason + * `hover-affordance.test.ts` reads a parsed stylesheet: the regression + * is someone tidying the declaration away, and nothing here renders + * differently when they do. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import '@components/track-list/track-list'; + +import { calls, flush, resetHarness, stub } from '@test/support/harness'; +import { fixture, shadow, shadowAll } from '@test/support/render'; +import { installTouchGestures } from '@utils/touch-gestures'; + +const wait = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +let uninstall: (() => void) | null = null; + +function track(n: number) { + return { + FilePath: `/music/track-${n}.mp3`, + TrackName: `Track ${n}`, + ArtistName: 'An Artist', + Album: 'An Album', + Duration: 100 + n, + ID: n, + }; +} + +const TRACKS = [track(1), track(2), track(3), track(4)]; + +async function mountList() { + const el = await fixture('track-list'); + + // A definite width, because the commit threshold is a fraction of + // the row and a list that has not been given one is not a list. + el.style.display = 'block'; + el.style.width = '400px'; + + (el as unknown as { tracks: unknown[] }).tracks = TRACKS; + await flush(); + await el.updateComplete; + await wait(60); + await el.updateComplete; + + return el; +} + +function rows(el: HTMLElement): HTMLElement[] { + return shadowAll(el, '.track-row'); +} + +/** + * Drag a row sideways by `dx` and lift, as one finger. + * + * `onStep` runs after each move and is awaited, which is how the + * reveal is observed: the component writes the travel straight to the + * row's style but renders the pane through Lit, so it exists a frame + * after the move that asked for it, not during it. + */ +async function swipe( + el: EventTarget, + dx: number, + dy = 0, + onStep?: () => Promise | void, +): Promise { + const at = (x: number, y: number) => + new Touch({ + identifier: 1, + target: el as Element, + clientX: x, + clientY: y, + }); + 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, 0)], + }), + ); + + send('touchstart', [at(0, 100)]); + + // Several steps, because the recogniser claims the gesture on the + // move that crosses its threshold and the component reads every one + // after it. + for (const step of [0.25, 0.5, 0.75, 1]) { + send('touchmove', [at(dx * step, 100 + dy * step)]); + if (onStep) await onStep(); + } + + send('touchend', []); +} + +/** Where the commit threshold falls for the row as rendered. */ +function threshold(row: HTMLElement): number { + return Math.max(72, row.getBoundingClientRect().width * 0.3); +} + +describe('a finger swiped right across a track row', () => { + beforeEach(() => { + resetHarness(); + stub('library.Library.GetTracks', TRACKS); + stub('library.Library.GetAllLibrariesWithTrackCounts', []); + stub('config.Config.GetShortcuts', {}); + stub('queue.Queue.SetQueue', null); + stub('queue.Queue.AddTracks', null); + uninstall = installTouchGestures(); + }); + + afterEach(() => { + uninstall?.(); + uninstall = null; + vi.restoreAllMocks(); + }); + + it('adds that row to the queue', async () => { + const el = await mountList(); + const row = rows(el)[1]; + + expect(row, 'the list rendered rows').toBeTruthy(); + + await swipe(row!, threshold(row!) + 40); + await flush(); + + const queued = calls('queue.Queue.AddTracks'); + + expect(queued.length, 'one swipe, one call').toBe(1); + expect(queued[0]?.args[0]).toEqual(['/music/track-2.mp3']); + + // It queues; it does not play. The row that was playing keeps + // playing, which is the difference from a tap. + expect(calls('queue.Queue.SetQueue').length).toBe(0); + }); + + it('does nothing when the finger did not get far enough', async () => { + const el = await mountList(); + const row = rows(el)[1]; + + await swipe(row!, Math.round(threshold(row!)) - 10); + await flush(); + + expect(calls('queue.Queue.AddTracks').length).toBe(0); + }); + + it('leaves a scroll that began on a row to the list', async () => { + // The same shape as `touch-selection.test.ts`'s "does not play a + // row the finger scrolled from", one gesture over: the failure + // this guards against makes the list unusable rather than wrong. + const el = await mountList(); + const row = rows(el)[1]; + + await swipe(row!, 30, 200); + await flush(); + + expect(calls('queue.Queue.AddTracks').length).toBe(0); + }); + + it('reveals what it will do, in words, while the finger is down', async () => { + const el = await mountList(); + const row = rows(el)[1]; + const reveals: (string | undefined)[] = []; + + await swipe(row!, threshold(row!) + 40, 0, async () => { + await el.updateComplete; + reveals.push( + shadow(el, '[data-testid="swipe-reveal"]')?.textContent?.trim(), + ); + }); + + // Not only a colour (WCAG 1.4.1): the pane says what it is for, + // and says something different once the gesture would commit. + expect(reveals.some((t) => t?.includes('Add to queue'))).toBe(true); + expect(reveals.some((t) => t?.includes('Release to add'))).toBe(true); + }); + + it('says what it did, for anyone not watching the row', async () => { + const el = await mountList(); + const row = rows(el)[1]; + + await swipe(row!, threshold(row!) + 40); + await flush(); + await el.updateComplete; + + const said = shadowAll(el, '[role="status"]') + .map((r) => r.textContent?.trim()) + .join(' '); + + expect(said).toContain('Track 2'); + expect(said).toContain('queue'); + }); + + it('queues the whole selection when the row is part of one', async () => { + // One row is a position; several rows are an explicit choice. A + // gesture that quietly queued the one row touched would contradict + // the bar above it saying how many are selected. + const el = await mountList(); + + rows(el)[1]?.dispatchEvent( + new MouseEvent('click', { bubbles: true, composed: true }), + ); + rows(el)[3]?.dispatchEvent( + new MouseEvent('click', { + bubbles: true, + composed: true, + ctrlKey: true, + }), + ); + await el.updateComplete; + + const row = rows(el)[1]; + + await swipe(row!, threshold(row!) + 40); + await flush(); + + expect(calls('queue.Queue.AddTracks')[0]?.args[0]).toEqual([ + '/music/track-2.mp3', + '/music/track-4.mp3', + ]); + }); + + it('queues only the row it touched when that row is outside the selection', async () => { + const el = await mountList(); + + rows(el)[3]?.dispatchEvent( + new MouseEvent('click', { bubbles: true, composed: true }), + ); + await el.updateComplete; + + const row = rows(el)[0]; + + await swipe(row!, threshold(row!) + 40); + await flush(); + + expect(calls('queue.Queue.AddTracks')[0]?.args[0]).toEqual([ + '/music/track-1.mp3', + ]); + + // And it did not become a way of selecting anything. + expect(rows(el)[3]?.getAttribute('aria-selected')).toBe('true'); + expect(rows(el)[0]?.getAttribute('aria-selected')).toBe('false'); + }); + + 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 }[]; + } + ).styles; + const css = sheets.map((s) => s.cssText).join('\n'); + const rule = css + .split('}') + .find((block) => /\[data-swipe\]\s*\{/.test(block)); + + 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', + ); + }); +});