fix: pane separator moves at half mouse speed during drag (#6468)#9999
fix: pane separator moves at half mouse speed during drag (#6468)#9999yingxuel wants to merge 1 commit intowarpdotdev:masterfrom
Conversation
…#6468) adjust_pane_size read pane_size() (last rendered bounds) on every drag event. When multiple events fire between render frames the stale sizes caused every event after the first to overwrite the flex values with the same result, effectively halving the resize speed. Fix: cache the sum of the two adjacent panes' pixel sizes on the first drag event (when the render is fresh) via cached_total_pixel_size on DraggedBorder. Subsequent events derive individual sizes from the current flex ratio instead of re-reading stale rendered bounds, so each delta accumulates correctly. Extract compute_new_flex as a pure function so the math can be unit tested without a ViewContext. Add 6 regression tests including one that explicitly demonstrates the old stale-size behaviour producing half the expected movement. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @yingxuel on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
github-actions
Bot
added
the
external-contributor
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR changes pane divider drag resizing to cache the adjacent panes' total pixel size during a drag and compute new flex values from the current flex ratio, with unit coverage for accumulated mouse deltas and minimum-size clamping.
Concerns
- For faster review, please upload screenshots or a video of the pane separator tracking the cursor end to end. This is a user-visible interaction change, and the PR currently says there is no visual diff rather than attaching evidence.
Verdict
Found: 0 critical, 0 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: @vkodithala. Comment Powered by Oz |
oz-for-oss
Bot
dismissed
their stale review
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR fixes split-pane divider resizing by caching the adjacent panes' combined rendered size for the drag session and deriving current per-pane sizes from flex ratios so multiple drag events between renders accumulate correctly. It also adds focused unit coverage for the pure flex redistribution helper.
Concerns
No blocking correctness, security, or performance concerns found in the annotated diff.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
1 participant
Closes #6468
Description
When dragging the pane separator, the divider moved at roughly half the speed of the mouse cursor — a 200 px mouse movement produced only ~100 px of divider movement.
Root cause:
PaneBranch::adjust_pane_sizecalledpane_size()(which returns last-rendered element bounds) on every drag event to compute the flex redistribution. Mouse drag events can fire at ~120 Hz while the app renders at ~60 Hz, meaning two events fire per render frame. Both events read the same stale rendered sizes and compute the same flex value, so the second event overwrites the first with an identical result — only one event's worth of movement is applied per frame, halving the effective speed.Fix: Add
cached_total_pixel_size: Option<f32>toDraggedBorder. On the first drag event after a render, the total pixel size of the two adjacent panes is read from the renderer (when fresh) and cached. All subsequent events in the same drag session derive individual pane sizes from the current flex ratio instead of re-reading stale rendered bounds, so each delta accumulates correctly.Extract the redistribution math into
compute_new_flex, a pure function with noViewContextdependency, enabling unit testing.Linked Issue
ready-to-specorready-to-implement.Screenshots / Videos
recording_6468.mov
Testing
Added 6 unit tests in
app/src/pane_group/tree_tests.rscoveringcompute_new_flex:test_compute_new_flex_single_event— basic single drag event correctnesstest_compute_new_flex_accumulates_across_events— 3 × 10 px events yield 430 px, not 410 pxtest_stale_size_bug_would_undercount_deltas— explicitly compares old formula (410 px) vs new (430 px) for N events before a re-render, proving the bug existed and is fixedtest_compute_new_flex_minimum_pane_size_respected— clamping at minimum pane sizetest_compute_new_flex_ignores_near_zero_delta— sub-epsilon delta returnsNonetest_compute_new_flex_asymmetric_flex— asymmetric flex values (600 px / 200 px split)Pre-existing presubmit failures (unrelated to this PR): 6 tests fail on
masterbefore applying these changes, confirmed by the PR author:shell_integration_tests::test_legacy_ssh_into_bashshell_integration_tests::test_legacy_ssh_into_zshshell_integration_tests::test_ssh_into_ashshell_integration_tests::test_ssh_into_shui_tests::test_ssh_with_shell_overridesettings::init::tests::test_migration_does_not_rerun_when_marker_presentThese require SSH infrastructure or specific local state not available in a standard dev environment and are not caused by this PR.
Agent Mode