fix(ui): the graph gets the 60% of the window it always asked for - #77
Merged
Conversation
`UI_SPEC`'s first principle is that the graph is the application — "centre of the window and the most pixels" — and `screen::repository` has asked for that since M1 with `FillPortion(6)` against the detail pane's `FillPortion(4)`. It never got it. Measured against the real layout pass, the graph was 145px and the detail 581px in a 727px column: 20/80, not 60/40. The cause is an asymmetry between the two panes. `ring` wraps its child in a container to draw the focus border, and a container with no height is `Shrink` in iced, so a `FillPortion` inside one never reaches the column that is meant to divide the space. The graph's portion sat inside its ring; the detail's sat on a container outside its own. That left the detail as the only child the column could see asking for a share, so it took everything the shrunk graph did not use. Moving the graph's portion outside its ring gives 436/291 — the ratio the code has been requesting all along. Pinned by a layout assertion rather than a text one, because no text assertion could ever have caught this: the graph is a canvas and carries no text, so the test measures the pane *below* it. `Target::Text` carries its `Rectangle` and `Bounded::bounds` is public, so the detail's centred placeholder can be found and its midpoint checked against the viewport. Sabotage puts the portion back inside the ring and the test reports `midpoint is 477.44 of 768`. Worth recording for the rest of this work: `Simulator::find` stops at the first match, but a closure selector that always returns `None` while pushing into a shared vector visits the whole tree and yields every container rectangle. That is how the 145/581 above was measured, and it is the general tool for asserting layout in this crate. The four committed screenshots are retaken, since this changes what all of them show — the graph goes from seven visible rows to nineteen.
youhide
force-pushed
the
d1-graph-gets-its-space
branch
from
August 21, 2026 01:04
5beb143 to
f093ea6
Compare
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.
First item of the UI refinement plan, and a defect rather than a preference.
The claim that was not true
UI_SPEC.md's first principle is that the graph is the application — "the centre of the window and the most pixels" — andscreen::repositoryhas asked for that since M1 withFillPortion(6)against the detail pane'sFillPortion(4).Measured against the real layout pass: graph 145px, detail 581px in a 727px column. 20/80, not 60/40. The primary surface was a fifth of the window for five milestones.
Why, and why the first hypothesis was wrong
I guessed the root
column!wasShrink. It was not — adding.height(Fill)there changed nothing, and measurement showed the content area already filled the window aty=41 h=727.The real cause is an asymmetry between the two panes:
ring()wraps its child in a container to draw the focus border. A container with no height isShrinkin iced, and aFillPortioninside aShrinknever reaches the column meant to divide the space.Moving the graph's portion outside its ring gives 436/291 — exactly the 6:4 the code always requested.
How it is pinned
No text assertion could have caught this, which is why nothing did: the graph is a canvas and carries no text. The test measures the pane below it instead —
Target::Textcarries itsRectangleandBounded::boundsis public, so the detail's centred placeholder is found and its midpoint checked against the viewport.Sabotage: putting the portion back inside the ring fails with
the detail pane's midpoint is 477.44 of 768, so it is taking the space the graph asked for.A technique worth reusing
Simulator::findstops at the first match, but a closure selector that always returnsNonewhile pushing into a shared vector visits the whole tree and yields every container rectangle. That is how145/581was measured. It is the general tool for asserting layout in this crate, and it should pin the resizable-split work later in the plan.Screenshots
All four retaken — this changes what every one of them shows. The graph goes from seven visible rows to nineteen.
graph.pngmainstaging.pnglight-theme.pnghidegit-lightinto a throwawayconfig.tomlrather than driving the UIcommand-palette.pngTaken the way
docs/screenshots/README.mddocuments: throwawayHOME,HIDEGIT_NO_KEYCHAIN=1, captured by window id.Docs
UI_SPEC.mdprinciple 1 now states the ratio concretely — six parts graph to four parts detail — and records that it is asserted rather than merely asked for, with a note on why it went unnoticed for five milestones.Gate
cargo fmt --all -- --check,cargo clippy --workspace --all-targets --all-features --locked -- -D warnings,cargo test --workspace --all-features --locked— 890 tests, all green.