Unverified Commit a628e4f4 authored by xubaolin's avatar xubaolin Committed by GitHub

`showModalBottomSheet` should not dispose the controller provided by user (#87707)

parent 66597ffb
......@@ -497,7 +497,12 @@ class _ModalBottomSheetRoute<T> extends PopupRoute<T> {
@override
AnimationController createAnimationController() {
assert(_animationController == null);
_animationController = transitionAnimationController ?? BottomSheet.createAnimationController(navigator!.overlay!);
if (transitionAnimationController != null) {
_animationController = transitionAnimationController;
willDisposeAnimationController = false;
} else {
_animationController = BottomSheet.createAnimationController(navigator!.overlay!);
}
return _animationController!;
}
......@@ -626,7 +631,8 @@ class _BottomSheetSuspendedCurve extends ParametricCurve<double> {
/// for more details).
///
/// The [transitionAnimationController] controls the bottom sheet's entrance and
/// exit animations if provided.
/// exit animations. It's up to the owner of the controller to call
/// [AnimationController.dispose] when the controller is no longer needed.
///
/// The optional `routeSettings` parameter sets the [RouteSettings] of the modal bottom sheet
/// sheet. This is particularly useful in the case that a user wants to observe
......
......@@ -2794,6 +2794,10 @@ class ScaffoldState extends State<Scaffold> with TickerProviderStateMixin, Resto
/// [ModalRoute] and a back button is added to the app bar of the [Scaffold]
/// that closes the bottom sheet.
///
/// The [transitionAnimationController] controls the bottom sheet's entrance and
/// exit animations. It's up to the owner of the controller to call
/// [AnimationController.dispose] when the controller is no longer needed.
///
/// To create a persistent bottom sheet that is not a [LocalHistoryEntry] and
/// does not add a back button to the enclosing Scaffold's app bar, use the
/// [Scaffold.bottomSheet] constructor parameter.
......
......@@ -149,9 +149,21 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
Animation<double>? get secondaryAnimation => _secondaryAnimation;
final ProxyAnimation _secondaryAnimation = ProxyAnimation(kAlwaysDismissedAnimation);
/// Whether to takeover the [controller] created by [createAnimationController].
///
/// If true, this route will call [AnimationController.dispose] when the
/// controller is no longer needed.
/// If false, the controller should be disposed by whoever owned it.
///
/// It defaults to `true`.
bool willDisposeAnimationController = true;
/// Called to create the animation controller that will drive the transitions to
/// this route from the previous one, and back to the previous route from this
/// one.
///
/// The returned controller will be disposed by [AnimationController.dispose]
/// if the [willDisposeAnimationController] is `true`.
AnimationController createAnimationController() {
assert(!_transitionCompleter.isCompleted, 'Cannot reuse a $runtimeType after disposing it.');
final Duration duration = transitionDuration;
......@@ -422,7 +434,9 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
void dispose() {
assert(!_transitionCompleter.isCompleted, 'Cannot dispose a $runtimeType twice.');
_animation?.removeStatusListener(_handleStatusChanged);
if (willDisposeAnimationController) {
_controller?.dispose();
}
_transitionCompleter.complete(_result);
super.dispose();
}
......
......@@ -771,6 +771,69 @@ void main() {
expect(find.text('BottomSheet'), findsNothing);
});
// Regression test for https://github.com/flutter/flutter/issues/87592
testWidgets('the framework do not dispose the transitionAnimationController provided by user.', (WidgetTester tester) async {
const Key tapTarget = Key('tap-target');
final AnimationController controller = AnimationController(
vsync: const TestVSync(),
duration: const Duration(seconds: 2),
reverseDuration: const Duration(seconds: 2),
);
await tester.pumpWidget(MaterialApp(
home: Scaffold(
body: Builder(
builder: (BuildContext context) {
return GestureDetector(
key: tapTarget,
onTap: () {
showModalBottomSheet<void>(
context: context,
// The default duration and reverseDuration is 1 second
transitionAnimationController: controller,
builder: (BuildContext context) {
return const Text('BottomSheet');
},
);
},
behavior: HitTestBehavior.opaque,
child: const SizedBox(
height: 100.0,
width: 100.0,
),
);
},
),
),
));
expect(find.text('BottomSheet'), findsNothing);
await tester.tap(find.byKey(tapTarget)); // Opening animation will start after tapping
await tester.pump();
expect(find.text('BottomSheet'), findsOneWidget);
await tester.pump(const Duration(milliseconds: 2000));
expect(find.text('BottomSheet'), findsOneWidget);
// Tapping above the bottom sheet to dismiss it.
await tester.tapAt(const Offset(20.0, 20.0)); // Closing animation will start after tapping
await tester.pump();
expect(find.text('BottomSheet'), findsOneWidget);
await tester.pump(const Duration(milliseconds: 2000));
// The bottom sheet should still be present at the very end of the animation.
expect(find.text('BottomSheet'), findsOneWidget);
await tester.pump(const Duration(milliseconds: 1));
// The bottom sheet should not be showing any longer.
expect(find.text('BottomSheet'), findsNothing);
controller.dispose();
// Double disposal will throw.
expect(tester.takeException(), isNull);
});
testWidgets('Verify persistence BottomSheet use AnimationController if provided.', (WidgetTester tester) async {
const Key tapTarget = Key('tap-target');
const Key tapTargetToClose = Key('tap-target-to-close');
......
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