Collapse the page header actions that do not fit, instead of clipping them #136

Merged
logan merged 2 commits from fix/69-page-header-action-overflow into main 2026-08-19 19:29:15 +00:00
Collaborator

Closes #69, the last thing plan 018 owed.

page-header gains an actions API, and collapses what does not fit into
one "More actions" menu.

What was wrong

Playlists slotted a <div class="header-actions"> of three plain
<button>s — Import (91px), New Playlist (122px), New Smart Playlist
(162px), 390px — into a header that gets 700px at 900×600.
Measured on main at bb7dde1, before this branch:

viewport main panel header needs result
900×600 (queue closed) 700 747 New Smart Playlist: 114 of 162px
800×600 (enforced min) 744 747 New Smart Playlist: 158 of 162px
390×780 (phone) 390 747 all three unreachable — the original report

Plan 018's size matrix promises the opposite: no action is ever
unreachable at any supported size
. #24/PR #132 removed the queue's
contribution (open and closed are identical now, which is all it owed)
and correctly left this residual behind.

Commits

commit issue what
a1ee967 #69 docs: the actions rule in CLAUDE.md beside the sort rule; plan 018 → completed/ with its recap
f1c066d #69 fix(page-header): the actions API, the overflow rule, the three host migrations, the specs

The decision this rests on

page-header cannot collapse a slotted action, and that is a fact
about the API rather than an effort estimate: a component cannot move
another component's light-DOM children into a dropdown and keep their
behaviour, and arbitrary markup offers nothing generic to render as a
menu item. So a host declares PageAction[]
({id, label, icon, onSelect, priority, drop?}) and the header chooses
the rendering — the same division the sort control already lives by:
the header decides what fits, the host decides what happens.

The slot survives for markup a data list genuinely cannot express, at
the stated cost that a slotted action does not collapse and must
therefore fit at 800×600. All three hosts migrated anyway
(playlist-view, home-view, downloads-view), which normalises the
plain-<button>/<wa-button> split — and lets the header measure a
button that has already upgraded rather than a wa-button whose shadow
DOM arrives in its own first update.

Two findings that changed the shape mid-way

"Fits" is not "the header does not overflow". The title has to be
able to ellipsis for the 320px case, and the moment it can it absorbs
the pressure: scrollWidth reports a header that fits perfectly while
the heading reads "Playlis…". That is this bug moved from the button to
the title, invisible to the same measurement that missed it the first
time. The fit test counts the heading's own truncation, so an action
collapses before the title gives way.

The actions were not the only thing competing for the width. Every
child of that flex row was flex-shrink: 0, so whatever came last lost
— and the actions come last. At 320px the sort control alone is 172px,
so with all three actions already in the menu the menu button was
76px off the right edge. Below 600px the word "Sort:" is now
visually hidden (kept in the accessibility tree, since it is the
select's accessible name — hiding it outright is config-field's bug
one component over), and the title yields last.

