Unverified Commit 4c5817c5 authored by Callum Moffat's avatar Callum Moffat Committed by GitHub

Reland "Fix DraggableScrollableSheet rebuilding during drag" (#112479)

parent 3f8c31fd
...@@ -21,6 +21,7 @@ import 'scroll_physics.dart'; ...@@ -21,6 +21,7 @@ import 'scroll_physics.dart';
import 'scroll_position.dart'; import 'scroll_position.dart';
import 'scroll_position_with_single_context.dart'; import 'scroll_position_with_single_context.dart';
import 'scroll_simulation.dart'; import 'scroll_simulation.dart';
import 'value_listenable_builder.dart';
/// The signature of a method that provides a [BuildContext] and /// The signature of a method that provides a [BuildContext] and
/// [ScrollController] for building a widget that may overflow the draggable /// [ScrollController] for building a widget that may overflow the draggable
...@@ -488,7 +489,6 @@ class _DraggableSheetExtent { ...@@ -488,7 +489,6 @@ class _DraggableSheetExtent {
required this.snap, required this.snap,
required this.snapSizes, required this.snapSizes,
required this.initialSize, required this.initialSize,
required this.onSizeChanged,
this.snapAnimationDuration, this.snapAnimationDuration,
ValueNotifier<double>? currentSize, ValueNotifier<double>? currentSize,
bool? hasDragged, bool? hasDragged,
...@@ -500,8 +500,7 @@ class _DraggableSheetExtent { ...@@ -500,8 +500,7 @@ class _DraggableSheetExtent {
assert(maxSize <= 1), assert(maxSize <= 1),
assert(minSize <= initialSize), assert(minSize <= initialSize),
assert(initialSize <= maxSize), assert(initialSize <= maxSize),
_currentSize = (currentSize ?? ValueNotifier<double>(initialSize)) _currentSize = currentSize ?? ValueNotifier<double>(initialSize),
..addListener(onSizeChanged),
availablePixels = double.infinity, availablePixels = double.infinity,
hasDragged = hasDragged ?? false, hasDragged = hasDragged ?? false,
hasChanged = hasChanged ?? false; hasChanged = hasChanged ?? false;
...@@ -515,7 +514,6 @@ class _DraggableSheetExtent { ...@@ -515,7 +514,6 @@ class _DraggableSheetExtent {
final Duration? snapAnimationDuration; final Duration? snapAnimationDuration;
final double initialSize; final double initialSize;
final ValueNotifier<double> _currentSize; final ValueNotifier<double> _currentSize;
final VoidCallback onSizeChanged;
double availablePixels; double availablePixels;
// Used to disable snapping until the user has dragged on the sheet. // Used to disable snapping until the user has dragged on the sheet.
...@@ -595,17 +593,12 @@ class _DraggableSheetExtent { ...@@ -595,17 +593,12 @@ class _DraggableSheetExtent {
return size / maxSize * availablePixels; return size / maxSize * availablePixels;
} }
void dispose() {
_currentSize.removeListener(onSizeChanged);
}
_DraggableSheetExtent copyWith({ _DraggableSheetExtent copyWith({
required double minSize, required double minSize,
required double maxSize, required double maxSize,
required bool snap, required bool snap,
required List<double> snapSizes, required List<double> snapSizes,
required double initialSize, required double initialSize,
required VoidCallback onSizeChanged,
Duration? snapAnimationDuration, Duration? snapAnimationDuration,
}) { }) {
return _DraggableSheetExtent( return _DraggableSheetExtent(
...@@ -615,7 +608,6 @@ class _DraggableSheetExtent { ...@@ -615,7 +608,6 @@ class _DraggableSheetExtent {
snapSizes: snapSizes, snapSizes: snapSizes,
snapAnimationDuration: snapAnimationDuration, snapAnimationDuration: snapAnimationDuration,
initialSize: initialSize, initialSize: initialSize,
onSizeChanged: onSizeChanged,
// Set the current size to the possibly updated initial size if the sheet // Set the current size to the possibly updated initial size if the sheet
// hasn't changed yet. // hasn't changed yet.
currentSize: ValueNotifier<double>(hasChanged currentSize: ValueNotifier<double>(hasChanged
...@@ -641,7 +633,6 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> { ...@@ -641,7 +633,6 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
snapSizes: _impliedSnapSizes(), snapSizes: _impliedSnapSizes(),
snapAnimationDuration: widget.snapAnimationDuration, snapAnimationDuration: widget.snapAnimationDuration,
initialSize: widget.initialChildSize, initialSize: widget.initialChildSize,
onSizeChanged: _setExtent,
); );
_scrollController = _DraggableScrollableSheetScrollController(extent: _extent); _scrollController = _DraggableScrollableSheetScrollController(extent: _extent);
widget.controller?._attach(_scrollController); widget.controller?._attach(_scrollController);
...@@ -683,24 +674,22 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> { ...@@ -683,24 +674,22 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
} }
} }
void _setExtent() {
setState(() {
// _extent has been updated when this is called.
});
}
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
return LayoutBuilder( return ValueListenableBuilder<double>(
valueListenable: _extent._currentSize,
builder: (BuildContext context, double currentSize, Widget? child) => LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) { builder: (BuildContext context, BoxConstraints constraints) {
_extent.availablePixels = widget.maxChildSize * constraints.biggest.height; _extent.availablePixels = widget.maxChildSize * constraints.biggest.height;
final Widget sheet = FractionallySizedBox( final Widget sheet = FractionallySizedBox(
heightFactor: _extent.currentSize, heightFactor: currentSize,
alignment: Alignment.bottomCenter, alignment: Alignment.bottomCenter,
child: widget.builder(context, _scrollController), child: child,
); );
return widget.expand ? SizedBox.expand(child: sheet) : sheet; return widget.expand ? SizedBox.expand(child: sheet) : sheet;
}, },
),
child: widget.builder(context, _scrollController),
); );
} }
...@@ -708,13 +697,11 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> { ...@@ -708,13 +697,11 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
void dispose() { void dispose() {
widget.controller?._detach(); widget.controller?._detach();
_scrollController.dispose(); _scrollController.dispose();
_extent.dispose();
super.dispose(); super.dispose();
} }
void _replaceExtent(covariant DraggableScrollableSheet oldWidget) { void _replaceExtent(covariant DraggableScrollableSheet oldWidget) {
final _DraggableSheetExtent previousExtent = _extent; final _DraggableSheetExtent previousExtent = _extent;
_extent.dispose();
_extent = _extent.copyWith( _extent = _extent.copyWith(
minSize: widget.minChildSize, minSize: widget.minChildSize,
maxSize: widget.maxChildSize, maxSize: widget.maxChildSize,
...@@ -722,7 +709,6 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> { ...@@ -722,7 +709,6 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
snapSizes: _impliedSnapSizes(), snapSizes: _impliedSnapSizes(),
snapAnimationDuration: widget.snapAnimationDuration, snapAnimationDuration: widget.snapAnimationDuration,
initialSize: widget.initialChildSize, initialSize: widget.initialChildSize,
onSizeChanged: _setExtent,
); );
// Modify the existing scroll controller instead of replacing it so that // Modify the existing scroll controller instead of replacing it so that
// developers listening to the controller do not have to rebuild their listeners. // developers listening to the controller do not have to rebuild their listeners.
......
...@@ -1465,4 +1465,51 @@ void main() { ...@@ -1465,4 +1465,51 @@ void main() {
closeTo(.6, precisionErrorTolerance), closeTo(.6, precisionErrorTolerance),
); );
}); });
testWidgets('DraggableScrollableSheet should not rebuild every frame while dragging', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/67219
int buildCount = 0;
await tester.pumpWidget(MaterialApp(
home: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) => Scaffold(
body: DraggableScrollableSheet(
initialChildSize: 0.25,
snap: true,
snapSizes: const <double>[0.25, 0.5, 1.0],
builder: (BuildContext context, ScrollController scrollController) {
buildCount++;
return ListView(
controller: scrollController,
children: <Widget>[
const Text('Drag me!'),
ElevatedButton(
onPressed: () => setState(() {}),
child: const Text('Rebuild'),
),
Container(
height: 10000,
color: Colors.blue,
),
],
);
},
),
),
),
));
expect(buildCount, 1);
await tester.fling(find.text('Drag me!'), const Offset(0, -300), 300);
await tester.pumpAndSettle();
// No need to rebuild the scrollable sheet, as only position has changed.
expect(buildCount, 1);
await tester.tap(find.text('Rebuild'));
await tester.pump();
// DraggableScrollableSheet has rebuilt, so expect the builder to be called.
expect(buildCount, 2);
});
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment