perf!: skip widget rebuilds during divider drag#134
Open
andyhorn wants to merge 6 commits into
Open
Conversation
andyhorn
added a commit
that referenced
this pull request
May 27, 2026
Address review findings on PR #134: - Replace `_PixelsListenable` wrapper with a `ValueNotifier<List<double>>` that publishes a fresh `UnmodifiableListView` snapshot per notification, restoring proper `ValueListenable` old/new comparison semantics. - Push a fresh snapshot from `_setAvailableSpace` so the live-pixel render path sees redistributed pixels after a LayoutBuilder-driven rebuild. - Rewrite the `pixelsListenable` dartdoc: drop private-symbol references and the negative contract, add a migration note for callers that watched the controller for size changes. - Acknowledge in the controller class doc that the main listener also fires on the post-layout rendered-size update. - Update the README size-tracking snippet to use `pixelsListenable`. - Assert `constraints.maxWidth.isFinite` in the RTL horizontal reversal path now that the assumption is no longer enforced by a `Flex` parent. - Add tests: `show()` notification, render-object detach/reattach lifecycle, vertical-axis rebuild and live-pixel coverage, and the `_canUseLivePixels` length-mismatch cold-path fallback.
Replaces the per-tick ChangeNotifier-driven rebuild of the entire container subtree with a render-object listener on a dedicated pixel channel. Drag updates now relayout the children directly without rebuilding any widgets. * `ResizableController` exposes `pixelsListenable` — a `ValueListenable<List<double>>` that fires on every pixel change (drag, post-layout, structural). The main controller listener no longer fires during a drag. * `ResizableLayoutRenderObject` accepts the listenable, subscribes on attach, and `markNeedsLayout`s on change. `performLayout` branches: when the listenable is present, lay out children directly from its value (fast path); otherwise resolve from the declared sizes (cold path) and report the resolved values via `onComplete`. * `ResizableContainer` replaces its outer `AnimatedBuilder` with `ListenableBuilder` and drops `_flexFromFullSizes` for the idle phase; hide-animation phases continue to use the flex path. Each child is wrapped in `RepaintBoundary` so paint dirtiness doesn't propagate to siblings during a drag. * RTL offset reversal moves into the render object, since the live path no longer goes through `Flex`. BREAKING CHANGE: `ResizableController`'s `addListener` no longer fires on divider drag updates — it now fires only on structural changes (children list, declared sizes, hidden set, layout invalidation). Callers that watched the controller for live size changes should subscribe to `controller.pixelsListenable` instead. Closes #120 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address review findings on PR #134: - Replace `_PixelsListenable` wrapper with a `ValueNotifier<List<double>>` that publishes a fresh `UnmodifiableListView` snapshot per notification, restoring proper `ValueListenable` old/new comparison semantics. - Push a fresh snapshot from `_setAvailableSpace` so the live-pixel render path sees redistributed pixels after a LayoutBuilder-driven rebuild. - Rewrite the `pixelsListenable` dartdoc: drop private-symbol references and the negative contract, add a migration note for callers that watched the controller for size changes. - Acknowledge in the controller class doc that the main listener also fires on the post-layout rendered-size update. - Update the README size-tracking snippet to use `pixelsListenable`. - Assert `constraints.maxWidth.isFinite` in the RTL horizontal reversal path now that the assumption is no longer enforced by a `Flex` parent. - Add tests: `show()` notification, render-object detach/reattach lifecycle, vertical-axis rebuild and live-pixel coverage, and the `_canUseLivePixels` length-mismatch cold-path fallback.
Expose `ResizableController.needsLayoutListenable: ValueListenable<bool>`, which fires when the controller's `needsLayout` flag transitions. The container now subscribes to this listenable alongside the controller via `Listenable.merge`, decoupling the cold↔live build-path swap from the main listener. With this in place, `_setRenderedSizes` no longer calls `notifyListeners`. The main controller listener is now reserved strictly for structural changes (children list, declared sizes, hidden set); post-layout updates flow through `needsLayoutListenable` (for the build-path swap) and `pixelsListenable` (for the rendered pixel values). Breaking: callers that watched the controller via `addListener` to react to *any* post-layout state change now see fires only on structural events. Subscribe to `needsLayoutListenable` for layout-state transitions or to `pixelsListenable` for live pixel values. Update the `notify count` tests in `resizable_container_test.dart` to assert the new contract on both listenables.
Cover the three-listenable surface from a consumer's POV: main ChangeNotifier fires only on structural changes, pixelsListenable carries drag updates and snapshot-stable values, needsLayoutListenable tracks build-path swap. Also pins the Listenable.merge coalescing the container relies on and the README migration pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Pin that drag clamping happens in the controller before publishing to pixelsListenable, not in the render-object live layout. A documenting test asserts the live layout renders constraint-violating pixels verbatim, so any future re-clamping there fails loudly. - Pin the hide/show animation contract under the new dual-listenable surface: pixelsListenable fires a bounded number of times per hide/show (the animation lives in HideAnimationCoordinator, not the controller, so it is not per-frame), main listener fires exactly once per call, and a concurrent drag does not bump main. - Pin controller swap via didUpdateWidget (old detached, new attached, consumer-owned), late subscriber delivery, dispose ordering, and the documented multi-container-sharing limitation (each initChildren wipes hidden indices, so containers cannot share a controller). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
c84543f to
c1c2316
Compare
The perf live path built dividers without the enabled flag, so locked dividers (resizable: false or ResizableDivider.enabled: false) still responded to drag, tap, and hover. Pass enabled through to match the hide-animation path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
ResizableController's notification surface: drag/post-layout pixel updates flow through a newpixelsListenable(ValueListenable<List<double>>); the main controller listener fires only on structural changes.ResizableLayoutRenderObjectsubscribes to the pixel listenable andmarkNeedsLayouts on change. The widget tree above is never rebuilt during a drag — onlyperformLayoutruns.ResizableContainerreplacesAnimatedBuilderwithListenableBuilder, drops_flexFromFullSizesfor the idle phase (still used for hide-animation phases), and wraps each child inRepaintBoundary.Flex.Closes #120.
Breaking change
ResizableController.addListener(...)no longer fires on divider drag — it now fires only on structural changes (children list, declared sizes, hidden set, layout invalidation). Callers that watched the controller for live size changes should subscribe tocontroller.pixelsListenableinstead.Test plan
adjustChildSize,setRenderedSizes,setSizes,setHidden,setChildren) and disposal semantics.🤖 Generated with Claude Code