Fix GridSplitter runaway resize when a neighboring star definition is clamped - #11900
Open
akon47 wants to merge 1 commit into
Open
Fix GridSplitter runaway resize when a neighboring star definition is clamped#11900akon47 wants to merge 1 commit into
akon47 wants to merge 1 commit into
Conversation
Contributor
|
LGTM |
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.
Fixes #1687
Description
When a
GridSplitterresizes only one of the two definitions (SplitBehavior.Resize1/Resize2, i.e. at least one of them is not star-sized),GetDeltaConstraintslimits the drag delta only by the Min/Max constraints of the definition being resized — the constraints of the neighboring star definition are ignored.The neighboring star definition is the one that absorbs the space freed up or consumed by the resized definition. Once it gets clamped by one of its own constraints (
MinWidth/MaxWidth/MinHeight/MaxHeight, including the implicit minimum of 0), the layout can no longer honor the lengths the splitter sets and the splitter stops moving on screen.Thumbreports drag changes relative to its own position, so from that point the reportedDragDeltachange keeps accumulating while the mouse moves, andMoveSplitterre-applies the ever-growing accumulated change to the current actual lengths on every mouse move. The result is that the resized definition changes much faster than the mouse (10px mouse steps produce 20/30/40/…px changes), or keeps growing forever past the Grid/window bounds (#1687).This change constrains the delta by the neighboring star definition's Min/Max as well, matching what
SplitBehavior.Splitalready does for both definitions. The extra clamp is only applied when the neighboring definition is star-sized — the only configurations in which the layout redistributes the space and the bug can occur. All other configurations keep the existing behavior (verified below).Minimal repro (columns)
Drag the splitter to the right. As soon as the first column reaches its
MaxWidth, the right column starts shrinking much faster than the mouse. The rows variant from #1687 (star row above a fixed-height row, splitter bottom-aligned in the star row, drag up past the window top) is the same bug with the implicit minimum of 0.Before / after
Top: current behavior, bottom: with this change. The red line is the (emulated) mouse position; every step moves the mouse by exactly 10px.
[ *(MaxWidth=520) | splitter | 300px ], dragging right — the right column collapses at an accelerating rate while the mouse moves at constant speed:[ *(MinWidth=400) | splitter | 300px ], dragging left — the right column grows at an accelerating rate and overflows the grid:Rows layout from #1687, dragging up past the top — the bottom row grows forever (reaches 832px in a 150px-tall grid); with the fix it stops once the star row is fully collapsed:
Excerpt of the per-step drag log (current behavior, MaxWidth scenario; full logs in the assets link below):
Customer Impact
Dragging a
GridSplitterwhose neighboring star column/row has a Min/Max constraint (or can collapse to 0) resizes the layout much faster than the mouse, and can push content far outside the Grid/window with no way to control it precisely. The rows variant is reported in #1687.Regression
No. The behavior is long-standing (reported against .NET Core 3.0 in #1687 and present in .NET Framework as well).
Testing
DragDeltaevents computed exactly the wayThumbreports them — change relative to the thumb) across the scenarios above, for both columns and rows.star(MaxWidth)|pixel, drag rightstar(MinWidth)|pixel, drag leftpixel|star, drag rightpixel|star, drag leftstar|star, drag rightpixel|pixel, drag rightpixel|auto, drag rightstar(MaxWidth)|star, drag rightThe repro/verification app, recordings, and full drag logs are available here: https://github.com/akon47/wpf/tree/gridsplitter-runaway-repro-assets
Risk
Low. The delta is additionally clamped only when
SplitBehaviorisResize1/Resize2and the neighboring definition is star-sized — exactly the configurations that are broken today.Resize2implies definition1 is star-sized (seeSetupDefinitionsToResize), so no condition is needed there. Keyboard resizing andShowsPreviewgo through the same constraint helper, so they get the same consistent clamping (the preview now stops where the actual resize would stop). The regression matrix above verifies non-affected configurations are unchanged.Microsoft Reviewers: Open in CodeFlow