From ef5574d18ba3a5768aa83c68ad274dd2743e9df2 Mon Sep 17 00:00:00 2001 From: Logan Date: Fri, 21 Aug 2026 02:24:04 -0400 Subject: [PATCH] docs(shell): record the clip, and the four things only a device showed CLAUDE.md gains the surface beside the keyboard model it shares, and NOTES.md the measurements: the 83px clip with its screenshot, the probe that established a top-layer dialog escapes paint containment from inside a view, the UA stylesheet's 354px, the focus steal a longer retry cannot beat, and the submenu this change pushed off-screen before it pulled it back. The last of those is also a note about scope: the issue was claimed saying the submenu would be measured and filed, and the measurement said fix it. --- .planning/NOTES.md | 78 ++++++++++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 76 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 152 insertions(+), 2 deletions(-) diff --git a/.planning/NOTES.md b/.planning/NOTES.md index f7bfee7..b51bde7 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -4707,3 +4707,81 @@ measurement taken was of a screen with `job-band` on it and the art at route other than its own** (#175) — it was still up, full-screen and intercepting pointer events, after `AddLibrary` succeeded through the binding, and was gone after a relaunch. Filed. + +## The context menu was clipped on the device, and the fix needed four measurements nothing here could make (measured 2026-08-21, TLP301 / Chrome 113 / 424x439) + +#60 had been diagnosed from the Web Awesome source and was right. What +the device added was the numbers, and three things the reading had not +reached. + +**The clip, reproduced before any code was written.** Long-press on the +lowest visible track row at 424x439: + +| | | +|---|---| +| viewport | 424x439 | +| `.main-panel` | 0 to **318**, computed `contain: content` | +| menu panel | 191 to **401**, 210px tall | +| clipped away | **83px, three of seven items** | +| `wa-popup` computed position | `fixed` | +| `HTMLElement.prototype.hasOwnProperty('popover')` | **false** | +| row height | **29px** (against a 44px floor and a 48px ask) | + +A screenshot shows the menu sliced off flush with the mini player's top +edge. Both halves of the diagnosis are therefore measured, not inferred. + +**"A dialog escapes containment" was the premise, and it was untested.** +Every dialog in this app is mounted in `index.html`, *outside* +`.main-panel` — so nothing here was evidence about a dialog opened from +inside a view, which is what this change needed. A probe `` +appended to `track-list`'s shadow root and `showModal()`n paints to +y=439, over the mini player and the tab bar. A top-layer element's +containing block is the viewport, paint-contained ancestor or not. +Checking that first cost ten minutes and would have cost a rebuild. + +**The UA stylesheet is the thing that makes a naive sheet look wrong.** +That same probe came out **354px wide on a 424px screen**, centred, +because a native `` carries `max-width: calc(100% - 6px - 2em)` +and `margin: auto`. `max-width: none` and explicit margins are four +declarations that are pure undoing. + +**A retry loop cannot win against a steal that happens later.** +`MenuKeyboard` focuses the first item and returns as soon as it lands; +`wa-dialog` then focuses `[autofocus]` or *itself* on the frame after +`showModal()`, and it cannot see our first item to prefer it — the +panel is slotted through `menu-surface`, so the dialog's own +`querySelector` stops at the ``. Measured: the sheet opened with +`document.activeElement` on the `` and every arrow key went +nowhere. Lengthening the retry budget does not help, because the first +attempt *succeeds*. The surface announcing `menu-shown` after +`wa-after-show`, and the keyboard re-asserting, is the fix. + +**The submenu was made worse before it was made better, and only a +measurement caught it.** `#playlist-submenu` is a +`placement="right-start"` flyout anchored to its row. Making the menu a +full-width sheet moved that anchor to x=0, so the flip put the playlist +picker at **x −182 to 0 — entirely off-screen**, and "Add to Playlist" +led nowhere at all. Before the change the anchor row started at x≈245 +and the same flip landed it on screen. It is a `menu-surface` too now +and stacks as a second sheet. Two lessons: a change that moves an +anchor changes every flip decision downstream of it, and *the scope I +declared on the issue was wrong* — I had said I would measure the +submenu and file it, and the measurement said fix it. + +**And the sweep found two call sites the conversion missed.** Twelve +were converted by hand; `menu-surface.test.ts` reads every source file +and fails on a `` outside a three-file allowlist, which +immediately named `queue-panel`'s add-to-playlist popup (a real menu, +converted) and `now-playing`'s cover preview (a hover affordance in the +bottom bar — allowlisted, since a touch device never opens it and +nothing clips it). A thirteenth menu written as a bare popup would pass +every tier here and be clipped on the device, which is precisely why +the guard is a source sweep rather than a rendered assertion. + +**What no tier here can see remains the clip itself.** This runner's +Chromium and CI's WebKit both have the Popover API, so the popup is +top-layered and correct and a "not clipped" assertion passes on the +broken build. The specs assert the *mechanism* — that the surface is a +native `` at phone width — which is the same move +`queue-as-a-screen.spec.ts` makes about containment and for the same +reason. diff --git a/CLAUDE.md b/CLAUDE.md index 4af3651..87510e7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1367,8 +1367,17 @@ against the real components: `wa-dropdown-item` sets its `role` in its *own* first update, so a `[role^="menuitem"]` query at `updateComplete` finds nothing — which reads exactly like a menu that opened and refused to take focus. -- **`focus()` on a popup that has not positioned itself is a silent - no-op**, so the first focus is retried across a few frames. +- **`focus()` on a surface that has not shown itself is a silent + no-op**, so the first focus is retried on a *time* budget rather + than a frame count — the thing being waited for is another + component's animation. And a retry is not enough on its own for the + sheet below: `wa-dialog` focuses `[autofocus]` or *itself* on the + frame after `showModal()`, and it cannot see the first menu item to + prefer it, because the panel is slotted through `menu-surface` and + the dialog's own `querySelector` stops at the ``. The first + attempt therefore *succeeds* and is then overwritten, which no + amount of waiting fixes — so the surface announces `menu-shown` when + it has settled and `MenuKeyboard.refocus()` re-asserts. - **Focus is only taken back if the menu had it.** A click elsewhere closes the menu too, and pulling focus to the row the user right-clicked a moment ago is worse than leaving it. @@ -1376,6 +1385,69 @@ against the real components: moving focus without setting it leaves the highlight on whichever item the mouse last touched. +**And a menu is drawn where it fits: a popup on a desktop, a bottom +sheet on a phone** (#60). `components/menu-surface/` is that one +decision. The host renders the panel it always rendered and slots it +into whichever surface is up, so `ContextMenuController` still drives +`.active` and `.anchor` as though it were talking to a `wa-popup`, and +fourteen call sites changed one tag name each and nothing else. + +**It is a correctness fix, not a taste one, and the failure was +measured on the device rather than inferred.** Chrome 113 has no +Popover API, so `wa-popup` takes its own documented fallback and +positions with `strategy: "fixed"`; `.main-panel` carries +`contain: layout style paint`, and paint containment *clips* fixed +descendants. On the reference device the main panel spans 0-318 of a +439px viewport while the open menu spanned 191-401 — three of its seven +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. + +**"Dialogs are fine" needed checking, because every other dialog in +this app is mounted in `index.html`** — outside `.main-panel` — so it +was not evidence about one opened from inside a view. A probe dialog +appended to `track-list`'s shadow root paints to y=439, over the mini +player and the tab bar, with the contained ancestor still in place. A +top-layer element's containing block is the viewport, contained +ancestor or not. + +**The sheet has to un-do the UA stylesheet.** A native `` +carries `max-width: calc(100% - 6px - 2em)` and `margin: auto`, which +drew a 354px panel floating in the middle of a 424px screen. +`max-width: none` plus explicit margins is what makes it a sheet. + +**The row sizing lives in `contextMenuStyles`, not in the component.** +The panel is the *host's* light DOM — it stays in the host's shadow +root, so only the host's stylesheet can reach it. `menu-surface` puts +`data-sheet` on the panel and that shared stylesheet does the rest, +which is how fourteen menus went from 29px rows to 48px ones in one +edit. + +**A dismissal has to travel back.** `wa-dialog` closes itself on +Escape, which would leave the controller believing the menu is open — +and the failure mode is not a stuck sheet but the *next* long-press +doing nothing, which reads as the gesture breaking. `menu-dismiss` is +that signal; the three surfaces that do not use `ContextMenuController` +bind it themselves. + +**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 +Playlist" led nowhere. It stacks as a second sheet over the first, +which is also why `menu-shown` does not re-assert focus while the +submenu is open. + +**And which call sites exist is swept, not remembered.** A thirteenth +menu written as a bare `` works perfectly in every tier here +and is clipped on the device, so `menu-surface.test.ts` reads the +source and fails on one outside a three-file allowlist — +`menu-surface` itself, `job-indicator` (in `.top-bar`, which no +ancestor contains — the contrast that proved the diagnosis on #62) and +`now-playing`'s cover preview (a hover affordance, which a touch device +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