Pin the queue panel's mouse model #152

Merged
logan merged 2 commits from fix/43-queue-panel-selection into main 2026-08-20 03:14:30 +00:00
Collaborator

#43 asks for single-click select and double-click play in the queue
panel, and reports both as broken. All four gestures work on current
main
— what was missing is that nothing pinned any of them, in
either tier, so the report could be made and could not be settled.

Commits

commit what issue
4f7529c e2e/specs/queue-selection.spec.ts (5 tests), and a note on onSelectionChanged recording the second repaint mechanism #43
70ab3dd two measurements in .planning/NOTES.md #43

What I measured

Real mouse events, not dispatchEvent — a synthetic click aimed at the
row bypasses the only thing that could be swallowing it:

gesture selected rows
click row 1 1
ctrl+click row 4 1, 4
shift+click row 7 1, 4, 5, 6, 7
click row 5 5
double-click row 3 plays row 3

Selection is visible too (.track-item.selected, confirmed in a
screenshot), so this is not a working model with no feedback.

The three candidates in the issue's Findings, each answered:

  1. The virtualizer repaint — already correct, and already correct on
    the day the issue was filed (checked against the tree at that commit).
  2. resolveTrackIndexFromEvent — reads data-index; DOM order and
    data order agree.
  3. A row control swallowing the click — real, but not the culprit.
    explore-link stops propagation deliberately ("the row must not also
    treat it as a selection"), so a click on a name navigates. A
    horizontal hit-scan asking elementFromPoint what is under each x
    makes the queue row 12% link and the track list's row 21% — the
    panel called broken is less link-covered than the list called
    correct. That measurement killed the fix this PR started out as,
    which was going to demote the queue's links.

Two traps that faked a defect while measuring

  • Fixture tracks are 2 seconds. "Double-click row 6" read a moment
    later reports row 7, because auto-advance moved on. I recorded that
    twice as an off-by-one in PlayIndex before spotting it. The spec
    uses LONG_TRACK (90 s), which fixtures.ts already exports for
    exactly this.
  • A generous poll cannot see a latency regression. queue-panel
    repaints two ways — the explicit requestUpdate() and a per-render
    keyFunction arrow — so removing either alone changes nothing
    observable, which is why this was not settleable by reading the code.
    With both removed the highlight still arrives, at 134ms, 3.9s
    and 5.8s
    against 5–17ms healthy. Four seconds is
    indistinguishable from broken. The selection assertions are bounded at
    500ms for that reason.

Verification

Mutation-tested, because a spec written against a working build proves
nothing until it has failed:

mutation caught by
playAtIndex(index + 1) both double-click tests
every click treated as ctrl+click both selection tests
both repaint mechanisms removed all three selection tests — only because of the 500ms bound; with the default 5s poll the degraded build passes all five
  • make lint — 0 issues, three build configurations
  • make test — green, three passes
  • make ui-test — 945 tests
  • make e2e — 178 passed (was 173), run twice against one
    make dev-headless
  • make css-check, make bindings-check, make skill-check
  • npx tsc --noEmit in both frontend/ and e2e/

Filed while here

  • #151album-dropdown's "can be scrolled" still races. It failed
    in the second of three full-suite runs and passed alone on the same
    app immediately after, with #133's exact signature. #133 strengthened
    the guard, which was necessary and cannot be sufficient: the guard
    and the write are separate CDP round trips, so the layout can settle
    out from under a poll that just passed. Not this branch's — its only
    changes are a spec that runs later and a comment.

On closing it

Closed as not reproducible, with the behaviour pinned and the two
likeliest explanations written onto the issue for the reporter, who can
tell them apart and I cannot from here: a 2-second fixture track, or a
late highlight. A note also went to #5, which is about to copy this
selection model into the detail lists — explore-album-details's
tracklist is number/title/artist/duration and is the one that plausibly
is mostly link, so it should get the hit-scan before the pattern is
copied.

Closes #43

#43 asks for single-click select and double-click play in the queue panel, and reports both as broken. **All four gestures work on current `main`** — what was missing is that nothing pinned any of them, in either tier, so the report could be made and could not be settled. ## Commits | commit | what | issue | |---|---|---| | `4f7529c` | `e2e/specs/queue-selection.spec.ts` (5 tests), and a note on `onSelectionChanged` recording the second repaint mechanism | #43 | | `70ab3dd` | two measurements in `.planning/NOTES.md` | #43 | ## What I measured Real mouse events, not `dispatchEvent` — a synthetic click aimed at the row bypasses the only thing that could be swallowing it: | gesture | selected rows | |---|---| | click row 1 | `1` | | ctrl+click row 4 | `1, 4` | | shift+click row 7 | `1, 4, 5, 6, 7` | | click row 5 | `5` | | double-click row 3 | plays row 3 | Selection is visible too (`.track-item.selected`, confirmed in a screenshot), so this is not a working model with no feedback. **The three candidates in the issue's Findings, each answered:** 1. **The virtualizer repaint** — already correct, and already correct on the day the issue was filed (checked against the tree at that commit). 2. **`resolveTrackIndexFromEvent`** — reads `data-index`; DOM order and data order agree. 3. **A row control swallowing the click** — real, but not the culprit. `explore-link` stops propagation deliberately ("the row must not also treat it as a selection"), so a click on a *name* navigates. A horizontal hit-scan asking `elementFromPoint` what is under each x makes the **queue row 12% link and the track list's row 21%** — the panel called broken is *less* link-covered than the list called correct. **That measurement killed the fix this PR started out as**, which was going to demote the queue's links. ## Two traps that faked a defect while measuring - **Fixture tracks are 2 seconds.** "Double-click row 6" read a moment later reports row 7, because auto-advance moved on. I recorded that twice as an off-by-one in `PlayIndex` before spotting it. The spec uses `LONG_TRACK` (90 s), which `fixtures.ts` already exports for exactly this. - **A generous poll cannot see a latency regression.** `queue-panel` repaints two ways — the explicit `requestUpdate()` *and* a per-render `keyFunction` arrow — so removing either alone changes nothing observable, which is why this was not settleable by reading the code. With **both** removed the highlight still arrives, at **134ms, 3.9s and 5.8s** against **5–17ms** healthy. Four seconds is indistinguishable from broken. The selection assertions are bounded at 500ms for that reason. ## Verification Mutation-tested, because a spec written against a working build proves nothing until it has failed: | mutation | caught by | |---|---| | `playAtIndex(index + 1)` | both double-click tests | | every click treated as ctrl+click | both selection tests | | both repaint mechanisms removed | all three selection tests — **only** because of the 500ms bound; with the default 5s poll the degraded build passes all five | - `make lint` — 0 issues, three build configurations - `make test` — green, three passes - `make ui-test` — 945 tests - `make e2e` — 178 passed (was 173), run **twice** against one `make dev-headless` - `make css-check`, `make bindings-check`, `make skill-check` - `npx tsc --noEmit` in both `frontend/` and `e2e/` ## Filed while here - **#151** — `album-dropdown`'s "can be scrolled" still races. It failed in the second of three full-suite runs and passed alone on the same app immediately after, with #133's exact signature. #133 strengthened the *guard*, which was necessary and cannot be sufficient: the guard and the write are separate CDP round trips, so the layout can settle out from under a poll that just passed. Not this branch's — its only changes are a spec that runs later and a comment. ## On closing it Closed as not reproducible, with the behaviour pinned and the two likeliest explanations written onto the issue for the reporter, who can tell them apart and I cannot from here: a 2-second fixture track, or a late highlight. A note also went to **#5**, which is about to copy this selection model into the detail lists — `explore-album-details`'s tracklist is number/title/artist/duration and is the one that plausibly *is* mostly link, so it should get the hit-scan before the pattern is copied. Closes #43
logan added 2 commits 2026-08-20 03:00:47 +00:00
Single click selects, ctrl and shift extend, double click plays from
that row — all four already worked, and nothing in either tier pinned
any of them, which is why the report could be made and could not be
settled. `queue-reorder.spec.ts` covers the keyboard and
`queue-overlay.spec.ts` the panel's mode; the pointer path had no
coverage at all, so "selection is broken here" and "selection is fine
here" were equally consistent with a green suite.

Measured with real mouse events rather than dispatched ones, because a
synthetic click aimed at the row bypasses the only thing that could be
swallowing it: click row 1 selects 1, ctrl+click 4 gives 1 and 4,
shift+click 7 extends to 1,4,5,6,7, a plain click collapses to one, and
a double click on row 3 leaves the backend playing row 3.

The three candidates the issue lists are all answered. The repaint was
already correct, and already correct on the day the issue was filed.
`resolveTrackIndexFromEvent` reads data-index, and DOM order matches
data order. A row control does swallow the click — `explore-link` stops
propagation on purpose, so a click on a name navigates and selects
nothing — but a hit-scan across a row makes the queue 12% link against
the track list's 21%, so the panel called broken is *less* covered by
links than the list called correct. That measurement killed the fix
this started out as.

Two traps are written into the spec because both faked a defect while
measuring. Fixture tracks are 2 seconds, so "double click row 3" read a
moment later reports whatever auto-advance moved on to — recorded twice
as an off-by-one that is not one, which is what `LONG_TRACK` exists
for. And the selection assertions are bounded at 500ms rather than
polled with the default 5s: `queue-panel` repaints two ways, the
explicit `requestUpdate()` and a per-render `keyFunction` arrow, and
with *both* removed the highlight still arrives — at 134ms, 3.9s and
5.8s against 5-17ms healthy. Four seconds is indistinguishable from
broken to a user and invisible to a generous poll.

Mutation-tested rather than trusted: `playAtIndex(index + 1)` fails both
double-click tests, treating every click as ctrl+click fails both
selection tests, and removing both repaint mechanisms fails all three
selection tests — the last only because of the bound.

Closes #43
docs: record two measurements from the queue selection work
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m41s
CI / e2e (pull_request) Successful in 8m1s
70ab3ddf94
The first is a second instance of a rule CLAUDE.md already states, with
numbers: a virtualized list can be repainting for a reason you are about
to delete, and here there are two such reasons — so removing either
alone changes nothing observable, and removing both leaves the highlight
seconds late rather than absent. That is the shape a poll cannot see,
which is the general lesson worth keeping.

The second is the hit-scan, because it stopped a wrong fix: the queue
panel is 12% link and the track list 21%, which is the opposite of the
assumption the fix was being built on.
logan merged commit 9ad1477b1e into main 2026-08-20 03:14:30 +00:00
Sign in to join this conversation.