Fix: DraggableScrollableSheet may not close if snapping is enabled (#165557)
<!-- Thanks for filing a pull request! Reviewers are typically assigned within a week of filing a request. To learn more about code review, see our documentation on Tree Hygiene: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md --> fixes #140701 This PR fixes an issue where a DraggableScrollableSheet with `snap` set to true and `shouldCloseOnMinExtent` set to true may not close when dragged downward. The issue was caused by round-off errors accumulated by `addPixelDelta` method, which could lead to `extent.currentSize` not matching the `extent.minSize` exactly when the bottom is reached. I added logic to correct it when the snapping ballistic animation is complete. <details> <summary>Sample code</summary> ```Dart import 'package:flutter/material.dart'; void main() { runApp(MyApp()); } class MyApp extends StatelessWidget { const MyApp({super.key}); @override Widget build(BuildContext context) { return MaterialApp( home: HomePage(), ); } } class HomePage extends StatelessWidget { const HomePage({super.key}); @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar( title: Text("DraggableScrollableSheet Test"), ), body: Center( child: ElevatedButton( child: Text("Open"), onPressed: () { showModalBottomSheet( context: context, showDragHandle: true, isScrollControlled: true, builder: (context) => _buildBottomSheet(), ); }, ), ), ); } Widget _buildBottomSheet() { return NotificationListener<DraggableScrollableNotification>( onNotification: (notification) { print(notification.extent); return false; }, child: DraggableScrollableSheet( expand: false, snap: true, shouldCloseOnMinExtent: true, maxChildSize: 0.9, minChildSize: 0.25, builder: (context, scrollController) { return ListView.builder( controller: scrollController, itemCount: 100, itemBuilder: (context, index) { return ListTile( title: Text("Item $index"), ); }, ); }, ), ); } } ``` </details> | Before applying fix | After | | --- | --- | | * Occurs with probability <video src="https://github.com/user-attachments/assets/ffd2d097-3ed5-4775-90d5-950092d49591"> | <video src="https://github.com/user-attachments/assets/0f20cb81-3444-40a3-a84d-ed4bff15887e"> | ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This commit is contained in:
parent
cc556aecae
commit
cd0082f04f
@ -15,6 +15,7 @@ library;
|
||||
|
||||
import 'dart:math' as math;
|
||||
|
||||
import 'package:collection/collection.dart';
|
||||
import 'package:flutter/foundation.dart';
|
||||
import 'package:flutter/gestures.dart';
|
||||
|
||||
@ -909,14 +910,18 @@ class _DraggableScrollableSheetScrollPosition extends ScrollPositionWithSingleCo
|
||||
}
|
||||
}
|
||||
|
||||
bool get _isAtSnapSize {
|
||||
return extent.snapSizes.any((double snapSize) {
|
||||
// Checks if the sheet's current size is close to a snap size, returning the
|
||||
// snap size if so; returns null otherwise.
|
||||
double? _getCurrentSnapSize() {
|
||||
return extent.snapSizes.firstWhereOrNull((double snapSize) {
|
||||
return (extent.currentSize - snapSize).abs() <=
|
||||
extent.pixelsToSize(physics.toleranceFor(this).distance);
|
||||
});
|
||||
}
|
||||
|
||||
bool get _shouldSnap => extent.snap && extent.hasDragged && !_isAtSnapSize;
|
||||
bool _isAtSnapSize() => _getCurrentSnapSize() != null;
|
||||
|
||||
bool _shouldSnap() => extent.snap && extent.hasDragged && !_isAtSnapSize();
|
||||
|
||||
@override
|
||||
void dispose() {
|
||||
@ -929,7 +934,7 @@ class _DraggableScrollableSheetScrollPosition extends ScrollPositionWithSingleCo
|
||||
|
||||
@override
|
||||
void goBallistic(double velocity) {
|
||||
if ((velocity == 0.0 && !_shouldSnap) ||
|
||||
if ((velocity == 0.0 && !_shouldSnap()) ||
|
||||
(velocity < 0.0 && listShouldScroll) ||
|
||||
(velocity > 0.0 && extent.isAtMax)) {
|
||||
super.goBallistic(velocity);
|
||||
@ -981,6 +986,13 @@ class _DraggableScrollableSheetScrollPosition extends ScrollPositionWithSingleCo
|
||||
super.goBallistic(velocity);
|
||||
ballisticController.stop();
|
||||
} else if (ballisticController.isCompleted) {
|
||||
// Update the extent value after the snap animation completes to
|
||||
// avoid rounding errors that could prevent the sheet from closing when
|
||||
// it reaches minSize.
|
||||
final double? snapSize = _getCurrentSnapSize();
|
||||
if (snapSize != null) {
|
||||
extent.updateSize(snapSize, context.notificationContext!);
|
||||
}
|
||||
super.goBallistic(0);
|
||||
}
|
||||
}
|
||||
|
@ -1892,4 +1892,30 @@ void main() {
|
||||
expect(receivedNotification!.shouldCloseOnMinExtent, isFalse);
|
||||
controller.dispose();
|
||||
});
|
||||
|
||||
// Regression test for https://github.com/flutter/flutter/issues/140701
|
||||
testWidgets('DraggableScrollableSheet snaps exactly to minChildSize', (
|
||||
WidgetTester tester,
|
||||
) async {
|
||||
double? lastExtent;
|
||||
|
||||
await tester.pumpWidget(
|
||||
boilerplateWidget(
|
||||
null,
|
||||
snap: true,
|
||||
onDraggableScrollableNotification: (DraggableScrollableNotification notification) {
|
||||
lastExtent = notification.extent;
|
||||
return false;
|
||||
},
|
||||
),
|
||||
);
|
||||
|
||||
// One of the conditions for reproducing the round-off error.
|
||||
await tester.fling(find.text('Item 1'), const Offset(0, 100), 2000);
|
||||
await tester.pumpFrames(
|
||||
tester.widget(find.byType(Directionality)),
|
||||
const Duration(milliseconds: 500),
|
||||
);
|
||||
expect(lastExtent, .25);
|
||||
});
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user