R8a: fix the app bar overflowing off-screen below ~1120px window width - #172
Merged
Conversation
Below roughly 1120px, AppBar's 13 controls (12 buttons plus a dead
provider-mode select) had no shrink/wrap/overflow handling at all. The
flex toolbar could never shrink below its own min-content width, so it
forced the whole document wider than the window - Settings, About, Help
and the connection status indicator ran off the right edge, and a
horizontal scrollbar appeared across the ENTIRE app, dragging the canvas
and composer sideways with it.
Also removes the provider-mode <select>: permanently disabled, exactly
one hardcoded option ("Ollama (Local)"), a literal no-op onChange, and no
setProviderMode intent anywhere in backend/ for it to call. Settings' own
provider pages are the real, complete switcher.
Fixed with a real overflow menu, not a bare overflow-x: auto stopgap.
Every collapsible button renders twice - once inline, once duplicated
inside an overflow menu - tagged with a shared data-tier. Pure CSS
@container queries (container-type: inline-size on .appbar) decide which
copy is visible at the current width, in three tiers: least-used
(About/Help/Export PNG) collapses first, then Pins/Organize/View/Plugins,
then the four zoom/fit controls. Library, Save and Settings never
collapse - those three going unreachable was the actual bug. No
ResizeObserver or width-measurement JS: container queries are
declarative, and this app targets one Chromium engine (WebView2), so
there's no compatibility reason to reach for JS instead.
Two real architectural mistakes were caught and corrected before this
shipped, both through independent review rather than by inspection alone:
- A NodeMenu-based (portal-to-body) version of the overflow menu was
built first and reverted: a portaled element is no longer a DOM
descendant of .appbar, so it falls outside the @container scope
entirely - every item would have stayed hidden forever regardless of
width. Switched to the shared Popover component instead, which stays in
the DOM subtree the container query needs.
- The Popover version then needed its own clipping fix, which introduced
a worse bug: overflow-x: hidden on the header, paired with an explicit
overflow-y: visible intended to protect the dropdown, does not do what
it says. Per the CSS overflow spec, when one axis computes to anything
other than visible, a visible value on the other axis is force-coerced
to auto instead - it cannot stay visible regardless of what was
authored. The menu was completely clipped and non-interactive at every
width it was supposed to appear, confirmed live via getComputedStyle,
scrollHeight/clientHeight, and elementFromPoint hit-testing - a defect
invisible to a JS-driven .click() call, which fires a handler
regardless of whether the element is actually paintable or hit-testable
at its screen position. There is no ancestor position for a horizontal-
only backstop that avoids this, since the popover must stay inside the
container-query subtree and therefore inside any such ancestor too. The
real backstop is the tier breakpoints being correctly margined instead
of a defensive CSS property - measured live, the four buttons that
never collapse need under 210px, well under the narrowest breakpoint's
420px floor.
That same review pass also caught: aria-haspopup="true" (menu shorthand)
on a trigger that actually opens role="dialog"; an undocumented "Reset"
vs "Reset Zoom" label split between a button's two copies that the test
suite's own comment incorrectly claimed didn't exist; the overflow
menu's Export PNG duplicate missing its close-the-menu call, contradicted
by the file's own doc comment, caught because the one action a test had
picked to represent "every plain action closes the menu" happened to be
a different, correct one; and the overlay-opening overflow duplicates
(Pins/View/Plugins/About/Help) not reflecting real open state the way
their inline counterparts do, unlike what the file's own doc comment
promised. All fixed; the every-plain-action gap is now closed with a
parameterized test over all six actions instead of one representative.
Verified independently: full backend suite (1033, unaffected - frontend-
only change), frontend typecheck/lint/tests/build (859 tests, 0 lint
errors), and a live browser check across the full range from 1440px down
to 500px (well below the app's documented 960px minimum) - real
coordinate-based hit-testing (elementFromPoint, not element.click()) at
every tier confirmed every overflow item is genuinely clickable, not just
present in the DOM; Library/Save/Settings never disappeared at any
tested width; document-level horizontal overflow never occurred; and a
synthetic pointer-event sequence dispatched at real screen coordinates
correctly resolved to and activated the intended button.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the app bar's toolbar overflowing off-screen below roughly 1120px window width, and removes a permanently-disabled provider-mode control.
Problem
Below ~1120px, AppBar's 13 controls (12 buttons plus a dead provider-mode select) had no shrink, wrap, or overflow handling. The flex toolbar could never shrink below its own min-content width, forcing the whole document wider than the window. Settings, About, Help, and the connection status indicator ran off the right edge, and a horizontal scrollbar appeared across the entire application, dragging the canvas and composer sideways with it.
The provider-mode
<select>was also removed: permanently disabled, exactly one hardcoded option ("Ollama (Local)"), a literal no-oponChange, and nosetProviderModeintent anywhere inbackend/for it to call. Settings' own provider pages are the real, complete switcher.Changes
A real overflow menu, not a bare
overflow-x: autostopgap. Every collapsible button renders twice — once inline, once duplicated inside an overflow menu — tagged with a shareddata-tier. Pure CSS@containerqueries (container-type: inline-sizeon.appbar) decide which copy is visible at the current width, across three tiers: least-used (About/Help/Export PNG) collapses first, then Pins/Organize/View/Plugins, then the four zoom/fit controls. Library, Save, and Settings never collapse — those three becoming unreachable was the actual bug. NoResizeObserveror width-measurement JS: container queries are declarative, and this app targets a single Chromium engine (WebView2), so there's no compatibility reason to reach for JS instead.Two architectural mistakes caught and corrected pre-merge
Both found through independent review rather than by inspection alone:
A portal-based version was built first and reverted. A
NodeMenu-style implementation (portal todocument.body) was tried first; a portaled element is no longer a DOM descendant of.appbar, so it falls outside the@containerscope entirely — every item would have stayed hidden forever regardless of width. Switched to the sharedPopovercomponent, which stays inside the DOM subtree the container query needs.The resulting clipping fix introduced a worse bug.
overflow-x: hiddenon the header, paired with an explicitoverflow-y: visibleintended to protect the dropdown, does not do what it says. Per the CSS overflow spec, when one axis computes to anything other thanvisible, avisiblevalue on the other axis is force-coerced toauto— it cannot stayvisibleregardless of what was authored. The menu was completely clipped and non-interactive at every width it was supposed to appear, confirmed live viagetComputedStyle,scrollHeight/clientHeight, andelementFromPointhit-testing — a defect invisible to a JS-driven.click()call, which fires a handler regardless of whether the element is actually paintable or hit-testable at its screen position.There is no ancestor position for a horizontal-only backstop that avoids this, since the popover must stay inside the container-query subtree and therefore inside any such ancestor too. The real backstop is the tier breakpoints being correctly margined instead of a defensive CSS property — measured live, the four buttons that never collapse need under 210px, well under the narrowest breakpoint's 420px floor.
Additional fixes from review
aria-haspopup="true"(the"menu"shorthand) on a trigger that actually opensrole="dialog"— corrected to"dialog"."Reset"vs"Reset Zoom"label split between a button's two copies, which the test suite's own comment incorrectly claimed didn't exist — labels now match.Verification
tsc --noEmit/ ESLint / tests / buildelementFromPoint, notelement.click()) at every tierRelated
Closes UI/UX issue list findings #5 (app bar overflow) and #8 (dead provider-mode select).