Compare commits

..
Author SHA1 Message Date
logan e5d0f2714b test(ui): make ui-visual-update honour its file filter
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m27s
CI / e2e (pull_request) Successful in 9m27s
vitest parses a bare `--update` as taking the next positional as its
value, so `make ui-visual-update UI_ARGS=<path>` handed the path to the
flag and ran with no filter at all: 99 files, every baseline in the repo
re-recorded, any stale one blessed in silence. Two paths were worse
still — the first was eaten and only the second ran.

That is #196's own hazard living in the tool meant to resolve it: the
rule is "refresh the reference your change moved and never one you did
not cause", and the documented way to refresh one refreshed the set.

`--update=true` is the whole fix, with the reason beside it because
`=true` reads like something to tidy away. `make ui-visual` and
`make ui-test` are unaffected — their `$(UI_ARGS)` follows `run`, with
no flag to swallow it — and no other target interpolates a variable
after a boolean flag.

Closes #204
2026-08-23 04:36:46 -04:00
5 changed files with 7 additions and 164 deletions
-46
View File
@@ -4912,49 +4912,3 @@ bridge leaves the wizard up with its "Get Started" button correctly
disabled — it gates on a directory chosen *in the wizard*, and the
existing-library check runs once, on mount. A reload clears it. Nothing
is broken; it cost twenty minutes of believing a tap had been swallowed.
## The sheet's scroll fade, and where a scrim may not go (measured 2026-08-23, headless)
#207's answer. The affordance is two background layers on
`wa-dialog::part(body)` and the conditionality is
`background-attachment`, not a scroll listener: a cover of the sheet's
own colour painted at the end of the *content* (`local`) over a shadow
pinned to the box (`scroll`), so the cover scrolls up and hides the
shadow exactly when there is nothing more to see.
Measured at 424x360 (which is where a menu overflows on `main`, since
`main` does not yet carry #67's eighth item — at 424x439 the track
list's seven items are `scrollHeight` 364 against `clientHeight` 364,
fitting exactly). Pixel at x=300, dark ramp, `bgElevated` `#343a40`:
| y | before | more below | at the end of the list |
|---|---|---|---|
| 330 | 52,58,64 | 50,56,62 | 52,58,64 |
| 340 | 52,58,64 | 43,48,53 | 52,58,64 |
| 350 | 52,58,64 | 33,37,40 | 52,58,64 |
| 359 | 52,58,64 | 22,24,27 | 52,58,64 |
Three things worth keeping.
**A menu that fits draws nothing**, which is the same measurement: at
424x439 the sheet is flat 52,58,64 to its bottom edge, because with no
overflow the `local` layer's positioning area *is* the padding box and
the cover lands on top of the shadow.
**A scrim over a menu row is that row's text surface**, so the 4.5:1
rule reaches it and this is why the curve is steep rather than linear.
A row is 48px with its label centred; 32px of scrim already down to a
quarter strength at 14px puts about 0.06 at the label. Checked on the
light ramp (`bgElevated` `#e9ecef`, text `#212529`) by overriding the
two custom properties on `:root`: background at the label 205,207,210,
which is **9.9:1**. The first draft — a linear 48px at 0.8 — put ~0.375
on that label, 5.0:1, passing but visibly greyed. The bottom few pixels
go to ~2.4:1 in either draft and are deliberately below where any
label of a *fully visible* row sits; a label that lands there belongs
to the half-cut row, which is the thing being signalled.
**A dark scrim on a dark surface reads far worse in a shrunk screenshot
than on screen.** The first two probes (24px/0.45, then 32px/0.75) were
measurably present — 52,58,64 down to 30,33,37 — and invisible in the
inline preview. Crop the bottom 70px and scale it up before judging;
the pixel values are the honest answer either way.
+1 -20
View File
@@ -1402,7 +1402,7 @@ descendants. On the reference device the main panel spans 0-318 of a
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.
Seven things about it are load-bearing.
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
@@ -1431,25 +1431,6 @@ doing nothing, which reads as the gesture breaking. `menu-dismiss` is
that signal; the three surfaces that do not use `ContextMenuController`
bind it themselves.
**A sheet that scrolls says so, and `background-attachment` is what
asks whether it does** (#207). The sheet is capped at 85vh — a surface
covering the whole screen is a page, not a sheet — so a long menu's
body scrolls, and for three phases it scrolled *silently*: measured at
424x439, eight items ended at y=470 with the fold at 439, and where the
cut lands on a row boundary the sheet ends in a clean edge that reads
as the end of the list. The fade is two background layers on
`wa-dialog::part(body)` — a shadow pinned to the box (`scroll`) under a
cover of the sheet's own colour painted at the end of the *content*
(`local`), which scrolls up over the shadow exactly when there is
nothing more to see. So it is absent on a menu that fits, present the
moment one does not, and gone again at the end of the list, with no
scroll listener and nothing reaching into `wa-dialog`'s shadow root for
the scroller. **The curve is steep because the rows under it stay
live**: a scrim over a menu item is that item's text surface, and the
4.5:1 rule applies to it — 32px already down to a quarter strength at
14px spends its weight below the last legible label, measured at 9.9:1
on the light ramp, whose `bgElevated` is `#e9ecef`.
**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
+5 -2
View File
@@ -160,8 +160,11 @@ ui-watch: ## Same suite, in watch mode
ui-visual: ## Run the suite including screenshot comparisons
@cd frontend && YJ_VISUAL=1 npx vitest run $(UI_ARGS)
ui-visual-update: ## Re-record the screenshot baselines
@cd frontend && YJ_VISUAL=1 npx vitest run --update $(UI_ARGS)
# `--update=true`, never a bare `--update`: vitest takes the following
# positional as the flag's value, so `--update <path>` swallows the path
# and re-records every baseline in the repo instead of the one named.
ui-visual-update: ## Re-record the screenshot baselines (UI_ARGS=<path> to filter)
@cd frontend && YJ_VISUAL=1 npx vitest run --update=true $(UI_ARGS)
ui-setup: ## Install the Vitest browser provider's own Chromium (once)
@cd frontend && pnpm install && npx playwright install chromium
@@ -155,55 +155,10 @@ export class MenuSurface extends LitElement {
bottom was at y=452 on a 439px screen -- the one row a
destructive action is most likely to be. The cap has to stay
(a sheet covering the whole screen is a page, not a sheet),
so the body is what gives.
**And a body that scrolls says so** (#207). Scrolling was the
whole of the fix above, which left the last item reachable
and nothing on screen admitting it was there -- measured at
424x439, eight items ending at y=470 with the fold at 439,
and worse when the cut lands on a row boundary, where the
sheet ends in a clean edge that reads as the end of the list.
Two layers, and the *order* is what asks the question: a
shadow pinned to the bottom of the box (attachment scroll),
and over it a cover of the sheet's own colour painted at the
end of the *content* (attachment local), which therefore
scrolls up over the shadow and hides it exactly when there is
nothing more to see. So the affordance is absent on a menu
that fits, present the moment one does not, and gone again at
the end of the list -- with no scroll listener, no
measurement, and nothing reaching into wa-dialog's shadow
root for the scroller. background-attachment is Chrome 4;
the reference device is Chrome 113.
**The curve is steep because the rows under it stay live.**
A scrim over a menu item is that item's text surface, and
this app's rule is that text clears 4.5:1 on every surface it
can sit on -- which the light ramp, whose bgElevated is
#e9ecef, is what makes non-theoretical. A row is 48px with
its label centred, so 32px of scrim that is already down to
a quarter strength at 14px reaches y-centre at about 0.06 and
spends its weight on the strip below the last legible label.
Measured on the dark ramp at x=300, flat 52,58,64 throughout
before: 50,56,62 at y=330, 33,37,40 at y=350 and 22,24,27 at
the bottom edge, and flat again at the end of the list. The
light ramp puts 9.9:1 on the last label. */
so the body is what gives. */
wa-dialog::part(body) {
padding: 0;
overflow-y: auto;
background:
linear-gradient(
var(--yj-bg-elevated, #343a40),
var(--yj-bg-elevated, #343a40)
)
bottom / 100% 32px no-repeat local,
linear-gradient(
to top,
rgba(0, 0, 0, 0.6) 0%,
rgba(0, 0, 0, 0.25) 45%,
rgba(0, 0, 0, 0) 100%
)
bottom / 100% 32px no-repeat scroll;
}
/* A sheet is dragged at with a thumb, so it says where its top
@@ -206,56 +206,6 @@ describe('menu-surface', () => {
expect(dismissed, 'no menu-dismiss reached the document').toBe(1);
});
/**
* The scroll affordance (#207), and this is the mechanism again
* rather than the symptom.
*
* The sheet's body has scrolled since #60 and said nothing about
* it: measured at 424x439, eight items ended at y=470 with the
* fold at 439, and where the cut lands on a row boundary the sheet
* ends in a clean edge that reads as the end of the list.
*
* What makes the fade *conditional* — absent on a menu that fits,
* present the moment one does not, gone again at the end of the
* list — is `background-attachment`, not a scroll listener: a cover
* of the sheet's own colour is painted at the end of the content
* and attached `local`, over a shadow pinned to the box and
* attached `scroll`. So the pair of attachments *is* the feature,
* and it is what this asserts. The rendered result was measured in
* the harness (dark ramp 52,58,64 flat before; 52,57,63 at the last
* label and 22,24,27 at the bottom edge with more below; flat again
* at the end of the list) and is on the PR.
*/
it('paints the fade only while there is more below', async () => {
const el = await surfaceWithPanel();
const wrapper = el.shadowRoot?.querySelector('wa-dialog');
await (wrapper as HTMLElement & { updateComplete: Promise<unknown> })
.updateComplete;
const body = wrapper?.shadowRoot?.querySelector('[part~="body"]');
expect(body, 'no body part to scroll').not.toBeNull();
const style = getComputedStyle(body as Element);
expect(style.overflowY, 'the body is what gives, not the cap').toBe(
'auto',
);
// The cover scrolls with the content; the shadow does not. Either
// one alone is a fade that is always there or never there.
expect(
style.backgroundAttachment,
'the cover must be local and the shadow must not',
).toBe('local, scroll');
// Both sit at the bottom, or the cover hides nothing.
expect(style.backgroundPosition).toBe('50% 100%, 50% 100%');
expect(style.backgroundSize).toBe('100% 32px, 100% 32px');
});
/**
* A dialog with no accessible name is what `utils/name-dialog.ts`
* exists for; here the name is already written on the panel, so no