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