yonlu/yellowjacket
1
1
Fork 0
Code Issues 55 Pull Requests 6 Actions Packages Projects Releases 12 Wiki Activity

Desktop: queue panel — single-click select and double-click to play from that spot #43

New Issue
Closed
opened 2026-08-18 05:57:03 +00:00 by logan · 2 comments
logan commented 2026-08-18 05:57:03 +00:00
Collaborator
Copy Link
Copy Source

Report

In the queue sidebar I should be able to single-click to select a row (with the normal multi-select behaviour) and double-click to start playing from that position.

Findings

Both appear to be implemented, which makes this a bug report rather than a feature request:

  • frontend/src/components/queue-panel/queue-panel.ts holds a SelectionController (line 76), delegates click/dblclick on the virtualizer (lines 631-632), and handleTrackClick → selection.handleItemClick, handleTrackDblClick → selection.clear(); queue.playAtIndex(index) (lines ~921-937).

Likely causes to check, in order:

  1. The virtualizer is not repainting on a selection change — <lit-virtualizer> renders through the virtualize directive and only reacts to its own properties changing, so a host state change never repaints the rows. Both playlist detail views had exactly this and must push virtualizer.requestUpdate() on a selection change; confirm queue-panel does.
  2. resolveTrackIndexFromEvent failing to find data-index for some rows.
  3. The row's own click targets (remove button, art) swallowing the event.

Direction

Reproduce, fix, and add a spec for select-then-multi-select and dblclick-plays-from-index. Note the known trap: a track started from the track list leaves currentIndex at −1, so the panel has no current row at all in that flow.

**Report** In the queue sidebar I should be able to single-click to select a row (with the normal multi-select behaviour) and double-click to start playing from that position. **Findings** Both appear to be *implemented*, which makes this a bug report rather than a feature request: - `frontend/src/components/queue-panel/queue-panel.ts` holds a `SelectionController` (line 76), delegates `click`/`dblclick` on the virtualizer (lines 631-632), and `handleTrackClick` → `selection.handleItemClick`, `handleTrackDblClick` → `selection.clear(); queue.playAtIndex(index)` (lines ~921-937). Likely causes to check, in order: 1. The virtualizer is not repainting on a selection change — `<lit-virtualizer>` renders through the `virtualize` directive and only reacts to *its own* properties changing, so a host state change never repaints the rows. Both playlist detail views had exactly this and must push `virtualizer.requestUpdate()` on a selection change; confirm `queue-panel` does. 2. `resolveTrackIndexFromEvent` failing to find `data-index` for some rows. 3. The row's own click targets (remove button, art) swallowing the event. **Direction** Reproduce, fix, and add a spec for select-then-multi-select and dblclick-plays-from-index. Note the known trap: a track started from the *track list* leaves `currentIndex` at −1, so the panel has no current row at all in that flow.
logan added the Area/QueueKind/BugPlatform/Desktop
Priority
High
2
 labels 2026-08-18 14:36:11 +00:00
logan self-assigned this 2026-08-20 02:36:27 +00:00
logan added the
Status
In Progress
 label 2026-08-20 02:36:27 +00:00
logan commented 2026-08-20 02:36:28 +00:00
Author
Collaborator
Copy Link
Copy Source

Claiming this. Branch: fix/43-queue-panel-selection.

Approach. Reproduce first against the running app, because the
report's own reading is that both behaviours are already wired — so the
value here is the diagnosis, not the feature. Working the three
candidates in the order given: the virtualizer repaint (which both
playlist detail views needed and which track-list has always done),
then resolveTrackIndexFromEvent, then a row control swallowing the
event.

Two traps from the issue and from CLAUDE.md that I will keep in
front of me: a track started from the track list leaves the queue's
currentIndex at −1, so the panel has no current row at all in that
flow and it reads exactly like this bug; and a closed panel renders no
list, so anything asserting on rows has to open it first.

Not #5, which is the same model in the other lists. This is the
existing wiring not behaving, and whatever the fault turns out to be is
worth knowing before #5 copies the pattern into five more places — I
will write it up on #5 either way.

