Repository navigation
fix(ui): text nobody chose the length of scrolls instead of being cut off - #88
Merged
Merged
Conversation
Three numbers decided how the main window was divided: a 230px sidebar, a 6:4 graph against the detail pane, and a 280px file list inside the working directory. Each was a guess about a window nobody had seen, and no window could argue with it — a long commit message on a laptop screen had four tenths of the height and no way to ask for more. Every divider between those panes is draggable now, the file list included. ## Two units, and the reason for each The sidebar and the file list are pixels: what they need is set by the length of a branch name or a path, and that does not grow because the window did. The graph and the detail pane are a fraction of the height they share, because they are two halves of one view — a taller window should keep the graph the subject and the commit under it the footnote. Nothing can be dragged out of existence. The pixel splits stop at a minimum and at half the space they divide; the fraction keeps 15% for each side. A pane with no height has no divider left to drag it back with. ## How a drag arrives `mouse_area` reports that a press happened, not where, so the first pointer movement after a grab is the anchor and every size is a delta from it. That is what makes `layout.rs` testable without a window: nothing in it knows where anything is on screen. The pointer subscription exists only while a divider is held — following every mouse movement for a session to serve a drag nobody is doing is the most expensive subscription this could have had. `extent` — the size of the space being divided — is the one thing the arithmetic cannot derive, so the view measures it with `responsive` and the grab carries it. ## Where it is written `state.toml`, beside the window's own geometry, on release rather than per frame. It is state, not a setting: it changes by dragging, and nobody is going to open a file to choose a number of pixels. It is clamped again on the way in, so a hand-edited file cannot open hideGit with a sidebar wider than the window. A state file written before this exists loads unchanged. Double-clicking a divider restores it, which is why none of this needs a settings entry: the control that moved it is the control that undoes it. ## What it cost `staging::view` reached fourteen arguments — already flagged in #86 as wanting a struct, and still not this change. Its file list became its own function, because the row it sits in is built inside a `responsive` closure that may run more than once and therefore cannot consume anything. ## Tests Thirteen on the arithmetic, three on the divider widget — including that a press beside it grabs nothing — and five driving `update`, one of which holds that a divider dragged inside the working directory resizes the window's layout rather than the repository's. Two on the config file: a round trip, and a `state.toml` from before the dividers existed.
… off Three places drew text of unbounded length at whatever height it asked for and let the window clip the rest: no scrollbar, no wheel, and no way to read the end of it short of making the window bigger. **A commit message.** The detail pane's header — title, author, body, stats — grew with the body. A release note of four paragraphs pushed the changed files and the diff off the bottom of the window, and the end of the message was not reachable either way. It scrolls now, and is capped at 240px so that reading a long message does not cost the diff underneath it. **Git's stderr in a failed pane.** A rejected push prints a paragraph and a merge prints the conflicted paths. The pane showed the lines that fitted and dropped the rest, which for an error message is the half that says what to do about it. **Git's stderr in a toast.** A toast is bottom-aligned, so its details grew *upwards*: thirty lines carried the summary and both controls — dismiss and Copy details — off the top of the window, on exactly the failures somebody needs to read and report. Both tests assert what is *visible* rather than what is in the tree. A widget pushed out of the window is still in the widget tree, so the usual `shows` helper passes against the bug, and the toast one lays out at 1000x500 rather than the harness's roomy default — the bug is about a toast outgrowing its window, so the window has to be one a toast can outgrow. Both were confirmed to fail with the cap removed.
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
1 participant
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.
Stacked on #87, which is stacked on #86.
Three places drew text of unbounded length at whatever height it asked for and let the window clip the rest. No scrollbar, no wheel, no way to read the end of it short of making the window bigger.
The third is the worst of them: it hid the report-this controls on exactly the failures worth reporting.
The tests assert visibility, not presence
A widget pushed out of the window is still in the widget tree, so
shows— the helper the rest of the render tests use — passes against every one of these bugs. Both new tests take the target'svisible_bounds()instead.The toast one also lays out at 1000x500 rather than the harness's 1024x768 default: the bug is about a toast outgrowing its window, so the window has to be one a toast can outgrow.
a_commit_message_of_any_length_leaves_the_diff_on_screena_toast_with_a_long_stderr_keeps_its_summary_on_screenBoth were confirmed failing before the fix went back in — the first version of each passed against the bug, which is how the visibility assertion got written.
Gate
cargo fmt --all -- --check,cargo clippy --workspace --all-targets --all-features -- -D warnings,cargo test --workspace— 957 tests, all green.