The top bar decides what it can afford to show #149

Merged
logan merged 2 commits from fix/143-top-bar-fits-its-window into main 2026-08-20 02:11:54 +00:00
Collaborator

Fixes the top bar overflowing its window at the bottom of the Compact
band (#143), by giving it the measured-fit treatment #69 gave
page-header.

Commits

commit what issue
ae85df0 services/top-bar-fit.ts, the collapse rules in index.css, job-indicator's [compact], and both specs #143
ead1354 the paragraph in CLAUDE.md beside the size bands, and two measured facts in NOTES.md #143

What was wrong

The report said 11px at 600x600. Re-measured with a job running it is
up to 262px, at every width from 600 to 899 and at 900
job-indicator is hidden when idle and 235px wide showing a real
library's scan title, and its label is capped at 12rem and gets there.
The idle number is what a seeded app that has finished scanning shows
you by the time you have resized it.

width idle with a long-titled scan
320, 390, 599 fits fits (the phone rules already drop the filter and the label)
600 611 862
700 / 800 fits 862
899 fits fits
900 fits 946
1100, 1440 fits fits

899 fitting while 900 does not is nav-history appearing — the worst
width for the header is not the narrowest one, the same way 900
rather than 800 is the worst width for the content area.

The four decisions

Measured, not breakpointed. Three of the bar's five children are as
wide as their content: the library filter is a <select> sized by the
longest library name, the indicator by the running job's title, the
search box by its view-scoped placeholder. Any width picked is right for
one library, one job and one view — and the sweep above shows a
breakpoint for "600 to 610" would have fixed whichever case happened to
be idle when it was measured.

Which child yields is decided by #24's own sentenceno action is
ever unreachable at any supported size
— which rules out the two
cheapest candidates in the Direction:

  • Hiding the library filter takes away an action. library-filter is
    the only caller of setSelectedLibrary in the app. That would
    have traded this promise for the same promise; filed as #148,
    because the phone rule already does it and its comment claims Settings
    covers it, which is false.
  • Collapsing search to an icon is #57's answer, and #57 is
    Status/Blocked behind #62. Building its modal here is building it
    without the thing that blocks it. Stated rather than skipped: it
    is the right eventual answer, so the yield order makes the search box
    the last thing that would ever go, and #57 inherits a position rather
    than an accident.

So the two that yield are the two that are not actions and whose
content survives elsewhere: the wordmark (a brand the window title
bar repeats, and which #48 wants down to "YJ" at every width anyway) and
then the job indicator's label, leaving the ring — not a new
judgement, since the component already drops it below 600px, and its
sr-only live region announces the state either way. The wordmark
yields its width, not its existence: the rule is visually-hidden
rather than display: none, because that h1 is the document's
top-level heading.

What "fits" means — and this is where #69's lesson earned its keep
twice. scrollWidth <= clientWidth is wrong here: scrollWidth counts
a box's left padding and not its right, so with 2em gutters it
under-reports by 32px. The first version read 700/700, a perfect fit,
with job-indicator ending 32px past the content edge and sitting in
the whole right gutter. The predicate is now the same one the spec
asserts — no in-flow child outside the parent's content box — and it was
found only because the spec measures per child rather than on the
container.

Verification

Pinned before fixed, as the Direction asks. On the unfixed build (the
install commented out) the new spec fails at 600 idle and at 600,
800 and 900 with a job
, and passes at 390, 899 and 1440;
layout-overflow.spec.ts gained 600x600 and failed there. 8 failures,
all in the measured places.

  • make lint — 0 issues, all three build configurations
  • make test — green, all three passes
  • make ui-test — 945 tests, 85 files
  • make e2e — 173 passed, and run twice against one
    make dev-headless
    , which is the cross-engine order condition CI's
    chromium-then-webkit pass reproduces
  • make css-check, make bindings-check, make skill-check
  • npx tsc --noEmit in both frontend/ and e2e/
  • Screenshots read at 600x600 and 900x600 with a long-titled scan
    staged, which is the half no assertion sees

One existing assertion changed and it is worth naming: no scrollbar appears at <size> now polls. Read once it passed alone and failed in
the full suite, because the fit is decided by a ResizeObserver and a
single read races it — the same reason the track-row assertion four
tests above it already polls.

Filed while here

  • #148 — the library filter is unreachable on a phone, and nothing
    else selects a library.

Closes #143

Fixes the top bar overflowing its window at the bottom of the Compact band (#143), by giving it the measured-fit treatment #69 gave `page-header`. ## Commits | commit | what | issue | |---|---|---| | `ae85df0` | `services/top-bar-fit.ts`, the collapse rules in `index.css`, `job-indicator`'s `[compact]`, and both specs | #143 | | `ead1354` | the paragraph in `CLAUDE.md` beside the size bands, and two measured facts in `NOTES.md` | #143 | ## What was wrong The report said 11px at 600x600. Re-measured with a job running it is up to **262px, at every width from 600 to 899 and at 900** — `job-indicator` is `hidden` when idle and **235px** wide showing a real library's scan title, and its label is capped at 12rem and gets there. The idle number is what a seeded app that has finished scanning shows you by the time you have resized it. | width | idle | with a long-titled scan | |---|---|---| | 320, 390, 599 | fits | fits (the phone rules already drop the filter and the label) | | 600 | 611 | **862** | | 700 / 800 | fits | 862 | | 899 | fits | fits | | 900 | fits | 946 | | 1100, 1440 | fits | fits | 899 fitting while 900 does not is `nav-history` appearing — the worst width for the *header* is not the narrowest one, the same way 900 rather than 800 is the worst width for the content area. ## The four decisions **Measured, not breakpointed.** Three of the bar's five children are as wide as their *content*: the library filter is a `<select>` sized by the longest library name, the indicator by the running job's title, the search box by its view-scoped placeholder. Any width picked is right for one library, one job and one view — and the sweep above shows a breakpoint for "600 to 610" would have fixed whichever case happened to be idle when it was measured. **Which child yields is decided by #24's own sentence** — *no action is ever unreachable at any supported size* — which rules out the two cheapest candidates in the Direction: - *Hiding the library filter* takes away an action. `library-filter` is the **only** caller of `setSelectedLibrary` in the app. That would have traded this promise for the same promise; filed as **#148**, because the phone rule already does it and its comment claims Settings covers it, which is false. - *Collapsing search to an icon* is #57's answer, and #57 is `Status/Blocked` behind #62. Building its modal here is building it without the thing that blocks it. **Stated rather than skipped**: it is the right eventual answer, so the yield order makes the search box the last thing that would ever go, and #57 inherits a position rather than an accident. So the two that yield are the two that are **not actions** and whose content survives elsewhere: the **wordmark** (a brand the window title bar repeats, and which #48 wants down to "YJ" at every width anyway) and then the **job indicator's label**, leaving the ring — not a new judgement, since the component already drops it below 600px, and its `sr-only` live region announces the state either way. The wordmark yields its *width*, not its existence: the rule is visually-hidden rather than `display: none`, because that `h1` is the document's top-level heading. **What "fits" means** — and this is where #69's lesson earned its keep twice. `scrollWidth <= clientWidth` is wrong here: `scrollWidth` counts a box's **left** padding and not its right, so with 2em gutters it under-reports by 32px. The first version read `700/700`, a perfect fit, with `job-indicator` ending 32px past the content edge and sitting in the whole right gutter. The predicate is now the same one the spec asserts — no in-flow child outside the parent's content box — and it was found only because the spec measures **per child** rather than on the container. ## Verification Pinned before fixed, as the Direction asks. On the unfixed build (the install commented out) the new spec fails at **600 idle** and at **600, 800 and 900 with a job**, and passes at 390, 899 and 1440; `layout-overflow.spec.ts` gained 600x600 and failed there. 8 failures, all in the measured places. - `make lint` — 0 issues, all three build configurations - `make test` — green, all three passes - `make ui-test` — 945 tests, 85 files - `make e2e` — 173 passed, and **run twice against one `make dev-headless`**, which is the cross-engine order condition CI's chromium-then-webkit pass reproduces - `make css-check`, `make bindings-check`, `make skill-check` - `npx tsc --noEmit` in **both** `frontend/` and `e2e/` - **Screenshots read** at 600x600 and 900x600 with a long-titled scan staged, which is the half no assertion sees One existing assertion changed and it is worth naming: `no scrollbar appears at <size>` now polls. Read once it passed alone and failed in the full suite, because the fit is decided by a ResizeObserver and a single read races it — the same reason the track-row assertion four tests above it already polls. ## Filed while here - **#148** — the library filter is unreachable on a phone, and nothing else selects a library. Closes #143
logan added 2 commits 2026-08-20 01:35:59 +00:00
The bar was 611px inside a 600px viewport at the bottom of the Compact
band, and 862px while a scan with a real library's title ran, because
`job-indicator` is `hidden` when idle and 235px wide when it is not.
`body` is `overflow-x: auto`, so a user got a horizontal scrollbar on a
shell #24 promised would not need one — and the band is 600 to 899 with
work in flight, not the 600 to 610 the idle measurement suggested.

`services/top-bar-fit.ts` is `page-header`'s treatment one bar up: a
ResizeObserver, every pass starting from all-visible, hiding the
lowest-priority child until it fits. Measured rather than breakpointed
because three of the five children are as wide as their content — the
library filter by the longest library name, the indicator by the
running job's title, the search box by its view-scoped placeholder — so
any width picked is right for one library, one job and one view.

What yields is decided by #24's own sentence, which rules out the two
cheapest candidates in the Direction. Hiding the library filter takes
away an action, since it is the only control in the app that selects a
library (filed as #148, which is the phone already doing it), and
collapsing search to an icon is #57's, which is blocked behind #62. So
the wordmark yields first — a brand the window title bar repeats, and
visually-hidden rather than `display: none` because that h1 is the
document's heading — and then the indicator's label, leaving the ring,
which the component already does below 600px and whose live region
announces the state either way.

"Fits" is the children against the content box, not `scrollWidth`
against `clientWidth`: `scrollWidth` counts the left padding and not
the right, so the first version read 700/700 with the indicator sitting
in the whole right gutter. And the bar does not resize when a job
starts, which is the case this is for, so every child is observed too.

Pinned before it was fixed, as the issue asks. On the unfixed build the
new spec fails at 600 idle and at 600, 800 and 900 with a job, and
passes at 390, 899 and 1440; `layout-overflow.spec.ts` gains 600x600
and failed there. That spec asserts on the *shell*, so it was green
throughout this defect — the per-child measurement is #69's lesson, and
it is what caught the gutter case above.

Closes #143
docs: record how the top bar decides what to drop
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m35s
CI / e2e (pull_request) Successful in 8m38s
ead1354e4d
The shell section already states the three size bands and the promise
that no action is unreachable at any of them; how the header chooses
what to give up belongs beside them, because the promise is what
decides it.

Two measured facts go to NOTES.md rather than here. `scrollWidth`
counts a box's left padding and not its right, so the obvious fit
predicate under-reports by a gutter and passed on a bar with a control
jammed against the window edge. And the overflow is 11px idle and 262px
while working, which is why the issue was filed twice with different
numbers — a seeded app that has finished scanning is idle by the time
you resize it.

Closes #143
Author
Collaborator

CI: green. check passed first time; e2e failed on attempt 1 and
passed on attempt 2 with no change to the commit — a re-run of the
same run id. The failure was phone-shell.spec.ts:115, on both engines,
with open-now-playing refused for the full 30s because the cover art's
placeholder icon was under the pointer.

It is not this branch's: nothing here touches now-playing or the
bottom bar, it does not reproduce locally (10/10 alone, three green full
suites, on a freshly rebuilt seed as well as an old one), and the same
commit is green on the re-run. Filed as #150 with the log, the
elementFromPoint check that rules out the obvious stacking
explanation, and the two remaining candidates.

Also filed from this work: #148 — the library filter is unreachable
on a phone and nothing else selects a library, which is why this PR
declines to hide it.

**CI: green.** `check` passed first time; `e2e` failed on attempt 1 and passed on attempt 2 with **no change to the commit** — a re-run of the same run id. The failure was `phone-shell.spec.ts:115`, on both engines, with `open-now-playing` refused for the full 30s because the cover art's placeholder icon was under the pointer. It is not this branch's: nothing here touches `now-playing` or the bottom bar, it does not reproduce locally (10/10 alone, three green full suites, on a freshly rebuilt seed as well as an old one), and the same commit is green on the re-run. Filed as **#150** with the log, the `elementFromPoint` check that rules out the obvious stacking explanation, and the two remaining candidates. Also filed from this work: **#148** — the library filter is unreachable on a phone and nothing else selects a library, which is why this PR declines to hide it.
logan merged commit bb21072386 into main 2026-08-20 02:11:54 +00:00
Sign in to join this conversation.