Compare commits

...
Author SHA1 Message Date
logan b5bdba2f38 fix(queue): name the queue header's two older actions
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m44s
CI / e2e (pull_request) Successful in 10m13s
Clear queue and Add queue to playlist were named by a `title` attribute
and nothing else, while the close button beside them has carried an
`aria-label` since #24. They get one too.

`title` is a name, so this is the weak-name case rather than the missing
one: it is the *last* fallback in the accname order, so any content put
inside the button later silently outranks it, and a phone has no hover
to show it. The `title`s stay — on a desktop they are also the tooltip
for an icon-only control, which is a different job.

The assertion is the part worth reading. The obvious spec — `getByRole`
by name, which is what `queue-overlay.spec.ts` already does for the
close button — is **green on the broken build**: measured against the
running pre-fix app, both buttons matched. So the second test states the
property as what it is, that the name is not the tooltip: it removes the
`title` attributes and asks again, which was 0 and 0 on main and is 1
and 1 now.

Closes #170
2026-08-26 05:39:42 -04:00
logan 245647f12b Merge pull request 'feat(ui): warm album art ahead of the scroll' (#215) from feat/65-art-prefetch-ahead into main
CI / check (push) Successful in 2m43s
CI / e2e (push) Successful in 10m14s
2026-08-25 17:58:42 +00:00
logan 3479ae8d39 feat(ui): warm album art ahead of the scroll
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m46s
CI / e2e (pull_request) Successful in 10m13s
Scrolling the albums grid pops art in: the cards already draw the
smallest adequate tier and are already lazy, so what was left is *when*
the request happens. The grids are virtualized, so the `<img>` — and
therefore the fetch — does not exist until the virtualizer renders its
card, which is about 1000px past the viewport, or two screens on the
reference device.

The issue asks for a larger overscan and that is not available:
`_overhang` is a hard-coded `protected` field on `BaseLayout` with no
configuration surface. So the request is issued ahead of the element
instead. `utils/image-prefetch.ts` warms a bounded window either side
of the rendered range, from `rangeChanged` rather than
`visibilityChanged` — the two report different ranges, and a window
measured from what is *visible* is spent on cards that already exist.

Cover and artist URLs are served under `Cache-Control: immutable`
(content-hashed filenames), so a prefetched image is a cache hit by the
time its card is drawn. The bytes are the browser's; what this holds is
the set of URLs asked for, capped and reported to `__yjCacheStats()`.

Measured on the bulk seed (4 988 albums), ten 2 400px jumps, covers in
the viewport with `naturalWidth === 0`: 254 of 258 blank one frame
after the jump and 214 two frames after, against 117 and 77 with the
prefetch.

Closes #65
2026-08-25 13:55:43 -04:00
logan e23e6f9a54 Merge pull request 'feat(android): the phone's "More" is a bottom sheet' (#211) from feat/71-more-as-a-bottom-sheet into main
CI / check (push) Canceled after 0s
CI / e2e (push) Canceled after 0s
2026-08-25 17:55:31 +00:00
logan 52d095e3c6 feat(android): the phone's "More" is a bottom sheet
CI / e2e (push) Skipped
CI / check (push) Skipped
CI / check (pull_request) Successful in 2m47s
CI / e2e (pull_request) Successful in 10m14s
The tab bar's fifth item opened `<app-sidebar>` in a `wa-drawer`
sliding in from the side, which is a desktop shape put on a phone: a
200px column of a 424px screen, opening away from the thumb that asked
for it, with the rest of its 400px band empty. It also had three nested
scrollers in it -- the dialog, its body, and the sidebar's own
`overflow-y: auto` host -- so which box a drag moved depended on where
the finger landed, which is the "only part of the screen scrolls under
my finger" in the report.

It is the same element with `placement="bottom"` and `without-header`,
so the surface is the sheet #60 already built rather than a second
pattern: a `wa-drawer` is a native `<dialog>` opened with `showModal()`,
which is exactly the top layer that finding rests on, so the focus
trap, Escape, tap-outside and `wa-after-hide` come along unchanged and
nothing new has to be proved about paint containment.

The sidebar is still mounted rather than re-listed as data, because the
shell's own copy is `display: none` below 600px rather than removed --
a second list drawing `nav-*` handles is the duplicate-testid failure
this component already renders conditionally to avoid. What `expanded`
means had to grow to say the host owns the *box*: `app-sidebar` writes
an inline width and caps itself at 400px, which beats any rule the host
could write, so the width, the scrolling and the mouse-only resize
handle now follow that attribute. The rows are 48px below 600px, stated
in the sidebar's own stylesheet since that is the only place it renders
there.

Measured in the running app at 424x439: the sheet is 424 wide, 373 tall
(85vh, so there is an outside to tap), rows 48px, one scroller with
`overscroll-behavior: contain`, and Settings' row reachable at the end
of it. Desktop and Compact are untouched.

Closes #71
2026-08-25 13:48:51 -04:00
logan 939915b1fa Merge pull request 'feat(android): the tap highlight goes, a press state replaces it' (#214) from feat/54-native-touch-feel into main
CI / check (push) Successful in 2m58s
CI / e2e (push) Canceled after 0s
2026-08-25 17:48:45 +00:00
logan 3aa2a434b4 feat(android): the tap highlight goes, a press state replaces it
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m44s
CI / e2e (pull_request) Successful in 10m10s
The phone drew a grey box over the bounding rect of whatever was
tapped, which is the web view saying what it is. It is gone in one
declaration: `-webkit-tap-highlight-color` is inherited and an
inherited property crosses a shadow boundary, so `html` in index.css
reaches every shadow root in the app. Measured three roots deep,
rgba(0, 0, 0, 0.18) before and rgba(0, 0, 0, 0) after.

Removing it removes the only touch feedback several surfaces had, so
the press state is part of the same change rather than a later polish
item — with the highlight gone a held row measured the *hover* tint,
which on a phone is synthesised by the hold itself and outlives it.
The four lists' rows, the tab bar, the sidebar's destinations and the
shared context-menu item take --yj-press-overlay on :active; the cards
already had scale(0.97). The press selector carries a state class
because a row is .track-row.selected.active, so a bare :active shows
nothing on the row a phone is most likely to press. And those
surfaces' hover tints move behind (hover: hover) and (pointer: fine),
which is #68's gate applied to a tint rather than a revealed control.

user-select, the other half of the Findings, was already done: the
first rule in index.css covers the shadow roots for the same reason.
touch-action: manipulation is declined — the 300ms delay it is offered
for is already absent on a width=device-width viewport, and what it
would really change is the gesture stack tuned by measurement on a
device this session cannot measure.

Closes #54
2026-08-25 12:51:28 -04:00
logan 944995dc3c Merge pull request 'feat(android): a name is not a link on a phone, the menu carries it' (#208) from feat/67-entity-links-into-menus into main
CI / check (push) Successful in 2m40s
CI / e2e (push) Successful in 10m18s
2026-08-25 16:51:16 +00:00
logan 8de412cf36 feat(android): a name is not a link on a phone, the menu carries it
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m5s
CI / e2e (pull_request) Successful in 10m3s
Every track, album and artist name in the app navigates through
`utils/explore-link.ts`, and every sentence of how it does that is a
desktop compromise: the navigation is held for one double-click
interval so double-clicking the row can still play it, and the target
is a few characters of text inside a row. On touch that is a delay on
an ambiguous target, and since #63 the row's own tap claims the click
anyway -- so the link was unreachable as well as fiddly.

So below the phone breakpoint a name renders as plain text and the
row's context menu carries the destination instead: `go-to-menu.ts`
draws "Go to Artist" / "Go to Album" under exactly the condition the
link is not, using `explore-link`'s own exported routing so an untagged
entity reaches the library page by the same lookup.

Three things this leans on. Suppressing a link with no menu behind it
is not a smaller affordance but a destination the phone cannot reach,
so `keepOnPhone` is the exception for the three surfaces with no row
menu. The items are drawn for a single selection only, which is the
Play item's rule one step on. And there is no "Go to Genre", because
no row renders a genre link to lose -- that would be new navigation
rather than a replacement.

Closes #67
2026-08-25 12:40:10 -04:00
logan aeb173c684 Merge pull request 'fix(ui): the phone's context sheet says when it scrolls' (#209) from fix/207-sheet-scroll-affordance into main
CI / e2e (push) Canceled after 0s
CI / check (push) Successful in 2m52s
2026-08-25 16:39:44 +00:00
logan 025ed59480 Merge pull request 'test(ui): refresh two stale baselines and settle whether they gate' (#205) from test/196-visual-tier-gates into main
CI / check (push) Canceled after 0s
CI / e2e (push) Canceled after 0s
2026-08-25 16:39:32 +00:00
logan a82d29abd7 docs(agent): drop #204's workaround from the baseline rule
CI / e2e (push) Skipped
CI / check (push) Skipped
CI / check (pull_request) Successful in 2m43s
CI / e2e (pull_request) Successful in 10m1s
#204 landed first, so the ui-tier rule can name `make ui-visual-update
UI_ARGS=<path>` rather than the raw vitest invocation it needed while
the recipe swallowed its filter.
2026-08-25 12:38:17 -04:00
logan d365850321 test(ui): refresh two stale baselines and settle whether they gate
`make ui-visual` had been red on main since #27, and nothing runs it,
so four references had drifted across three unrelated merges. Two were
refreshed with #186; these are the other two.

Each recorded two changes, not one. `app-sidebar` lost Jobs (#27,
shipped) and moved its highlight from Home to Tracks; `now-playing`
gained the source line (shipped) and was playing from "a dynamic mix".
Both are singleton stores read by a case that sets nothing, so the shot
photographs whatever the case above it left behind — blessing that
would have pinned the file's own ordering into a PNG. Both cases state
their world now, and only then are the references re-recorded.

The second half of the issue asks whether this tier should gate, and
the answer is measured rather than preferred: replayed in a bare
ubuntu:24.04 container — CI's `check` image — three of the ten
baselines fail on rendering alone (`track-info` and one `page-header`
shot at ratio 0.03 against a 0.02 allowance, `seek-bar` one pixel
shorter), and the two stale ones disagree about their new height
between the machines. So CI cannot run this suite without a second,
container-recorded baseline set that every local run would then fail
against, and a pre-push hook is the same fault with the machines
swapped. It stays local and opt-in; what replaces the gate is the rule
that a change moving a component's geometry refreshes that component's
baseline in the same commit, having read the image, and never one it
did not cause. Written where a person meets it: the skill's tier doc
has the table, SKILL.md has the obligation, CLAUDE.md has the
constraint.

Deleting the baselines was the third option and is declined: this tier
has caught one thing no other could, the `<span>` that lost the UA
stylesheet's `box-sizing` and grew a badge 36→38px.

Closes #196
2026-08-25 12:38:06 -04:00
logan 3a2d3e8ef8 Merge pull request 'fix(metadata): read a WAV's tags out of its RIFF id3 chunk' (#218) from fix/104-wav-tags-read into main
CI / check (push) Canceled after 1m18s
CI / e2e (push) Canceled after 0s
2026-08-25 16:37:56 +00:00
logan 871a3b7aac Merge pull request 'test(ui): make ui-visual-update honour its file filter' (#206) from fix/204-ui-visual-update-filter into main
CI / check (push) Canceled after 0s
CI / e2e (push) Canceled after 0s
2026-08-25 16:37:53 +00:00
logan f3207e8bf9 Merge pull request 'test(ui): clear localStorage between component tests' (#219) from fix/138-ui-test-storage-leak into main
CI / check (push) Canceled after 3s
CI / e2e (push) Canceled after 0s
2026-08-25 16:37:42 +00:00
logan 772c71c49f test(ui): clear localStorage between component tests
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m29s
CI / e2e (pull_request) Successful in 9m41s
A test file does not get its own origin. `@vitest/browser-playwright`
opens one BrowserContext per session and runs several files in it, one
after another, so everything a component persists survives from file to
file. `track-list` restores its sort in `connectedCallback` and
`aria-tail.test.ts` activates the Title column header, so any file that
mounts a track list later in that tab opens sorted by title — where
`track-11` precedes `track-3`, which is #138's failure exactly.

Nothing about it is specific to that pair: a probe that throws when a
test starts with a non-empty `localStorage` failed 24 test-starts in one
full run (11 with the two sort keys, 8 with `cover-grid-size`, 5 with
`track-list-column-widths`) and cascaded into 248 failures. Which files
share a tab, and in what order, changes run to run, which is the whole
of why this reads as a 1-in-3 flake and passes in isolation.

The clear belongs in `setup.ts` rather than in the specs that write,
because the spec that reads is never the one that knows — and it is
safe for the same reason the leak exists: files within a session are
sequential, so it cannot wipe storage a concurrent file is using.

The spec now also states the order it asserts rather than inheriting a
default, and checks the row it is about to double-click carries the path
it expects, so a stray sort fails by naming itself instead of as an
off-by-eight file path.

Closes #138
2026-08-24 06:47:05 -04:00
logan c56eae2959 fix(metadata): read a WAV's tags out of its RIFF id3 chunk
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m32s
CI / e2e (pull_request) Successful in 9m46s
tagwriter has always written a WAV's tags into a RIFF "id3 " chunk
correctly, and dhowden/tag -- which metadata.ExtractTags is built on --
has no RIFF reader at all.  So the app could not see tags it had just
written: editing tags on a WAV, autotagging a WAV folder or importing a
WAV download all appeared to succeed and changed nothing the library
could show, while the file on disk really was tagged and other players
read it.

backend/riff is a new package rather than a move into either half,
because tagwriter already imports metadata: reaching back for parseRIFF
is an import cycle, not merely the wrong direction.  backend/tagtotals
is the precedent.

Its two readers are deliberately different.  Parse holds every chunk in
memory, which is what rewriting a file needs -- and a WAV's audio *is*
a chunk, so doing that on the scan path would read every WAV in the
library in full.  ID3Chunk seeks over what it is not looking for.

The container is asked before tag.ReadFrom rather than after it fails,
because that library's last resort is an ID3v1 trailer and a WAV
carrying both would otherwise be read by the wrong one.  An untagged
WAV -- no chunk, an RF64 container, a tag with every frame cleared --
reads as empty metadata with no TagReadWarning: the scanner's filename
fallback is the right answer there, and a warning would report a fault
on a healthy file.

The gap was pinned by TestWAVTagsAreNotReadableYet, which failed the
moment the reader learned and said in its own comment what to update.
So it goes, TestFixturesMatchManifest no longer skips wav, and
totals_test.go's WAV case reads through metadata.ExtractTags like the
other three formats -- a round trip asserted through the writer's own
parser was a test of the writer, which is why nothing caught this.

Closes #104
2026-08-24 05:44:39 -04:00
logan 8c85db8968 docs(ui): quote the shipped fade's own measurement
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m29s
CI / e2e (pull_request) Successful in 9m33s
The comment on `wa-dialog::part(body)` carried bottom-edge pixels from
an intermediate probe (29,33,36) while `.planning/NOTES.md` recorded
the final sample against the shipped rule (22,24,27) — the same
gradient, read a few pixels higher up the box. A measurement written in
two places has to agree, or neither can be trusted.
2026-08-23 07:04:04 -04:00
logan 02e2251bb2 fix(ui): the phone's context sheet says when it scrolls
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m28s
CI / e2e (pull_request) Successful in 9m40s
The bottom sheet's body has scrolled since #60 and said nothing about
it. Measured at 424x439, the track list's menu ended at y=470 with the
fold at 439 — reachable, since the body is `overflow-y: auto`, but with
no affordance saying so, and worst where the cut lands on a row
boundary and the sheet ends in a clean edge that reads as the end of
the list.

The cap stays: `menu-surface`'s own comment says a surface covering the
whole screen is a page, not a sheet. What changes is that the body
draws a fade, from two background layers whose *attachments* are the
feature — a shadow pinned to the box (`scroll`) under a cover of the
sheet's own colour painted at the end of the content (`local`), which
scrolls up over the shadow exactly when there is nothing more to see.
So the fade is absent on a menu that fits, present the moment one does
not, and gone again at the end of the list, with no scroll listener and
nothing reaching into `wa-dialog`'s shadow root for the scroller.
`background-attachment` is Chrome 4; the reference device is Chrome 113.

The other two options in the report — a shortened last row, or a max
height that makes the cut obvious — both need `height mod 48`, which
CSS cannot express, and the observed case is exactly the one where the
cut already lands on a row boundary.

The curve is steep rather than linear because the rows under it stay
live: a scrim over a menu item is that item's text surface, so the
4.5:1 rule reaches it, and the light ramp is what makes that real.
A 48px linear scrim at 0.8 greyed the last label to 5.0:1; 32px already
down to a quarter strength at 14px measures 9.9:1 there and spends its
weight on the strip below it.

The test asserts the pair of attachments rather than the pixels, on
this file's existing grounds that no tier here renders like the device
— it fails on the pre-fix stylesheet with `expected 'scroll' to be
'local, scroll'`. The rendered result was measured in the harness and
is recorded in `.planning/NOTES.md`.

Closes #207
2026-08-23 06:45:46 -04:00
logan e5d0f2714b test(ui): make ui-visual-update honour its file filter
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m27s
CI / e2e (pull_request) Successful in 9m27s
vitest parses a bare `--update` as taking the next positional as its
value, so `make ui-visual-update UI_ARGS=<path>` handed the path to the
flag and ran with no filter at all: 99 files, every baseline in the repo
re-recorded, any stale one blessed in silence. Two paths were worse
still — the first was eaten and only the second ran.

That is #196's own hazard living in the tool meant to resolve it: the
rule is "refresh the reference your change moved and never one you did
not cause", and the documented way to refresh one refreshed the set.

`--update=true` is the whole fix, with the reason beside it because
`=true` reads like something to tidy away. `make ui-visual` and
`make ui-test` are unaffected — their `$(UI_ARGS)` follows `run`, with
no flag to swallow it — and no other target interpolates a variable
after a boolean flag.

Closes #204
2026-08-23 04:36:46 -04:00
logan ee1d8b3179 Merge pull request 'Android touch model, phases 2-4: swipe to queue, and the other three lists' (#201) from 63-touch-model-phase-2 into main
CI / e2e (push) Successful in 9m28s
CI / check (push) Successful in 2m31s
Build & publish the Android APK / apk (push) Successful in 1m27s
Build & publish Arch package / arch-package (push) Successful in 2m39s
Attach the desktop build to the release / linux (push) Successful in 58s
Sync Homebrew formula / sync-formula (push) Successful in 6s
2026-08-22 05:54:47 +00:00
logan 29feb4b94b feat(android): the touch model reaches the other three lists
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m33s
CI / e2e (pull_request) Successful in 9m20s
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
2026-08-22 01:41:21 -04:00
logan 4e667759c4 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.
2026-08-22 01:23:19 -04:00
logan ff3875b55d Merge pull request 'Android touch model, phase 1: tap to play, hold to select' (#199) from 63-android-touch-model into main
CI / check (push) Successful in 2m36s
CI / e2e (push) Successful in 9m29s
2026-08-22 04:36:37 +00:00
59 changed files with 6039 additions and 286 deletions
+8 -1
View File
@@ -158,7 +158,7 @@ only climb when it cannot.
| You changed | Run | Cost |
|---|---|---|
| A Lit component, a store, the shortcut service | `make ui-test` | ~2 s, no app |
| …and it renders differently | `make ui-visual` | + 6 baselines, opt-in |
| …and it renders differently | `make ui-visual` | + 10 baselines, opt-in, never gates |
| Any Go code | `make test` | 3 passes, ~2 min |
| A service that emits events | `make test` — assert on the payload, see `backend/queue/emit_test.go` | in-process, no app |
| A bound method or a bound struct field | `make bindings` then `make ui-test` | ~1.5 s + 2 s |
@@ -180,6 +180,13 @@ less than it looks.)
Two rules about climbing:
- **If you moved a component's geometry, run `make ui-visual` and
refresh that component's baseline in the same commit.** Nothing else
will: it is the one tier in this repo no hook and no CI job runs, and
it cannot be one — its references are machine-specific, measured in
[references/ui-tier.md](references/ui-tier.md). Four of them drifted
across three merges before anyone noticed (#196). Read the image;
never bless a reference you did not cause.
- **A component test passing is not the app rendering.** If you touched
anything in `frontend/src`, verify it in the real app too — start it
headless, `screenshot --filename=/tmp/shot.png`, and *read the PNG*.
@@ -78,9 +78,58 @@ synchronously.
Microtasks and not a timer, deliberately: a timer hangs forever under
the suites that install fake ones.
Visual baselines are font-hinting and compositing sensitive, which is
why they are opt-in: they only mean anything on the machine that
recorded them.
## The visual tier does not gate, and that is measured (#196)
`make ui-visual` is the same suite with nine `toMatchScreenshot`
baselines switched on. **Nothing runs it but a person**, deliberately,
and the reason is a number rather than a preference: the committed
baselines were recorded on Arch, and replayed in a bare `ubuntu:24.04`
container — CI's `check` image — three of them fail for reasons that
have nothing to do with any component.
| baseline | Arch | ubuntu:24.04 |
|---|---|---|
| `page-header` filtered-by-search | passes | ratio 0.03 differ, against a 0.02 allowance |
| `track-info` | passes | ratio 0.03 differ |
| `seek-bar` | 1152×18 | 1152×17 |
The two references that were genuinely stale did not even agree about
their *new* size — `now-playing` renders 1152×65 on Arch and 1152×64 in
the container. So moving CI's `check` job from `make ui-test` to
`make ui-visual` is not a one-line change: it needs a second,
container-recorded baseline set, which every local run would then fail
against. That is the same trap the other way round, and a pre-push hook
is the same fault again — one machine's baselines against everybody
else's renderer.
So the tier stays local and opt-in, and the rule that replaces the gate
is:
- **A change that moves a component's geometry refreshes that
component's reference in the same commit, having read the image.**
Look at the PNG; the dimensions in the failure message are the cheap
half of the answer.
- **Never refresh a reference you did not cause.** #196 exists because
four of them drifted across three unrelated merges, and every red run
made the next person likelier to stop running the tier than to read
it.
- **State the world the shot is taken in.** The stores are singletons,
so a visual case that sets nothing photographs whatever the previous
case left behind — which is how the sidebar's baseline came to have
Tracks lit and `now-playing`'s to be playing from a dynamic mix.
- **Record one file with `make ui-visual-update UI_ARGS=<path>`**, and
check `git status` before committing either way. That filter is only
honoured since #204: the recipe was a bare `--update`, and vitest
takes the following positional as the flag's value, so the path was
swallowed and *every* baseline was re-recorded — blessing any stale
one in silence.
What the tier is worth, for the record: it is a *layout* check, blind to
colour (the component tier has no `:root`, so it renders the fallbacks —
`make ui-visual` passed unchanged through a whole palette rewrite,
twice), and it has caught one thing nothing else could — swapping
`library-status-indicator`'s `<button>` for a `<span>` lost the UA
stylesheet's `box-sizing` and grew the badge 36→38px.
## Bindings
+166
View File
@@ -4912,3 +4912,169 @@ bridge leaves the wizard up with its "Get Started" button correctly
disabled — it gates on a directory chosen *in the wizard*, and the
existing-library check runs once, on mount. A reload clears it. Nothing
is broken; it cost twenty minutes of believing a tap had been swallowed.
## The sheet's scroll fade, and where a scrim may not go (measured 2026-08-23, headless)
#207's answer. The affordance is two background layers on
`wa-dialog::part(body)` and the conditionality is
`background-attachment`, not a scroll listener: a cover of the sheet's
own colour painted at the end of the *content* (`local`) over a shadow
pinned to the box (`scroll`), so the cover scrolls up and hides the
shadow exactly when there is nothing more to see.
Measured at 424x360 (which is where a menu overflows on `main`, since
`main` does not yet carry #67's eighth item — at 424x439 the track
list's seven items are `scrollHeight` 364 against `clientHeight` 364,
fitting exactly). Pixel at x=300, dark ramp, `bgElevated` `#343a40`:
| y | before | more below | at the end of the list |
|---|---|---|---|
| 330 | 52,58,64 | 50,56,62 | 52,58,64 |
| 340 | 52,58,64 | 43,48,53 | 52,58,64 |
| 350 | 52,58,64 | 33,37,40 | 52,58,64 |
| 359 | 52,58,64 | 22,24,27 | 52,58,64 |
Three things worth keeping.
**A menu that fits draws nothing**, which is the same measurement: at
424x439 the sheet is flat 52,58,64 to its bottom edge, because with no
overflow the `local` layer's positioning area *is* the padding box and
the cover lands on top of the shadow.
**A scrim over a menu row is that row's text surface**, so the 4.5:1
rule reaches it and this is why the curve is steep rather than linear.
A row is 48px with its label centred; 32px of scrim already down to a
quarter strength at 14px puts about 0.06 at the label. Checked on the
light ramp (`bgElevated` `#e9ecef`, text `#212529`) by overriding the
two custom properties on `:root`: background at the label 205,207,210,
which is **9.9:1**. The first draft — a linear 48px at 0.8 — put ~0.375
on that label, 5.0:1, passing but visibly greyed. The bottom few pixels
go to ~2.4:1 in either draft and are deliberately below where any
label of a *fully visible* row sits; a label that lands there belongs
to the half-cut row, which is the thing being signalled.
**A dark scrim on a dark surface reads far worse in a shrunk screenshot
than on screen.** The first two probes (24px/0.45, then 32px/0.75) were
measurably present — 52,58,64 down to 30,33,37 — and invisible in the
inline preview. Crop the bottom 70px and scale it up before judging;
the pixel values are the honest answer either way.
## The phone's context sheet is now longer than the phone (measured 2026-08-23, headless at 424x439)
#67 moves two destinations into every row menu, and the track list's
menu is where that runs out of screen. Measured against the running
app at the reference viewport, one row selected:
| menu | items | first item top | last item bottom |
|---|---|---|---|
| queue panel | 7 | 95 | 431 |
| track list | 8 | 86 | **470** |
The viewport is 439. So the track list's last item — "Remove from
Library" — is below the fold. It is **not unreachable**: the sheet is a
`wa-dialog` whose body is `overflow-y: auto`, measured `scrollHeight`
412 against `clientHeight` 373, and scrolling it 39px brings that item
fully into view (383431). What it has is no *affordance*: nothing on
screen says the list continues.
Two things worth knowing before adding a ninth item anywhere.
**The limit was already reached, and this is what crossed it.** Seven
48px rows in a 373px body is 364px — the queue's menu fits with 8px to
spare and the track list's fitted exactly. Any item added to any of the
fourteen menus after #60 was going to be the one that overflowed; the
first one simply happened to be this.
**The measurement has to be taken with a row selected**, since the
`Go to` items are drawn for a single selection only, and on the *first*
track of the fixture library — which has no album (`01 Tone A`,
`02 Tone B`) — only "Go to Artist" appears. That is the 8 above; an
ordinary track makes it 9.
Filed as its own issue rather than fixed in #67's diff: it is a
property of the shared sheet (`components/menu-surface/`), not of the
items.
## The tap highlight is one inherited declaration (measured 2026-08-24)
`-webkit-tap-highlight-color` is an **inherited** property, and an
inherited property crosses a shadow boundary — so `html { … :
transparent }` in `index.css` reaches every shadow root in the app and
no component needs a rule of its own. Measured in the running app
(Chromium, `app-sidebar`'s `li button`, which is three shadow roots
from the document): `rgba(0, 0, 0, 0)` with the rule, and
`rgba(0, 0, 0, 0.18)` with it removed. That 0.18 grey over the bounding
rect of whatever was tapped is what #54 reported.
The same argument was already spent once and is worth not
re-deriving: `index.css`'s first rule is `*, *::before, *::after {
user-select: none }`, which for the same reason already covers the
shadow roots — #54's Findings ask for `user-select` on interactive
surfaces and it has been done since before the issue was filed.
**What the highlight was, on the surfaces that had nothing else, is the
press feedback.** Measured on a track row with the press rule removed
and the button held down: `rgba(255, 255, 255, 0.05)` — the *hover*
tint, arriving because the pointer is over the row, which is a
synthesised hover on a phone and outlives the press. With the rule:
0.12 while held, and the neighbouring row unchanged. So the press state
is part of removing the highlight rather than a separate polish item,
and the hover tints on those same surfaces moved behind
`(hover: hover) and (pointer: fine)`, which is #68's gate applied to a
tint rather than to a revealed control.
**`touch-action: manipulation` was considered and not taken.** The
Findings offer it for the 300ms tap delay; this app's viewport is
`width=device-width`, which is what removes that delay in Chrome, so
the stated benefit is not there to win. What it would change is the
gesture stack #63 tuned by measurement on the device (`pan-y` plus a
non-passive `preventDefault`), and that is not measurable from here.
## Art pop-in is measurable in a browser, if you count frames rather than milliseconds (measured 2026-08-24)
#65 is an Android report ("scrolling through albums, the art pops in")
and the desktop harness can measure it, which was not obvious: the
first attempt waited 220 ms after each scroll jump and found **zero**
blank covers on either build. The metric only discriminates at one and
two animation frames after the jump, which is where a pop-in actually
lives.
Protocol, on `make dev-headless SEED=bulk` (4 988 albums), ten
2 400px jumps of `.grid-scroll-container`, counting covers whose rect
intersects the viewport with `naturalWidth === 0`:
| build | blank at frame 1 | at frame 2 | at 50 ms |
|---|---|---|---|
| `main` | 254 / 258 | 214 / 258 | 0 |
| `main`, second run | 254 / 258 | 190 / 258 | 0 |
| prefetch | 117 / 258 | 77 / 258 | 0 |
| prefetch, second run | 118 / 258 | 96 / 258 | 0 |
Two things this protocol gets wrong if repeated carelessly. **A second
run in the same browser session measures the HTTP cache**, not the
build — the skill already warns about this for `make perf`, and it
applies to any image measurement; every row above is a fresh
`playwright-cli close` + `open`. And **the frontend is embedded**, so
comparing builds is a `git stash` *and* a rebuild, not a stash.
**The bulk library's covers are 300x300 and ~3.7 kB**, which is why
both builds are clean by 50 ms here and why the phone's number cannot
be inferred from this one — same caveat the skill already records
about full-size artwork.
**`rangeChanged` and `visibilityChanged` are different ranges**, and
the difference is the whole of this fix's value.
`@lit-labs/virtualizer` reports `_first`/`_last` (rendered, including
the ~1000px overhang) on the former and `_firstVisible`/`_lastVisible`
on the latter. Both grids listen to `visibilityChanged` for scroll
persistence, which wants the visible range and is correct; a prefetch
window measured from it lands mostly on cards that already exist.
Anchored there, the component test could see only one row past the
last rendered card.
**`_overhang` is not configurable.** It is a `protected` field set to
1000 in `BaseLayout` and read by every layout; there is no option on
`grid()`/`flow()` and no property on the element. The issue's Direction
("ask the virtualizer for a larger overscan") is therefore not
available without patching a private, which is why the request is
issued ahead of the element instead.
@@ -5,7 +5,7 @@
**Relates:** #67 (inline links into the menu), #71 ("More" nav), #54
(native feel), #5/#8 (selection, drag to queue — the desktop semantics
being diverged from)
**Status:** in flight.
**Status:** shipped (phases 1-4). Its one deliberate remainder is #200.
#73 puts #60 first in Phase 4 because it is "the presentation every
other item needs", and this is the next one. The Direction on #63 asks
@@ -225,29 +225,184 @@ still works. `SelectionController` gains a mode. `track-list` acts on
tap and enters the mode on long press. The action bar.
**Phase 2 — swipe right to queue**, with the `touch-action: pan-y`
finding above and a reveal-and-snap affordance.
finding above and a reveal-and-snap affordance. **Shipped**; what the
device said about it is the section below.
**Phase 3 — the other three surfaces**, which is mostly wiring, since
they already share the controller.
they already share the controller. **Shipped**, and it was not entirely
wiring — see below.
**Phase 4 — what this leaves behind.** The inline `explore-link`s in a
row are a single-click target inside a row whose single tap now plays;
that conflict is #67's, and this plan should not pre-empt its answer
beyond making tap-to-play win on touch.
beyond making tap-to-play win on touch. **Shipped.**
## Phase 3 was not symmetric, in two places
**A tap on a queue row plays that position**, not the list. Copying
`track-list`'s tap — which sets the queue to the list the row is in —
would rebuild the queue *from* the queue, discarding its source, its
shuffle order and everything a user had inserted by hand. It reads as a
no-op and is not one.
**The queue panel has no swipe, deliberately.** A right swipe means
*add to the queue* everywhere else it exists, and a queue row is
already in the queue; the only thing it could mean there is *remove*,
which is the same gesture with the opposite effect one screen away —
the fault `utils/icon-language.ts` exists to have fixed for glyphs.
Removing a queue row is on the row itself (the ×), on its bottom sheet
since #60, and on the selection bar this phase gave it. The assertion
is that its rows do **not** carry `data-swipe`, so a swipe there cannot
silently become a second meaning for the app's one horizontal gesture.
And the affordance became `utils/swipe-to-queue.ts` rather than being
copied twice. Three lists want it; three copies of "how far is far
enough" is three chances for them to disagree, which is what
`utils/library-status.ts` and `utils/ownership.ts` each exist to have
stopped happening. The shared stylesheet is keyed on `[data-swipe]`
rather than on a class name, because the three lists call their rows
two different things and the `touch-action` half of the device fix has
to reach all of them.
## Phase 4 was already true, which is why it is asserted
A claimed tap has its click swallowed at document capture, so an
`explore-link` inside the row never sees one and tap-to-play wins with
no rule of its own. Nothing in the suite would have failed if that
stopped covering the link, and the symptom — tapping a track's *title*
navigating to its album instead of playing it — is one a phone user
meets constantly and a mouse user never does.
**Its test was vacuous when written**, in the way this file keeps
finding: the tap helper dispatched `pointerdown` and `pointerup` and no
`click`, so there was nothing to swallow and the assertion held on any
build. It sends the trailing click now, which also strengthened phase
1's "a tap plays and does not also select". The fixture needed an MBID
for the same reason — without one the link asks the backend for a local
album first and gives up when nothing answers, so "it did not navigate"
was true of a working build and a broken one alike.
---
## What phase 2 measured, which was not what phase 2 predicted
The `touch-action` finding above is **half** of the answer, and
shipping only that half would have been the exact failure it warns
about. Driving a real finger with `adb shell input swipe` across a
track row, three values, all three on the device:
```
touch-action: auto pointerdown, 1 move, pointercancel
touch-action: pan-y pointerdown, 2 moves, pointercancel
touch-action: none pointerdown, 2 moves, pointercancel
```
`touchmove` kept firing in all three. So **Chrome 113's WebView
cancels the pointer stream ~16px into any drag whatever `touch-action`
says**, and a swipe recognised from `pointermove` — which is what the
rest of this module is built on — is a swipe that dies 16px in.
The other half is a **non-passive `touchmove` calling
`preventDefault()`**: with it, the same swipe ran to 12 moves and a
`pointerup` at full travel. And both halves are required, which was
measured rather than assumed — with the `preventDefault` in place and
`touch-action` back at `auto`, the gesture died after **one** move.
The reading is that `auto` lets the browser commit to a horizontal pan
on the first move past slop, before any threshold of ours can have
been crossed, while `pan-y` leaves it undecided long enough for the
second move to claim it.
`none` is the one value to avoid: the list stopped scrolling at all.
With the shipped pair, a vertical drag still scrolls the virtualizer
81px on the same run that a horizontal one survives.
**`draggable="true"` is not a competitor**, which is the other thing
the device was asked. No `dragstart` fires from a touch drag on this
WebView at all, so the drag-to-playlist attribute on every row needs no
pointer-type gate.
### And it found a phase 1 defect that no tier can see
The native `contextmenu` arrives in **either** order, and phase 1 only
handled one of them. `nativeSeen` covers the browser's menu arriving
*during* the hold. The reverse — our 500ms timer firing first, a
component claiming it, and Chrome delivering its own `contextmenu`
5070ms *later* — was suppressed by nothing, so the context menu
opened on top of the selection bar. Measured over four holds:
```
hold 1 yj-long-press, then contextmenu isTrusted=true menu open
hold 2 yj-long-press clean
hold 3 yj-long-press, then contextmenu isTrusted=true menu open
hold 4 yj-long-press clean
```
Two in four, on the one surface #63 exists to have changed, and
invisible to both browser tiers because neither synthesises a
`contextmenu` from a dispatched press. A press that has produced its
outcome now suppresses a late one whichever branch it took; six holds
on the fixed build, six clean.
### The rules phase 2 settled
- **A swipe is not a selection.** It queues the row it was made on,
unless that row is one of several *explicitly* selected — the same
rule the context menu answers with, because a bar reading "40
selected" beside a gesture that quietly queues one of them is two
answers to one question. It never changes the selection, which is
where it differs from a right-click.
- **Rightward only.** Nothing is bound to a leftward swipe and
claiming one would take a gesture away to do nothing with it.
- **The commit threshold is a fraction of the row** (0.3, floor 72px),
because the row is 424x52 on this device and a bare pixel count is a
fraction of a row height on one screen and a third of the width on
the next.
- **The affordance is not only a colour** (WCAG 1.4.1, the rule the
playing-row marker exists for): the pane carries the queue icon and
words, the words change at the threshold ("Add to queue" → "Release
to add" → "Added"), and the outcome is announced in a live region.
- **The row does not move; its cells do.** `.track-row` is
`contain: strict` with `overflow: hidden`, so a pane held at the
row's original position while the row translates is a pane at a
negative offset inside a clipping box and is simply not painted.
Sliding the cells needs no wrapper element in a row that is already
a grid.
- **The travel is written to the row's own style, not rendered.** One
render at the start, one at the threshold, one at the end; a
virtualizer re-rendering every visible row per frame of one finger's
travel is the thing `perf.m1` is about.
## Open questions
1. **Does selection mode have an escape other than the bar's own
close?** Back is the platform's answer and the shell already owns
the history stack (#6/#55). Pushing an entry for a *mode* rather
than a place is the same argument #55 settled for the overlaid
queue, and it should probably be settled the same way — but the
queue is a screen and a selection mode is not, so it wants its own
paragraph rather than an assumption.
close?** *Settled: Escape, here; back, not here.*
Escape leaves the mode, from `selection-bar` rather than from each
of the four hosts — that element exists only while the mode does, so
it is the one place a dismissal can be attached and detached with
the thing it dismisses. It is the same documented exception the
overlaid queue's Escape is: **a dismissal, not a shortcut**, so it
is not a panel-scoped binding.
The back gesture is the half that is *not* done, and deliberately.
The obvious version — `selection-bar` pushing a history entry — is
precisely the fault `navStack` was deleted for: the shell owns the
stack (#6/#55) and is the only thing that calls `pushState`, so that
two stacks cannot disagree about what one press means. Four lists
each reaching for `history` is four stacks. It is also wrong on its
own terms, since a mode is per-component and a user who enters one,
navigates away and returns has an entry for a mode that no longer
exists. #55 settled the shape for a *place*; a mode is not one,
which is why it could not simply inherit that answer.
What it wants is one shell-owned register of dismissible surfaces,
which would retro-fit the queue overlay, the dialogs and this alike
rather than adding a fourth private answer. **#200.**
2. **Does a tap on a row's favourite icon still toggle it in normal
mode?** It is inside the row and the row now plays. It has to keep
working — it is a 44px target since #56 — so the gesture layer needs
the same "a control inside the row wins" rule the keyboard service
has for a focused control that owns a key.
mode?** *Settled in phase 1: yes.* A control inside the row keeps
its own tap — the gesture is simply not claimed there, so the click
behind it falls through untouched. It is the same rule the shortcut
service has for a focused control that owns a key, and it is what
keeps the 44px favourite target (#56) from becoming a 44px play
target. The queue row's × is the second instance of it.
+352 -21
View File
@@ -262,6 +262,26 @@ real store code. A binding carries an **ID**, not a name
`yellowjacket/backend/home.Service.GetShelves`), so the fake derives
that map from the generated tree rather than writing it down.
**A test file does not get its own origin, so `setup.ts` clears
`localStorage` between tests.** `@vitest/browser-playwright` opens one
BrowserContext per session and runs several files in it one after
another, so everything a component persists — the track list's sort and
column widths, the cover size, `now-playing`'s scroll mode — is still
there when the next file mounts the same component. Which files share a
tab, and in what order, changes run to run, so the symptom is a spec
that fails about one test in three and passes every time it is run on
its own: #138 cost three scheduled runs, one of them a PR whose diff
held no frontend code at all. Measured on the build before the fix, a
single full run started **24** tests with storage already set. Two
things follow. The clear is safe precisely because the leak is
sequential — files in a session do not overlap, so it cannot wipe
storage a concurrently-running file is in the middle of using — and it
belongs in `setup.ts` rather than in the specs that write, because the
spec that *reads* is never the one that knows. And a spec whose
assertion depends on an order still **states that order** rather than
inheriting a default, or the next change to a default is the same
mystery again.
**`frontend/bindings/` is generated by `wails3`, not `go generate`**, so
the pre-commit codegen check does not cover it. `make bindings-check`
(~3.5 s warm, ~20 s on a cold build cache, also a pre-commit hook)
@@ -1093,9 +1113,9 @@ not the fix and cannot be: that function is the `document` listener for
`navigate`, so it is an infinite loop.
**It is a store rather than an event, because a component that mounts
after a navigation still has to know.** `bottom-nav`'s "More" drawer
after a navigation still has to know.** `bottom-nav`'s "More" sheet
creates its `<app-sidebar>` on open, and that copy had heard no
`navigate` at all — standing on Albums, the drawer opened highlighting
`navigate` at all — standing on Albums, it opened highlighting
Home. An event has no answer for a listener that was not there.
**A detail view is not a view here**, so the destination it was opened
@@ -1196,7 +1216,7 @@ and then vanishing.
than a general rule about phones.** `PHONE_COLUMN_IDS` is the precedent
for "what a phone shows is a different question", and it would apply —
except that `bottom-nav`'s "More" opens the *same* `<app-sidebar>`,
which filters, so an unfiltered bar would contradict its own drawer one
which filters, so an unfiltered bar would contradict its own sheet one
tap away. Which four tabs is still plan 016's committed subset; this
only removes from it, and "More" is never filtered because it is how
everything else stays reachable.
@@ -1402,7 +1422,7 @@ descendants. On the reference device the main panel spans 0-318 of a
items cut off, with no way to reach them. `showModal()` is Chrome 37
and uses the real top layer, so a dialog is immune by construction.
Six things about it are load-bearing.
Seven things about it are load-bearing.
**"Dialogs are fine" needed checking, because every other dialog in
this app is mounted in `index.html`** — outside `.main-panel` — so it
@@ -1431,6 +1451,25 @@ doing nothing, which reads as the gesture breaking. `menu-dismiss` is
that signal; the three surfaces that do not use `ContextMenuController`
bind it themselves.
**A sheet that scrolls says so, and `background-attachment` is what
asks whether it does** (#207). The sheet is capped at 85vh — a surface
covering the whole screen is a page, not a sheet — so a long menu's
body scrolls, and for three phases it scrolled *silently*: measured at
424x439, eight items ended at y=470 with the fold at 439, and where the
cut lands on a row boundary the sheet ends in a clean edge that reads
as the end of the list. The fade is two background layers on
`wa-dialog::part(body)` — a shadow pinned to the box (`scroll`) under a
cover of the sheet's own colour painted at the end of the *content*
(`local`), which scrolls up over the shadow exactly when there is
nothing more to see. So it is absent on a menu that fits, present the
moment one does not, and gone again at the end of the list, with no
scroll listener and nothing reaching into `wa-dialog`'s shadow root for
the scroller. **The curve is steep because the rows under it stay
live**: a scrim over a menu item is that item's text surface, and the
4.5:1 rule applies to it — 32px already down to a quarter strength at
14px spends its weight below the last legible label, measured at 9.9:1
on the light ramp, whose `bgElevated` is `#e9ecef`.
**The playlist submenu is a sheet too, and it had to be.** It is a
`placement="right-start"` flyout, and making the menu full-width moved
its anchor — measured at x 182 to 0, entirely off-screen, so "Add to
@@ -1449,18 +1488,112 @@ never opens). **The sweep found two of the fourteen**; twelve were
converted by hand.
**And a menu opens from a finger, through the event it already has.**
`utils/long-press.ts` is one document-capture listener installed once
from `index.ts`: a touch that holds still for 500 ms dispatches a
synthetic `contextmenu` at the touch point, so all six components that
bind one — delegated on a virtualizer, per row, per card — gained the
gesture without changing. The target is `composedPath()[0]` rather than
`utils/touch-gestures.ts` is one document-capture listener installed
once from `index.ts``utils/long-press.ts` until #63 replaced it,
rather than adding a second listener claiming the same 500 ms hold. It
**announces** rather than acts: `yj-tap`, `yj-long-press` and
`yj-swipe-start` are composed and cancelable, and a component claims
one with `preventDefault()`. That is what let #63 reassign the hold
without touching one of the fourteen context menus: an *unclaimed*
`yj-long-press` still becomes a synthetic `contextmenu`, so all six
components that bind one — delegated on a virtualizer, per row, per
card — behave exactly as they did, and only the lists that opt in get
selection mode. The target is `composedPath()[0]` rather than
`elementFromPoint`, which stops at the outermost shadow host and so
reaches a delegated listener and no per-row one; a browser that fires
its own long-press `contextmenu` (Chromium does, WebKit and the WebView
vary) wins, ours being told from theirs by **identity** rather than
`isTrusted`, since no test can dispatch a trusted event; and the click
that ends the gesture is swallowed, keyed on the gesture rather than on
a time window so the first tap on the menu it opened is not eaten too.
reaches a delegated listener and no per-row one; and the click that
ends a *claimed* gesture is swallowed, keyed on the gesture rather than
on a time window so the first tap on the menu it opened is not eaten
too.
Three things about it are load-bearing, and all three were found on the
device rather than in a tier.
**A browser that fires its own long-press `contextmenu` is a trigger,
not a competitor.** Chromium does, WebKit and the WebView vary. The old
rule was to stand down when a trusted one arrived, which was right
while both paths ended in a context menu and is wrong the moment a hold
can mean something else — standing down silently does the *old* thing.
So the gesture is announced from the native event, and only a component
that claims it suppresses that event. Ours and the browser's are told
apart by **identity** rather than `isTrusted`, since no test can
dispatch a trusted event.
**That arrives in either order, and both have to be handled.** The
native `contextmenu` mid-hold is one case; the other is our own 500 ms
timer firing first and Chrome delivering its menu **5070 ms later**,
which nothing suppressed — measured over four holds on the reference
phone, two took that order, so the context menu opened over the
selection bar intermittently, on the one surface #63 changed. A press
that has produced its outcome therefore suppresses a late
`contextmenu` whichever branch it took.
**A horizontal swipe runs on touch events, and needs two things that
look like one.** Chrome 113's WebView cancels the *pointer* stream
~16 px into any drag — measured at `auto`, `pan-y` and `none` alike,
one or two `pointermove`s and then `pointercancel`, while `touchmove`
kept firing throughout. So the recogniser is `touchmove`, the surface
declares **`touch-action: pan-y`** *and* a claimed swipe calls
**`preventDefault()`** on a non-passive listener. Neither works alone:
with the `preventDefault` in place but `touch-action` back at `auto`
the gesture died after one move, because `auto` lets the browser commit
to a horizontal pan before any threshold can be crossed. `none` is the
value to avoid — it takes the list's own vertical scrolling with it.
**Both are correct in Chromium either way**, which is why this is
written down rather than tested. The tie breaks toward scrolling, in
that order: vertical drift past the tolerance vetoes the swipe for the
rest of the press (a scroll that curves is still a scroll), and a
gesture that is not *strictly* more horizontal than vertical is the
scroller's.
**And what a finger *means* on a row is the inversion of what a mouse
means, decided per event** (#63). A click selects and a double-click
plays; a tap **plays** and a hold enters **selection mode**, in which a
tap toggles. The predicate is `pointerType`, never a viewport width and
never a platform flag — #64's rule, and with #64's warning: keyed on a
width, an Android tablet over 600px gets desktop semantics on a
touchscreen, a touchscreen laptop cannot be described at all, and a
narrow desktop window gets phone semantics with a mouse.
There is deliberately **no double-tap**, which #63 asked for. The first
tap of one is indistinguishable from a single tap until the interval
expires, so tapping would have to wait `DOUBLE_CLICK_GRACE_MS` before
acting — 250ms on top of a measured ~100ms play, 3.5x the app's primary
interaction, to reach a menu a hold already reaches. So the menu and
the action bar are the same surface: `components/selection-bar/` is
presentational (a count and a list of actions, no store, no selection),
`SelectionController` carries the mode for all four selecting surfaces,
and #60's bottom sheet is the overflow behind "More" — so
`contextMenuStyles`, `MenuKeyboard` and `menu-surface` are reused
rather than reimplemented.
Three things about it are load-bearing. **A control inside a row keeps
its own tap**: the gesture is simply not claimed there, so the click
behind it falls through, which is what stops the 44px favourite target
(#56) becoming a 44px play target — the queue row's × is the second
instance. **A swipe right queues**, and its affordance is
`utils/swipe-to-queue.ts` once rather than in each of the three lists
that draw it: the row does not move, its *children* do (a row here is
`contain: strict` with `overflow: hidden`, so a pane held at the row's
original position while the row translates is at a negative offset
inside a clipping box and is not painted), the travel is written to the
row's own style rather than rendered, and the threshold is a fraction
of the row because the row is 424x52 on the reference device. **The
queue panel takes the tap and the hold and refuses the swipe**, because
a right swipe means *add to the queue* everywhere it exists and a queue
row is already in it — the only thing it could mean there is *remove*,
which is the same gesture with the opposite effect one screen away.
A tap there plays that *position*, too: setting the queue to the queue
reads as a no-op and discards its source, its shuffle order and
anything inserted by hand.
Escape leaves the mode, from `selection-bar` rather than from each
host, since that element exists only while the mode does — the same
exception the overlaid queue's Escape is, *a dismissal, not a
shortcut*. The platform's back gesture deliberately does **not** reach
it: the shell owns the history stack and four lists each reaching for
`history` is four stacks, which is the fault `navStack` was deleted
for. That wants one shell-owned register of dismissible surfaces, which
is #200.
**A control revealed by `:hover` is gated on the device having hover,
and which way round depends on whether it is the only route to its
@@ -1498,6 +1631,51 @@ sits inside which media query — and says so; the regression it exists
for is someone hoisting a rule out of its query as a tidy-up, which
nothing on a desktop renders differently.
**The web view's own tap highlight is gone, and what replaced it is a
press state** (#54). `-webkit-tap-highlight-color` is an *inherited*
property, so one declaration on `html` in `index.css` reaches every
shadow root in the app and takes away the grey box a phone drew over
the bounding rect of whatever was tapped — measured at
`rgba(0, 0, 0, 0.18)` with the rule removed. `user-select` is the same
argument and was already done: `index.css`'s first rule is `*, *::before,
*::after { user-select: none }`, which reaches the shadow roots for the
same reason.
Three things about it are load-bearing.
**Removing the highlight removes the only touch feedback several
surfaces had**, so the press state is part of the same change rather
than a later polish item: the four lists' rows, `bottom-nav`'s tabs,
`app-sidebar`'s destinations (which are also the phone's "More" sheet)
and the shared `contextMenuStyles` menu item all take
`--yj-press-overlay` on `:active`. The cards already had one
(`transform: scale(0.97)`) and are untouched.
**A press selector carries a state class or it does nothing where it
matters.** A row is `.track-row.selected.active`, so a bare
`.track-row:active` is one class short of it and the press is invisible
on exactly the row a phone is most likely to press — the one it has
just selected. The rule is last and lists `.selected:active` /
`.active:active` beside the bare form.
**And the hover tints on those same surfaces moved behind
`(hover: hover) and (pointer: fine)`**, which is #68's gate applied to
a tint rather than to a revealed control and for the same mechanism: a
hold synthesises a hover in the WebView, so an ungated tint arrives
because a finger touched the row and stays there after it has gone —
measured, since with the press rule removed a held row reads
`rgba(255, 255, 255, 0.05)`, the hover tint, rather than nothing.
`touch-action: manipulation` was considered and declined: the 300ms
delay it is offered for is already absent on a `width=device-width`
viewport, and what it would really change is the gesture stack #63
tuned by measurement on a device this session cannot measure.
The split of tiers is `hover-affordance.test.ts`'s: `press-feedback.
test.ts` reads the parsed stylesheet, because `:active` cannot be
forced there either, and `native-touch-feel.spec.ts` *measures* — it
holds the button down on a real row of the real list, and it is the
only tier that loads `index.css` at all.
Three lists had no focused row to open a menu *from* — the queue panel
and both playlist detail views — and gained a roving tab stop through
`utils/roving-rows.ts`. **`track-list` deliberately does not use it**:
@@ -1654,6 +1832,21 @@ is not it.** A `placeholder` is an accname fallback, so an
Explore's search box — the audit's own `a11y.26` — as clean. A sweep
for *empty* names cannot see a *weak* one.
**`title` is the same trap one rung lower, and it defeats the obvious
spec as well as the obvious sweep.** `queue-panel`'s Clear queue and
Add queue to playlist were named by `title` alone, so
`getByRole('button', { name: 'Clear queue' })` matched them **before**
the fix as well as after — a `getByRole` assertion, which is what
catches every other nameless control in this app, would have been
green on the broken build. `title` is the *last* fallback in the
accname order, so content put inside the button later silently
outranks it, and it is the one name a phone cannot show, having no
hover. The property is therefore asserted as *the name is not the
tooltip*: `queue-overlay.spec.ts` removes the `title` attributes and
asks again, which is 1 and 1 with `aria-label` and was measured at 0
and 0 without it. The `title`s stay, because on a desktop they are
also the tooltip for an icon-only control and that is a different job.
**The shell scrolls sideways and not down.** `body` is
`overflow-x: auto; overflow-y: hidden`, and both halves are measured.
Vertically there is nothing to fix: the middle grid row is `1fr` and
@@ -1696,7 +1889,7 @@ listing the destinations again — but rendering it unconditionally put a
second copy of every `data-testid="nav-*"` in the DOM, and 30 existing
specs failed with "strict mode violation: resolved to 2 elements" on a
desktop viewport where the element is not even visible. It renders only
while the drawer is open, and `bottom-nav.test.ts` asserts its absence
while the sheet is open, and `bottom-nav.test.ts` asserts its absence
before that.
**The tab bar is four destinations and a way to the rest.** Three to
@@ -1705,6 +1898,33 @@ is 32px each. Which four is plan 016's committed subset, and everything
else — Settings included, because a phone still needs it — is behind
"More".
**And "More" rises from the bottom, on #60's sheet rather than a
second one** (#71). It was a `wa-drawer placement="start"`: a 200px
column of a 424px screen, opening away from the thumb that asked for
it, with the rest of its 400px band empty. It is the *same element*
with `placement="bottom"` and `without-header`, which is what keeps
the change to where it comes from — `wa-drawer` renders a native
`<dialog>` and opens it with `showModal()`, so #60's containment
finding carries over with nothing new to prove, and the focus trap,
Escape, tap-outside and `wa-after-hide` all come along. Measured at
424x439: 424 wide, 373 tall (85vh, so there is an outside to tap),
48px rows.
Three things about it are load-bearing. **The sidebar is mounted
rather than re-listed as data**, which the issue offers as the
alternative: the shell's own `<app-sidebar>` is `display: none` below
600px rather than removed, so a second list drawing `nav-*` handles is
the duplication above, and it would be a second place to add the next
view to. **There is one scroller, and it is the sheet's body** — the
reported "only part of the screen scrolls under my finger" is three
nested ones (the dialog, its body, and the sidebar's own
`overflow-y: auto` host), so which box a drag moves depends on where
the finger landed; `overscroll-behavior: contain` is the other half.
And **`expanded` means the host owns the box, not just the labels**:
`app-sidebar` writes an *inline* width and caps itself at 400px, which
beats any rule the host could write, so the width, the scrolling and
the mouse-only resize handle all follow that attribute.
**There are three supported size bands, and the queue is part of the
promise.** Plan 018 (#24) wrote them down: **Phone** below 600 (bottom
nav, reflows, fits 320px exactly), **Compact** 600899 (icon sidebar),
@@ -2242,6 +2462,31 @@ a list or a detail view:
and dropped if a second click arrives, because the title is the
widest thing in a row and double-clicking a row plays it. Rows do
not need to know links exist.
**Below 600px a name is not a link, and the row's menu is where it
went** (#67). Every sentence above is a *desktop* compromise: the
double-click grace means nothing on touch, a few characters of text
is not a touch target, and since #63 a claimed `yj-tap` has its click
swallowed, so the link was unreachable as well as fiddly. The rule is
in the utility rather than at twenty call sites, and
`utils/go-to-menu.ts` is the other half — "Go to Artist" / "Go to
Album", drawn under exactly the condition the link is not, from
`explore-link`'s own exported routing so an untagged artist reaches
the library page by the same lookup.
Three things about it are load-bearing. **Suppressing a link without
a menu behind it is not a smaller affordance**, it is a destination
the phone cannot reach — so `keepOnPhone` is the documented exception
for the three surfaces with no row menu (`now-playing-view`,
`explore-album-details`' header credit, `top-results-row`), and
nothing else may pass it. **One row or none**: the items are the Play
item's rule one step on, since "go to the album" of five different
albums means nothing. And **there is no "Go to Genre"**, because
there is no genre link anywhere to lose — that would be new
navigation rather than a replacement, and belongs in its own issue.
`track-list` is the one list that gains rather than moves: its phone
column set stacks title over artist as plain text already, so those
names have never been links there.
- **`<catalog-scope-notice>`** is how a detail page admits what it is
showing: catalog data (silent), a library stand-in while a fetch is
in flight, library-only because the entity has no MBID, or a failed/
@@ -2455,11 +2700,38 @@ Five things about it are load-bearing, and four of them fail silently:
correctly. Confidently wrong is worse than absent here, which is the
same rule `Known` exists for.
One gap this did not close, and it is older: **`dhowden/tag` has no
RIFF reader**, so nothing the tag writer puts in a WAV's `id3 ` chunk
is visible to `metadata.ExtractTags` — not the totals and not the title
either. `wav_test.go` reads that chunk itself, which is why no test
ever noticed.
One gap this did not close and #104 did: **`dhowden/tag` has no RIFF
reader**, so nothing the tag writer put in a WAV's `id3 ` chunk was
visible to `metadata.ExtractTags` — not the totals and not the title
either, on files the app itself had just tagged. `wav_test.go` read
that chunk itself, which is why no test noticed: a round trip asserted
through the writer's own parser is a test of the writer.
`backend/riff` is where the container is now read, and it is its own
package because the alternative is an import cycle — `tagwriter`
imports `metadata`, so `metadata` cannot reach back for `parseRIFF`.
`backend/tagtotals` is the precedent.
Three things about it are load-bearing. **The two readers are
deliberately different**: `Parse` holds every chunk in memory, which is
what rewriting a file needs, and a WAV's audio *is* a chunk — so the
scan path uses `ID3Chunk`, which seeks over what it is not looking for.
**The container decides, before `tag.ReadFrom` rather than after it
fails**, because that library's last resort is an ID3v1 trailer and a
WAV carrying both would otherwise be read by the wrong one. And **an
untagged WAV is a file with no tags, not a file with a problem**: no
chunk, an RF64 container or a tag holding no frames all read as empty
metadata with no `TagReadWarning`, since the scanner's filename
fallback is the right answer and a warning would put a fault on a file
that has none.
The gap was pinned by a test that said so, which failed the moment the
reader learned and carried the instructions for what to update in its
own comment. So it is deleted, `TestFixturesMatchManifest` no longer
skips `wav`, and `totals_test.go`'s WAV case goes through
`metadata.ExtractTags` like the other three formats. The fixture
library's two WAV tracks scan with their tags and their cover now,
which is a change to what every seeded tier sees.
**The absence is what gets marked, not the presence.** The tracklist
put a green tick against every owned track and a legend underneath
@@ -3146,6 +3418,46 @@ rather than searching it — the store replaces that array when its
contents change and shares the unchanged members, which is the same
signal `track-list`'s memoized caches key on.
**And the right tier arriving late still reads as no art at all**, so
the two grids ask for it before the card exists (#65).
`utils/image-prefetch.ts` warms the images a scroll is about to reach,
from `cover-grid`'s and `artists-view`'s virtualizers. Measured on the
50 000-track bulk seed over ten 2 400px jumps: of 258 covers arriving
in view, **254 were still blank one frame later and 214 two frames
later**; with the prefetch, 117 and 77. Both builds are clean by 50 ms
on a desktop with 3.7 kB fixture covers, which is where the reference
device's slower engine and 27 kB covers spend their pop-in.
Four things about it are load-bearing.
**The overscan the obvious fix asks for does not exist.**
`@lit-labs/virtualizer`'s `_overhang` is a hard-coded 1000px
`protected` field on `BaseLayout` with no configuration surface, so
raising it means monkey-patching a private. 1000px is about two
screens on a 439px viewport, and the *image* cannot be requested until
the card it lives in is rendered — which is what this asks for
instead.
**It hangs off `rangeChanged`, not `visibilityChanged`.** Those report
different ranges: visibility is what is on screen, and the virtualizer
has already rendered that 1000px past it. Anchored to the visible
range the window is spent on cards that already exist and have already
asked for their own art — measured as the difference between the
prefetch reaching one row past the last card and reaching a full
window past it.
**It is not the `LRUMap` path, and saying so is the bound.** That
ceiling holds Explore's base64 data URLs in JS; a library cover is a
plain URL under `Cache-Control: immutable` (the filenames are content
hashes), so what retains the bytes is the browser's own cache. What
this module retains is the *set of URLs already asked for*, capped at
512 and reported to `window.__yjCacheStats()` — 497 entries and 15 407
chars after the run above.
**The prefetch asks for what the card will draw.** `artists-view`'s
tier ladder moved into `artistAvatarURL()` so the two cannot disagree;
a second copy would be a warm cache for a tier nothing renders.
**The same rule, on the selection path, was the worst stall in the
app.** Five components turned selected file paths back into tracks with
`filePaths.map(fp => tracks.find(…))`, so "Select all → Edit tags" at
@@ -3330,6 +3642,25 @@ Pre-commit hooks verify generated code is fresh — always run `make generate` a
Tests use `database.NewTestDB(t)` for in-memory SQLite, built by the same
`applySchema` production uses so the two cannot diverge. Test audio fixtures live in `test_data/music_library_test/`. Table-driven tests are the norm.
**`make ui-visual` is the one tier nothing but a person runs, and it
cannot become one.** Its ten `toMatchScreenshot` baselines were recorded
on a developer's Arch box; replayed in a bare `ubuntu:24.04` container
— CI's `check` image — three of them fail on font metrics and
compositing alone (`track-info` and one `page-header` shot at a 0.03
mismatch ratio against a 0.02 allowance, `seek-bar` one pixel shorter),
and two components disagree about their own height between the two
machines. So CI keeps running `make ui-test`, which is the same suite
with the comparisons off, and a pre-push hook would be the same fault
with the machines swapped. What replaces the gate is a rule, in
`.pi/skills/yellowjacket-dev/references/ui-tier.md`: **a change that
moves a component's geometry refreshes that component's baseline in the
same commit, having read the image, and never one it did not cause**.
That is #196, which was four stale references accumulated across three
unrelated merges — a red tier nobody could read, which is how it stayed
red. A visual case must also **state the world it photographs**, since
the stores are singletons and a case that sets nothing records whatever
the previous one left in them.
## Git Workflow
Feature branches and PRs are the only way in: **`main` is a protected
+5 -2
View File
@@ -160,8 +160,11 @@ ui-watch: ## Same suite, in watch mode
ui-visual: ## Run the suite including screenshot comparisons
@cd frontend && YJ_VISUAL=1 npx vitest run $(UI_ARGS)
ui-visual-update: ## Re-record the screenshot baselines
@cd frontend && YJ_VISUAL=1 npx vitest run --update $(UI_ARGS)
# `--update=true`, never a bare `--update`: vitest takes the following
# positional as the flag's value, so `--update <path>` swallows the path
# and re-records every baseline in the repo instead of the one named.
ui-visual-update: ## Re-record the screenshot baselines (UI_ARGS=<path> to filter)
@cd frontend && YJ_VISUAL=1 npx vitest run --update=true $(UI_ARGS)
ui-setup: ## Install the Vitest browser provider's own Chromium (once)
@cd frontend && pnpm install && npx playwright install chromium
+8
View File
@@ -74,6 +74,14 @@ func ExtractTags(path string) (*TrackMetadata, error) {
// ExtractTagsFromReader reads metadata from an io.ReadSeeker.
func ExtractTagsFromReader(r io.ReadSeeker) (*TrackMetadata, error) {
// The container decides, so this is asked before tag.ReadFrom and
// not after its failure: a WAV's tags live in a RIFF chunk that
// dhowden/tag cannot see, and its fallback -- an ID3v1 trailer --
// would otherwise outrank them.
if meta, ok := wavTags(r); ok {
return meta, nil
}
m, err := tag.ReadFrom(r)
if err != nil {
// No tags found is not necessarily an error - return empty metadata
+68
View File
@@ -0,0 +1,68 @@
package metadata
import (
"bytes"
"errors"
"fmt"
"io"
"strings"
"yellowjacket/backend/riff"
)
// wavTags reads the ID3v2 tag a WAV carries in its RIFF "id3 " chunk,
// which is where backend/tagwriter puts it and where dhowden/tag --
// having no RIFF reader at all -- cannot look. Without this a WAV
// scans as an untagged file however carefully it was tagged.
//
// ok is false when r is not a RIFF/WAVE container, and the read
// position is restored either way so the caller can carry on.
func wavTags(r io.ReadSeeker) (*TrackMetadata, bool) {
start, err := r.Seek(0, io.SeekCurrent)
if err != nil {
return nil, false
}
id3Data, chunkErr := riff.ID3Chunk(r)
if _, err := r.Seek(start, io.SeekStart); err != nil {
return nil, false
}
switch {
case chunkErr == nil:
return wavTagsFrom(id3Data), true
// Not ours to read: let the ordinary dispatch have the file.
case errors.Is(chunkErr, riff.ErrNotRIFF), errors.Is(chunkErr, riff.ErrNotWAVE):
return nil, false
// A RIFF container we cannot get a tag out of -- no chunk, an RF64
// file, a truncated header. That is a file with no readable tags,
// which is what the scanner's filename fallback is for.
default:
return &TrackMetadata{}, true
}
}
// wavTagsFrom parses the bytes of a WAV's ID3v2 chunk.
func wavTagsFrom(id3Data []byte) *TrackMetadata {
meta, err := extractID3v2Lenient(bytes.NewReader(id3Data))
if err != nil {
// A tag holding no frames is not a damaged tag: writing every
// field back out empty leaves one, and warning about it would
// put a fault on a file that has none.
if errors.Is(err, ErrTagsUnreadable) {
return &TrackMetadata{}
}
return &TrackMetadata{
TagReadWarning: fmt.Errorf("%w: %w", ErrTagsUnreadable, err),
}
}
// extractID3v2Lenient names MP3, being the recovery path for one.
meta.FileFormat = strings.ToUpper(strings.TrimPrefix(string(WAV), "."))
return meta
}
+183
View File
@@ -0,0 +1,183 @@
// Package riff reads the chunk layout of a RIFF/WAVE container.
//
// It exists because both halves of WAV tagging need it and neither can
// import the other: backend/tagwriter writes a WAV's tags into a RIFF
// "id3 " chunk and already imports backend/metadata, which is what has
// to read them back out. backend/tagtotals is the precedent.
//
// The two readers here are deliberately different. Parse holds every
// chunk's data in memory, which is what rewriting a file needs; a WAV's
// audio *is* the "data" chunk, so doing that on the scan path would
// read every library file in full. ID3Chunk seeks over what it is not
// looking for instead. Both walk the same headers.
package riff
import (
"bytes"
"encoding/binary"
"errors"
"fmt"
"io"
"strings"
)
// Sentinel errors describing a container this package will not read.
var (
ErrRF64NotSupported = errors.New("RF64 files are not yet supported")
ErrNotRIFF = errors.New("not a RIFF file")
ErrNotWAVE = errors.New("not a WAVE file")
ErrNoID3Chunk = errors.New("no ID3 chunk in RIFF file")
)
// Chunk holds a single RIFF sub-chunk (ID + raw data).
type Chunk struct {
ID [4]byte
Data []byte
}
// IsID3 reports whether id is that of an ID3v2 RIFF chunk. Both
// lowercase "id3 " and uppercase "ID3 " are accepted.
func IsID3(id [4]byte) bool {
return strings.ToLower(string(id[:3])) == "id3"
}
// Parse reads every RIFF sub-chunk from r, in order, starting at the
// reader's current position. It rejects RF64 files and non-WAVE
// containers with descriptive errors. The parser is lenient: it
// tolerates a missing final padding byte and ignores the declared
// RIFF size.
func Parse(r io.Reader) ([]Chunk, error) {
if err := readContainer(r); err != nil {
return nil, err
}
var chunks []Chunk
for {
id, size, err := nextHeader(r)
if errors.Is(err, io.EOF) {
break
}
if err != nil {
return nil, err
}
data := make([]byte, size)
if _, err := io.ReadFull(r, data); err != nil {
return nil, fmt.Errorf("read chunk data for %q: %w", id, err)
}
chunks = append(chunks, Chunk{ID: id, Data: data})
// Odd-length chunks have a padding byte. Lenient: if the
// read fails (e.g. EOF), just break rather than error.
if size%2 != 0 {
var pad [1]byte
if _, err := r.Read(pad[:]); err != nil {
break
}
}
}
return chunks, nil
}
// ID3Chunk returns the payload of the ID3v2 chunk of the RIFF/WAVE
// container at the reader's current position, seeking over every other
// chunk rather than reading it. It returns ErrNoID3Chunk when the
// container carries no such chunk, and leaves the read position
// unspecified either way.
func ID3Chunk(r io.ReadSeeker) ([]byte, error) {
if err := readContainer(r); err != nil {
return nil, err
}
for {
id, size, err := nextHeader(r)
if errors.Is(err, io.EOF) {
return nil, ErrNoID3Chunk
}
if err != nil {
return nil, err
}
if !IsID3(id) {
// Odd-length chunks carry a padding byte. Seeking past
// the end of the file is not an error; the next header
// read is what reports the end.
if _, err := r.Seek(int64(size)+int64(size%2), io.SeekCurrent); err != nil {
return nil, fmt.Errorf("skip chunk %q: %w", id, err)
}
continue
}
// Copied rather than allocated up front: a truncated file is
// free to declare a chunk larger than the whole of itself.
var data bytes.Buffer
if _, err := io.CopyN(&data, r, int64(size)); err != nil {
return nil, fmt.Errorf("read chunk data for %q: %w", id, err)
}
return data.Bytes(), nil
}
}
// readContainer consumes the 12-byte RIFF/WAVE header at the reader's
// current position.
func readContainer(r io.Reader) error {
var magic [4]byte
if _, err := io.ReadFull(r, magic[:]); err != nil {
return fmt.Errorf("read RIFF magic: %w", err)
}
if string(magic[:]) == "RF64" {
return ErrRF64NotSupported
}
if string(magic[:]) != "RIFF" {
return fmt.Errorf("%w: got %q", ErrNotRIFF, magic)
}
// Read (and discard) RIFF size — lenient, do not enforce.
var riffSize uint32
if err := binary.Read(r, binary.LittleEndian, &riffSize); err != nil {
return fmt.Errorf("read RIFF size: %w", err)
}
var form [4]byte
if _, err := io.ReadFull(r, form[:]); err != nil {
return fmt.Errorf("read WAVE form type: %w", err)
}
if string(form[:]) != "WAVE" {
return fmt.Errorf("%w: got %q", ErrNotWAVE, form)
}
return nil
}
// nextHeader reads one sub-chunk header. It returns io.EOF once the
// chunks are exhausted, including for a header cut short.
func nextHeader(r io.Reader) ([4]byte, uint32, error) {
var id [4]byte
_, err := io.ReadFull(r, id[:])
if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
return id, 0, io.EOF
}
if err != nil {
return id, 0, fmt.Errorf("read chunk ID: %w", err)
}
var size uint32
if err := binary.Read(r, binary.LittleEndian, &size); err != nil {
return id, 0, fmt.Errorf("read chunk size for %q: %w", id, err)
}
return id, size, nil
}
+211
View File
@@ -0,0 +1,211 @@
package riff_test
import (
"bytes"
"encoding/binary"
"errors"
"testing"
"yellowjacket/backend/riff"
)
// chunk is one sub-chunk to put in a test container.
type chunk struct {
id string
data []byte
}
// buildRIFF assembles a container from magic, form type and chunks,
// padding odd-length chunks the way a writer must.
func buildRIFF(magic, form string, chunks []chunk) []byte {
var body bytes.Buffer
body.WriteString(form)
for _, c := range chunks {
body.WriteString(c.id)
_ = binary.Write(&body, binary.LittleEndian, uint32(len(c.data)))
body.Write(c.data)
if len(c.data)%2 != 0 {
body.WriteByte(0)
}
}
var out bytes.Buffer
out.WriteString(magic)
_ = binary.Write(&out, binary.LittleEndian, uint32(body.Len()))
out.Write(body.Bytes())
return out.Bytes()
}
func TestID3Chunk_FindsTheTagPastTheAudio(t *testing.T) {
t.Parallel()
tests := []struct {
name string
chunks []chunk
want string
}{
{
name: "after an odd-length chunk",
chunks: []chunk{
{id: "fmt ", data: make([]byte, 16)},
{id: "LIST", data: []byte("INFOodd")},
{id: "data", data: make([]byte, 200)},
{id: "id3 ", data: []byte("ID3vTAG")},
},
want: "ID3vTAG",
},
{
// The chunk ID is written both ways in the wild, and the
// writer accepts either, so the reader must too.
name: "uppercase ID3",
chunks: []chunk{
{id: "data", data: make([]byte, 8)},
{id: "ID3 ", data: []byte("upper")},
},
want: "upper",
},
{
name: "first chunk",
chunks: []chunk{
{id: "id3 ", data: []byte("first")},
{id: "data", data: make([]byte, 8)},
},
want: "first",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
r := bytes.NewReader(buildRIFF("RIFF", "WAVE", tc.chunks))
got, err := riff.ID3Chunk(r)
if err != nil {
t.Fatalf("ID3Chunk: %v", err)
}
if string(got) != tc.want {
t.Errorf("chunk data: got %q, want %q", got, tc.want)
}
})
}
}
func TestID3Chunk_RejectsWhatItCannotRead(t *testing.T) {
t.Parallel()
tests := []struct {
name string
bytes []byte
want error
}{
{
name: "no ID3 chunk",
bytes: buildRIFF("RIFF", "WAVE", []chunk{{id: "data", data: []byte{1, 2}}}),
want: riff.ErrNoID3Chunk,
},
{
name: "no chunks at all",
bytes: buildRIFF("RIFF", "WAVE", nil),
want: riff.ErrNoID3Chunk,
},
{
name: "not RIFF",
bytes: []byte("ID3\x03\x00\x00\x00\x00\x00\x00\x00\x00"),
want: riff.ErrNotRIFF,
},
{
name: "not WAVE",
bytes: buildRIFF("RIFF", "AVI ", []chunk{{id: "id3 ", data: []byte("x")}}),
want: riff.ErrNotWAVE,
},
{
name: "RF64",
bytes: buildRIFF("RF64", "WAVE", []chunk{{id: "id3 ", data: []byte("x")}}),
want: riff.ErrRF64NotSupported,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
_, err := riff.ID3Chunk(bytes.NewReader(tc.bytes))
if !errors.Is(err, tc.want) {
t.Errorf("ID3Chunk error: got %v, want %v", err, tc.want)
}
})
}
}
// A file cut short mid-chunk is a file with no tag, not a reason to
// allocate the size it claims: the declared size is four bytes any
// truncation can leave saying 4 GB.
func TestID3Chunk_ToleratesATruncatedFile(t *testing.T) {
t.Parallel()
full := buildRIFF("RIFF", "WAVE", []chunk{
{id: "data", data: make([]byte, 64)},
{id: "id3 ", data: []byte("tag")},
})
t.Run("cut inside the audio", func(t *testing.T) {
t.Parallel()
_, err := riff.ID3Chunk(bytes.NewReader(full[:32]))
if !errors.Is(err, riff.ErrNoID3Chunk) {
t.Errorf("ID3Chunk error: got %v, want %v", err, riff.ErrNoID3Chunk)
}
})
t.Run("cut inside the tag", func(t *testing.T) {
t.Parallel()
if _, err := riff.ID3Chunk(bytes.NewReader(full[:len(full)-2])); err == nil {
t.Error("ID3Chunk: got nil error for a truncated tag chunk")
}
})
}
// Parse is the writer's half and reads every chunk into memory, which
// is what preserving them needs.
func TestParse_ReadsEveryChunkInOrder(t *testing.T) {
t.Parallel()
raw := buildRIFF("RIFF", "WAVE", []chunk{
{id: "fmt ", data: make([]byte, 16)},
{id: "LIST", data: []byte("INFOodd")},
{id: "id3 ", data: []byte("tag")},
})
chunks, err := riff.Parse(bytes.NewReader(raw))
if err != nil {
t.Fatalf("Parse: %v", err)
}
want := []string{"fmt ", "LIST", "id3 "}
if len(chunks) != len(want) {
t.Fatalf("chunk count: got %d, want %d", len(chunks), len(want))
}
for i, id := range want {
if got := string(chunks[i].ID[:]); got != id {
t.Errorf("chunk %d: got %q, want %q", i, got, id)
}
}
if !riff.IsID3(chunks[2].ID) || string(chunks[2].Data) != "tag" {
t.Errorf("id3 chunk: got %q", chunks[2].Data)
}
// The padding byte after an odd chunk is not part of its data.
if string(chunks[1].Data) != "INFOodd" {
t.Errorf("odd chunk data: got %q, want %q", chunks[1].Data, "INFOodd")
}
}
+5 -4
View File
@@ -13,9 +13,10 @@ import (
// indistinguishable from never having written one. So these assert the
// round trip through the *reader the scan uses*, not the bytes.
//
// WAV is the exception and it is not this change's: dhowden/tag has no
// RIFF reader at all, so metadata.ExtractTags cannot see a WAV's ID3
// chunk -- which is why every other test here reads that chunk itself.
// WAV was the exception until #104 -- dhowden/tag has no RIFF reader,
// so metadata.ExtractTags could not see a WAV's ID3 chunk and this
// case read the chunk itself, which is a test of the writer wearing
// the shape of a round trip. All four go through the scanner now.
func TestWriteTotals_RoundTripsInEveryFormat(t *testing.T) {
t.Parallel()
@@ -91,7 +92,7 @@ func TestWriteTotals_RoundTripsInEveryFormat(t *testing.T) {
},
{
name: "wav",
read: readWavID3Tags,
read: viaScanner,
write: func(t *testing.T, dir string) string {
t.Helper()
+11 -105
View File
@@ -8,116 +8,22 @@ import (
"io"
"log/slog"
"os"
"strings"
id3v2 "github.com/bogem/id3v2/v2"
"yellowjacket/backend/fileutil"
"yellowjacket/backend/riff"
)
// Sentinel errors for WAV RIFF operations.
var (
errRF64NotSupported = errors.New("RF64 files are not yet supported")
errNotRIFF = errors.New("not a RIFF file")
errNotWAVE = errors.New("not a WAVE file")
errFileTooLargeForWAV = errors.New("file too large for WAV format (>4GB)")
)
// riffChunk holds a single RIFF sub-chunk (ID + raw data).
type riffChunk struct {
id [4]byte
data []byte
}
// parseRIFF reads all RIFF sub-chunks from r. It rejects RF64 files
// and non-WAVE containers with descriptive errors. The parser is
// lenient on read: it tolerates missing padding bytes and ignores
// the declared RIFF size.
func parseRIFF(r io.ReadSeeker) ([]riffChunk, error) {
// Read 4-byte container magic.
var magic [4]byte
if _, err := io.ReadFull(r, magic[:]); err != nil {
return nil, fmt.Errorf("read RIFF magic: %w", err)
}
if string(magic[:]) == "RF64" {
return nil, errRF64NotSupported
}
if string(magic[:]) != "RIFF" {
return nil, fmt.Errorf("%w: got %q", errNotRIFF, magic)
}
// Read (and discard) RIFF size — lenient, do not enforce.
var riffSize uint32
if err := binary.Read(r, binary.LittleEndian, &riffSize); err != nil {
return nil, fmt.Errorf("read RIFF size: %w", err)
}
// Read 4-byte form type.
var form [4]byte
if _, err := io.ReadFull(r, form[:]); err != nil {
return nil, fmt.Errorf("read WAVE form type: %w", err)
}
if string(form[:]) != "WAVE" {
return nil, fmt.Errorf("%w: got %q", errNotWAVE, form)
}
// Read sub-chunks until EOF.
var chunks []riffChunk
for {
var chunkID [4]byte
_, err := io.ReadFull(r, chunkID[:])
if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
break
}
if err != nil {
return nil, fmt.Errorf("read chunk ID: %w", err)
}
var chunkSize uint32
if err := binary.Read(r, binary.LittleEndian, &chunkSize); err != nil {
return nil, fmt.Errorf("read chunk size for %q: %w", chunkID, err)
}
data := make([]byte, chunkSize)
if _, err := io.ReadFull(r, data); err != nil {
return nil, fmt.Errorf("read chunk data for %q: %w", chunkID, err)
}
chunks = append(chunks, riffChunk{id: chunkID, data: data})
// Odd-length chunks have a padding byte. Lenient: if the
// read fails (e.g. EOF), just break rather than error.
if chunkSize%2 != 0 {
var pad [1]byte
if _, err := r.Read(pad[:]); err != nil {
break
}
}
}
return chunks, nil
}
// isID3ChunkID returns true if id represents an ID3v2 RIFF chunk.
// Both lowercase "id3 " and uppercase "ID3 " are accepted.
func isID3ChunkID(id [4]byte) bool {
s := strings.ToLower(string(id[:3]))
return s == "id3"
}
// errFileTooLargeForWAV is the one RIFF error that belongs to the
// writer; reading rejects a container in backend/riff.
var errFileTooLargeForWAV = errors.New("file too large for WAV format (>4GB)")
// writeRIFF writes a complete RIFF/WAVE container to w, preserving
// the given chunks in order and appending the id3Data as the final
// "id3 " chunk. Returns errFileTooLargeForWAV if the result would
// exceed the 4 GB RIFF limit.
func writeRIFF(w io.Writer, chunks []riffChunk, id3Data []byte) error {
func writeRIFF(w io.Writer, chunks []riff.Chunk, id3Data []byte) error {
// Calculate total RIFF payload size:
// 4 bytes (WAVE form type)
// + for each preserved chunk: 8 (header) + len(data) + padding
@@ -125,7 +31,7 @@ func writeRIFF(w io.Writer, chunks []riffChunk, id3Data []byte) error {
riffPayload := uint64(4)
for _, c := range chunks {
sz := uint64(len(c.data))
sz := uint64(len(c.Data))
riffPayload += 8 + sz
if sz%2 != 0 {
@@ -162,7 +68,7 @@ func writeRIFF(w io.Writer, chunks []riffChunk, id3Data []byte) error {
// Write each preserved chunk.
for _, c := range chunks {
if err := writeChunk(w, c.id, c.data); err != nil {
if err := writeChunk(w, c.ID, c.Data); err != nil {
return err
}
}
@@ -225,7 +131,7 @@ func writeWavTags(
return fmt.Errorf("open wav for reading: %w", err)
}
allChunks, err := parseRIFF(f)
allChunks, err := riff.Parse(f)
// Close immediately — we need the handle released before
// AtomicWrite creates the replacement file.
@@ -237,13 +143,13 @@ func writeWavTags(
// Separate preserved chunks from existing ID3 data.
var (
preserved []riffChunk
preserved []riff.Chunk
existingID3 []byte
)
for _, c := range allChunks {
if isID3ChunkID(c.id) {
existingID3 = c.data
if riff.IsID3(c.ID) {
existingID3 = c.Data
} else {
preserved = append(preserved, c)
}
+103 -17
View File
@@ -12,6 +12,7 @@ import (
id3v2 "github.com/bogem/id3v2/v2"
"yellowjacket/backend/metadata"
"yellowjacket/backend/riff"
)
// createTestWAV builds a minimal valid WAV file with an optional
@@ -270,6 +271,88 @@ func TestWriteWavTags_PartialUpdate(t *testing.T) {
assertStrField(t, "Composer", meta.Composer, "Original Composer")
}
// The writer has always been correct and the reader could not see it:
// a WAV tagged by this app scanned as an untagged file, so editing
// tags, autotagging a folder or importing a WAV download all appeared
// to work and changed nothing the library could show (#104). So this
// asserts the write through metadata.ExtractTags -- the reader the
// scan uses -- rather than through the id3 chunk.
func TestWriteWavTags_ReadBackByTheScanner(t *testing.T) {
t.Parallel()
dir := t.TempDir()
path := createTestWAV(t, dir, "scanner.wav", nil)
art := tinyJPEG(t)
changes := TagChanges{
FieldTitle: "Some Song",
FieldArtist: "Some Artist",
FieldAlbum: "Some Album",
FieldAlbumArtist: "Some Album Artist",
FieldGenre: "Rock",
FieldYear: 2024,
FieldTrackNumber: 3,
FieldComposer: "Some Composer",
FieldCoverArt: art,
}
if err := writeWavTags(testLogger(), path, changes); err != nil {
t.Fatalf("writeWavTags: %v", err)
}
meta, err := metadata.ExtractTags(path)
if err != nil {
t.Fatalf("ExtractTags: %v", err)
}
if meta.TagReadWarning != nil {
t.Errorf("TagReadWarning: %v", meta.TagReadWarning)
}
assertStrField(t, "Title", meta.Title, "Some Song")
assertStrField(t, "Artist", meta.Artist, "Some Artist")
assertStrField(t, "Album", meta.Album, "Some Album")
assertStrField(t, "AlbumArtist", meta.AlbumArtist, "Some Album Artist")
assertStrField(t, "Genre", meta.Genre, "Rock")
assertStrField(t, "Composer", meta.Composer, "Some Composer")
assertStrField(t, "FileFormat", meta.FileFormat, "WAV")
assertIntField(t, "Year", meta.Year, 2024)
assertIntField(t, "TrackNumber", meta.TrackNumber, 3)
if !strings.HasPrefix(meta.TagFormat, "ID3v2") {
t.Errorf("TagFormat: got %q, want an ID3v2 version", meta.TagFormat)
}
if meta.Picture == nil {
t.Fatal("expected cover art, got nil")
}
if !bytes.Equal(meta.Picture.Data, art) {
t.Errorf("picture data mismatch: got %d bytes, want %d",
len(meta.Picture.Data), len(art))
}
}
// An untagged WAV is a file with no tags, not a file with a problem:
// the scanner falls back to the filename and must not be handed a
// warning to surface about it.
func TestUntaggedWav_ReadsAsEmptyWithoutAWarning(t *testing.T) {
t.Parallel()
path := createTestWAV(t, t.TempDir(), "bare.wav", nil)
meta, err := metadata.ExtractTags(path)
if err != nil {
t.Fatalf("ExtractTags: %v", err)
}
if meta.TagReadWarning != nil {
t.Errorf("TagReadWarning: %v", meta.TagReadWarning)
}
assertStrField(t, "Title", meta.Title, "")
}
func TestWriteWavTags_ChunkPreservation(t *testing.T) {
t.Parallel()
@@ -282,7 +365,7 @@ func TestWriteWavTags_ChunkPreservation(t *testing.T) {
t.Fatalf("open original: %v", err)
}
origChunks, err := parseRIFF(origFile)
origChunks, err := riff.Parse(origFile)
_ = origFile.Close()
if err != nil {
@@ -292,7 +375,7 @@ func TestWriteWavTags_ChunkPreservation(t *testing.T) {
// Record original chunk data by ID string.
origData := map[string][]byte{}
for _, c := range origChunks {
origData[string(c.id[:])] = c.data
origData[string(c.ID[:])] = c.Data
}
// Write a tag to trigger RIFF rewrite.
@@ -309,7 +392,7 @@ func TestWriteWavTags_ChunkPreservation(t *testing.T) {
t.Fatalf("open after write: %v", err)
}
newChunks, err := parseRIFF(newFile)
newChunks, err := riff.Parse(newFile)
_ = newFile.Close()
if err != nil {
@@ -320,7 +403,7 @@ func TestWriteWavTags_ChunkPreservation(t *testing.T) {
origNonID3 := 0
for _, c := range origChunks {
if !isID3ChunkID(c.id) {
if !riff.IsID3(c.ID) {
origNonID3++
}
}
@@ -328,7 +411,7 @@ func TestWriteWavTags_ChunkPreservation(t *testing.T) {
newNonID3 := 0
for _, c := range newChunks {
if !isID3ChunkID(c.id) {
if !riff.IsID3(c.ID) {
newNonID3++
}
}
@@ -359,17 +442,17 @@ func TestWriteWavTags_ChunkPreservation(t *testing.T) {
// in chunks and its data matches want byte-for-byte.
func checkChunkPreserved(
t *testing.T,
chunks []riffChunk,
chunks []riff.Chunk,
idStr string,
want []byte,
) {
t.Helper()
for _, c := range chunks {
if string(c.id[:]) == idStr {
if !bytes.Equal(c.data, want) {
if string(c.ID[:]) == idStr {
if !bytes.Equal(c.Data, want) {
t.Errorf("chunk %q data changed: got %d bytes, want %d",
idStr, len(c.data), len(want))
idStr, len(c.Data), len(want))
}
return
@@ -430,7 +513,7 @@ func TestWriteWavTags_RejectsRF64(t *testing.T) {
buf.WriteString("WAVE")
// Minimal ds64 chunk (required for RF64 but we just need
// enough bytes for parseRIFF to hit the RF64 rejection).
// enough bytes for riff.Parse to hit the RF64 rejection).
buf.WriteString("ds64")
_ = binary.Write(&buf, binary.LittleEndian, uint32(28)) //nolint:mnd
buf.Write(make([]byte, 28)) //nolint:mnd
@@ -454,9 +537,12 @@ func TestWriteWavTags_RejectsRF64(t *testing.T) {
// readWavID3Tags extracts ID3v2 metadata from a WAV file by parsing
// the RIFF structure and reading the id3 chunk with bogem/id3v2.
// dhowden/tag's ReadFrom does not support WAV files, and its
// ReadID3v2Tags fails on empty tags (after clearing all frames).
// Using bogem/id3v2.ParseReader handles all cases correctly.
//
// metadata.ExtractTags reads a WAV since #104 and is what the round
// trips assert through. This stays for the two cases that are about
// the bytes rather than about the scan: a tag with every frame
// cleared, which no reader reports as anything, and the chunk
// preservation test, which is already parsing the container itself.
func readWavID3Tags(
t *testing.T,
path string,
@@ -470,17 +556,17 @@ func readWavID3Tags(
defer func() { _ = f.Close() }()
chunks, err := parseRIFF(f)
chunks, err := riff.Parse(f)
if err != nil {
t.Fatalf("parseRIFF: %v", err)
t.Fatalf("riff.Parse: %v", err)
}
// Find the id3 chunk.
var id3Data []byte
for _, c := range chunks {
if isID3ChunkID(c.id) {
id3Data = c.data
if riff.IsID3(c.ID) {
id3Data = c.Data
break
}
+140
View File
@@ -0,0 +1,140 @@
import { test, expect } from '../support/fixtures.js';
/**
* The web view's own tap highlight, and what replaced it (#54).
*
* Two halves, and each is here because no other tier can see it.
*
* **The highlight is killed by one declaration on `html`**, which
* reaches the app's shadow roots because `-webkit-tap-highlight-color`
* is inherited and inheritance crosses a shadow boundary. That is a
* property of `index.css`, and `index.css` is loaded by the real app
* and by nothing else — the component tier mounts a component with no
* page stylesheet at all, which is the same reason the theme's ramps
* are invisible to it.
*
* **The press state is measured rather than read.** The component tier
* asserts the shape of the stylesheet (which rule is inside which
* query, and that the press selector carries a state class), because
* `:active` cannot be forced there. Here there is a real pointer: hold
* the button down on a real row of the real list and read what the row
* became. That is the assertion that would fail if the rule were
* hoisted, renamed, or lost to `.selected`.
*
* What neither half is, is the device. Chrome 113's WebView is where
* the grey box was reported and where a finger is; the numbers from it
* are on the PR.
*/
type Page = import('@playwright/test').Page;
/** The phone this work was measured against, in CSS pixels. */
const DEVICE = { width: 424, height: 439 };
/** The computed tap-highlight colour of a node inside a shadow root. */
const tapHighlight = (page: Page, host: string, inner: string) =>
page.evaluate(
([hostSel, innerSel]) => {
const el = document
.querySelector(hostSel!)
?.shadowRoot?.querySelector(innerSel!);
if (!el) return null;
return getComputedStyle(el).getPropertyValue(
'-webkit-tap-highlight-color',
);
},
[host, inner],
);
test.describe('the tap highlight', () => {
test('is transparent inside a shadow root, from one rule on html', async ({
app,
browserName,
}) => {
await app.getByTestId('nav-tracks').click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'tracks',
);
const row = await tapHighlight(app, 'track-list', '.track-row');
expect(row).not.toBeNull();
// The property is a WebKit extension that only iOS honours, so an
// engine is free not to report one at all. Chromium always does —
// measured at rgba(0, 0, 0, 0.18) with the rule removed, which is
// the grey box the report describes — so the assertion is not
// skippable there, and nothing this app can do makes the property
// disappear on an engine that has it.
test.skip(
row === '',
`${browserName} reports no -webkit-tap-highlight-color to read`,
);
expect(row).toBe('rgba(0, 0, 0, 0)');
});
});
test.describe('the press state that replaced it', () => {
test.beforeEach(async ({ app }) => {
await app.setViewportSize(DEVICE);
await app.getByTestId('tab-tracks').click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'tracks',
);
await expect(app.locator('track-list').first()).toBeVisible();
});
test.afterEach(async ({ app }) => {
await app.mouse.up();
await app.setViewportSize({ width: 1440, height: 900 });
});
test('shows on the row being pressed, and on that row only', async ({
app,
}) => {
const rows = await app.evaluate(() => {
const found = document
.querySelector('track-list')
?.shadowRoot?.querySelectorAll('.track-row');
if (!found || found.length < 2) return null;
const rect = found[1]!.getBoundingClientRect();
return {
x: Math.round(rect.x + rect.width / 2),
y: Math.round(rect.y + rect.height / 2),
};
});
expect(rows).not.toBeNull();
const backgrounds = () =>
app.evaluate(() => {
const found = document
.querySelector('track-list')!
.shadowRoot!.querySelectorAll('.track-row');
return {
pressed: getComputedStyle(found[1]!).backgroundColor,
neighbour: getComputedStyle(found[2]!).backgroundColor,
};
});
await app.mouse.move(rows!.x, rows!.y);
await app.mouse.down();
const held = await backgrounds();
// The press overlay, from the theme rather than from a literal in
// a component: rgba(255, 255, 255, 0.12) on both dark ramps.
expect(held.pressed).toBe('rgba(255, 255, 255, 0.12)');
expect(held.neighbour).not.toBe(held.pressed);
await app.mouse.up();
});
});
+135
View File
@@ -0,0 +1,135 @@
import {
test,
expect,
callBinding,
openTheQueue,
NO_QUEUE_SOURCE,
} from '../support/fixtures.js';
import type { Page } from '@playwright/test';
/**
* #67 — a name is not a link on a phone, and the menu is where it went.
*
* The queue panel is the surface this is visible on: its rows draw a
* track title and an artist credit as `explore-link`s at every width,
* unlike `track-list`, whose phone column set stacks title over artist
* as plain text already.
*
* **The pair is what makes either assertion mean anything.** A link
* that is gone and a menu item that never arrived is not a smaller
* affordance — it is a destination the phone cannot reach, which is
* what plan 018's "no action is unreachable at any supported size"
* refuses. So each test asserts the phone and the desktop in the same
* breath: text *and* an item here, a link *and* no item there.
*
* The desktop half is also the regression guard for the change: menus
* above the breakpoint must be exactly what they were, because the name
* beside them is still a link and a menu that repeats the row is
* furniture.
*/
/** The reference device's real viewport, not a resized desktop. */
const DEVICE = { width: 424, height: 439 };
/** Wide enough that the queue is a column beside the content. */
const DESKTOP = { width: 1280, height: 800 };
const row = (app: Page, index: number) =>
app.locator(`queue-panel .track-item[data-index="${index}"]`);
/** The queue panel's own context menu, as a list of item labels. */
async function menuLabels(app: Page): Promise<string[]> {
return app.evaluate(() =>
[
...document
.querySelector('queue-panel')!
.shadowRoot!.querySelectorAll('wa-dropdown-item'),
].map((item) => item.textContent?.replace(/\s+/g, ' ').trim() ?? ''),
);
}
/**
* Queue three tracks that have an album, for the reason
* `queue-selection.spec.ts` states at length: `explore-link` routes a
* title to its *album's* page and renders plain text where it cannot
* route, so a track with no album answers this file's question with
* the wrong "no link".
*/
async function queueThree(app: Page): Promise<void> {
const paths = await app.evaluate(async () => {
const tracks = (await window.__yjEvents.call(
'library.Library.GetTracks',
[0],
10_000,
)) as { FilePath: string; Album: string; ArtistName: string }[];
return tracks
.filter((t) => t.Album !== '' && t.ArtistName !== '')
.slice(0, 3)
.map((t) => t.FilePath);
});
await callBinding(app, 'queue.Queue.SetQueue', [
paths,
0,
false,
NO_QUEUE_SOURCE,
]);
}
/** Open the row's context menu and read the items back. */
async function openRowMenu(app: Page, index: number): Promise<string[]> {
await row(app, index).click({ button: 'right' });
await expect
.poll(async () => (await menuLabels(app)).length)
.toBeGreaterThan(0);
return menuLabels(app);
}
test.describe('an inline name and the menu that replaces it', () => {
test.afterEach(async ({ app }) => {
await app.keyboard.press('Escape');
await callBinding(app, 'queue.Queue.Clear').catch(() => {
/* an empty queue is the state we were asking for */
});
await app.setViewportSize(DESKTOP);
});
test('a queue row is plain text on a phone and carries the destination', async ({
app,
}) => {
await app.setViewportSize(DEVICE);
await queueThree(app);
await openTheQueue(app);
await expect(row(app, 0)).toBeVisible();
// The name is text: nothing in the row is a link at all.
await expect(app.locator('queue-panel .track-item .explore-link')).toHaveCount(
0,
);
const labels = await openRowMenu(app, 0);
expect(labels).toContain('Go to Artist');
expect(labels).toContain('Go to Album');
});
test('the same row on a desktop is a link, and its menu is untouched', async ({
app,
}) => {
await app.setViewportSize(DESKTOP);
await queueThree(app);
await openTheQueue(app);
await expect(row(app, 0)).toBeVisible();
await expect(
row(app, 0).locator('.track-title .explore-link'),
).toHaveCount(1);
const labels = await openRowMenu(app, 0);
expect(labels).not.toContain('Go to Artist');
expect(labels).not.toContain('Go to Album');
});
});
+52
View File
@@ -98,6 +98,58 @@ test.describe('the shell on a phone', () => {
).toBeVisible();
});
test('draws "More" as a sheet on the bottom edge (#71)', async ({ app }) => {
await app.getByTestId('tab-more').click();
await expect(app.getByTestId('nav-drawer').locator('app-sidebar'))
.toBeVisible();
// What the report is about is geometry, and geometry is what no
// other assertion here can see: the side drawer was a 200px column
// opening away from the thumb that asked for it, with the rest of
// its 400px band empty. Measured rather than screenshotted, since
// the failure is a number.
//
// Polled, because a sheet *arrives*: the drawer's show animation
// translates it a full height below the fold, so a measurement
// taken the moment its content is visible reports a box hanging
// 412px off the bottom of the screen. Asking for the settled
// number is the assertion; asking once is a race.
const measure = () => app.evaluate(() => {
const nav = document.querySelector('bottom-nav');
const drawer = nav?.shadowRoot?.querySelector('wa-drawer');
const dialog = drawer?.shadowRoot?.querySelector('[part~="dialog"]');
const sidebar = nav?.shadowRoot?.querySelector('app-sidebar');
const row = sidebar?.shadowRoot?.querySelector('li button');
const box = dialog?.getBoundingClientRect();
return {
left: Math.round(box?.left ?? -1),
right: Math.round(box?.right ?? -1),
bottom: Math.round(box?.bottom ?? -1),
height: Math.round(box?.height ?? -1),
row: Math.round(row?.getBoundingClientRect().height ?? -1),
viewport: [window.innerWidth, window.innerHeight],
};
});
await expect
.poll(async () => (await measure()).bottom)
.toBe(PHONE.height);
const sheet = await measure();
expect(sheet.left).toBe(0);
expect(sheet.right).toBe(sheet.viewport[0]);
// A surface covering the whole screen is a page, not a sheet --
// which is also what leaves an outside to tap on, the only pointer
// route out of it (#171 is the same question one surface over).
expect(sheet.height).toBeLessThan(sheet.viewport[1]);
// 48px rows, from #186's touch floor and #60's context sheet.
expect(sheet.row).toBeGreaterThanOrEqual(48);
});
for (const vp of [PHONE, SMALL_PHONE]) {
test(`does not scroll sideways at ${vp.width}×${vp.height}`, async ({ app }) => {
await app.setViewportSize(vp);
+56
View File
@@ -157,6 +157,62 @@ test.describe('an overlaid queue says it is over the content', () => {
});
});
/**
* #170 — the other two buttons in that same row.
*
* Clear queue and Add queue to playlist predate the close button and
* were named by a `title` attribute and nothing else. Unlike the
* sliders in `control-names.spec.ts`, that is not a *missing* name:
* `title` is the last fallback in the accname order, so
* `getByRole('button', { name: 'Clear queue' })` matched them before
* this fix as well as after it — measured, 1 and 1. A sweep for empty
* names cannot see a weak one, which is `a11y.26`'s complaint and the
* reason this file could have grown a green test that proved nothing.
*
* So the name is asserted twice, and the second assertion is the one
* that fails on the broken build. Taking the tooltip away and asking
* again is the property in words: **the name is not the tooltip**. It
* is what makes the button survive content being put inside it later,
* and it is the only one of the two a phone has — there is no hover on
* the surface #55 turned into a full screen. Measured on `main` before
* the fix: 0 and 0.
*
* Both buttons are disabled here, because the queue starts empty and
* naming is not enablement. A disabled button is still in the
* accessibility tree, which is exactly where the complaint was.
*/
test.describe('the queue header says what its actions do', () => {
const ACTIONS = ['Clear queue', 'Add queue to playlist'];
test('names both of the older actions', async ({ app }) => {
await openQueue(app);
for (const name of ACTIONS) {
await expect(
app.getByRole('button', { name, exact: true }),
).toHaveCount(1);
}
});
test('and the names do not come from the tooltip', async ({ app }) => {
await openQueue(app);
await app.locator('#queue-panel').evaluate((el) => {
for (const button of el.shadowRoot!.querySelectorAll(
'.header-action-button',
)) {
button.removeAttribute('title');
}
});
for (const name of ACTIONS) {
await expect(
app.getByRole('button', { name, exact: true }),
).toHaveCount(1);
}
});
});
/**
* The inline panel is the mode that already worked, and the one every
* other queue spec is written against. It keeps its resize handle and
+118 -1
View File
@@ -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);
});
});
+20
View File
@@ -7,6 +7,26 @@
html {
height: 100%;
/* #54. The web view's own tap highlight -- the grey box a phone
draws over the bounding rect of whatever was tapped -- gone in
one declaration, because `-webkit-tap-highlight-color` is an
*inherited* property and an inherited property crosses a shadow
boundary. So this reaches every one of the app's shadow roots
without a rule in any of them; before it, exactly one component
(`library-status-indicator`) set it and the box appeared
everywhere else.
What it costs is the only touch feedback several surfaces had,
which is why the rows, the tab bar and the shared menu items
grew a `:active` state in the same change: removing the wrong
feedback and leaving none is not an improvement. The cards
already had one (`transform: scale(0.97)`).
`user-select` is the same argument one rule up and was already
done: the `*` rule at the top of this file is inherited into the
shadow roots too. */
-webkit-tap-highlight-color: transparent;
}
body {
@@ -7,6 +7,7 @@ import {
import '@lit-labs/virtualizer';
import type {
LitVirtualizer,
RangeChangedEvent,
VisibilityChangedEvent,
} from '@lit-labs/virtualizer';
import { grid } from '@lit-labs/virtualizer/layouts/grid.js';
@@ -30,6 +31,7 @@ import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller
import { FavoritesController } from '@store/controllers/favorites-controller';
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
import { RovingGridController } from '@utils/roving-grid';
import { prefetchImageWindow } from '@utils/image-prefetch';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@awesome.me/webawesome/dist/components/popup/popup.js';
@@ -582,6 +584,26 @@ export class ArtistsView
* Scroll position persistence
* ================================================================ */
/**
* Warm the avatars just past the rendered range (#65).
*
* `rangeChanged` is the rendered range and `visibilityChanged` is
* what is on screen; the virtualizer has already drawn about
* 1000px past the latter, so that is the wrong anchor to measure a
* prefetch window from. It is deliberately outside the
* `restoringScroll` guard below: a restored scroll lands in the
* middle of the grid, which is exactly when nothing around it is
* cached.
*/
private onRangeChanged = (e: RangeChangedEvent) => {
prefetchImageWindow(
this.cachedGridEntries,
e.first,
e.last,
(entry) => this.artistAvatarURL(entry.artist),
);
};
/**
* Save the first visible item index on scroll.
*/
@@ -1145,7 +1167,16 @@ export class ArtistsView
* Helpers
* ================================================================ */
private renderArtistAvatar(artist: library.Artist) {
/**
* The image this artist's card will draw, or `''` for the initial
* placeholder.
*
* Split out of `renderArtistAvatar` so the prefetch (#65) asks for
* exactly what the card is going to ask for — a second copy of the
* tier ladder would be a second thing to keep in step, and warming
* the wrong tier is a download that buys nothing.
*/
private artistAvatarURL(artist: library.Artist): string {
const needed = (this.imageSize ?? 176) * window.devicePixelRatio;
let imageURL = '';
@@ -1172,6 +1203,12 @@ export class ArtistsView
) ?? '';
}
return imageURL;
}
private renderArtistAvatar(artist: library.Artist) {
const imageURL = this.artistAvatarURL(artist);
if (imageURL) {
return html`<img
class="avatar-image"
@@ -1531,6 +1568,7 @@ export class ArtistsView
.keyFunction=${(entry: ArtistEntry) => entry.artist.ID}
.layout=${this.gridLayout}
@visibilityChanged=${this.onVisibilityChanged}
@rangeChanged=${this.onRangeChanged}
></lit-virtualizer>
</div>
${this.renderContextMenu()}
@@ -27,15 +27,43 @@ interface Tab {
* three to five items before the targets stop being thumb-sized —
* 360 px over eleven sidebar entries is 32 px each — so the four here
* are the ones plan 016's subset says a phone is *for*, and "More"
* opens the existing `<app-sidebar>` in a drawer. That is deliberately
* opens the existing `<app-sidebar>` in a sheet. That is deliberately
* a reuse rather than a second nav: two lists of destinations is two
* places to add the next view to, and the sidebar already carries the
* drag-to-navigate behaviour, the active state and the labels.
*
* **"More" rises from the bottom, and it is the same sheet a context
* menu is** (#71). It was a `wa-drawer` sliding in from the side: a
* 200px column of a 424px screen, opening away from the thumb that
* asked for it, with three nested scrollers in it — the dialog, its
* body, and the sidebar's own `overflow-y: auto` host — which is the
* "only part of the screen scrolls under my finger" in the report.
*
* Three things about the replacement are load-bearing.
*
* **It is the same element with another `placement`, not a new
* surface.** `wa-drawer` renders a native `<dialog>` and opens it with
* `showModal()`, which is exactly what `menu-surface`'s sheet relies
* on — Chrome 37, the real top layer — so #60's containment finding
* carries over with nothing new to prove, and the focus trap, Escape,
* tap-outside and `wa-after-hide` all come along unchanged.
*
* **The body is the only scroller**, with `overscroll-behavior:
* contain`, and the sidebar is told to stop being one. Nesting them is
* what makes a drag scroll the wrong box.
*
* **The sidebar is still mounted rather than re-listed as data**,
* which the issue offers as an alternative. Its `data-testid` per
* destination is the reason: the shell's own sidebar is `display:
* none` below 600px rather than removed, so a second list drawing
* `nav-*` handles is the duplication this component already renders
* conditionally to avoid — and it would be a second place to add the
* next view to, with its own copy of #25's visibility filter.
*
* It emits the same bubbling, composed `navigate` event the sidebar
* does, so `index.ts` needs no knowledge of it, and it listens for that
* event globally for the same reason the sidebar does: a navigation it
* did not send (a card click, a detail view, the drawer) still has to
* did not send (a card click, a detail view, the sheet) still has to
* move the highlight.
*/
@customElement('bottom-nav')
@@ -89,6 +117,17 @@ export class BottomNav extends LitElement {
color: var(--yj-accent, #ffd43b);
}
/* The press state (#54). This bar is the phone's primary
navigation and had no feedback of its own at all -- what a
tap produced was the web view's tap highlight, a grey box
over the whole 48px cell, which index.css has now taken
away. The .active rule above is which tab you are *on*; this
is the tab being pressed, so they are a colour and a
background rather than two colours. */
button:active {
background-color: var(--yj-press-overlay, rgba(255, 255, 255, 0.12));
}
button:focus-visible {
outline: 2px solid var(--yj-accent, #ffd43b);
outline-offset: -2px;
@@ -104,15 +143,50 @@ export class BottomNav extends LitElement {
white-space: nowrap;
}
wa-drawer::part(body) {
padding: 0;
/* The sheet. --size is the drawer's own API for the axis its
placement uses, so auto is what makes it hug its content
instead of being a fixed 25rem band; the rest is the shape
the menu-surface context sheet already has, so a phone meets
one sheet rather than two. 85vh for its reason too: a surface
covering the whole screen is a page, not a sheet. */
wa-drawer {
--size: auto;
}
app-sidebar {
/* The sidebar sizes itself inline and collapses to icons
below 900px, which is every phone. In the drawer there
is room for the labels, so it is told not to. */
height: 100%;
wa-drawer::part(dialog) {
max-height: 85vh;
border-radius: 12px 12px 0 0;
/* The sidebar paints its own surface, so the sheet takes
that colour rather than the menus' elevated one: two
greys in one sheet is a seam across the middle of it. */
background-color: var(--yj-bg-surface, #212529);
/* One scroller, and it is the body below. The dialog's own
overflow: auto is what let the sheet scroll as well as
its content, and it is also what would square off the
corners this rule just rounded. */
overflow: hidden;
}
wa-drawer::part(body) {
padding: 0;
/* A scroll that reaches the end of this list must not
become a scroll of the page underneath it. */
overscroll-behavior: contain;
/* The sheet sits on the bottom edge, so the last
destination would otherwise be under the home indicator
on a gesture-navigation phone -- the same allowance the
bar itself makes above. */
padding-bottom: env(safe-area-inset-bottom, 0);
}
/* A sheet is dragged at with a thumb, so it says where its top
edge is. Decorative: the destinations are below it. */
.grip {
width: 36px;
height: 4px;
margin: 8px auto 4px;
border-radius: 2px;
background: var(--yj-text-tertiary, #888);
}
`];
@@ -147,7 +221,7 @@ export class BottomNav extends LitElement {
private visibilityCtrl = new ViewVisibilityController(this);
/**
* Whether the drawer has been asked for.
* Whether the sheet has been asked for.
*
* The sidebar inside it is rendered only while this is true, and
* that is not an optimisation. `app-sidebar` carries a
@@ -189,15 +263,17 @@ export class BottomNav extends LitElement {
override updated() {
// Web Awesome renders its heading into its own shadow root and
// never points aria-labelledby at it, so the drawer would
// never points aria-labelledby at it, so the sheet would
// otherwise be announced unnamed -- the same fix, and the same
// reason, as every wa-dialog in the app. A drawer's shadow root
// has the same shape, so the helper needs no change.
// has the same shape, so the helper needs no change; under
// `without-header` there is no heading to point at, which is
// that helper's documented `aria-label` path.
nameDialog(this.drawer);
}
private onGlobalNavigate = () => {
// A navigation from inside the drawer is the drawer's job done.
// A navigation from inside the sheet is the sheet's job done.
// The highlight is not this listener's business any more.
this.drawerOpen = false;
};
@@ -263,12 +339,14 @@ export class BottomNav extends LitElement {
</nav>
<wa-drawer
placement="start"
placement="bottom"
without-header
label="All views"
data-testid="nav-drawer"
?open=${this.drawerOpen}
@wa-after-hide=${this.onDrawerHide}
>
<div class="grip"></div>
${this.drawerOpen
? html`<app-sidebar expanded></app-sidebar>`
: nothing}
@@ -8,6 +8,7 @@ import {
import '@lit-labs/virtualizer';
import type {
LitVirtualizer,
RangeChangedEvent,
VisibilityChangedEvent,
} from '@lit-labs/virtualizer';
import { grid } from '@lit-labs/virtualizer/layouts/grid.js';
@@ -30,6 +31,7 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@components/playlist-picker/playlist-picker.js';
import { loadTrackDetails } from '@utils/lazy-track-details.js';
import { tracksByFilePath, tracksForPaths } from '@utils/track-index.js';
import { prefetchImageWindow } from '@utils/image-prefetch.js';
import type { TrackDetails } from '@components/track-details/track-details.js';
import type { CoverArtUrls } from '@components/track-details/track-details.js';
import { AlbumSelectionManager } from './album-selection.js';
@@ -55,6 +57,8 @@ import {
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
import { FavoritesController } from '@store/controllers/favorites-controller';
import { creditLink, exploreLinkStyles } from '../../utils/explore-link';
import { goToMenuItems } from '../../utils/go-to-menu';
import type { GoToTarget } from '../../utils/go-to-menu';
import { creditStore } from '@store/credit-store';
import {
createAlbumArtDragImage,
@@ -908,6 +912,31 @@ export class CoverGrid
);
};
/**
* Warm the covers just past the rendered range (#65).
*
* `rangeChanged` rather than `visibilityChanged`, because the two
* report different ranges and only one of them is the right
* anchor: visibility is what is on screen, and the virtualizer has
* already rendered about 1000px past that. Measured from the
* visible range this would spend most of its window on cards that
* already exist and have already asked for their own art.
*
* The entry lists are memoized, so asking for one here costs a
* reference compare.
*/
private onRangeChanged = (e: RangeChangedEvent) => {
const entries = this.splitMode
? this.getBeforeEntries()
: this.buildGridEntries();
prefetchImageWindow(entries, e.first, e.last, (entry) =>
entry.album.CoverArtPath
? this.getCoverUrl(entry.album)
: '',
);
};
/* ====================================================================
* Virtualizer items
* ==================================================================== */
@@ -2001,6 +2030,7 @@ export class CoverGrid
@keydown=${this.onGridAlbumKeydown}
@contextmenu=${this.onGridAlbumContextMenu}
@visibilityChanged=${this.onVisibilityChanged}
@rangeChanged=${this.onRangeChanged}
></lit-virtualizer>
`;
}
@@ -2035,6 +2065,7 @@ export class CoverGrid
@keydown=${this.onGridAlbumKeydown}
@contextmenu=${this.onGridAlbumContextMenu}
@visibilityChanged=${this.onVisibilityChanged}
@rangeChanged=${this.onRangeChanged}
></lit-virtualizer>
<album-dropdown
@@ -2084,6 +2115,30 @@ export class CoverGrid
);
}
/**
* The artist an album card's menu can navigate to — the card's own
* credit line, which stops being a link below the phone breakpoint
* (#67).
*
* A *track* target gets nothing: the dropdown's rows carry no
* links of their own, and the album they sit under is the card
* that opened them.
*/
private get goToTarget(): GoToTarget | undefined {
if (this.contextMenuTarget.kind !== 'album') return undefined;
const album = this.albums.find(
(a) => a.ID === this.contextMenuAlbumId,
);
if (!album) return undefined;
return {
artistName: album.ArtistName,
artistMBID: album.ArtistMBID,
};
}
private renderContextMenu() {
const { ctxMenu } = this;
@@ -2199,6 +2254,11 @@ export class CoverGrid
</wa-dropdown-item>
`
: nothing}
${goToMenuItems(this.goToTarget, {
onSelect: () => ctxMenu.close(),
onHover: () =>
ctxMenu.closePlaylistSubmenu(),
})}
</div>
`
: nothing}
@@ -3191,10 +3191,14 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
return html`
${artist
? html`<div class="album-artist">
<!-- keepOnPhone: the page header is not a row and
has no menu of its own, so this credit is the
only route from an album to its artist (#67). -->
${creditLink(
creditStore.credits(this.releaseGroupMBID),
artist,
artistMbid,
{ keepOnPhone: true },
)}
</div>`
: nothing}
@@ -30,6 +30,7 @@ import { libraryStore } from '../../store/library-store';
import { downloadStore } from '../../store/download-store';
import '@awesome.me/webawesome/dist/components/button/button.js';
import { trackLink, exploreLinkStyles } from '../../utils/explore-link';
import { goToMenuItems } from '../../utils/go-to-menu';
import { describeError } from '../../utils/describe-error';
import {
GetAlbumsByArtist,
@@ -2705,6 +2706,16 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
<wa-icon slot="icon" name="globe"></wa-icon>
View on MusicBrainz
</wa-dropdown-item>
<!-- The track title links to its album, and below the phone
breakpoint it is plain text (#67). The artist is this
page, so there is nothing to go to. -->
${goToMenuItems(
{ albumName: track.releaseName, albumMBID: track.releaseGroupMbid ?? '' },
{
onSelect: () => this.ctxMenu.close(),
onHover: () => this.ctxMenu.closePlaylistSubmenu(),
},
)}
`;
}
@@ -23,6 +23,8 @@ import { queueStore } from '../../store/queue-store';
import { notificationStore } from '../../store/notification-store';
import '../notifications/inline-notice';
import { creditLink, trackLink, exploreLinkStyles } from '../../utils/explore-link';
import { goToMenuItems } from '../../utils/go-to-menu';
import type { GoToTarget } from '../../utils/go-to-menu';
import { creditStore } from '@store/credit-store';
import { describeError } from '../../utils/describe-error';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
@@ -54,10 +56,28 @@ export const ExploreRegion = 'explore';
* is present only when owned — that's what gates the playback items,
* while `mbid` (always present) is what "View on MusicBrainz" uses, so
* a catalog-only card still gets a menu with somewhere useful to go.
*
* `goTo` is the names the card draws -- an artist credit, and for a
* recording row the release its title links to. Below the phone
* breakpoint those are plain text, so the menu is where they went
* (#67); an album card carries no album of its own, because tapping
* the card is already that.
*/
type ExploreMenuTarget =
| { kind: 'album'; mbid: string; localId?: number; title: string }
| { kind: 'recording'; mbid: string; localId?: number; title: string };
| {
kind: 'album';
mbid: string;
localId?: number;
title: string;
goTo?: GoToTarget;
}
| {
kind: 'recording';
mbid: string;
localId?: number;
title: string;
goTo?: GoToTarget;
};
type ThumbnailRequest = explore.ThumbnailRequest;
type MBSearchResult = explore.MBSearchResult;
type LyricsResult = explore.LyricsResult;
@@ -1386,6 +1406,9 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
<wa-icon slot="icon" name="globe"></wa-icon>
View on MusicBrainz
</wa-dropdown-item>
${goToMenuItems(target.goTo, {
onSelect: () => this.ctxMenu.close(),
})}
</div>
`
: nothing}
@@ -2188,6 +2211,10 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
mbid: rg.mbid,
localId: rg.localId,
title: rg.title,
goTo: {
artistName: rg.artistCredit,
artistMBID: rg.artistMbid ?? '',
},
})}
role="button"
tabindex="0"
@@ -2200,6 +2227,10 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
mbid: rg.mbid,
localId: rg.localId,
title: rg.title,
goTo: {
artistName: rg.artistCredit,
artistMBID: rg.artistMbid ?? '',
},
},
)}
>
@@ -2278,6 +2309,12 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
mbid: r.mbid,
localId: r.localId,
title: r.title,
goTo: {
artistName: r.artistCredit,
artistMBID: r.artistMbid ?? '',
albumName: r.releaseName ?? '',
albumMBID: r.releaseGroupMbid ?? '',
},
})}
@keydown=${(e: KeyboardEvent) =>
this.onCardKeydown(
@@ -2288,6 +2325,12 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
mbid: r.mbid,
localId: r.localId,
title: r.title,
goTo: {
artistName: r.artistCredit,
artistMBID: r.artistMbid ?? '',
albumName: r.releaseName ?? '',
albumMBID: r.releaseGroupMbid ?? '',
},
},
)}
>
@@ -155,10 +155,55 @@ export class MenuSurface extends LitElement {
bottom was at y=452 on a 439px screen -- the one row a
destructive action is most likely to be. The cap has to stay
(a sheet covering the whole screen is a page, not a sheet),
so the body is what gives. */
so the body is what gives.
**And a body that scrolls says so** (#207). Scrolling was the
whole of the fix above, which left the last item reachable
and nothing on screen admitting it was there -- measured at
424x439, eight items ending at y=470 with the fold at 439,
and worse when the cut lands on a row boundary, where the
sheet ends in a clean edge that reads as the end of the list.
Two layers, and the *order* is what asks the question: a
shadow pinned to the bottom of the box (attachment scroll),
and over it a cover of the sheet's own colour painted at the
end of the *content* (attachment local), which therefore
scrolls up over the shadow and hides it exactly when there is
nothing more to see. So the affordance is absent on a menu
that fits, present the moment one does not, and gone again at
the end of the list -- with no scroll listener, no
measurement, and nothing reaching into wa-dialog's shadow
root for the scroller. background-attachment is Chrome 4;
the reference device is Chrome 113.
**The curve is steep because the rows under it stay live.**
A scrim over a menu item is that item's text surface, and
this app's rule is that text clears 4.5:1 on every surface it
can sit on -- which the light ramp, whose bgElevated is
#e9ecef, is what makes non-theoretical. A row is 48px with
its label centred, so 32px of scrim that is already down to
a quarter strength at 14px reaches y-centre at about 0.06 and
spends its weight on the strip below the last legible label.
Measured on the dark ramp at x=300, flat 52,58,64 throughout
before: 50,56,62 at y=330, 33,37,40 at y=350 and 22,24,27 at
the bottom edge, and flat again at the end of the list. The
light ramp puts 9.9:1 on the last label. */
wa-dialog::part(body) {
padding: 0;
overflow-y: auto;
background:
linear-gradient(
var(--yj-bg-elevated, #343a40),
var(--yj-bg-elevated, #343a40)
)
bottom / 100% 32px no-repeat local,
linear-gradient(
to top,
rgba(0, 0, 0, 0.6) 0%,
rgba(0, 0, 0, 0.25) 45%,
rgba(0, 0, 0, 0) 100%
)
bottom / 100% 32px no-repeat scroll;
}
/* A sheet is dragged at with a thumb, so it says where its top
@@ -420,11 +420,20 @@ export class NowPlayingView extends LitElement {
<h2 class="title" data-testid="npv-title">
${track.title || track.fileName}
</h2>
<!-- keepOnPhone: this screen is the phone's,
and it has no context menu to carry the
destination the way a row does (#67).
Suppressing these takes the artist and the
album away rather than moving them, and
they are two lines of their own here
rather than a few characters inside a
row. -->
<p class="artist">
${creditLink(
creditStore.credits(track.recordingMbid),
track.artist,
track.artistMbid,
{ keepOnPhone: true },
)}
</p>
${track.album
@@ -434,6 +443,7 @@ export class NowPlayingView extends LitElement {
track.releaseGroupMbid,
undefined,
track.artist,
{ keepOnPhone: true },
)}
</p>`
: nothing}
@@ -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';
@@ -70,12 +74,17 @@ import {
trackLink,
exploreLinkStyles,
} from '@utils/explore-link';
import { goToMenuItems } from '@utils/go-to-menu';
import type { GoToTarget } from '@utils/go-to-menu';
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 +431,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`
<selection-bar
.count=${this.selection.selectionCount}
.actions=${PlaylistDetails.SELECTION_ACTIONS}
@selection-exit=${this.onSelectionExit}
@selection-action=${(e: CustomEvent<{ id: string }>) =>
this.onContextMenuAction(e.detail.id)}
@selection-more=${(e: CustomEvent<{ x: number; y: number }>) =>
this.ctxMenu.openAt(e.detail.x, e.detail.y)}
></selection-bar>
`;
}
private onSelectionExit = () => {
this.selection.exitSelectionMode();
this.virtualizer?.requestUpdate();
};
private handleTrackContextMenu(
e: MouseEvent,
trackIndex: number,
@@ -461,6 +591,28 @@ export class PlaylistDetails
.map((i) => this.tracks[i]!.FilePath);
}
/**
* The row "Go to Artist" / "Go to Album" navigate from one row
* or none, and only below the phone breakpoint, where the row's
* own names stopped being links (#67).
*/
private get goToTarget(): GoToTarget | undefined {
const indices = this.selection.getSelectedIndices();
if (indices.length !== 1) return undefined;
const track = this.tracks[indices[0]!];
if (!track) return undefined;
return {
artistName: track.Artist,
artistMBID: track.ArtistMBID,
albumName: track.Album,
albumMBID: track.ReleaseGroupMBID,
};
}
// =================================================================
// Context menu actions
// =================================================================
@@ -955,9 +1107,11 @@ export class PlaylistDetails
static override styles = [
designTokens,
srOnly,
backButton,
contextMenuStyles,
exploreLinkStyles,
swipeRevealStyles,
css`
:host {
display: flex;
@@ -1136,6 +1290,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 {
@@ -1200,8 +1357,14 @@ export class PlaylistDetails
user-select: none;
}
.track-item:hover {
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
/* A hover tint is for a device that hovers (#54). A hold
synthesises a hover in the WebView, so ungated this arrives
because a finger touched the row and stays after it has
gone; the press state below is what a tap gets instead. */
@media (hover: hover) and (pointer: fine) {
.track-item:hover {
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
}
}
.track-item.selected {
@@ -1221,11 +1384,13 @@ export class PlaylistDetails
cursor: pointer;
}
.track-item.phantom:hover {
background-color: var(
--yj-hover-overlay,
rgba(255, 255, 255, 0.05)
);
@media (hover: hover) and (pointer: fine) {
.track-item.phantom:hover {
background-color: var(
--yj-hover-overlay,
rgba(255, 255, 255, 0.05)
);
}
}
.track-item.phantom.selected {
@@ -1235,6 +1400,19 @@ export class PlaylistDetails
);
}
/* The press state (#54): the feedback a tap has now that the
web view's own highlight box is gone (index.css). Last, and
carrying a class, because a selected or playing row is two
classes deep and a bare :active would lose to it. */
.track-item.selected:active,
.track-item.active:active,
.track-item:active {
background-color: var(
--yj-press-overlay,
rgba(255, 255, 255, 0.12)
);
}
.phantom-row {
grid-column: 1 / -1;
display: flex;
@@ -1433,6 +1611,9 @@ export class PlaylistDetails
<div class="header-cell col-album">Album</div>
<div class="header-cell col-duration">Duration</div>
</div>
<div class="sr-only" role="status" aria-live="polite">
${this.swipe.announcement}
</div>
<lit-virtualizer
class="track-scroller"
role="listbox"
@@ -1442,7 +1623,13 @@ export class PlaylistDetails
.renderItem=${this.renderRow}
.keyFunction=${this.rowKey}
.layout=${this.flowLayout}
@yj-tap=${this.onRowTap}
@yj-long-press=${this.onRowLongPress}
@yj-swipe-start=${this.swipe.onSwipeStart}
@yj-swipe-move=${this.swipe.onSwipeMove}
@yj-swipe-end=${this.swipe.onSwipeEnd}
></lit-virtualizer>
${this.renderSelectionBar()}
`;
}
@@ -1464,6 +1651,7 @@ export class PlaylistDetails
active ? 'active' : '',
selected ? 'selected' : '',
isPhantom ? 'phantom' : '',
this.swipe.isSwiping(trackIndex) ? 'swiping' : '',
]
.filter(Boolean)
.join(' ');
@@ -1474,6 +1662,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 +1709,7 @@ export class PlaylistDetails
? nothing
: this.onTrackDragEnd}
>
${this.swipe.renderReveal(trackIndex)}
${isPhantom
? html`<div
class="phantom-row"
@@ -1755,6 +1945,14 @@ export class PlaylistDetails
Track
Details
</wa-dropdown-item>
${goToMenuItems(this.goToTarget, {
onSelect: () => {
this.selection.clear();
this.ctxMenu.close();
},
onHover: () =>
this.ctxMenu.closePlaylistSubmenu(),
})}
</div>
`
: nothing}
@@ -62,11 +62,18 @@ import {
trackLink,
exploreLinkStyles,
} from '@utils/explore-link';
import { goToMenuItems } from '@utils/go-to-menu';
import type { GoToTarget } from '@utils/go-to-menu';
import {
ICON_NEW,
ICON_PLAY,
ICON_PLAYLIST,
ICON_QUEUE,
ICON_REMOVE,
} from '@utils/icon-language';
import type { GestureEvent } from '@utils/touch-gestures';
import '@components/selection-bar/selection-bar';
import type { SelectionAction } from '@components/selection-bar/selection-bar';
/** Above this many tracks, clearing the queue asks first. */
const CLEAR_CONFIRM_THRESHOLD = 20;
@@ -574,8 +581,14 @@ export class QueuePanel
contain: strict;
}
.track-item:hover {
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
/* A hover tint is for a device that hovers (#54). A hold
synthesises a hover in the WebView, so ungated this arrives
because a finger touched the row and stays after it has
gone; the press state below is what a tap gets instead. */
@media (hover: hover) and (pointer: fine) {
.track-item:hover {
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
}
}
.track-item.selected {
@@ -590,6 +603,19 @@ export class QueuePanel
background-color: var(--yj-selection-bg, rgba(100, 160, 255, 0.15));
}
/* The press state (#54): the feedback a tap has now that the
web view's own highlight box is gone (index.css). Last, and
carrying a class, because a selected or playing row is two
classes deep and a bare :active would lose to it. */
.track-item.selected:active,
.track-item.active:active,
.track-item:active {
background-color: var(
--yj-press-overlay,
rgba(255, 255, 255, 0.12)
);
}
.track-position {
font-size: var(--yj-text-sm);
color: var(--yj-text-tertiary, #888);
@@ -844,6 +870,8 @@ export class QueuePanel
virtEl.addEventListener('dragstart', this.onDelegatedDragStart);
virtEl.addEventListener('dragend', this.onTrackDragEnd);
virtEl.addEventListener('keydown', this.onDelegatedKeydown);
virtEl.addEventListener('yj-tap', this.onRowTap);
virtEl.addEventListener('yj-long-press', this.onRowLongPress);
this.delegationAttached = true;
}
@@ -962,6 +990,8 @@ export class QueuePanel
virtEl.removeEventListener('dragstart', this.onDelegatedDragStart);
virtEl.removeEventListener('dragend', this.onTrackDragEnd);
virtEl.removeEventListener('keydown', this.onDelegatedKeydown);
virtEl.removeEventListener('yj-tap', this.onRowTap);
virtEl.removeEventListener('yj-long-press', this.onRowLongPress);
}
this.delegationAttached = false;
}
@@ -1247,6 +1277,92 @@ export class QueuePanel
this.queue.playAtIndex(index);
}
// =================================================================
// A finger on a queue row (plan 019 phase 3, #63)
// =================================================================
/**
* A tap plays this position in the queue.
*
* `track-list`'s tap sets the queue to the list it was made in;
* copying that here would rebuild the queue from the queue, which
* is not the no-op it looks like -- it would discard the queue's
* source, its shuffle order and everything a user had inserted by
* hand. `playAtIndex` is what a double-click already does, and it
* is what a tap means.
*/
private onRowTap = (e: GestureEvent) => {
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`
<selection-bar
.count=${this.selection.selectionCount}
.actions=${QueuePanel.SELECTION_ACTIONS}
@selection-exit=${this.onSelectionExit}
@selection-action=${(e: CustomEvent<{ id: string }>) =>
this.onContextMenuAction(e.detail.id)}
@selection-more=${(e: CustomEvent<{ x: number; y: number }>) =>
this.ctxMenu.openAt(e.detail.x, e.detail.y)}
></selection-bar>
`;
}
private onSelectionExit = () => {
this.selection.exitSelectionMode();
this.virtualizer?.requestUpdate();
};
private handleTrackContextMenu(
e: MouseEvent,
index: number,
@@ -1529,6 +1645,29 @@ export class QueuePanel
.map((i) => tracks[i]!.filePath);
}
/**
* The row "Go to Artist" / "Go to Album" navigate from, which is
* one row or none the rule the Play item already follows. Both
* items are drawn only below the phone breakpoint, where the row's
* own names stopped being links (#67).
*/
private get goToTarget(): GoToTarget | undefined {
const indices = this.selection.getSelectedIndices();
if (indices.length !== 1) return undefined;
const track = this.queue.tracks[indices[0]!];
if (!track) return undefined;
return {
artistName: track.artist,
artistMBID: track.artistMbid,
albumName: track.album,
albumMBID: track.releaseGroupMbid,
};
}
// =================================================================
// Drop target (tracks dropped into queue)
// =================================================================
@@ -2062,11 +2201,25 @@ export class QueuePanel
`
: nothing}
</div>
<!-- **Every action here is named by aria-label**, like
the close button #24 added beside them (#170). A
title alone *is* a name, which is why a sweep for
empty names reports these clean and why an
assertion by role and name is green either way --
but it is the weakest one: title is the last
fallback in the accname order, so any content put
inside the button later silently outranks it, and
a phone has no hover to show it as a tooltip.
The titles stay. On a desktop they are the tooltip
for an icon-only control, which is a different job
from naming it, and aria-label does not do it. -->
<div class="header-actions">
<button
class="header-action-button"
@click=${() => void this.handleClearQueue()}
?disabled=${tracks.length === 0}
aria-label="Clear queue"
title="Clear queue"
>
<wa-icon
@@ -2077,6 +2230,7 @@ export class QueuePanel
class="header-action-button add-to-playlist-button"
@click=${this.handleAddToPlaylist}
?disabled=${tracks.length === 0}
aria-label="Add queue to playlist"
title="Add queue to playlist"
>
<wa-icon
@@ -2157,6 +2311,7 @@ export class QueuePanel
></lit-virtualizer>
`}
</div>
${this.renderSelectionBar()}
</div>
<menu-surface
@@ -2245,6 +2400,14 @@ export class QueuePanel
Track
Details
</wa-dropdown-item>
${goToMenuItems(this.goToTarget, {
onSelect: () => {
this.selection.clear();
this.ctxMenu.close();
},
onHover: () =>
this.ctxMenu.closePlaylistSubmenu(),
})}
</div>
`
: nothing}
@@ -39,6 +39,11 @@ import { ICON_MORE_ACTIONS } from '@utils/icon-language';
* region: the number changes under the user's finger as they tap rows,
* and nothing else on screen announces it.
*
* **Escape leaves the mode, from here rather than from each host.**
* This 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 exception the overlaid queue's Escape is.
*
* **It renders nothing at zero.** The mode ends when the last row is
* deselected `SelectionController.toggleInMode` is where that is
* decided so a bar with a count of none is a state this should never
@@ -120,6 +125,43 @@ export class SelectionBar extends LitElement {
);
}
/**
* Escape leaves the mode.
*
* A mode changes what a tap means, so it has to have an exit that
* is not "find the ×" -- and this is the documented exception to
* the app's one-keyboard-authority rule, on exactly the grounds
* the overlaid queue's Escape is: **it is a dismissal, not a
* shortcut**, so it is not a panel-scoped binding and it is
* attached only while there is something to dismiss. Putting it
* here rather than in each host is what gives all four surfaces
* the same answer, since this element exists only while the mode
* does.
*
* The platform's own back gesture is the other half of that and is
* deliberately *not* here: the shell owns the history stack
* (#6/#55), and a component reaching for `history` itself is how
* two stacks come to disagree about what one press means -- the
* fault that deleted `navStack`. See #200.
*/
private onKeydown = (e: KeyboardEvent) => {
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;
+73 -7
View File
@@ -42,6 +42,26 @@ export class AppSidebar extends LitElement {
scrollbar-width: thin;
}
/* A host that has made room owns the box, not just the labels
(#71). The bottom-nav sheet is the width of the screen and
provides the one scroll container it needs; left to itself
the sidebar is a 200px column with a second scroller inside
it, which is what a nested scroll region feels like under a
thumb -- part of the surface moves and part of it does not. */
:host([expanded]) {
max-width: none;
height: auto;
overflow: visible;
}
/* And the width is not draggable there. It is a mouse
affordance (mousedown, col-resize) sitting on the right edge
of a touch surface, where the compatibility mouse events a
tap synthesises can start a resize nobody asked for. */
:host([expanded]) .resize-handle {
display: none;
}
.resize-handle {
position: absolute;
top: 0;
@@ -95,8 +115,15 @@ export class AppSidebar extends LitElement {
text-align: center;
}
li button:hover {
background-color: var(--yj-bg-elevated, #343a40);
/* A hover tint is for a device that hovers (#54), and this
component is on a phone too: below 600px it is what
bottom-nav's "More" sheet mounts, where a hold
synthesises a hover and leaves a destination looking picked
after the finger has gone. */
@media (hover: hover) and (pointer: fine) {
li button:hover {
background-color: var(--yj-bg-elevated, #343a40);
}
}
li button:focus-visible {
@@ -108,6 +135,14 @@ export class AppSidebar extends LitElement {
background-color: var(--yj-bg-overlay, #495057);
}
/* The press state (#54), after the .active rule and at the same
specificity, so pressing the destination you are already on
still says something. It is what a tap gets now that
index.css has taken the web view's own highlight box away. */
li button:active {
background-color: var(--yj-press-overlay, rgba(255, 255, 255, 0.12));
}
li button p {
margin: 0;
white-space: nowrap;
@@ -145,6 +180,25 @@ export class AppSidebar extends LitElement {
:host(.collapsed) li button wa-icon {
font-size: var(--yj-icon-md);
}
/* Below 600px the only place this renders is the bottom-nav
sheet -- the shell's own copy is display: none there -- so
the rows are sized for the thumb that opened it: 48px, which
is #186's floor and the height every row in #60's context
sheet already has. A media query inside a shadow root is
answered by the viewport, so the component states this
itself rather than the sheet reaching in. */
@media (max-width: 599px) {
ul {
padding: 4px 8px 8px;
}
li button {
min-height: 48px;
padding: 12px 10px;
gap: 14px;
}
}
`];
/** Delay in ms before a drag-hover triggers navigation. */
@@ -173,11 +227,14 @@ export class AppSidebar extends LitElement {
/**
* Keep the labels regardless of the viewport, for a host that has
* made room for them -- `bottom-nav`'s drawer, which is the whole
* made room for them -- `bottom-nav`'s sheet, which is the whole
* screen wide on the phone where this would otherwise auto-collapse
* to icons. The auto-collapse is a *width* response to a narrow
* shell, and inside a drawer the shell is not what the sidebar is
* shell, and inside a sheet the shell is not what the sidebar is
* sharing space with.
*
* It says the host owns the *box*, not only the labels: the width,
* the scrolling and the resize handle all follow it (#71).
*/
@property({ type: Boolean, reflect: true })
expanded = false;
@@ -350,9 +407,18 @@ export class AppSidebar extends LitElement {
* be a media query in the stylesheet.
*/
private applyViewportWidth() {
const narrow =
!this.expanded &&
(this.narrowViewport?.matches ?? false);
// A host that made room decides how much: `bottom-nav`'s sheet
// is the whole screen wide, and the inline width below -- which
// beats any rule the host could write -- would draw the old
// 200px side drawer inside it.
if (this.expanded) {
this.style.width = '100%';
this.collapsed = false;
return;
}
const narrow = this.narrowViewport?.matches ?? false;
const width = narrow
? MIN_WIDTH
: this.userWidth;
@@ -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,
@@ -60,11 +64,16 @@ import {
trackLink,
exploreLinkStyles,
} from '@utils/explore-link';
import { goToMenuItems } from '@utils/go-to-menu';
import type { GoToTarget } from '@utils/go-to-menu';
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 +252,11 @@ export class SmartPlaylistDetails
static override styles = [
designTokens,
srOnly,
backButton,
contextMenuStyles,
exploreLinkStyles,
swipeRevealStyles,
css`
:host {
display: flex;
@@ -444,6 +455,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 {
@@ -502,8 +516,14 @@ export class SmartPlaylistDetails
user-select: none;
}
.track-item:hover {
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
/* A hover tint is for a device that hovers (#54). A hold
synthesises a hover in the WebView, so ungated this arrives
because a finger touched the row and stays after it has
gone; the press state below is what a tap gets instead. */
@media (hover: hover) and (pointer: fine) {
.track-item:hover {
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
}
}
.track-item.selected {
@@ -519,6 +539,19 @@ export class SmartPlaylistDetails
background-color: var(--yj-selection-bg, rgba(100, 160, 255, 0.15));
}
/* The press state (#54): the feedback a tap has now that the
web view's own highlight box is gone (index.css). Last, and
carrying a class, because a selected or playing row is two
classes deep and a bare :active would lose to it. */
.track-item.selected:active,
.track-item.active:active,
.track-item:active {
background-color: var(
--yj-press-overlay,
rgba(255, 255, 255, 0.12)
);
}
/* Phantom rows span the full grid */
.track-item.phantom {
display: grid;
@@ -916,10 +949,136 @@ export class SmartPlaylistDetails
.map((i) => this.tracks[i]!.FilePath);
}
/**
* The row "Go to Artist" / "Go to Album" navigate from one row
* or none, and only below the phone breakpoint, where the row's
* own names stopped being links (#67).
*/
private get goToTarget(): GoToTarget | undefined {
const indices = this.selection.getSelectedIndices();
if (indices.length !== 1) return undefined;
const track = this.tracks[indices[0]!];
if (!track) return undefined;
return {
artistName: track.Artist,
artistMBID: track.ArtistMBID,
albumName: track.Album,
albumMBID: track.ReleaseGroupMBID,
};
}
// =================================================================
// 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`
<selection-bar
.count=${this.selection.selectionCount}
.actions=${SmartPlaylistDetails.SELECTION_ACTIONS}
@selection-exit=${this.onSelectionExit}
@selection-action=${(e: CustomEvent<{ id: string }>) =>
this.onContextMenuAction(e.detail.id)}
@selection-more=${(e: CustomEvent<{ x: number; y: number }>) =>
this.ctxMenu.openAt(e.detail.x, e.detail.y)}
></selection-bar>
`;
}
private onSelectionExit = () => {
this.selection.exitSelectionMode();
this.virtualizer?.requestUpdate();
};
private onContextMenuAction(action: string) {
const filePaths = this.getSelectedFilePaths();
@@ -1334,6 +1493,9 @@ export class SmartPlaylistDetails
<div class="header-cell col-album">Album</div>
<div class="header-cell col-duration">Duration</div>
</div>
<div class="sr-only" role="status" aria-live="polite">
${this.swipe.announcement}
</div>
<lit-virtualizer
role="listbox"
aria-label="Smart playlist tracks"
@@ -1342,7 +1504,13 @@ export class SmartPlaylistDetails
.renderItem=${this.renderRow}
.keyFunction=${this.rowKey}
.layout=${this.flowLayout}
@yj-tap=${this.onRowTap}
@yj-long-press=${this.onRowLongPress}
@yj-swipe-start=${this.swipe.onSwipeStart}
@yj-swipe-move=${this.swipe.onSwipeMove}
@yj-swipe-end=${this.swipe.onSwipeEnd}
></lit-virtualizer>
${this.renderSelectionBar()}
`;
}
@@ -1364,6 +1532,7 @@ export class SmartPlaylistDetails
active ? 'active' : '',
selected ? 'selected' : '',
isPhantom ? 'phantom' : '',
this.swipe.isSwiping(trackIndex) ? 'swiping' : '',
]
.filter(Boolean)
.join(' ');
@@ -1374,6 +1543,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 +1578,7 @@ export class SmartPlaylistDetails
? nothing
: this.onTrackDragEnd}
>
${this.swipe.renderReveal(trackIndex)}
${isPhantom
? html`<div class="phantom-row">
<wa-icon
@@ -1555,6 +1726,14 @@ export class SmartPlaylistDetails
></wa-icon>
Track Details
</wa-dropdown-item>
${goToMenuItems(this.goToTarget, {
onSelect: () => {
this.selection.clear();
this.ctxMenu.close();
},
onHover: () =>
this.ctxMenu.closePlaylistSubmenu(),
})}
</div>
`
: nothing}
@@ -371,8 +371,11 @@ export class TopResultsRow extends LitElement {
<span class="card-name">${r.name}</span>
${artistPart || metaPart
? html`<span class="card-subtitle"
>${artistPart
? creditLink(creditStore.credits(r.mbid), artistPart, r.artistMbid ?? '')
><!-- keepOnPhone: this card has no
context menu, so the credit is
the only route to the artist of
a top result (#67). -->${artistPart
? creditLink(creditStore.credits(r.mbid), artistPart, r.artistMbid ?? '', { keepOnPhone: true })
: nothing}${artistPart && metaPart
? ' · '
: ''}${metaPart}</span
@@ -11,6 +11,7 @@ import {
import { SelectionController } from '@utils/selection-controller';
import type { SelectionHost } from '@utils/selection-controller';
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 { ViewLifecycleMixin } from '@utils/view-lifecycle';
@@ -50,6 +51,8 @@ import {
trackLink,
exploreLinkStyles,
} from '@utils/explore-link';
import { goToMenuItems } from '@utils/go-to-menu';
import type { GoToTarget } from '@utils/go-to-menu';
import {
setDragPayload,
emitDragActive,
@@ -282,6 +285,29 @@ export class TrackList
* path into it at all (H-5). */
@state() private focusedIndex = 0;
/**
* Swipe a row right to queue it (plan 019 phase 2, #63).
*
* The affordance and the arithmetic are `utils/swipe-to-queue.ts`,
* shared with both playlist detail views; what stays here is what
* only this list knows -- which row an event is on, and what a
* swipe on it means when other rows are selected.
*/
private swipe = new SwipeToQueue(this, {
resolve: (e) => {
const hit = this.resolveTrackFromEvent(e);
if (!hit) return null;
return {
index: hit.index,
filePaths: this.swipeTargetKeys(hit.track.FilePath),
label: hit.track.TrackName,
};
},
repaint: () => this.virtualizer?.requestUpdate(),
});
private handleSelectAll = (): void => {
this.selection.selectAll();
};
@@ -1016,7 +1042,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;
@@ -1175,8 +1201,16 @@ export class TrackList
padding-left: 6px;
}
.track-row:hover {
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
/* A hover tint is for a device that hovers (#54): a hold
synthesises a hover in the WebView, so ungated this is a
highlight that arrives because a finger touched the row and
then stays there after it has gone -- which reads as a
selection the user did not make. Same gate, and the same
mechanism, as #68's revealed controls. */
@media (hover: hover) and (pointer: fine) {
.track-row:hover {
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
}
}
.track-row.selected {
@@ -1213,6 +1247,24 @@ export class TrackList
background-color: var(--yj-selection-bg, rgba(100, 160, 255, 0.15));
}
/* The press state (#54), and the only feedback a tap has now that
the web view's tap highlight is gone (index.css).
**Last, and as specific as the state rules above**: a row that
is selected and playing is .track-row.selected.active, so a
bare .track-row:active is one class short of it and a press
on the row a phone is most likely to press -- the one it just
selected -- would show nothing. Instant rather than
transitioned, because the only measured statement here about
transitions on a list is that two card grids removed theirs
for software-rendering repaint cost. */
.track-row.selected:active,
.track-row.active:active,
.track-row:active {
background-color: var(--yj-press-overlay, rgba(255, 255, 255, 0.12));
}
.cell {
overflow: hidden;
text-overflow: ellipsis;
@@ -1311,6 +1363,9 @@ export class TrackList
virt.removeEventListener('contextmenu', this.onDelegatedContextMenu);
virt.removeEventListener('yj-tap', this.onRowTap);
virt.removeEventListener('yj-long-press', this.onRowLongPress);
virt.removeEventListener('yj-swipe-start', this.swipe.onSwipeStart);
virt.removeEventListener('yj-swipe-move', this.swipe.onSwipeMove);
virt.removeEventListener('yj-swipe-end', this.swipe.onSwipeEnd);
virt.removeEventListener('dragstart', this.onDelegatedDragStart);
virt.removeEventListener('dragend', this.onTrackDragEnd);
}
@@ -1457,6 +1512,9 @@ export class TrackList
// through the same path a real click takes (plan 019).
virt.addEventListener('yj-tap', this.onRowTap);
virt.addEventListener('yj-long-press', this.onRowLongPress);
virt.addEventListener('yj-swipe-start', this.swipe.onSwipeStart);
virt.addEventListener('yj-swipe-move', this.swipe.onSwipeMove);
virt.addEventListener('yj-swipe-end', this.swipe.onSwipeEnd);
this.delegationAttached = true;
}
@@ -1776,6 +1834,30 @@ export class TrackList
this.virtualizer?.requestUpdate();
};
/**
* What a swipe on this row would queue.
*
* The same rule the context menu answers with, and it has to be:
* **one row is a position, several rows are an explicit choice.**
* A finger that swipes a row which is part of a selection of forty
* has not un-made that selection, and queueing the one row it
* touched would quietly contradict the bar above saying forty are
* selected. A swipe on a row *outside* the selection is a statement
* about that row, exactly as a right-click on one is -- and unlike
* a right-click it does not move the selection, because a swipe is
* not a way of selecting anything.
*/
private swipeTargetKeys(filePath: string): string[] {
if (
this.selection.selectionCount > 1 &&
this.selection.isSelected(filePath)
) {
return this.selection.getSelectedKeysOrdered();
}
return [filePath];
}
private onDelegatedDragStart = (e: DragEvent) => {
const hit = this.resolveTrackFromEvent(e);
@@ -1899,6 +1981,33 @@ export class TrackList
emitDragActive(false);
};
/**
* The row the menu can navigate from, for "Go to Artist" / "Go to
* Album" which exist only below the phone breakpoint, where the
* row's own names are no longer links (#67).
*
* One row only, on the rule the Play item already states: one row
* is a position, several are an explicit choice of *those* tracks,
* and "go to the album" of five different albums means nothing.
*/
private get goToTarget(): GoToTarget | undefined {
if (this.selection.selectionCount !== 1) return undefined;
const [path] = this.selection.selectedItems;
const track = path
? tracksByFilePath(this.tracks).get(path)
: undefined;
if (!track) return undefined;
return {
artistName: track.ArtistName,
artistMBID: track.ArtistMBID,
albumName: track.Album,
albumMBID: track.ReleaseGroupMBID,
};
}
private onContextMenuAction(action: string) {
const filePaths =
this.selection.getSelectedKeysOrdered();
@@ -2197,6 +2306,7 @@ export class TrackList
'track-row': true,
active,
selected,
swiping: this.swipe.isSwiping(index),
})}
role="row"
aria-rowindex=${index + 1}
@@ -2206,8 +2316,10 @@ export class TrackList
draggable="true"
data-index=${index}
data-testid="track-row"
data-swipe
data-file-path=${track.FilePath}
>
${this.swipe.renderReveal(index)}
<div
role="gridcell"
class=${classMap({
@@ -2354,6 +2466,9 @@ export class TrackList
<div class="sr-only" role="status" aria-live="polite">
${this.liveStatus(visibleTracks.length)}
</div>
<div class="sr-only" role="status" aria-live="polite">
${this.swipe.announcement}
</div>
${this.tracks.length === 0
? this.renderPlaceholder()
: html`
@@ -2502,6 +2617,13 @@ export class TrackList
></wa-icon>
Track Details
</wa-dropdown-item>
${goToMenuItems(this.goToTarget, {
onSelect: () => {
this.selection.clear();
this.ctxMenu.close();
},
onHover: () => this.ctxMenu.closePlaylistSubmenu(),
})}
<wa-dropdown-item
@click=${() =>
this.onContextMenuAction(
+15
View File
@@ -46,6 +46,17 @@ export interface ShadePalette {
border: string;
borderSubtle: string;
hoverOverlay: string;
/**
* The tint a surface takes while it is being pressed (#54).
*
* Separate from `hoverOverlay` because the two answer different
* questions and only one of them a phone can ask: a hover is a
* pointer resting somewhere, a press is a finger on the thing it
* is about to activate. It is deliberately the stronger of the
* two a press that reads the same as a hover says nothing on a
* device where the hover is synthesised by the press itself.
*/
pressOverlay: string;
selectionBg: string;
}
@@ -95,6 +106,7 @@ export const SHADE_PALETTES: Record<BackgroundShade, ShadePalette> = {
border: '#333333',
borderSubtle: '#222222',
hoverOverlay: 'rgba(255, 255, 255, 0.05)',
pressOverlay: 'rgba(255, 255, 255, 0.12)',
selectionBg: 'rgba(100, 160, 255, 0.15)',
},
dark: {
@@ -114,6 +126,7 @@ export const SHADE_PALETTES: Record<BackgroundShade, ShadePalette> = {
border: '#444444',
borderSubtle: '#333333',
hoverOverlay: 'rgba(255, 255, 255, 0.05)',
pressOverlay: 'rgba(255, 255, 255, 0.12)',
selectionBg: 'rgba(100, 160, 255, 0.15)',
},
light: {
@@ -133,6 +146,7 @@ export const SHADE_PALETTES: Record<BackgroundShade, ShadePalette> = {
border: '#ced4da',
borderSubtle: '#dee2e6',
hoverOverlay: 'rgba(0, 0, 0, 0.05)',
pressOverlay: 'rgba(0, 0, 0, 0.12)',
selectionBg: 'rgba(100, 160, 255, 0.15)',
},
};
@@ -285,6 +299,7 @@ function deriveThemeVariables(
// Interactive overlays
'--yj-hover-overlay': palette.hoverOverlay,
'--yj-press-overlay': palette.pressOverlay,
'--yj-selection-bg': palette.selectionBg,
// Semantic *fills* — the background of a solid button or badge.
+28 -3
View File
@@ -710,10 +710,35 @@ export const contextMenuStyles = css`
font-size: 13px;
}
.context-menu-panel wa-dropdown-item:hover {
/* A hover tint is for a device that hovers (#54).
Below the query is a phone, where a hold *synthesises* a hover
in the WebView -- the same mechanism #68 gates the revealed
controls on -- so an ungated tint is a highlight that arrives
because a finger touched the row and then stays on it after the
finger has gone. Which is indistinguishable from the press
state below, and outlives it. */
@media (hover: hover) and (pointer: fine) {
.context-menu-panel wa-dropdown-item:hover {
background-color: var(
--yj-hover-overlay,
rgba(255, 255, 255, 0.1)
);
}
}
/* And a press state is for every device, because it is the one
piece of feedback a tap has now that the web view's own
highlight box is gone (index.css). Stronger than the hover tint
on purpose, and instant rather than transitioned: the only
measured statement this repo has about transitions on these
surfaces is the two card grids that removed theirs because
software rendering repaints per frame, and the phone is not
something this session can measure. */
.context-menu-panel wa-dropdown-item:active {
background-color: var(
--yj-hover-overlay,
rgba(255, 255, 255, 0.1)
--yj-press-overlay,
rgba(255, 255, 255, 0.12)
);
}
+135 -29
View File
@@ -13,11 +13,35 @@
* bug, not as a statement about metadata. The only case that still
* renders as text is one we genuinely cannot route (no name at all, or
* nothing in the library by that name).
*
* ## Below the phone breakpoint a name is not a link (#67)
*
* A few characters of text inside a row is not a touch target, and the
* click handling below is explicitly a *desktop* compromise: the
* navigation is held for one double-click interval so double-clicking
* the row can still play it, which means nothing at all on touch. On
* a phone the row's own gesture wins anyway a claimed `yj-tap` has
* its click swallowed by `utils/touch-gestures.ts`, so the link was
* unreachable as well as fiddly.
*
* So the rule lives here rather than at twenty call sites, which is
* what the Findings on #67 ask for: a name renders as plain text below
* `PHONE_QUERY`, and the row's context menu carries "Go to Artist" /
* "Go to Album" in its place (`goToMenuItems`).
*
* The exception is `keepOnPhone`, and it is not a preference. Three
* surfaces render a name with **no menu to carry the destination**
* `now-playing-view`, `explore-album-details`' header credit and
* `top-results-row` so suppressing the link there takes the action
* away entirely rather than moving it, which is what plan 018's "no
* action is unreachable at any supported size" refuses. Each of those
* call sites says so.
*/
import { html, css } from 'lit';
import type { TemplateResult } from 'lit';
import { libraryStore } from '../store/library-store';
import { PHONE_QUERY } from './breakpoints';
/** Shared CSS for explore link styling. Import into component styles. */
export const exploreLinkStyles = css`
@@ -32,6 +56,57 @@ export const exploreLinkStyles = css`
}
`;
/**
* Options every link function takes, for the one case that is not the
* default.
*/
export interface LinkOptions {
/**
* Keep the name navigable at phone width.
*
* For a surface with no context menu to carry the destination
* see the header of this file. A row must not pass it: the row's
* tap already means "play", and the menu is where the destination
* went.
*/
keepOnPhone?: boolean;
}
/**
* The live phone breakpoint, made once and read per link.
*
* A `MediaQueryList` is live, so one object answers for the life of
* the page and a resize needs nothing from here. The identity check
* is the test seam: this tier's viewport is fixed by the runner, so a
* spec answers the query by replacing `window.matchMedia` (the same
* stub `now-playing-phone.test.ts` installs), and swapping the
* function is what tells us to ask again.
*/
let phoneQuery: MediaQueryList | undefined;
let phoneQuerySource: typeof window.matchMedia | undefined;
/**
* Whether an inline name still navigates.
*
* Exported because the menus that carry the destination in its place
* are drawn under exactly the same condition -- one answer, not two.
*/
export function inlineLinksSuppressed(): boolean {
if (!window.matchMedia) return false;
if (phoneQuerySource !== window.matchMedia) {
phoneQuerySource = window.matchMedia;
phoneQuery = window.matchMedia(PHONE_QUERY);
}
return phoneQuery?.matches ?? false;
}
/** Whether this call site should render plain text rather than a link. */
function plainText(options?: LinkOptions): boolean {
return !options?.keepOnPhone && inlineLinksSuppressed();
}
/** Fire a navigate event from the clicked element. */
function navigate(target: EventTarget, detail: Record<string, unknown>): void {
target.dispatchEvent(
@@ -153,39 +228,19 @@ function singleClick(
* @param mbid - The MusicBrainz artist ID. Empty string = local only.
* @param content - Optional custom content to render inside the link
* (e.g. highlighted search result). Defaults to artistName.
* @param options - See `LinkOptions`.
*/
export function artistLink(
artistName: string,
mbid: string,
content?: TemplateResult | string,
options?: LinkOptions,
): TemplateResult | string {
if (!artistName) return artistName;
if (plainText(options)) return content ?? artistName;
const onClick = singleClick((target) => {
void (async () => {
if (mbid) {
navigate(target, {
view: 'explore-artist-details',
artistMBID: mbid,
artistName,
});
return;
}
const local = await findLocalArtist(artistName);
if (!local) return;
// The caller's row had no MBID, but the library row for the
// same artist may — the grid routes by exactly this field,
// so reading it here is what keeps the two paths agreeing.
navigate(target, {
view: 'explore-artist-details',
artistMBID: local.MBID || '',
artistName,
localArtistId: local.ID,
});
})();
void openArtistPage(target, artistName, mbid);
});
return html`<a
@@ -203,19 +258,22 @@ export function artistLink(
* @param mbid - The MusicBrainz release group ID. Empty = local only.
* @param content - Optional custom content to render inside the link.
* @param artistName - Disambiguates same-named albums in the library.
* @param options - See `LinkOptions`.
*/
export function albumLink(
albumName: string,
mbid: string,
content?: TemplateResult | string,
artistName?: string,
options?: LinkOptions,
): TemplateResult | string {
if (!albumName) return albumName;
if (plainText(options)) return content ?? albumName;
return html`<a
class="explore-link"
@click=${singleClick((target) => {
void openAlbum(target, albumName, mbid, artistName);
void openAlbumPage(target, albumName, mbid, artistName);
})}
title=${mbid ? 'View album on Explore' : 'View album in your library'}
>${content ?? albumName}</a>`;
@@ -232,6 +290,7 @@ export function albumLink(
* @param recordingMBID - The track's MusicBrainz recording ID.
* @param content - Optional custom content (e.g. highlighted text).
* @param artistName - Disambiguates same-named albums in the library.
* @param options - See `LinkOptions`.
*/
export function trackLink(
trackName: string,
@@ -240,14 +299,16 @@ export function trackLink(
recordingMBID: string,
content?: TemplateResult | string,
artistName?: string,
options?: LinkOptions,
): TemplateResult | string {
if (!trackName) return trackName;
if (!albumName) return content ?? trackName;
if (plainText(options)) return content ?? trackName;
return html`<a
class="explore-link"
@click=${singleClick((target) => {
void openAlbum(
void openAlbumPage(
target,
albumName,
releaseGroupMBID,
@@ -262,11 +323,48 @@ export function trackLink(
>${content ?? trackName}</a>`;
}
/**
* Route to an artist page, preferring the catalog and falling back to
* the library copy.
*
* Exported because a menu item goes to the same place a name does, and
* two routings of "go to this artist" is how the two come to disagree
* about an untagged one.
*/
export async function openArtistPage(
target: EventTarget,
artistName: string,
mbid: string,
): Promise<void> {
if (mbid) {
navigate(target, {
view: 'explore-artist-details',
artistMBID: mbid,
artistName,
});
return;
}
const local = await findLocalArtist(artistName);
if (!local) return;
// The caller's row had no MBID, but the library row for the
// same artist may — the grid routes by exactly this field,
// so reading it here is what keeps the two paths agreeing.
navigate(target, {
view: 'explore-artist-details',
artistMBID: local.MBID || '',
artistName,
localArtistId: local.ID,
});
}
/**
* Route to an album page, preferring the catalog and falling back to
* the library copy. `highlight*` marks one track on arrival.
*/
async function openAlbum(
export async function openAlbumPage(
target: EventTarget,
albumName: string,
releaseGroupMBID: string,
@@ -337,23 +435,31 @@ export interface CreditPart {
* @param parts - The credit's parts in position order, if known.
* @param fallbackName - The credit as a single string.
* @param fallbackMbid - The primary artist's MBID.
* @param options - See `LinkOptions`.
*/
export function creditLink(
parts: readonly CreditPart[] | undefined,
fallbackName: string,
fallbackMbid: string,
options?: LinkOptions,
): TemplateResult | string {
// One part is one link, so it is the fallback rather than a special
// case — and a zero-part credit reaching here would otherwise
// render as nothing at all, which is worse than the single-artist
// answer it replaced.
if (!parts || parts.length < 2) {
return artistLink(fallbackName, fallbackMbid);
return artistLink(fallbackName, fallbackMbid, undefined, options);
}
// A decomposed credit is rendered from the same parts either way,
// so the join phrases survive the suppression and the text reads
// as it did — which is `creditText`'s job, and it is the string
// the `title=` beside these already uses.
if (plainText(options)) return creditText(parts, fallbackName);
return html`${parts.map(
(part) =>
html`${artistLink(part.creditedName, part.artistMbid)}${part.joinPhrase}`,
html`${artistLink(part.creditedName, part.artistMbid, undefined, options)}${part.joinPhrase}`,
)}`;
}
+109
View File
@@ -0,0 +1,109 @@
/**
* "Go to Artist" / "Go to Album", for the menus that carry a name the
* phone stopped drawing as a link (#67).
*
* `utils/explore-link.ts` renders a plain string below the phone
* breakpoint, because a few characters inside a row is not a touch
* target and the row's own tap already means "play". That takes a
* destination away, so the row's context menu gives it back which is
* the whole of this issue: the navigation moves, it does not go.
*
* Three things about it are load-bearing.
*
* **It is drawn under exactly the condition the link is not.**
* `inlineLinksSuppressed()` answers both, so a desktop menu is
* untouched (the name beside it is still a link, and a menu that
* repeats what the row already offers is furniture) and a phone menu
* cannot be missing what the row lost.
*
* **It goes where the name went.** `openArtistPage` / `openAlbumPage`
* are `explore-link`'s own routing, exported rather than reimplemented,
* so an untagged artist reaches the library page here for the same
* reason and by the same lookup it does from a link.
*
* **The host says when it is over**, through `onSelect` every menu in
* this app closes itself and most clear their selection, and both are
* the host's bookkeeping rather than something a shared item may do on
* its behalf. `onHover` is for the four hosts with a playlist submenu,
* which closes on any other item being pointed at.
*/
import { html, nothing } from 'lit';
import type { TemplateResult } from 'lit';
import { inlineLinksSuppressed, openArtistPage, openAlbumPage } from './explore-link';
/**
* The entities one row or card can send you to.
*
* Everything is optional because the hosts differ: a track row knows
* both, an album card knows only its artist, and an artist page's own
* tracklist knows only the album.
*/
export interface GoToTarget {
artistName?: string;
artistMBID?: string;
albumName?: string;
albumMBID?: string;
}
export interface GoToHandlers {
/** Called before navigating: close the menu, clear the selection. */
onSelect?: () => void;
/** Called on hover: close a playlist submenu, where the host has one. */
onHover?: () => void;
}
/**
* The menu items for a target, or nothing at all where the name beside
* them is still a link.
*/
export function goToMenuItems(
target: GoToTarget | undefined,
handlers: GoToHandlers = {},
): TemplateResult | typeof nothing {
if (!target || !inlineLinksSuppressed()) return nothing;
const artist = target.artistName?.trim();
const album = target.albumName?.trim();
if (!artist && !album) return nothing;
return html`
${artist
? html`<wa-dropdown-item
data-testid="go-to-artist"
@click=${(e: Event) => {
handlers.onSelect?.();
void openArtistPage(
e.currentTarget as EventTarget,
artist,
target.artistMBID ?? '',
);
}}
@mouseenter=${() => handlers.onHover?.()}
>
<wa-icon slot="icon" name="user-group"></wa-icon>
Go to Artist
</wa-dropdown-item>`
: nothing}
${album
? html`<wa-dropdown-item
data-testid="go-to-album"
@click=${(e: Event) => {
handlers.onSelect?.();
void openAlbumPage(
e.currentTarget as EventTarget,
album,
target.albumMBID ?? '',
artist,
);
}}
@mouseenter=${() => handlers.onHover?.()}
>
<wa-icon slot="icon" name="compact-disc"></wa-icon>
Go to Album
</wa-dropdown-item>`
: nothing}
`;
}
+156
View File
@@ -0,0 +1,156 @@
/**
* Warm the browser's image cache for the cards a scroll is about to
* reach.
*
* #65: album art pops in while scrolling. The rule this app already
* follows is that a row image is `loading="lazy" decoding="async"` and
* draws the smallest adequate tier, and both halves are in place
* `cover-grid.getCoverUrl()` and `artists-view`'s avatar both pick
* `_sm`/`_md`/`_lg` from the card size and the device pixel ratio. What
* is left is *when* the fetch starts: the grids are virtualized, so the
* `<img>` does not exist at all until the virtualizer decides to render
* its card, and only then can the browser ask for anything.
*
* The issue's Direction asks for a larger overscan, and that is not
* available: `@lit-labs/virtualizer`'s `_overhang` is a hard-coded
* 1000px `protected` field on `BaseLayout` with no configuration
* surface, so raising it means monkey-patching a private. 1000px is
* about two screens on the reference device's 439px viewport, which is
* a fraction of a second at speed.
*
* So the request is issued ahead of the element instead. Cover art and
* artist images are plain URLs served by `coverart.Handler` /
* `explore`'s image handler under `Cache-Control: public,
* max-age=31536000, immutable` — the filenames are content hashes — so
* a prefetched image is a cache hit by the time the card is drawn, and
* a second pass over the same rows costs nothing at all.
*
* Three things about it are load-bearing.
*
* **This is not the `LRUMap` path the issue's Findings warn about.**
* That ceiling (`ARTIST_IMAGE_CACHE_LIMIT` and friends) bounds
* Explore's base64 data URLs, which are held in JS. A library cover is
* a URL, and what retains the bytes is the browser's own HTTP cache,
* which evicts on its own terms. What this module retains is the *set
* of URLs already asked for*, which is why that set has a cap and
* reports itself to `window.__yjCacheStats()` the measurement the
* issue asks for.
*
* **A window is warmed on both sides of the rendered range.** The
* event carries no direction, and scrolling back up needs the same
* treatment; the rows behind are already in `requested` from the pass
* that rendered them, so the backward half issues nothing in the
* common case and is free.
*
* **An in-flight image is held.** `new Image().src = url` and drop it
* is the usual idiom and usually survives, but "usually" is an engine
* detail and the engine that matters here is a two-year-old WebView.
* The element is kept until it loads or fails, and no longer nothing
* here holds a decoded bitmap on purpose.
*/
import { registerCacheProbe } from './cache-stats.js';
import { LRUMap } from './lru-map.js';
/**
* How many entries past each edge of the rendered range to warm.
*
* Entries rather than pixels, because that is what the event reports
* and what the caller has an array of. Twelve rows on the phone's
* two-column grid and four on a desktop's six, on top of the
* virtualizer's own 1000px enough to cover a flick, and bounded so a
* fast scroll through 5 000 albums cannot ask for 5 000 covers.
*/
export const PREFETCH_AHEAD = 24;
/** Ceiling on the record of what has already been asked for. */
export const PREFETCH_MEMORY = 512;
/** URLs already requested; the value is a placeholder, the key is the record. */
const requested = new LRUMap<string, true>(PREFETCH_MEMORY);
/** Images still loading, held so the request cannot be collected. */
const inFlight = new Set<HTMLImageElement>();
registerCacheProbe('imagePrefetch', () => {
let chars = 0;
for (const url of requested.keys()) chars += url.length;
return { entries: requested.size, chars, limit: PREFETCH_MEMORY };
});
/** Whether this URL has already been asked for. */
export function imagePrefetched(url: string): boolean {
return requested.has(url);
}
/**
* Ask the browser for `url` unless it has already been asked for.
* Returns whether a request was issued.
*/
export function prefetchImage(url: string): boolean {
if (!url || requested.has(url)) return false;
requested.set(url, true);
const img = new Image();
inFlight.add(img);
const done = () => {
inFlight.delete(img);
};
img.addEventListener('load', done, { once: true });
img.addEventListener('error', done, { once: true });
img.decoding = 'async';
img.src = url;
return true;
}
/**
* Warm the images either side of a virtualizer's rendered range.
*
* `first`/`last` are the indices the `visibilityChanged` event
* reported; `urlOf` returns the image the card at that index will
* draw, or `''` where it draws a placeholder. Returns how many
* requests were issued, which is what a test can assert on and what
* makes "a second run does approximately nothing" checkable.
*/
export function prefetchImageWindow<T>(
items: readonly T[],
first: number,
last: number,
urlOf: (item: T) => string,
ahead: number = PREFETCH_AHEAD,
): number {
if (items.length === 0 || first < 0 || last < first) return 0;
const from = Math.max(0, first - ahead);
const to = Math.min(items.length - 1, last + ahead);
let issued = 0;
// Forward first: it is the direction a scroll is usually going, so
// it is the half that has to win the race.
for (let i = last + 1; i <= to; i++) {
const item = items[i];
if (item !== undefined && prefetchImage(urlOf(item))) issued++;
}
for (let i = from; i < first; i++) {
const item = items[i];
if (item !== undefined && prefetchImage(urlOf(item))) issued++;
}
return issued;
}
/** Forget what has been asked for. For tests; the app never needs it. */
export function resetImagePrefetch(): void {
requested.clear();
inFlight.clear();
}
+362
View File
@@ -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 `<lit-virtualizer>` 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`
<div
class=${classMap({ 'swipe-reveal': true, armed: this.armed })}
aria-hidden="true"
data-testid="swipe-reveal"
>
<wa-icon name=${ICON_QUEUE}></wa-icon>
<span>${words}</span>
</div>
`;
}
/**
* 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;
}
}
`;
+297 -5
View File
@@ -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<GestureDetail>;
export type SwipeEvent = CustomEvent<SwipeDetail>;
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<SwipeDetail>(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;
};
Binary file not shown.

Before

Width:  |  Height:  |  Size: 15 KiB

After

Width:  |  Height:  |  Size: 14 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 4.4 KiB

After

Width:  |  Height:  |  Size: 6.7 KiB

@@ -0,0 +1,190 @@
/**
* The grids ask for the art below the fold before the card exists
* (#65).
*
* Reported as "scrolling through albums, the art pops in". The cards
* already draw the smallest adequate tier and are already
* `loading="lazy"`, so what was left is *when*: `<lit-virtualizer>`
* renders about 1000px past the viewport and the `<img>` and
* therefore the request does not exist until it does. On the
* reference device that is about two screens.
*
* These assert the mechanism, since no tier here can photograph a
* pop-in: that the rows past the rendered range are requested, that
* the request is for the same tier the card will draw, and that the
* window has an end an unbounded prefetch of a 5 000-album library
* is the failure this trades against.
*
* What is *not* asserted here is that a rendered card was never
* prefetched. It often was, honestly: the grid lays out more than once
* on mount, so a row warmed by the first pass is drawn by the second,
* which is the whole point. The rule that a single pass skips its own
* rendered range is `image-prefetch.test.ts`'s, where one call can be
* looked at on its own.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import type { LitElement } from 'lit';
import '@components/cover-grid/cover-grid';
import '@components/artists-view/artists-view';
import { emit, stub, flush, resetHarness } from '@test/support/harness';
import { Events } from '../../src/events';
import { fixture, shadowAll } from '@test/support/render';
import {
PREFETCH_AHEAD,
imagePrefetched,
resetImagePrefetch,
} from '@utils/image-prefetch';
/** Enough albums that the virtualizer's own window is nowhere near the end. */
const ALBUMS = Array.from({ length: 400 }, (_, i) => {
const n = String(i + 1).padStart(4, '0');
return {
ID: i + 1,
Name: `Album ${n}`,
ArtistName: 'Aurora Fields',
Year: 2020,
CoverArtPath: `/covers/${n}.jpg`,
CoverArtSmall: `/covers/${n}_sm.jpg`,
CoverArtMedium: `/covers/${n}_md.jpg`,
CoverArtLarge: `/covers/${n}_lg.jpg`,
};
});
const ARTISTS = Array.from({ length: 400 }, (_, i) => {
const n = String(i + 1).padStart(4, '0');
return {
ID: i + 1,
Name: `Artist ${n}`,
AlbumCount: 2,
TrackCount: 9,
ImageSmall: `/artists/${n}_sm.jpg`,
ImageMedium: `/artists/${n}_md.jpg`,
ImageLarge: `/artists/${n}_lg.jpg`,
};
});
/** Give the virtualizer a viewport; a zero-height host renders nothing. */
function sized(el: HTMLElement): void {
el.style.display = 'block';
el.style.height = '600px';
el.style.width = '900px';
}
async function settle(el: LitElement): Promise<void> {
await flush();
await el.updateComplete;
await new Promise((r) => setTimeout(r, 200));
}
/** The `src` of every card the grid actually rendered. */
function renderedSources(el: LitElement, selector: string): string[] {
return shadowAll(el, selector)
.map((img) => (img as HTMLImageElement).getAttribute('src') ?? '')
.filter(Boolean);
}
/**
* The last index the virtualizer has rendered, read off the cards
* rather than counted: the rendered range is what the prefetch window
* is measured from, and a count assumes it starts at 0 and has no
* gaps.
*/
function lastRenderedIndex(el: LitElement, selector: string): number {
const indices = shadowAll(el, selector).map((card) =>
Number(card.getAttribute('data-index')),
);
return Math.max(...indices);
}
/**
* The tier the cards chose, read off a rendered card rather than
* recomputed the point of the assertion is that the prefetch and the
* card agree, so deriving both from the same ladder here would prove
* nothing.
*/
function tierSuffix(src: string): string {
const m = /_(sm|md|lg)\.jpg$/.exec(src);
return m ? `_${m[1]}` : '';
}
beforeEach(() => {
resetHarness();
resetImagePrefetch();
localStorage.clear();
stub('library.Library.GetAlbums', ALBUMS);
stub('library.Library.GetArtists', ARTISTS);
stub('library.Library.GetTracks', []);
stub('library.Library.GetGenres', []);
emit(Events.LibraryScanComplete);
});
describe('the albums grid warms the covers below the fold', () => {
it('asks for the covers past the rendered range, in the tier the card draws', async () => {
const el = await fixture<LitElement>('cover-grid');
sized(el);
await settle(el);
const rendered = renderedSources(el, 'img.cover-image');
expect(rendered.length).toBeGreaterThan(0);
const tier = tierSuffix(rendered[0]!);
const url = (index: number) =>
`/covers/${String(index + 1).padStart(4, '0')}${tier}.jpg`;
// The grid starts at the top and never scrolls here, so the whole
// window lies past the last card drawn.
const last = lastRenderedIndex(el, '.album-card');
expect(imagePrefetched(url(last + 1))).toBe(true);
expect(imagePrefetched(url(last + PREFETCH_AHEAD))).toBe(true);
});
it('stops at the end of the window rather than warming the library', async () => {
const el = await fixture<LitElement>('cover-grid');
sized(el);
await settle(el);
const rendered = renderedSources(el, 'img.cover-image');
const tier = tierSuffix(rendered[0]!);
const url = (index: number) =>
`/covers/${String(index + 1).padStart(4, '0')}${tier}.jpg`;
// Not "exactly `last + PREFETCH_AHEAD`": the grid lays out more
// than once on mount and each pass warms a window from wherever
// the rendered range was then, so the reachable set is a few
// windows wide. The property that matters is that it is a window
// at all rather than the library.
expect(imagePrefetched(url(399))).toBe(false);
expect(window.__yjCacheStats?.()['imagePrefetch']?.entries ?? 0)
.toBeLessThan(ALBUMS.length / 2);
});
});
describe('the artists grid warms its avatars the same way', () => {
it('asks for the avatars past the rendered range', async () => {
const el = await fixture<LitElement>('artists-view');
sized(el);
await settle(el);
const rendered = renderedSources(el, 'img.avatar-image');
expect(rendered.length).toBeGreaterThan(0);
const tier = tierSuffix(rendered[0]!);
const last = lastRenderedIndex(el, '.artist-card');
const url = (index: number) =>
`/artists/${String(index + 1).padStart(4, '0')}${tier}.jpg`;
expect(imagePrefetched(url(last + 1))).toBe(true);
expect(imagePrefetched(url(399))).toBe(false);
});
});
+115
View File
@@ -193,4 +193,119 @@ describe('bottom-nav', () => {
expect(shadow<HTMLElement>(el, 'app-sidebar')?.hasAttribute('expanded'))
.toBe(true);
});
it('draws "More" as a sheet rising from the bottom', async () => {
const el = await fixture<Nav>('bottom-nav');
const drawer = shadow<HTMLElement>(el, 'wa-drawer');
// #71. A side drawer is a desktop shape: it opened away from the
// thumb that asked for it and drew a 200px column of a 424px
// screen. `placement` is the whole of the change to *where* it
// comes from, and `without-header` is what makes it the same sheet
// `menu-surface` draws rather than a second pattern with a title
// bar and a close button.
expect(drawer?.getAttribute('placement')).toBe('bottom');
expect(drawer?.hasAttribute('without-header')).toBe(true);
// Named all the same: `nameDialog`'s documented aria-label path,
// since without-header renders no heading to point at.
expect(drawer?.getAttribute('label')).toBe('All views');
});
it('leaves exactly one scroll container, and it is the sheet body', async () => {
const el = await fixture<Nav>('bottom-nav');
const drawer = shadow<HTMLElement & { open: boolean }>(el, 'wa-drawer');
if (!drawer) throw new Error('no drawer');
const shown = once(drawer, 'wa-after-show');
shadow<HTMLButtonElement>(el, '[data-testid="tab-more"]')?.click();
await shown;
await update(el, {});
// The report is "only part of the screen scrolls under my finger",
// and the cause is three boxes that each scroll: the dialog, its
// body, and the sidebar's own overflow-y host. Which one a drag
// moves depends on where the finger landed.
const dialog = drawer.shadowRoot?.querySelector('[part~="dialog"]');
const body = drawer.shadowRoot?.querySelector('[part~="body"]');
const sidebar = shadow<HTMLElement>(el, 'app-sidebar');
if (!dialog || !body || !sidebar) throw new Error('no sheet');
expect(getComputedStyle(dialog).overflowY).toBe('hidden');
expect(getComputedStyle(body).overflowY).toBe('auto');
expect(getComputedStyle(sidebar).overflowY).toBe('visible');
// And the one that does scroll keeps it to itself, or reaching the
// end of the destinations scrolls the page behind the sheet.
expect(getComputedStyle(body).overscrollBehaviorY).toBe('contain');
});
it('gives the sheet the whole width, which the sidebar does not take', async () => {
const el = await fixture<Nav>('bottom-nav');
shadow<HTMLButtonElement>(el, '[data-testid="tab-more"]')?.click();
await update(el, {});
const sidebar = shadow<HTMLElement>(el, 'app-sidebar');
if (!sidebar) throw new Error('no sidebar');
// `app-sidebar` writes an *inline* width and caps itself at 400px,
// which beats any rule this host could write — so "the host owns
// the box" has to be part of what `expanded` means, or the sheet
// draws the old 200px column inside a full-width surface.
expect(sidebar.style.width).toBe('100%');
expect(getComputedStyle(sidebar).maxWidth).toBe('none');
});
it('sizes the sheet rows for a thumb, below the phone breakpoint', async () => {
const el = await fixture<Nav>('bottom-nav');
shadow<HTMLButtonElement>(el, '[data-testid="tab-more"]')?.click();
await update(el, {});
const sidebar = shadow<HTMLElement>(el, 'app-sidebar');
const sheets = sidebar?.shadowRoot?.adoptedStyleSheets ?? [];
const phoneRules: string[] = [];
for (const sheet of sheets) {
for (const rule of Array.from(sheet.cssRules)) {
if (!(rule instanceof CSSMediaRule)) continue;
if (!/max-width:\s*599px/.test(rule.conditionText)) continue;
for (const inner of Array.from(rule.cssRules)) {
phoneRules.push(inner.cssText);
}
}
}
// Asserted against the parsed stylesheet, like
// `hover-affordance.test.ts` and for the same reason: this tier's
// iframe is not 599px wide, so the rule cannot be *rendered* here —
// but the regression worth catching is someone moving it out of the
// query, which nothing on a desktop draws differently.
expect(phoneRules.length).toBeGreaterThan(0);
expect(phoneRules.some((r) => /min-height:\s*48px/.test(r))).toBe(true);
});
it('takes the resize handle out of the sheet', async () => {
const el = await fixture<Nav>('bottom-nav');
shadow<HTMLButtonElement>(el, '[data-testid="tab-more"]')?.click();
await update(el, {});
const handle = shadow<HTMLElement>(el, 'app-sidebar')
?.shadowRoot?.querySelector('.resize-handle');
if (!handle) throw new Error('no resize handle');
// A col-resize strip on the right edge of a touch surface: the
// compatibility mouse events a tap synthesises reach its
// `mousedown`, so it can start a resize nobody asked for.
expect(getComputedStyle(handle).display).toBe('none');
});
});
+5
View File
@@ -137,6 +137,11 @@ describe('<app-sidebar>', () => {
});
it('looks the way it did last time', async () => {
// Stated rather than inherited: `activeViewStore` is a singleton, so
// without this the shot records whichever view the *previous* case
// left in it and the reference moves when the file is reordered.
activeViewStore.setView('home', true);
const el = await fixture('app-sidebar');
await visual(el, 'app-sidebar');
@@ -206,6 +206,56 @@ describe('menu-surface', () => {
expect(dismissed, 'no menu-dismiss reached the document').toBe(1);
});
/**
* The scroll affordance (#207), and this is the mechanism again
* rather than the symptom.
*
* The sheet's body has scrolled since #60 and said nothing about
* it: measured at 424x439, eight items ended at y=470 with the
* fold at 439, and where the cut lands on a row boundary the sheet
* ends in a clean edge that reads as the end of the list.
*
* What makes the fade *conditional* absent on a menu that fits,
* present the moment one does not, gone again at the end of the
* list is `background-attachment`, not a scroll listener: a cover
* of the sheet's own colour is painted at the end of the content
* and attached `local`, over a shadow pinned to the box and
* attached `scroll`. So the pair of attachments *is* the feature,
* and it is what this asserts. The rendered result was measured in
* the harness (dark ramp 52,58,64 flat before; 52,57,63 at the last
* label and 22,24,27 at the bottom edge with more below; flat again
* at the end of the list) and is on the PR.
*/
it('paints the fade only while there is more below', async () => {
const el = await surfaceWithPanel();
const wrapper = el.shadowRoot?.querySelector('wa-dialog');
await (wrapper as HTMLElement & { updateComplete: Promise<unknown> })
.updateComplete;
const body = wrapper?.shadowRoot?.querySelector('[part~="body"]');
expect(body, 'no body part to scroll').not.toBeNull();
const style = getComputedStyle(body as Element);
expect(style.overflowY, 'the body is what gives, not the cap').toBe(
'auto',
);
// The cover scrolls with the content; the shadow does not. Either
// one alone is a fade that is always there or never there.
expect(
style.backgroundAttachment,
'the cover must be local and the shadow must not',
).toBe('local, scroll');
// Both sit at the bottom, or the cover hides nothing.
expect(style.backgroundPosition).toBe('50% 100%, 50% 100%');
expect(style.backgroundSize).toBe('100% 32px, 100% 32px');
});
/**
* A dialog with no accessible name is what `utils/name-dialog.ts`
* exists for; here the name is already written on the panel, so no
@@ -366,6 +366,16 @@ describe('<now-playing>', () => {
const el = await fixture('now-playing');
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 6 });
// Stated rather than inherited: the queue store is a singleton, so
// without this the shot records whichever source the *previous*
// case left in it and the reference moves when the file is
// reordered. Three lines is what the bar renders while playing
// from somewhere, which is the arrangement worth recording.
setQueue([queueTrack(1, 'Ashes to Ashes')], 0, {
type: 'album',
id: 7,
label: 'Scary Monsters',
});
await flush();
await el.updateComplete;
@@ -0,0 +1,275 @@
/**
* A name is not a link on a phone, and the menu is where it went (#67).
*
* `utils/explore-link.ts` makes every track, album and artist name
* navigable, with click handling that is explicitly a desktop
* compromise the navigation is held for one double-click interval so
* double-clicking the row can still play it. On touch that is a delay
* on an ambiguous target, and since #63 the row's own tap claims the
* click anyway, so the link was unreachable as well as fiddly.
*
* So below the phone breakpoint a name renders as plain text and the
* row's context menu carries "Go to Artist" / "Go to Album" instead.
* The two halves are asserted together on purpose: a suppressed link
* with no menu item behind it is not a smaller affordance, it is a
* destination that cannot be reached, which is what plan 018 promises
* against.
*
* The breakpoint is stubbed rather than emulated for the reason
* `now-playing-phone.test.ts` states: this tier's viewport is fixed at
* 1280x800 by the runner, and `matchMedia` is the seam.
*/
import { describe, expect, it, beforeEach, afterEach } from 'vitest';
import { html, render } from 'lit';
import type { LitElement } from 'lit';
import '@components/playlist-details/playlist-details';
import {
albumLink,
artistLink,
creditLink,
trackLink,
} from '@utils/explore-link';
import { goToMenuItems } from '@utils/go-to-menu';
import { stub, flush, resetHarness } from '@test/support/harness';
import { fixture, shadowAll } from '@test/support/render';
/** Answer the phone breakpoint, and hand back the undo. */
function atPhone(phone: boolean): () => void {
const real = window.matchMedia.bind(window);
window.matchMedia = ((q: string) =>
q.includes('max-width: 599px')
? {
matches: phone,
media: q,
addEventListener() {},
removeEventListener() {},
}
: real(q)) as typeof window.matchMedia;
return () => {
window.matchMedia = real as typeof window.matchMedia;
};
}
/** Render a template into a detached container and hand it back. */
function draw(template: unknown): HTMLElement {
const host = document.createElement('div');
document.body.append(host);
render(html`${template}`, host);
return host;
}
describe('an inline name below the phone breakpoint', () => {
let restore: () => void = () => {};
afterEach(() => {
restore();
document.querySelectorAll('body > div').forEach((el) => el.remove());
});
it('is a link on a desktop', () => {
restore = atPhone(false);
const host = draw(artistLink('Cocteau Twins', 'artist-mbid'));
expect(host.querySelector('a.explore-link')).not.toBeNull();
expect(host.textContent?.trim()).toBe('Cocteau Twins');
});
it('is plain text on a phone, for all four shapes', () => {
restore = atPhone(true);
const host = draw(html`
${artistLink('Cocteau Twins', 'artist-mbid')}
${albumLink('Heaven or Las Vegas', 'rg-mbid')}
${trackLink('Iceblink Luck', 'Heaven or Las Vegas', 'rg-mbid', 'rec-mbid')}
${creditLink(
[
{
creditedName: 'Skrillex',
artistMbid: 'a1',
joinPhrase: ' feat. ',
},
{ creditedName: 'Swae Lee', artistMbid: 'a2', joinPhrase: '' },
],
'Skrillex & Swae Lee',
'a1',
)}
`);
expect(host.querySelectorAll('a.explore-link')).toHaveLength(0);
// The words survive, join phrases included — a decomposed credit is
// still assembled from its parts, so the text does not change with
// the affordance.
expect(host.textContent).toContain('Cocteau Twins');
expect(host.textContent).toContain('Heaven or Las Vegas');
expect(host.textContent).toContain('Iceblink Luck');
expect(host.textContent).toContain('Skrillex feat. Swae Lee');
});
it('stays a link where the caller has no menu to carry it', () => {
restore = atPhone(true);
const host = draw(
albumLink('Heaven or Las Vegas', 'rg-mbid', undefined, 'Cocteau Twins', {
keepOnPhone: true,
}),
);
expect(host.querySelector('a.explore-link')).not.toBeNull();
});
});
describe('the "Go to" menu items', () => {
let restore: () => void = () => {};
afterEach(() => {
restore();
document.querySelectorAll('body > div').forEach((el) => el.remove());
});
it('are absent on a desktop, where the name beside them is a link', () => {
restore = atPhone(false);
const host = draw(
goToMenuItems({ artistName: 'Cocteau Twins', albumName: 'Treasure' }),
);
expect(host.querySelectorAll('wa-dropdown-item')).toHaveLength(0);
});
it('offer only what the target knows', () => {
restore = atPhone(true);
const both = draw(
goToMenuItems({ artistName: 'Cocteau Twins', albumName: 'Treasure' }),
);
const artistOnly = draw(goToMenuItems({ artistName: 'Cocteau Twins' }));
const neither = draw(goToMenuItems({}));
expect(both.querySelectorAll('wa-dropdown-item')).toHaveLength(2);
expect(artistOnly.querySelectorAll('wa-dropdown-item')).toHaveLength(1);
expect(neither.querySelectorAll('wa-dropdown-item')).toHaveLength(0);
});
});
// =====================================================================
// The menu that carries the destination
// =====================================================================
function playlistTracks(n: number) {
return Array.from({ length: n }, (_, i) => ({
ID: i + 1,
FilePath: `/music/track-${i}.mp3`,
Title: `Track ${i}`,
Artist: 'Cocteau Twins',
ArtistMBID: 'artist-mbid',
Album: 'Heaven or Las Vegas',
ReleaseGroupMBID: 'rg-mbid',
Duration: 180000,
Phantom: false,
}));
}
describe('a playlist rows context menu on a phone', () => {
let el: LitElement;
let restore: () => void = () => {};
beforeEach(async () => {
resetHarness();
restore = atPhone(true);
stub('playlist.Service.GetPlaylistTracks', playlistTracks(8));
stub('playlist.Service.GetAllPlaylists', []);
el = await fixture<LitElement>('playlist-details', {
playlistId: 1,
playlistName: 'A playlist',
});
el.style.display = 'block';
el.style.height = '600px';
await flush();
await el.updateComplete;
await new Promise((r) => setTimeout(r, 60));
});
afterEach(() => {
restore();
});
/** Right-click a row and hand back the menu's items. */
async function openMenu(index: number): Promise<HTMLElement[]> {
const row = shadowAll(el, '.track-item').find(
(r) => r.getAttribute('data-index') === String(index),
);
row!.dispatchEvent(
new MouseEvent('contextmenu', { bubbles: true, composed: true }),
);
await el.updateComplete;
return shadowAll<HTMLElement>(el, 'wa-dropdown-item');
}
it('carries the artist and the album the row stopped linking to', async () => {
const labels = (await openMenu(3)).map((i) => i.textContent?.trim());
expect(labels).toContain('Go to Artist');
expect(labels).toContain('Go to Album');
});
it('navigates where the name would have', async () => {
const seen: CustomEvent[] = [];
const listen = (e: Event) => seen.push(e as CustomEvent);
document.addEventListener('navigate', listen);
try {
const items = await openMenu(3);
items
.find((i) => i.textContent?.trim() === 'Go to Artist')!
.click();
await flush();
} finally {
document.removeEventListener('navigate', listen);
}
expect(seen.map((e) => e.detail)).toEqual([
{
view: 'explore-artist-details',
artistMBID: 'artist-mbid',
artistName: 'Cocteau Twins',
},
]);
});
it('is absent while several rows are selected', async () => {
// "Go to the album" of five different albums means nothing, which
// is the rule the Play item already follows: one row is a
// position, several are an explicit choice of those tracks.
const rows = shadowAll(el, '.track-item');
const click = (i: number, modifiers: MouseEventInit) =>
rows
.find((r) => r.getAttribute('data-index') === String(i))!
.dispatchEvent(
new MouseEvent('click', {
bubbles: true,
composed: true,
...modifiers,
}),
);
click(1, {});
click(4, { ctrlKey: true });
await el.updateComplete;
const labels = (await openMenu(4)).map((i) => i.textContent?.trim());
expect(labels).not.toContain('Go to Artist');
});
});
@@ -161,6 +161,15 @@ describe('double-clicking a row in the track list', () => {
localStorage.removeItem('track-list-column-widths');
el = await fixture<LitElement>('track-list', { externalTracks: LIST });
// Say which order is being asserted rather than inheriting one.
// `restoreSortPreferences()` runs in `connectedCallback`, so the
// list opens in whatever sort was last persisted -- and
// `track-11` sorts before `track-3` by title, which is the shape
// of #138. The row below is the fixture's third track only while
// nothing is sorting the list.
(el as unknown as { sortField: string | null }).sortField = null;
el.style.display = 'block';
el.style.height = '600px';
await flush();
@@ -172,6 +181,10 @@ describe('double-clicking a row in the track list', () => {
const rows = shadowAll(el, '.track-row');
const row = rows.find((r) => r.getAttribute('data-index') === '3');
// Stated first, so a list that is not in the order this asserts
// fails by saying so rather than as an off-by-eight file path.
expect(row?.getAttribute('data-file-path')).toBe('/music/track-3.mp3');
dblclick(row!);
await flush();
@@ -0,0 +1,191 @@
/**
* What a tap looks like now that the web view's own highlight is gone
* (#54).
*
* `index.css` sets `-webkit-tap-highlight-color: transparent` on
* `html`, which the property being inherited reaches every shadow
* root in the app. That takes away the grey box a phone drew over the
* bounding rect of whatever was tapped, and with it the only touch
* feedback the rows, the tab bar, the sidebar's destinations and the
* shared menu items had. So the press states below are not decoration:
* without them this change trades wrong feedback for none.
*
* **Asserted against the parsed stylesheet**, on `hover-affordance`'s
* precedent and with the same limitation stated out loud: CDP's
* `Emulation.setEmulatedMedia` does not reach this tier's iframe, so
* there is no way here to render a component as a phone would, and
* `:active` cannot be forced from a test either. What the browser will
* answer is the shape it built from the `css` literal which rule sits
* inside which media query, and what the press selector actually is.
*
* Two regressions are worth catching that way, and both are silent on a
* desktop:
*
* - someone hoisting a hover tint back out of its query as a tidy-up,
* which on a phone is a highlight that arrives because a finger
* touched the row and stays after it has gone;
* - someone simplifying the press selector to a bare `:active`, which
* is one class short of `.selected` / `.active` and so does nothing
* on the row a phone is most likely to press the one it has just
* selected.
*
* The pixels are the Android tier's, and the tap highlight itself is
* `e2e/specs/native-touch-feel.spec.ts`, since only the real app loads
* `index.css` at all.
*/
import { describe, expect, it } from 'vitest';
import '@components/track-list/track-list';
import '@components/queue-panel/queue-panel';
import '@components/playlist-details/playlist-details';
import '@components/smart-playlist-details/smart-playlist-details';
import '@components/bottom-nav/bottom-nav';
import '@components/sidebar/app-sidebar';
import { fixture } from '@test/support/render';
/** Every rule in the element's own adopted stylesheets, flattened. */
function rulesOf(host: Element): { text: string; condition: string | null }[] {
const sheets = host.shadowRoot?.adoptedStyleSheets ?? [];
const out: { text: string; condition: string | null }[] = [];
for (const sheet of sheets) {
for (const rule of Array.from(sheet.cssRules)) {
if (rule instanceof CSSMediaRule) {
for (const inner of Array.from(rule.cssRules)) {
out.push({ text: inner.cssText, condition: rule.conditionText });
}
continue;
}
out.push({ text: rule.cssText, condition: null });
}
}
return out;
}
/** The four lists, their row selector, and the tag that draws them. */
const LISTS: Array<[string, string]> = [
['track-list', '.track-row'],
['queue-panel', '.track-item'],
['playlist-details', '.track-item'],
['smart-playlist-details', '.track-item'],
];
describe('a row says it is being pressed', () => {
for (const [tag, row] of LISTS) {
it(`${tag} draws a press state that survives its state classes`, async () => {
const el = await fixture(tag, {});
const rules = rulesOf(el);
// Worth nothing if it read no rules at all — the first assertion
// icon-language.test.ts makes, for the same reason.
expect(rules.length).toBeGreaterThan(0);
const press = rules.filter(
(r) => r.text.includes(`${row}:active`) && r.text.includes('background-color'),
);
expect(press.length).toBeGreaterThan(0);
for (const rule of press) {
// A press is not a hover: it is the one thing a touch device
// can say, so it must not sit behind a pointer query.
expect(rule.condition).toBeNull();
expect(rule.text).toContain('--yj-press-overlay');
}
// The load-bearing half: the selector carries a state class, or
// it loses to `.selected` / `.selected.active` and the press is
// invisible on a selected or playing row.
expect(press.some((r) => r.text.includes(`${row}.selected:active`))).toBe(true);
});
it(`${tag} keeps its hover tint for devices that hover`, async () => {
const el = await fixture(tag, {});
const rules = rulesOf(el);
expect(rules.length).toBeGreaterThan(0);
const hover = rules.filter(
(r) =>
r.text.includes(`${row}:hover`) &&
r.text.includes('--yj-hover-overlay'),
);
expect(hover.length).toBeGreaterThan(0);
for (const rule of hover) {
expect(rule.condition).toMatch(/hover:\s*hover/);
expect(rule.condition).toMatch(/pointer:\s*fine/);
}
});
}
});
describe('the two navigations say they are being pressed', () => {
it('the phone tab bar, which had no state of its own at all', async () => {
const el = await fixture('bottom-nav', {});
const rules = rulesOf(el);
expect(rules.length).toBeGreaterThan(0);
const press = rules.filter((r) => r.text.startsWith('button:active'));
expect(press.length).toBe(1);
expect(press[0]!.condition).toBeNull();
expect(press[0]!.text).toContain('--yj-press-overlay');
});
it("the sidebar, which is also the phone's More sheet", async () => {
const el = await fixture('app-sidebar', {});
const rules = rulesOf(el);
expect(rules.length).toBeGreaterThan(0);
const press = rules.filter((r) => r.text.startsWith('li button:active'));
expect(press.length).toBe(1);
expect(press[0]!.condition).toBeNull();
expect(press[0]!.text).toContain('--yj-press-overlay');
// Its hover tint is a destination looking picked, if it is left to
// a synthesised hover inside the More sheet.
const hover = rules.filter((r) => r.text.startsWith('li button:hover'));
expect(hover.length).toBeGreaterThan(0);
for (const rule of hover) {
expect(rule.condition).toMatch(/hover:\s*hover/);
}
});
});
describe('the shared context menu', () => {
// One stylesheet, fourteen menus — the same reason the sheet's row
// height lives there rather than in each host.
it('presses its items, in the one place every menu includes', async () => {
const el = await fixture('queue-panel', {});
const rules = rulesOf(el);
const press = rules.filter((r) =>
r.text.startsWith('.context-menu-panel wa-dropdown-item:active'),
);
expect(press.length).toBe(1);
expect(press[0]!.condition).toBeNull();
expect(press[0]!.text).toContain('--yj-press-overlay');
const hover = rules.filter((r) =>
r.text.startsWith('.context-menu-panel wa-dropdown-item:hover'),
);
expect(hover.length).toBeGreaterThan(0);
for (const rule of hover) {
expect(rule.condition).toMatch(/hover:\s*hover/);
expect(rule.condition).toMatch(/pointer:\s*fine/);
}
});
});
@@ -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);
});
});
@@ -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('<selection-bar>', () => {
}
});
});
/**
* 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');
});
});
@@ -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<HTMLElement>(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<HTMLElement>(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');
});
});
@@ -0,0 +1,299 @@
/**
* Swipe right on a track row to queue it (plan 019 phase 2, #63).
*
* `touch-gestures.test.ts` holds the recogniser the axis rule, the
* claim, the guaranteed end. What is here is what the *list* does with
* it, and the two things that are only true of a list:
*
* **A swipe is not a selection.** It acts on the row it was made on,
* unless that row is one of several the user has explicitly chosen, in
* which case it acts on all of them the same rule the context menu
* answers with, because a bar saying "40 selected" and a gesture that
* quietly queues one of them is two answers to the same question.
*
* **A short swipe is a no-op**, and that is the only thing standing
* between "add to queue" and a scroll that drifted sideways. The
* threshold is a fraction of the row, so it is measured from the row
* here rather than written down twice.
*
* What this tier cannot see is the device, and the reason is in the
* module's own header: Chrome 113's WebView cancels the pointer stream
* ~16px into any drag whatever `touch-action` says, so the gesture
* runs on touch events and needs `touch-action: pan-y` *and* a
* non-passive `preventDefault`. Both are correct in Chromium either
* way. The stylesheet half is asserted below for the same reason
* `hover-affordance.test.ts` reads a parsed stylesheet: the regression
* is someone tidying the declaration away, and nothing here renders
* differently when they do.
*/
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import '@components/track-list/track-list';
import { calls, flush, resetHarness, stub } from '@test/support/harness';
import { fixture, shadow, shadowAll } from '@test/support/render';
import { installTouchGestures } from '@utils/touch-gestures';
const wait = (ms: number) => new Promise((r) => setTimeout(r, ms));
let uninstall: (() => void) | null = null;
function track(n: number) {
return {
FilePath: `/music/track-${n}.mp3`,
TrackName: `Track ${n}`,
ArtistName: 'An Artist',
Album: 'An Album',
Duration: 100 + n,
ID: n,
};
}
const TRACKS = [track(1), track(2), track(3), track(4)];
async function mountList() {
const el = await fixture('track-list');
// A definite width, because the commit threshold is a fraction of
// the row and a list that has not been given one is not a list.
el.style.display = 'block';
el.style.width = '400px';
(el as unknown as { tracks: unknown[] }).tracks = TRACKS;
await flush();
await el.updateComplete;
await wait(60);
await el.updateComplete;
return el;
}
function rows(el: HTMLElement): HTMLElement[] {
return shadowAll<HTMLElement>(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> | void,
): Promise<void> {
const at = (x: number, y: number) =>
new Touch({
identifier: 1,
target: el as Element,
clientX: x,
clientY: y,
});
const send = (type: string, points: Touch[]) =>
el.dispatchEvent(
new TouchEvent(type, {
bubbles: true,
composed: true,
cancelable: true,
touches: points,
changedTouches: points.length > 0 ? points : [at(0, 0)],
}),
);
send('touchstart', [at(0, 100)]);
// Several steps, because the recogniser claims the gesture on the
// move that crosses its threshold and the component reads every one
// after it.
for (const step of [0.25, 0.5, 0.75, 1]) {
send('touchmove', [at(dx * step, 100 + dy * step)]);
if (onStep) await onStep();
}
send('touchend', []);
}
/** Where the commit threshold falls for the row as rendered. */
function threshold(row: HTMLElement): number {
return Math.max(72, row.getBoundingClientRect().width * 0.3);
}
describe('a finger swiped right across a track row', () => {
beforeEach(() => {
resetHarness();
stub('library.Library.GetTracks', TRACKS);
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
stub('config.Config.GetShortcuts', {});
stub('queue.Queue.SetQueue', null);
stub('queue.Queue.AddTracks', null);
uninstall = installTouchGestures();
});
afterEach(() => {
uninstall?.();
uninstall = null;
vi.restoreAllMocks();
});
it('adds that row to the queue', async () => {
const el = await mountList();
const row = rows(el)[1];
expect(row, 'the list rendered rows').toBeTruthy();
await swipe(row!, threshold(row!) + 40);
await flush();
const queued = calls('queue.Queue.AddTracks');
expect(queued.length, 'one swipe, one call').toBe(1);
expect(queued[0]?.args[0]).toEqual(['/music/track-2.mp3']);
// It queues; it does not play. The row that was playing keeps
// playing, which is the difference from a tap.
expect(calls('queue.Queue.SetQueue').length).toBe(0);
});
it('does nothing when the finger did not get far enough', async () => {
const el = await mountList();
const row = rows(el)[1];
await swipe(row!, Math.round(threshold(row!)) - 10);
await flush();
expect(calls('queue.Queue.AddTracks').length).toBe(0);
});
it('leaves a scroll that began on a row to the list', async () => {
// The same shape as `touch-selection.test.ts`'s "does not play a
// row the finger scrolled from", one gesture over: the failure
// this guards against makes the list unusable rather than wrong.
const el = await mountList();
const row = rows(el)[1];
await swipe(row!, 30, 200);
await flush();
expect(calls('queue.Queue.AddTracks').length).toBe(0);
});
it('reveals what it will do, in words, while the finger is down', async () => {
const el = await mountList();
const row = rows(el)[1];
const reveals: (string | undefined)[] = [];
await swipe(row!, threshold(row!) + 40, 0, async () => {
await el.updateComplete;
reveals.push(
shadow(el, '[data-testid="swipe-reveal"]')?.textContent?.trim(),
);
});
// Not only a colour (WCAG 1.4.1): the pane says what it is for,
// and says something different once the gesture would commit.
expect(reveals.some((t) => t?.includes('Add to queue'))).toBe(true);
expect(reveals.some((t) => t?.includes('Release to add'))).toBe(true);
});
it('says what it did, for anyone not watching the row', async () => {
const el = await mountList();
const row = rows(el)[1];
await swipe(row!, threshold(row!) + 40);
await flush();
await el.updateComplete;
const said = shadowAll(el, '[role="status"]')
.map((r) => r.textContent?.trim())
.join(' ');
expect(said).toContain('Track 2');
expect(said).toContain('queue');
});
it('queues the whole selection when the row is part of one', async () => {
// One row is a position; several rows are an explicit choice. A
// gesture that quietly queued the one row touched would contradict
// the bar above it saying how many are selected.
const el = await mountList();
rows(el)[1]?.dispatchEvent(
new MouseEvent('click', { bubbles: true, composed: true }),
);
rows(el)[3]?.dispatchEvent(
new MouseEvent('click', {
bubbles: true,
composed: true,
ctrlKey: true,
}),
);
await el.updateComplete;
const row = rows(el)[1];
await swipe(row!, threshold(row!) + 40);
await flush();
expect(calls('queue.Queue.AddTracks')[0]?.args[0]).toEqual([
'/music/track-2.mp3',
'/music/track-4.mp3',
]);
});
it('queues only the row it touched when that row is outside the selection', async () => {
const el = await mountList();
rows(el)[3]?.dispatchEvent(
new MouseEvent('click', { bubbles: true, composed: true }),
);
await el.updateComplete;
const row = rows(el)[0];
await swipe(row!, threshold(row!) + 40);
await flush();
expect(calls('queue.Queue.AddTracks')[0]?.args[0]).toEqual([
'/music/track-1.mp3',
]);
// And it did not become a way of selecting anything.
expect(rows(el)[3]?.getAttribute('aria-selected')).toBe('true');
expect(rows(el)[0]?.getAttribute('aria-selected')).toBe('false');
});
it('declares pan-y on the row, which is half of what makes it work', async () => {
// The other half is the gesture module's non-passive
// `preventDefault`. Neither works alone on Chrome 113 and both are
// irrelevant here, so this reads the stylesheet and the attribute
// rather than the rendering — the regression is someone tidying
// one of them away, and nothing in this browser looks different
// when they do.
const el = await mountList();
expect(
rows(el)[0]?.hasAttribute('data-swipe'),
'the row opts into the shared rule',
).toBe(true);
const sheets = (
customElements.get('track-list') as unknown as {
styles: { cssText: string }[];
}
).styles;
const css = sheets.map((s) => s.cssText).join('\n');
const rule = css
.split('}')
.find((block) => /\[data-swipe\]\s*\{/.test(block));
expect(rule, 'the shared rule is in this component').toBeTruthy();
expect(rule).toContain('touch-action: pan-y');
expect(css, 'never none: it takes the scrolling too').not.toContain(
'touch-action: none',
);
});
});
+17
View File
@@ -72,6 +72,23 @@ document.body.style.margin = '0';
beforeEach(() => {
resetHarness();
// A test file does not get its own origin. `@vitest/browser-playwright`
// opens one BrowserContext per session and runs several files in it,
// one after another, so everything a component persists — the track
// list's sort and column widths, the cover size, `now-playing`'s
// scroll mode — is still there when the next file mounts the same
// component. That is invisible until it is intermittent, because
// which files share a tab and in what order changes run to run: it
// cost #138 three scheduled runs, one of them a PR with no frontend
// code in it at all.
//
// Clearing here rather than in the specs that write is deliberate —
// the spec that *reads* is never the one that knows. It is safe for
// the same reason the leak exists: files in a session are
// sequential, so this cannot wipe storage a concurrent file is in
// the middle of using.
localStorage.clear();
});
afterEach(() => {
+2
View File
@@ -0,0 +1,2 @@
<!-- A real, servable image for the prefetch tests: one transparent pixel. -->
<svg xmlns="http://www.w3.org/2000/svg" width="1" height="1"></svg>

After

Width:  |  Height:  |  Size: 147 B

+113
View File
@@ -0,0 +1,113 @@
/**
* What the grids ask for ahead of the scroll (#65).
*
* The virtualizer renders about 1000px past its viewport and nothing
* else can be asked for, because the `<img>` does not exist until the
* card does two screens on the reference device, which is a fraction
* of a second at speed. `prefetchImageWindow` issues the request
* before the element, so the assertions here are about *which* rows
* are asked for, that none is asked for twice, and that a request is
* really made rather than merely recorded.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import {
PREFETCH_MEMORY,
imagePrefetched,
prefetchImage,
prefetchImageWindow,
resetImagePrefetch,
} from '@utils/image-prefetch';
/** A hundred cards, each with its own cover URL. */
const CARDS = Array.from({ length: 100 }, (_, i) => ({ url: `/covers/${i}_sm.jpg` }));
const urlOf = (card: { url: string }) => card.url;
beforeEach(() => {
resetImagePrefetch();
});
describe('warming the images a scroll is about to reach', () => {
it('asks for the rows just past the rendered range, and no further', () => {
const issued = prefetchImageWindow(CARDS, 40, 50, urlOf, 3);
// Three past each edge: 51-53 and 37-39.
expect(issued).toBe(6);
expect(imagePrefetched('/covers/51_sm.jpg')).toBe(true);
expect(imagePrefetched('/covers/53_sm.jpg')).toBe(true);
expect(imagePrefetched('/covers/54_sm.jpg')).toBe(false);
expect(imagePrefetched('/covers/39_sm.jpg')).toBe(true);
expect(imagePrefetched('/covers/37_sm.jpg')).toBe(true);
expect(imagePrefetched('/covers/36_sm.jpg')).toBe(false);
});
it('leaves the rendered rows alone — they have their own <img>', () => {
prefetchImageWindow(CARDS, 40, 50, urlOf, 3);
expect(imagePrefetched('/covers/45_sm.jpg')).toBe(false);
});
it('asks for nothing twice, so a scroll back over the same rows is free', () => {
prefetchImageWindow(CARDS, 40, 50, urlOf, 3);
expect(prefetchImageWindow(CARDS, 40, 50, urlOf, 3)).toBe(0);
});
it('clamps at both ends of the list', () => {
// At the top of a five-item list nothing precedes the range, and
// the tail runs out after two.
expect(prefetchImageWindow(CARDS.slice(0, 5), 0, 2, urlOf, 10)).toBe(2);
});
it('asks for nothing when the virtualizer reports an empty range', () => {
// `visibilityChanged` reports -1/-1 before anything is laid out.
expect(prefetchImageWindow(CARDS, -1, -1, urlOf)).toBe(0);
});
it('skips a card that draws a placeholder rather than an image', () => {
expect(prefetchImageWindow(CARDS, 40, 50, () => '', 3)).toBe(0);
});
it('really issues the request, rather than only recording it', async () => {
// A served file, so the load succeeds and the resource timing entry
// is unambiguous; the query string keeps it distinct per run.
const url = `/test/support/pixel.svg?prefetch=${Date.now()}`;
const href = new URL(url, location.href).href;
expect(prefetchImage(url)).toBe(true);
for (let i = 0; i < 100; i++) {
if (performance.getEntriesByName(href).length > 0) break;
await new Promise((r) => setTimeout(r, 20));
}
expect(performance.getEntriesByName(href)).toHaveLength(1);
expect(prefetchImage(url)).toBe(false);
expect(performance.getEntriesByName(href)).toHaveLength(1);
});
it('reports what it is holding, with its cap, to the cache stats', () => {
prefetchImageWindow(CARDS, 40, 50, urlOf, 3);
const stat = window.__yjCacheStats?.()['imagePrefetch'];
expect(stat).toBeTruthy();
expect(stat!.entries).toBe(6);
expect(stat!.limit).toBe(PREFETCH_MEMORY);
// It holds URLs, not images — the bytes are the browser's cache.
expect(stat!.chars).toBe(6 * '/covers/51_sm.jpg'.length);
});
it('keeps its record bounded, so a 50 000-album scroll cannot grow it', () => {
const many = Array.from(
{ length: PREFETCH_MEMORY * 2 },
(_, i) => ({ url: `/covers/bulk-${i}_sm.jpg` }),
);
prefetchImageWindow(many, 0, 0, urlOf, many.length);
expect(window.__yjCacheStats?.()['imagePrefetch']?.entries).toBe(PREFETCH_MEMORY);
});
});
-39
View File
@@ -30,12 +30,6 @@ func TestFixturesMatchManifest(t *testing.T) {
m := testfixtures.Load(t)
for _, want := range m.Tracks {
// WAV tags are write-only today; see
// TestWAVTagsAreNotReadableYet.
if want.Format == "wav" {
continue
}
t.Run(want.Path, func(t *testing.T) {
t.Parallel()
@@ -161,39 +155,6 @@ func TestDuplicateFixturesAreIndistinguishable(t *testing.T) {
}
}
// TestWAVTagsAreNotReadableYet pins a known gap rather than hiding it.
//
// backend/tagwriter writes WAV tags into a RIFF "id3 " chunk, but
// backend/metadata reads through dhowden/tag, which recognises MP3,
// FLAC, OGG, MP4 and DSF and has no RIFF parser at all. So every tag
// the app writes to a WAV is invisible to the app that wrote it, and
// WAV tracks always scan in as untitled.
//
// The fixtures are tagged correctly on disk, so when the reader learns
// to unwrap the RIFF chunk this test starts failing — which is the
// point. Delete it then and drop the "wav" skip in
// TestFixturesMatchManifest.
func TestWAVTagsAreNotReadableYet(t *testing.T) {
t.Parallel()
m := testfixtures.Load(t)
for _, path := range m.Case(t, testfixtures.CaseWAVTracks) {
got, err := metadata.ExtractTags(path)
if err != nil {
t.Fatalf("extract tags from %s: %v", path, err)
}
if got.Title != "" {
t.Errorf(
"%s: WAV tags are now readable (%q) — good news; "+
"see this test's comment for what to update",
filepath.Base(path), got.Title,
)
}
}
}
func assertTag(t *testing.T, field string, want testfixtures.Track, got string) {
t.Helper()