fix(balance-sizes): preserve total weight instead of resetting to a constant - #2211
Open
MsfPablo wants to merge 1 commit into
Open
fix(balance-sizes): preserve total weight instead of resetting to a constant#2211MsfPablo wants to merge 1 commit into
MsfPablo wants to merge 1 commit into
Conversation
…onstant balance(_:) assigned a hardcoded weight of 1 to every child, discarding whatever total the container's weights previously summed to. Weights are only converted to real pixel sizes by the layout pass, which runs once after a whole binding's command list finishes — so a later command in the same binding (e.g. resize) still sees the old, larger total via sibling weights, while the balanced child was reset to a small constant. Chaining `balance-sizes` then `resize width N` in one keybind therefore produced a much wider window than running the same two commands as separate keybind presses. Redistribute the container's existing weight total evenly across its children instead of assigning an arbitrary constant, so later commands in the same list see a consistent scale. Fixes nikitabobko#1837
Author
|
Ping on this. The current |
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 #1837.
balance(_:)assigned a hardcoded weight of1to every child, discarding whatever total the container's weights previously summed to.Weights are only converted to real pixel sizes by the layout pass, which runs once after a whole binding's command list finishes. So when
balance-sizesandresizeare chained in the same binding,resize's delta calculation still operates against the pre-existing (larger) weight scale of sibling containers, while the just-balanced children were reset to a tiny constant (1) — producing a much wider window than running the same two commands as separate keybind presses.Fix
Redistribute the container's existing weight total evenly across its children (
total / children.count) instead of assigning an arbitrary constant, so later commands in the same list see a consistent scale.Test plan
testBalanceSizes(total 1+2+3=6 → each child becomes 2, not the old hardcoded 1)testBalanceSizes_nestedContainerPreservesItsOwnTotal, confirming each nested container's total is redistributed independentlytestBalanceSizesThenResize_behavesLikeResizeAlone, the regression test for the exact chained-command scenario from the issueI couldn't run the test suite itself in my environment —
swift testfails witherror: no such module 'XCTest', which I confirmed is pre-existing on a cleanmaincheckout with no changes at all (I only have Xcode Command Line Tools, not a full Xcode install).swift build(the actual app code) compiles cleanly with this change. Happy to have CI confirm the test suite.