Claiming this. Branch: `fix/43-queue-panel-selection`. **Approach.** Reproduce first against the running app, because the report's own reading is that both behaviours are already wired — so the value here is the diagnosis, not the feature. Working the three candidates in the order given: the virtualizer repaint (which both playlist detail views needed and which `track-list` has always done), then `resolveTrackIndexFromEvent`, then a row control swallowing the event. Two traps from the issue and from `CLAUDE.md` that I will keep in front of me: a track started from the **track list** leaves the queue's `currentIndex` at −1, so the panel has no current row at all in that flow and it reads exactly like this bug; and a closed panel renders no list, so anything asserting on rows has to open it first. **Not #5**, which is the same model in the *other* lists. This is the existing wiring not behaving, and whatever the fault turns out to be is worth knowing before #5 copies the pattern into five more places — I will write it up on #5 either way.
logan commented 2026-08-20 02:58:58 +00:00
Author
Collaborator
Copy Link
Copy Source

All four behaviours work on current main, and I could not reproduce
the report. Here is everything I measured, because "works for me" is
not an answer anybody can check.

Driven against make dev-headless SEED=default with real mouse
events (Playwright, not dispatchEvent — a synthetic click aimed at the
row bypasses the very thing that could be swallowing it):

gesture result
click row 1 .1......
ctrl+click row 4 1..1....
shift+click row 7 1..1111.
click row 5 ....1...
double-click row 3 currentIndex 2, playing Harbour Lights

Selection is also visible — .track-item.selected renders, confirmed
in a screenshot, so this is not a working model with no feedback.

The three candidates in the Findings, each checked:

  1. The virtualizer repaint — already correct, and already correct
    when this was filed: onSelectionChanged() has called
    virtualizer.requestUpdate() since well before 2026-08-18. So this
    was never it.
  2. resolveTrackIndexFromEvent — reads data-index off
    .track-item; DOM order and data order agree (0,1,2,...), so it
    resolves correctly for every row.
  3. A row control swallowing the event — this one is real but is not
    what it looks like. explore-link stops propagation on purpose
    ("the row must not also treat it as a selection"), so a click landing
    on a track or artist name navigates and selects nothing.

Candidate 3 is not the culprit either, and the measurement is the
reason. A horizontal hit-scan across a row at three heights, asking
elementFromPoint what is actually under each x:

row link coverage
queue panel 12%
track list 21%

The panel this issue calls broken is less covered by links than the
list it calls correct. I had assumed the opposite and was about to fix
it; the scan says don't.

What I think you actually hit. Two candidates, both of which produce
exactly this impression:

  • The fixture library is 2-second tracks. Double-click a queue row,
    look up two seconds later, and auto-advance has moved on — I recorded
    "double-clicked row 6, row 7 playing" twice before spotting it, which
    reads precisely like an off-by-one in PlayIndex and is not one.
    e2e/support/fixtures.ts already names this trap and exports
    LONG_TRACK (90 s) for it.
  • A late highlight. The rows repaint two ways — the explicit
    requestUpdate() above, and .keyFunction being a per-render arrow,
    which is a changed property the virtualizer reacts to by itself.
    Removing either alone changes nothing observable, which is why
    this could not be settled by reading the code. With both removed the
    highlight still arrives, on whatever unrelated render happens next:
    134ms, 3,866ms and 5,816ms for three clicks, against 5, 16 and
    17ms     healthy. Four seconds is indistinguishable from broken.

So the deliverable is the pin, not a patch — the Direction asks for
a spec and that is the part that was genuinely missing.
e2e/specs/queue-selection.spec.ts covers single click, ctrl, shift,
replace-on-plain-click, double-click-plays-from-index, and both halves
of the explore-link exception (a single click navigates; a double
click plays rather than navigating).

It is 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 the assertion is bounded at 500ms; with the default 5s poll the degraded build passes

That last row is the one worth keeping: a poll generous enough to be
stable is generous enough to miss this entire class of defect.

Closing as not reproducible, with the behaviour now pinned. If you can
still make it happen, please reopen with the library you were on and
whether the highlight arrived late or not at all — the two candidates
above have different fixes and the distinction is exactly what I cannot
get from here.

