Keep Tool and browser controls usable in narrow panes - #675
Conversation
The browser header now keeps a quantized tier in state instead of a raw width, through a shared `useHeaderTier` hook, so a sash drag or tween no longer re-renders the whole header every frame. The overflow popover follows the pane-header dismissal contract (`useDismissOverlay`): a press in another pane's header now dismisses it, a press on its trigger toggles it, and the trigger anchors it by ref instead of a cached rect. It reuses the popover height token, `chromeButton`, `POPOVER_FOCUSABLE_SELECTOR`, and a shared note-count phrase. Tests split the one long popover case into one per rule, share a driveable ResizeObserver stub with the terminal header suite, and drop assertions the pure format function already pins. The width rule moves into layout.md's responsive-sizing section as a table beside the terminal tiers; the rationale lead no longer states the superseded approach. The QC fixture is allowlisted in the loopback lint, which it had been failing, and named from the QC doc that runs it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Deploying mouseterm with
|
| Latest commit: |
229c08a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d978d8bb.mouseterm.pages.dev |
| Branch Preview URL: | https://dor-tool-qc.mouseterm.pages.dev |
# Conflicts: # lib/src/host/tool-open.test.ts
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress, not a merge verdict — mark the PR ready when you want the full review.
The header rework reads well and the popover's dismissal contract holds up: the trigger's pointerdown/mousedown stops keep the window-level listeners in use-dismiss-overlay and use-popover-focus-trap from dismiss-then-reopening, the capture-phase Escape wins over the bubble-phase one so a single close runs, and the deferred onClose correctly leaves focus with a modal the action opened. Two notes inline, both about where the tiering rule now lives, plus one question on the QC report.
docs/testing/dor-tool-qc.md — durable doc or run log? Worth settling before this lands. The "Harness and isolation" section and the fixture's flags are reusable, but the rest records one run: a baseline table the file itself marks superseded, and evidence reachable only from an ignored local directory (pdf-zoom.png, approval-error.png, final-before-reload.json), including a development-state reset whose trigger the file says was never captured. AGENTS.md routes evidence to <foo>.rationale.md, and the two findings that matter are already in layout.rationale.md and dor-tool.rationale.md. docs/testing/ is also a doc category nothing in AGENTS.md indexes, and scripts/dor-tool-qc/server.mjs now carries a standing loopback-lint allowlist entry to support it. Splitting the harness recipe from the run log — or keeping only the recipe — would leave a file the next reader can act on.
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
One note inline, on the content-box mode that sharing the tier hook needed.
Holding the verdict on the merge order you stated rather than on the code: the
description gates this behind #670 and its predecessors, which are all still
open. That same paragraph still reads "This PR remains a draft", which the
un-drafted PR now contradicts — worth rewriting so the stacking order is
plainly the only thing holding it.
|
@dormouse-bot The stack order is a merge-order instruction only. Please assess this PR's diff against #670 on its own merits; the parents remain open so the maintainer can merge the reviewed stack bottom-up. The current description no longer calls this a draft. The tiny-header issue is fixed in |
|
All three of my earlier notes are closed at
The PDF policy holds end to end: One nit, not a blocker: the 13px is now a constant in a comment. Nothing fails if This is a read of the diff against #670, not a local run — |
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Assessed against #670 on its own merits; the stack order isn't holding anything.
The tiny-header fix in 0ef0c804 is right: >293/>173 border-box picks the
same pixels as the old >280/>160 content-box for this header's pl-2 pr-[5px], positive border-box width now separates an 8px visible leaf from a
hidden one, and the getBoundingClientRect fallback keeps the observer
border-box on engines without borderBoxSize (Safari 15.0–15.3), where the old
contentRect fallback would have been 13px short.
One finding on the new popover, inline.
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
The reclamp closes the stale-clamp thread: positionPopover re-runs on every border-box change of the popover, and clampOverlayPosition returns only position/left/top, so the new position can't feed back into the observed size.
One correction to the reply on that thread — the new test does not verify observer cleanup. The assertion meant to pin it can't fail; fix inline. One note on the spec line too.
2 participants
Innerdogfood testing found browser and Tool controls overflowing narrow panes, and PDFs failing inside the sandboxed iframe. Header controls now use the pane width and an overflow popup. Hiding a Workspace or parking a pane dismisses the popup without focusing the hidden header. PDFs require a configured user Tool, including source-like filenames such as
README.pdf.Terminal and browser headers share border-box observation, synchronous initial measurement, and a hidden-zero-width guard. Terminal thresholds account for its padding, preserving the same control transitions while correctly handling tiny visible panes. The reusable harness recipe is in
docs/testing/dor-tool-qc.md, with its fixture inscripts/dor-tool-qc/server.mjs. Historical observations and verification limits are recorded in the Tool and layout rationale files.Validation: Claude simplification and independent review are complete. The QC pass checked 210 library and 26 CLI tests; parent integration checked 32 host/header tests; the final shared-header follow-up checked 31 header tests, library TypeScript, spec lint/self-tests, loopback lint/self-tests, and diff checks. Current-head CI and Chromatic run on this ready PR; visual approval is handled by the maintainer.
Tool stack 6/7, based on #670 (
dor-file-viewer). Next: #676. Merge bottom-up: #493 → #514 → #668 → #669 → #670 → #675 → #676.