Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
8c85db8968 | ||
|
|
02e2251bb2 |
@@ -158,7 +158,7 @@ only climb when it cannot.
|
||||
| You changed | Run | Cost |
|
||||
|---|---|---|
|
||||
| A Lit component, a store, the shortcut service | `make ui-test` | ~2 s, no app |
|
||||
| …and it renders differently | `make ui-visual` | + 10 baselines, opt-in, never gates |
|
||||
| …and it renders differently | `make ui-visual` | + 6 baselines, opt-in |
|
||||
| Any Go code | `make test` | 3 passes, ~2 min |
|
||||
| A service that emits events | `make test` — assert on the payload, see `backend/queue/emit_test.go` | in-process, no app |
|
||||
| A bound method or a bound struct field | `make bindings` then `make ui-test` | ~1.5 s + 2 s |
|
||||
@@ -180,13 +180,6 @@ less than it looks.)
|
||||
|
||||
Two rules about climbing:
|
||||
|
||||
- **If you moved a component's geometry, run `make ui-visual` and
|
||||
refresh that component's baseline in the same commit.** Nothing else
|
||||
will: it is the one tier in this repo no hook and no CI job runs, and
|
||||
it cannot be one — its references are machine-specific, measured in
|
||||
[references/ui-tier.md](references/ui-tier.md). Four of them drifted
|
||||
across three merges before anyone noticed (#196). Read the image;
|
||||
never bless a reference you did not cause.
|
||||
- **A component test passing is not the app rendering.** If you touched
|
||||
anything in `frontend/src`, verify it in the real app too — start it
|
||||
headless, `screenshot --filename=/tmp/shot.png`, and *read the PNG*.
|
||||
|
||||
@@ -78,58 +78,9 @@ synchronously.
|
||||
Microtasks and not a timer, deliberately: a timer hangs forever under
|
||||
the suites that install fake ones.
|
||||
|
||||
## The visual tier does not gate, and that is measured (#196)
|
||||
|
||||
`make ui-visual` is the same suite with nine `toMatchScreenshot`
|
||||
baselines switched on. **Nothing runs it but a person**, deliberately,
|
||||
and the reason is a number rather than a preference: the committed
|
||||
baselines were recorded on Arch, and replayed in a bare `ubuntu:24.04`
|
||||
container — CI's `check` image — three of them fail for reasons that
|
||||
have nothing to do with any component.
|
||||
|
||||
| baseline | Arch | ubuntu:24.04 |
|
||||
|---|---|---|
|
||||
| `page-header` filtered-by-search | passes | ratio 0.03 differ, against a 0.02 allowance |
|
||||
| `track-info` | passes | ratio 0.03 differ |
|
||||
| `seek-bar` | 1152×18 | 1152×17 |
|
||||
|
||||
The two references that were genuinely stale did not even agree about
|
||||
their *new* size — `now-playing` renders 1152×65 on Arch and 1152×64 in
|
||||
the container. So moving CI's `check` job from `make ui-test` to
|
||||
`make ui-visual` is not a one-line change: it needs a second,
|
||||
container-recorded baseline set, which every local run would then fail
|
||||
against. That is the same trap the other way round, and a pre-push hook
|
||||
is the same fault again — one machine's baselines against everybody
|
||||
else's renderer.
|
||||
|
||||
So the tier stays local and opt-in, and the rule that replaces the gate
|
||||
is:
|
||||
|
||||
- **A change that moves a component's geometry refreshes that
|
||||
component's reference in the same commit, having read the image.**
|
||||
Look at the PNG; the dimensions in the failure message are the cheap
|
||||
half of the answer.
|
||||
- **Never refresh a reference you did not cause.** #196 exists because
|
||||
four of them drifted across three unrelated merges, and every red run
|
||||
made the next person likelier to stop running the tier than to read
|
||||
it.
|
||||
- **State the world the shot is taken in.** The stores are singletons,
|
||||
so a visual case that sets nothing photographs whatever the previous
|
||||
case left behind — which is how the sidebar's baseline came to have
|
||||
Tracks lit and `now-playing`'s to be playing from a dynamic mix.
|
||||
- **`make ui-visual-update UI_ARGS=<path>` does not filter** and
|
||||
re-records *every* baseline, blessing any stale one in silence:
|
||||
vitest's `--update` takes the following positional as its value. Until
|
||||
#204 lands, record one file with
|
||||
`cd frontend && YJ_VISUAL=1 npx vitest run --update=true <path>`, and
|
||||
check `git status` before committing either way.
|
||||
|
||||
What the tier is worth, for the record: it is a *layout* check, blind to
|
||||
colour (the component tier has no `:root`, so it renders the fallbacks —
|
||||
`make ui-visual` passed unchanged through a whole palette rewrite,
|
||||
twice), and it has caught one thing nothing else could — swapping
|
||||
`library-status-indicator`'s `<button>` for a `<span>` lost the UA
|
||||
stylesheet's `box-sizing` and grew the badge 36→38px.
|
||||
Visual baselines are font-hinting and compositing sensitive, which is
|
||||
why they are opt-in: they only mean anything on the machine that
|
||||
recorded them.
|
||||
|
||||
## Bindings
|
||||
|
||||
|
||||
@@ -4912,3 +4912,49 @@ 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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
Six things about it are load-bearing.
|
||||
Seven 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,6 +1431,25 @@ 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
|
||||
@@ -3424,25 +3443,6 @@ Pre-commit hooks verify generated code is fresh — always run `make generate` a
|
||||
Tests use `database.NewTestDB(t)` for in-memory SQLite, built by the same
|
||||
`applySchema` production uses so the two cannot diverge. Test audio fixtures live in `test_data/music_library_test/`. Table-driven tests are the norm.
|
||||
|
||||
**`make ui-visual` is the one tier nothing but a person runs, and it
|
||||
cannot become one.** Its ten `toMatchScreenshot` baselines were recorded
|
||||
on a developer's Arch box; replayed in a bare `ubuntu:24.04` container
|
||||
— CI's `check` image — three of them fail on font metrics and
|
||||
compositing alone (`track-info` and one `page-header` shot at a 0.03
|
||||
mismatch ratio against a 0.02 allowance, `seek-bar` one pixel shorter),
|
||||
and two components disagree about their own height between the two
|
||||
machines. So CI keeps running `make ui-test`, which is the same suite
|
||||
with the comparisons off, and a pre-push hook would be the same fault
|
||||
with the machines swapped. What replaces the gate is a rule, in
|
||||
`.pi/skills/yellowjacket-dev/references/ui-tier.md`: **a change that
|
||||
moves a component's geometry refreshes that component's baseline in the
|
||||
same commit, having read the image, and never one it did not cause**.
|
||||
That is #196, which was four stale references accumulated across three
|
||||
unrelated merges — a red tier nobody could read, which is how it stayed
|
||||
red. A visual case must also **state the world it photographs**, since
|
||||
the stores are singletons and a case that sets nothing records whatever
|
||||
the previous one left in them.
|
||||
|
||||
## Git Workflow
|
||||
|
||||
Feature branches and PRs are the only way in: **`main` is a protected
|
||||
|
||||
@@ -155,10 +155,55 @@ 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. */
|
||||
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. */
|
||||
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
|
||||
|
||||
BIN
Binary file not shown.
|
Before Width: | Height: | Size: 14 KiB After Width: | Height: | Size: 15 KiB |
BIN
Binary file not shown.
|
Before Width: | Height: | Size: 6.7 KiB After Width: | Height: | Size: 4.4 KiB |
@@ -137,11 +137,6 @@ describe('<app-sidebar>', () => {
|
||||
});
|
||||
|
||||
it('looks the way it did last time', async () => {
|
||||
// Stated rather than inherited: `activeViewStore` is a singleton, so
|
||||
// without this the shot records whichever view the *previous* case
|
||||
// left in it and the reference moves when the file is reordered.
|
||||
activeViewStore.setView('home', true);
|
||||
|
||||
const el = await fixture('app-sidebar');
|
||||
|
||||
await visual(el, 'app-sidebar');
|
||||
|
||||
@@ -206,6 +206,56 @@ 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
|
||||
|
||||
@@ -366,16 +366,6 @@ describe('<now-playing>', () => {
|
||||
const el = await fixture('now-playing');
|
||||
|
||||
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 6 });
|
||||
// Stated rather than inherited: the queue store is a singleton, so
|
||||
// without this the shot records whichever source the *previous*
|
||||
// case left in it and the reference moves when the file is
|
||||
// reordered. Three lines is what the bar renders while playing
|
||||
// from somewhere, which is the arrangement worth recording.
|
||||
setQueue([queueTrack(1, 'Ashes to Ashes')], 0, {
|
||||
type: 'album',
|
||||
id: 7,
|
||||
label: 'Scary Monsters',
|
||||
});
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user