**All four behaviours work on current `main`, and I could not reproduce the report. Here is everything I measured, because "works for me" is not an answer anybody can check.** Driven against `make dev-headless SEED=default` with **real** mouse events (Playwright, not `dispatchEvent` — a synthetic click aimed at the row bypasses the very thing that could be swallowing it): | gesture | result | |---|---| | click row 1 | `.1......` | | ctrl+click row 4 | `1..1....` | | shift+click row 7 | `1..1111.` | | click row 5 | `....1...` | | double-click row 3 | `currentIndex 2`, playing *Harbour Lights* | Selection is also *visible* — `.track-item.selected` renders, confirmed in a screenshot, so this is not a working model with no feedback. **The three candidates in the Findings, each checked:** 1. **The virtualizer repaint** — already correct, and already correct **when this was filed**: `onSelectionChanged()` has called `virtualizer.requestUpdate()` since well before 2026-08-18. So this was never it. 2. **`resolveTrackIndexFromEvent`** — reads `data-index` off `.track-item`; DOM order and data order agree (`0,1,2,...`), so it resolves correctly for every row. 3. **A row control swallowing the event** — this one is real but is not what it looks like. `explore-link` **stops propagation on purpose** ("the row must not also treat it as a selection"), so a click landing on a track or artist *name* navigates and selects nothing. **Candidate 3 is not the culprit either, and the measurement is the reason.** A horizontal hit-scan across a row at three heights, asking `elementFromPoint` what is actually under each x: | row | link coverage | |---|---| | queue panel | **12%** | | track list | **21%** | The panel this issue calls broken is *less* covered by links than the list it calls correct. I had assumed the opposite and was about to fix it; the scan says don't. **What I think you actually hit.** Two candidates, both of which produce exactly this impression: - **The fixture library is 2-second tracks.** Double-click a queue row, look up two seconds later, and auto-advance has moved on — I recorded "double-clicked row 6, row 7 playing" twice before spotting it, which reads precisely like an off-by-one in `PlayIndex` and is not one. `e2e/support/fixtures.ts` already names this trap and exports `LONG_TRACK` (90 s) for it. - **A late highlight.** The rows repaint two ways — the explicit `requestUpdate()` above, *and* `.keyFunction` being a per-render arrow, which is a changed property the virtualizer reacts to by itself. Removing **either** alone changes nothing observable, which is why this could not be settled by reading the code. With both removed the highlight still arrives, on whatever unrelated render happens next: **134ms, 3,866ms and 5,816ms** for three clicks, against **5, 16 and 17ms** healthy. Four seconds is indistinguishable from broken. **So the deliverable is the pin, not a patch** — the Direction asks for a spec and that is the part that was genuinely missing. `e2e/specs/queue-selection.spec.ts` covers single click, ctrl, shift, replace-on-plain-click, double-click-plays-from-index, and both halves of the `explore-link` exception (a single click navigates; a double click plays rather than navigating). It is **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 the assertion is bounded at 500ms; with the default 5s poll the degraded build passes | That last row is the one worth keeping: a poll generous enough to be stable is generous enough to miss this entire class of defect. Closing as not reproducible, with the behaviour now pinned. **If you can still make it happen, please reopen with the library you were on and whether the highlight arrived late or not at all** — the two candidates above have different fixes and the distinction is exactly what I cannot get from here.
logan closed this issue 2026-08-20 03:14:30 +00:00
gitea-actions bot removed the
Status
In Progress
 label 2026-08-20 03:14:40 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Area/Design
Area/Downloads
Area/Explore
Area/Library-UI
Area/Metadata
Area/Packaging
Area/Player
Area/Queue
Area/Settings
Area/Shell-Nav
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
 Platform/Android
Platform/Desktop
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Medium
3
The priority is medium
Priority
Low
4
The priority is low
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
In Progress
Somebody is actively working on this right now
No labels Area/Queue Kind/Bug Platform/Desktop
Priority
High
2
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
logan yonlu
No Assignees logan
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: yonlu/yellowjacket#43