Skip to content

Commit

Permalink
showModalBottomSheet should not dispose the controller provided by …
Browse files Browse the repository at this point in the history
…user (#87707)
  • Loading branch information
xu-baolin committed Aug 11, 2021
1 parent 66597ff commit a628e4f
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 3 deletions.
10 changes: 8 additions & 2 deletions packages/flutter/lib/src/material/bottom_sheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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!;
}

Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions packages/flutter/lib/src/material/scaffold.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
16 changes: 15 additions & 1 deletion packages/flutter/lib/src/widgets/routes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -422,7 +434,9 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
void dispose() {
assert(!_transitionCompleter.isCompleted, 'Cannot dispose a $runtimeType twice.');
_animation?.removeStatusListener(_handleStatusChanged);
_controller?.dispose();
if (willDisposeAnimationController) {
_controller?.dispose();
}
_transitionCompleter.complete(_result);
super.dispose();
}
Expand Down
63 changes: 63 additions & 0 deletions packages/flutter/test/material/bottom_sheet_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down

0 comments on commit a628e4f

Please sign in to comment.