fix(queue): name the queue header's two older actions #223

Merged
logan merged 1 commits from fix/170-queue-header-action-names into main 2026-08-26 16:03:02 +00:00
Collaborator

The issue

#170. queue-panel's two header actions — Clear queue and Add queue to playlist — were named by a title attribute and nothing else. The close button beside them, added by #24, has carried an aria-label since.

What changed and why

aria-label on both, matching the close button. The titles stay: on a desktop they are also the tooltip for an icon-only control, which is a different job from naming it and one aria-label does not do.

This is the weak-name case rather than the missing one, and the issue is explicit about why that still matters. title is the last fallback in the accname order, so any content put inside the button later silently outranks it; and it is the one name a phone cannot show, there being no hover on the surface #55 turned into a full screen.

The assertion is the part worth reading

The issue asks for a getByRole assertion in e2e/specs/queue-overlay.spec.ts — the file that already makes exactly that assertion about the third button in the same row. That spec is green on the broken build, because title is an accname fallback. Measured against the running pre-fix app before anything was edited:

getByRole button "Clear queue"          -> 1
getByRole button "Add queue to playlist" -> 1
getByRole button "Close queue"           -> 1

That is a11y.26's "a sweep for empty names cannot see a weak one", one rung lower and now defeating the spec as well as the sweep — the same trap as layout-overflow.spec.ts and back-navigation.spec.ts, a spec named for the behaviour that measures the plumbing.

So the property is asserted as what it actually is: the name is not the tooltip. The second test strips the title attributes and asks again.

names both of the older actions and the names do not come from the tooltip
pre-fix bundle passed failed (0 matches, 0 matches)
this branch passed passed

Both rows were run, not reasoned about: the component change was stashed, the frontend rebuilt, make dev-headless restarted and the committed spec run against it.

Verification

tier result
make e2e (chromium, against make dev-headless SEED=default) 246 passed — 244 before, plus the two here
the new spec, against the pre-fix bundle 1 passed, 1 failed — i.e. non-vacuous
make ui-test 1129 passed, 103 files
npx tsc --noEmit (frontend/ and e2e/) clean
make css-check, make skill-check clean

Port 34115 was free before starting, so no other worktree's app was attached to.

Not run, and why: no Go, no .sql, no .templ and no bound signature, so make lint, make test, make generate and make bindings do not apply. make ui-visual was not run — two attributes moved no geometry, and no baseline covers queue-panel.

Deliberately not done

  • The titles were not removed. "Matching the close button" could be read as dropping them, but that would take the desktop tooltip off an icon-only control, which the issue does not ask about and which aria-label does not replace.
  • No sweep for other weakly-named controls. title-only naming elsewhere in the app is a real piece of work and is not this issue's surface. Nothing was found incidentally while here.

CLAUDE.md gains a paragraph under the existing "0 unnamed is not it" rule, because the technique — strip the fallback and re-ask — is the reusable half.

Closes #170

## The issue #170. `queue-panel`'s two header actions — **Clear queue** and **Add queue to playlist** — were named by a `title` attribute and nothing else. The close button beside them, added by #24, has carried an `aria-label` since. ## What changed and why `aria-label` on both, matching the close button. The `title`s stay: on a desktop they are also the *tooltip* for an icon-only control, which is a different job from naming it and one `aria-label` does not do. This is the **weak**-name case rather than the missing one, and the issue is explicit about why that still matters. `title` is the last fallback in the accname order, so any content put inside the button later silently outranks it; and it is the one name a phone cannot show, there being no hover on the surface #55 turned into a full screen. ## The assertion is the part worth reading The issue asks for a `getByRole` assertion in `e2e/specs/queue-overlay.spec.ts` — the file that already makes exactly that assertion about the third button in the same row. **That spec is green on the broken build**, because `title` *is* an accname fallback. Measured against the running pre-fix app before anything was edited: ``` getByRole button "Clear queue" -> 1 getByRole button "Add queue to playlist" -> 1 getByRole button "Close queue" -> 1 ``` That is `a11y.26`'s "a sweep for *empty* names cannot see a *weak* one", one rung lower and now defeating the spec as well as the sweep — the same trap as `layout-overflow.spec.ts` and `back-navigation.spec.ts`, a spec named for the behaviour that measures the plumbing. So the property is asserted as what it actually is: **the name is not the tooltip**. The second test strips the `title` attributes and asks again. | | `names both of the older actions` | `and the names do not come from the tooltip` | |---|---|---| | pre-fix bundle | ✅ passed | ❌ failed (0 matches, 0 matches) | | this branch | ✅ passed | ✅ passed | Both rows were run, not reasoned about: the component change was stashed, the frontend rebuilt, `make dev-headless` restarted and the committed spec run against it. ## Verification | tier | result | |---|---| | `make e2e` (chromium, against `make dev-headless SEED=default`) | **246 passed** — 244 before, plus the two here | | the new spec, against the **pre-fix** bundle | 1 passed, 1 failed — i.e. non-vacuous | | `make ui-test` | 1129 passed, 103 files | | `npx tsc --noEmit` (`frontend/` and `e2e/`) | clean | | `make css-check`, `make skill-check` | clean | Port 34115 was free before starting, so no other worktree's app was attached to. **Not run, and why:** no Go, no `.sql`, no `.templ` and no bound signature, so `make lint`, `make test`, `make generate` and `make bindings` do not apply. `make ui-visual` was not run — two attributes moved no geometry, and no baseline covers `queue-panel`. ## Deliberately not done - **The `title`s were not removed.** "Matching the close button" could be read as dropping them, but that would take the desktop tooltip off an icon-only control, which the issue does not ask about and which `aria-label` does not replace. - **No sweep for other weakly-named controls.** `title`-only naming elsewhere in the app is a real piece of work and is not this issue's surface. Nothing was found incidentally while here. `CLAUDE.md` gains a paragraph under the existing "0 unnamed is not it" rule, because the *technique* — strip the fallback and re-ask — is the reusable half. Closes #170
logan added 1 commit 2026-08-26 09:40:32 +00:00
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
b5bdba2f38
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
Author
Collaborator

CI is green. Run 18004, both required jobs, on first attempt.

job result
check success — commit messages, lint, test, frontend typecheck, CSS literals, component suite, bindings, skill-check
e2e success — both browser steps, E2E — chromium and E2E — webkit

The WebKit step is called out because it is the one that goes quiet: it carries if: ${{ !cancelled() }} precisely because a chromium failure used to skip it, and this repo has spent two sessions with no WebKit signal at all while a plan recorded it as covered. It ran here, so the two new assertions are checked against the renderer that actually ships as well as against Chromium.

Nothing to fix. Not merging — leaving it for a human.

**CI is green.** Run [18004](https://git.ljones.me/yonlu/yellowjacket/actions/runs/18004), both required jobs, on first attempt. | job | result | |---|---| | `check` | success — commit messages, lint, test, frontend typecheck, CSS literals, component suite, bindings, skill-check | | `e2e` | success — **both** browser steps, `E2E — chromium` and `E2E — webkit` | The WebKit step is called out because it is the one that goes quiet: it carries `if: ${{ !cancelled() }}` precisely because a chromium failure used to skip it, and this repo has spent two sessions with no WebKit signal at all while a plan recorded it as covered. It ran here, so the two new assertions are checked against the renderer that actually ships as well as against Chromium. Nothing to fix. Not merging — leaving it for a human.
logan merged commit d21b842d86 into main 2026-08-26 16:03:02 +00:00
Sign in to join this conversation.