Unverified Commit 7b5241b3 authored by Callum Moffat's avatar Callum Moffat Committed by GitHub

Fix DraggableScrollableSheet rebuilding during drag (#111010)

parent 3e074119
......@@ -21,6 +21,7 @@ import 'scroll_physics.dart';
import 'scroll_position.dart';
import 'scroll_position_with_single_context.dart';
import 'scroll_simulation.dart';
import 'value_listenable_builder.dart';
/// The signature of a method that provides a [BuildContext] and
/// [ScrollController] for building a widget that may overflow the draggable
......@@ -488,7 +489,6 @@ class _DraggableSheetExtent {
required this.snap,
required this.snapSizes,
required this.initialSize,
required this.onSizeChanged,
this.snapAnimationDuration,
ValueNotifier<double>? currentSize,
bool? hasDragged,
......@@ -500,8 +500,7 @@ class _DraggableSheetExtent {
assert(maxSize <= 1),
assert(minSize <= initialSize),
assert(initialSize <= maxSize),
_currentSize = (currentSize ?? ValueNotifier<double>(initialSize))
..addListener(onSizeChanged),
_currentSize = currentSize ?? ValueNotifier<double>(initialSize),
availablePixels = double.infinity,
hasDragged = hasDragged ?? false,
hasChanged = hasChanged ?? false;
......@@ -515,7 +514,6 @@ class _DraggableSheetExtent {
final Duration? snapAnimationDuration;
final double initialSize;
final ValueNotifier<double> _currentSize;
final VoidCallback onSizeChanged;
double availablePixels;
// Used to disable snapping until the user has dragged on the sheet.
......@@ -595,17 +593,12 @@ class _DraggableSheetExtent {
return size / maxSize * availablePixels;
}
void dispose() {
_currentSize.removeListener(onSizeChanged);
}
_DraggableSheetExtent copyWith({
required double minSize,
required double maxSize,
required bool snap,
required List<double> snapSizes,
required double initialSize,
required VoidCallback onSizeChanged,
Duration? snapAnimationDuration,
}) {
return _DraggableSheetExtent(
......@@ -615,7 +608,6 @@ class _DraggableSheetExtent {
snapSizes: snapSizes,
snapAnimationDuration: snapAnimationDuration,
initialSize: initialSize,
onSizeChanged: onSizeChanged,
// Set the current size to the possibly updated initial size if the sheet
// hasn't changed yet.
currentSize: ValueNotifier<double>(hasChanged
......@@ -641,7 +633,6 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
snapSizes: _impliedSnapSizes(),
snapAnimationDuration: widget.snapAnimationDuration,
initialSize: widget.initialChildSize,
onSizeChanged: _setExtent,
);
_scrollController = _DraggableScrollableSheetScrollController(extent: _extent);
widget.controller?._attach(_scrollController);
......@@ -683,24 +674,22 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
}
}
void _setExtent() {
setState(() {
// _extent has been updated when this is called.
});
}
@override
Widget build(BuildContext context) {
return LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
_extent.availablePixels = widget.maxChildSize * constraints.biggest.height;
final Widget sheet = FractionallySizedBox(
heightFactor: _extent.currentSize,
alignment: Alignment.bottomCenter,
child: widget.builder(context, _scrollController),
);
return widget.expand ? SizedBox.expand(child: sheet) : sheet;
},
return ValueListenableBuilder<double>(
valueListenable: _extent._currentSize,
builder: (BuildContext context, double currentSize, Widget? child) => LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
_extent.availablePixels = widget.maxChildSize * constraints.biggest.height;
final Widget sheet = FractionallySizedBox(
heightFactor: currentSize,
alignment: Alignment.bottomCenter,
child: child,
);
return widget.expand ? SizedBox.expand(child: sheet) : sheet;
},
),
child: widget.builder(context, _scrollController),
);
}
......@@ -708,13 +697,11 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
void dispose() {
widget.controller?._detach();
_scrollController.dispose();
_extent.dispose();
super.dispose();
}
void _replaceExtent(covariant DraggableScrollableSheet oldWidget) {
final _DraggableSheetExtent previousExtent = _extent;
_extent.dispose();
_extent = _extent.copyWith(
minSize: widget.minChildSize,
maxSize: widget.maxChildSize,
......@@ -722,7 +709,6 @@ class _DraggableScrollableSheetState extends State<DraggableScrollableSheet> {
snapSizes: _impliedSnapSizes(),
snapAnimationDuration: widget.snapAnimationDuration,
initialSize: widget.initialChildSize,
onSizeChanged: _setExtent,
);
// Modify the existing scroll controller instead of replacing it so that
// developers listening to the controller do not have to rebuild their listeners.
......
......@@ -1455,4 +1455,51 @@ void main() {
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