Answers to #69's four open questions

  1. The slot survives, documented as not collapsible.
  2. A ResizeObserver, measuring the header's own overflow — the count
    and sort are flex-shrink: 0 and so are inside the budget.
  3. Not a bottom sheet, and not blocked on #60. #60's Direction is an
    <action-sheet> "selected by viewport width so every existing menu
    call site gains it without changing" — a phone-specific surface built
    here is either thrown away by #60 or becomes the second one it exists
    to prevent. This uses the shared MenuKeyboard pattern, so Android: context menus should be a bottom sheet, not a popover that gets clipped (#60)
    inherits it for free. Said so on #60.
  4. No phone mode for the actions. PHONE_COLUMN_IDS exists because
    the track list's columns cannot be derived from a width; these can.

After

Measured on this branch, Playlists, actions clipped at every band:

viewport buttons menu clipped title
1280×800 all three 0 ok
900×600 New Playlist Import, New Smart Playlist 0 ok
800×600 New Playlist, New Smart Playlist Import 0 ok
424×439 (the reference device) all three 0 ok
390×780 all three 0 ok
320×600 all three 0 truncated (last resort)

Verification actually run

  • make ui-test915 passed (80 files), up from 906. Nine new
    page-header cases; home-view.test.ts updated (its wa-button
    shadow query is now a header action).
  • make e2e136 passed, chromium, full suite.
    e2e/specs/header-action-overflow.spec.ts is new: 10 cases.
  • tsc --noEmit in both frontend/ and e2e/.
  • make css-check — 127 files, no broken literals.
  • Screenshots read at 900×600, 800×600, 390×780 plus Home and
    Downloads, and the menu open at both desktop and phone width.
  • No Go files touched, so make lint/make test are unaffected.

Two things the test tiers could not have caught, and now do

layout-overflow.spec.ts passes on the broken build. It asserts the
shell needs no sideways scrolling; clipping inside a component is
invisible to it, which is exactly why this defect survived a spec named
for it. The new spec measures each button against its own header, and
asserts buttons plus menu account for every declared action —
without that half it would pass vacuously on a build that renders no
actions at all, which is the shape the old build had.

The drop target had no test in any tier. New Playlist is a
dragover/dragleave/drop target, and it is the one behaviour this
migration could have destroyed silently — dragging still looks fine
against a button that no longer accepts anything. Both tiers cover it
now, end to end (highlight applies, drop opens the create form).

Two bugs found while doing it

  • My first detachOutsideClose() removed a document listener it had
    never added; view-lifecycle.test.ts caught it as held: -1. That
    accounting is what would hide a real leak in the other direction, so
    it is guarded now rather than made to balance.
  • onNewButtonDragLeave looked its own button up by
    .new-playlist-button in the host's shadow root. It uses
    e.currentTarget now — the button is in page-header's shadow root,
    so the selector would have found nothing and left the highlight stuck
    on.

Filed nothing new: nothing was tripped over that this PR did not fix.

Closes #69

Closes #69, the last thing plan 018 owed. `page-header` gains an actions API, and collapses what does not fit into one "More actions" menu. ## What was wrong Playlists slotted a `<div class="header-actions">` of three plain `<button>`s — Import (91px), New Playlist (122px), New Smart Playlist (162px), **390px** — into a header that gets **700px** at 900×600. Measured on `main` at `bb7dde1`, before this branch: | viewport | main panel | header needs | result | |---|---|---|---| | 900×600 (queue closed) | 700 | 747 | **New Smart Playlist: 114 of 162px** | | 800×600 (enforced min) | 744 | 747 | New Smart Playlist: 158 of 162px | | 390×780 (phone) | 390 | 747 | all three unreachable — the original report | Plan 018's size matrix promises the opposite: *no action is ever unreachable at any supported size*. #24/PR #132 removed the **queue's** contribution (open and closed are identical now, which is all it owed) and correctly left this residual behind. ## Commits | commit | issue | what | |---|---|---| | `a1ee967` | #69 | `docs:` the actions rule in CLAUDE.md beside the sort rule; plan 018 → `completed/` with its recap | | `f1c066d` | #69 | `fix(page-header):` the actions API, the overflow rule, the three host migrations, the specs | ## The decision this rests on **`page-header` cannot collapse a slotted action**, and that is a fact about the API rather than an effort estimate: a component cannot move another component's light-DOM children into a dropdown and keep their behaviour, and arbitrary markup offers nothing generic to render as a menu item. So a host declares `PageAction[]` (`{id, label, icon, onSelect, priority, drop?}`) and the header chooses the rendering — the same division the sort control already lives by: **the header decides what fits, the host decides what happens.** The slot survives for markup a data list genuinely cannot express, at the stated cost that **a slotted action does not collapse** and must therefore fit at 800×600. All three hosts migrated anyway (`playlist-view`, `home-view`, `downloads-view`), which normalises the plain-`<button>`/`<wa-button>` split — and lets the header measure a button that has already upgraded rather than a `wa-button` whose shadow DOM arrives in its own first update. ### Two findings that changed the shape mid-way **"Fits" is not "the header does not overflow".** The title has to be able to ellipsis for the 320px case, and the moment it can it *absorbs* the pressure: `scrollWidth` reports a header that fits perfectly while the heading reads "Playlis…". That is this bug moved from the button to the title, invisible to the same measurement that missed it the first time. The fit test counts the heading's own truncation, so an action collapses before the title gives way. **The actions were not the only thing competing for the width.** Every child of that flex row was `flex-shrink: 0`, so whatever came last lost — and the actions come last. At 320px the sort control alone is 172px, so with all three actions already in the menu the *menu button* was **76px off the right edge**. Below 600px the word "Sort:" is now visually hidden (kept in the accessibility tree, since it is the select's accessible name — hiding it outright is `config-field`'s bug one component over), and the title yields last. ### Answers to #69's four open questions 1. **The slot survives**, documented as not collapsible. 2. **A ResizeObserver**, measuring the header's own overflow — the count and sort are `flex-shrink: 0` and so are inside the budget. 3. **Not a bottom sheet, and not blocked on #60.** #60's Direction is an `<action-sheet>` "selected by viewport width so every existing menu call site gains it without changing" — a phone-specific surface built here is either thrown away by #60 or becomes the second one it exists to prevent. This uses the shared `MenuKeyboard` pattern, so #60 inherits it for free. Said so on #60. 4. **No phone mode for the actions.** `PHONE_COLUMN_IDS` exists because the track list's columns cannot be derived from a width; these can. ## After Measured on this branch, Playlists, actions clipped at every band: | viewport | buttons | menu | clipped | title | |---|---|---|---|---| | 1280×800 | all three | — | 0 | ok | | 900×600 | New Playlist | Import, New Smart Playlist | 0 | ok | | 800×600 | New Playlist, New Smart Playlist | Import | 0 | ok | | 424×439 (the reference device) | — | all three | 0 | ok | | 390×780 | — | all three | 0 | ok | | 320×600 | — | all three | 0 | truncated (last resort) | ## Verification actually run - `make ui-test` — **915 passed** (80 files), up from 906. Nine new `page-header` cases; `home-view.test.ts` updated (its `wa-button` shadow query is now a header action). - `make e2e` — **136 passed**, chromium, full suite. `e2e/specs/header-action-overflow.spec.ts` is new: **10 cases**. - `tsc --noEmit` in **both** `frontend/` and `e2e/`. - `make css-check` — 127 files, no broken literals. - **Screenshots read at 900×600, 800×600, 390×780** plus Home and Downloads, and the menu open at both desktop and phone width. - No Go files touched, so `make lint`/`make test` are unaffected. ### Two things the test tiers could not have caught, and now do **`layout-overflow.spec.ts` passes on the broken build.** It asserts the *shell* needs no sideways scrolling; clipping *inside* a component is invisible to it, which is exactly why this defect survived a spec named for it. The new spec measures each button against its own header, and asserts buttons **plus** menu account for every declared action — without that half it would pass vacuously on a build that renders no actions at all, which is the shape the old build had. **The drop target had no test in any tier.** `New Playlist` is a `dragover`/`dragleave`/`drop` target, and it is the one behaviour this migration could have destroyed silently — dragging still *looks* fine against a button that no longer accepts anything. Both tiers cover it now, end to end (highlight applies, drop opens the create form). ## Two bugs found while doing it - My first `detachOutsideClose()` removed a document listener it had never added; `view-lifecycle.test.ts` caught it as `held: -1`. That accounting is what would hide a real leak in the other direction, so it is guarded now rather than made to balance. - `onNewButtonDragLeave` looked its own button up by `.new-playlist-button` in the host's shadow root. It uses `e.currentTarget` now — the button is in `page-header`'s shadow root, so the selector would have found nothing and left the highlight stuck on. Filed nothing new: nothing was tripped over that this PR did not fix. Closes #69
logan added 2 commits 2026-08-19 19:09:37 +00:00
The `page-header` paragraph already stated "the header asks for a sort,
it does not perform one"; actions now follow the same division and it
belongs beside it — the header decides what fits, the host decides what
happens.

Plan 018 moves to completed/ because #69 was the last thing it owed:
its size matrix promised "no action is ever unreachable at any
supported size" and the residual 114/162px clip was that promise
outstanding. Its recap also corrects a claim the plan made — the queue
and the actions were not the only two things competing for the header's
width, since every child of that flex row was flex-shrink: 0 and the
actions come last.
fix(page-header): collapse the actions that do not fit into a menu
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m36s
CI / e2e (pull_request) Successful in 7m17s
f967916550
Playlists slotted three buttons totalling 390px into a header that gets
700px at 900x600, so "New Smart Playlist" rendered 114 of its 162px
with the queue closed, and 158 of 162 at the 800x600 enforced minimum.
On a phone none of the three could be reached at all, which is what the
Android report said. Plan 018's size matrix promises the opposite: no
action is ever unreachable at any supported size.

The header could not fix that for slotted markup, and that is a fact
about the API rather than an effort estimate — a component cannot move
another component's light-DOM children into a dropdown and keep their
behaviour, and arbitrary markup offers nothing generic to render as a
menu item. So a host passes `PageAction[]` and the header chooses the
rendering; the slot survives for markup a data list cannot express, at
the stated cost that a slotted action does not collapse.

All three hosts that slot actions migrated, which also normalises the
plain-<button>/<wa-button> split between them onto one shape the header
styles — and lets it measure a button that has already upgraded, rather
than a wa-button whose shadow DOM arrives in its own first update.

Four things in it are load-bearing:

- Every measuring pass starts from all-visible, so the collapsed set is
  a pure function of the current width and an action comes back when
  the window grows. It flips `hidden` imperatively rather than
  re-rendering between steps, or the intermediate state paints and the
  fix flashes the overflow it exists to prevent.
- "Fits" means nothing is clipped, not that the header does not
  overflow. Once the title can ellipsis it absorbs the pressure and
  scrollWidth reports a perfect fit while the heading reads "Playlis…"
  — this bug moved from the button to the title, and invisible to the
  same measurement that missed it the first time.
- New Playlist has the highest priority because it is the drop target
  and a closed menu cannot be one. `PageAction.drop` therefore carries
  the host's own handlers; the affordance is absent from the overflow
  rather than approximated there.
- The overflow trigger is a named button with aria-expanded and an
  aria-controls naming a panel that is always in the DOM, and the
  keyboard model is the shared `MenuKeyboard`.

`layout-overflow.spec.ts` passes on the broken build — it asserts the
shell needs no sideways scrolling, and clipping inside a component is
invisible to it, which is why this defect survived a spec named for it.
The new spec measures each button against its own header at four
viewports and asserts buttons plus menu account for every declared
action, without which it would pass vacuously on a build rendering none.

Closes #69
logan force-pushed fix/69-page-header-action-overflow from f1c066db6e to f967916550 2026-08-19 19:09:37 +00:00 Compare
Author
Collaborator

Rebased onto cceeb40 (main, after #139).

#139's note about a conflict with this PR was half right. Both touch
home-view.ts and the code does merge cleanly — #139 changes
static styles (the .play hover gate), this changes render() and
the imports. But it also predicted a conflict in .planning/NOTES.md,
and there is none: this PR does not touch that file. Its decisions went
to CLAUDE.md (the actions rule belongs beside the sort rule it
mirrors) and to plan 018's recap.

Verified on the merged tree before merging either PR, rather than
trusting the clean merge:

  • make ui-test921 passed (82 files)
  • make e2e137 passed (chromium)
  • make test and make lint — pass, all three build configurations
    (#139 touches backend/explore; this PR touches no Go)
  • make bindings-check, make skill-check — current
  • tsc --noEmit in frontend/ and e2e/; make css-check — 128 files

One incidental agreement worth recording. #139 adds
utils/breakpoints.ts with PHONE_QUERY = '(max-width: 599px)', for
the cases where "the decision is not a style" — track-list's computed
grid, now-playing's different content. This PR's phone rule (the
"Sort:" label) is a plain @media (max-width: 599px) in the component's
own stylesheet, which is precisely the case that file says needs nothing
from it. The two are consistent by that rule rather than by coincidence,
and the values match.

Rebased onto `cceeb40` (main, after #139). **#139's note about a conflict with this PR was half right.** Both touch `home-view.ts` and the code does merge cleanly — #139 changes `static styles` (the `.play` hover gate), this changes `render()` and the imports. But it also predicted a conflict in `.planning/NOTES.md`, and there is none: this PR does not touch that file. Its decisions went to `CLAUDE.md` (the actions rule belongs beside the sort rule it mirrors) and to plan 018's recap. **Verified on the merged tree before merging either PR**, rather than trusting the clean merge: - `make ui-test` — **921 passed** (82 files) - `make e2e` — **137 passed** (chromium) - `make test` and `make lint` — pass, all three build configurations (#139 touches `backend/explore`; this PR touches no Go) - `make bindings-check`, `make skill-check` — current - `tsc --noEmit` in `frontend/` and `e2e/`; `make css-check` — 128 files **One incidental agreement worth recording.** #139 adds `utils/breakpoints.ts` with `PHONE_QUERY = '(max-width: 599px)'`, for the cases where "the decision is not a style" — `track-list`'s computed grid, `now-playing`'s different *content*. This PR's phone rule (the "Sort:" label) is a plain `@media (max-width: 599px)` in the component's own stylesheet, which is precisely the case that file says needs nothing from it. The two are consistent by that rule rather than by coincidence, and the values match.
logan merged commit c84a9069ef into main 2026-08-19 19:29:15 +00:00
Sign in to join this conversation.