Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions packages/animations/lib/src/open_container.dart
Original file line number Diff line number Diff line change
Expand Up @@ -620,16 +620,10 @@ class _OpenContainerRoute<T> extends ModalRoute<T> {
_currentAnimationStatus = status;
switch (status) {
case AnimationStatus.dismissed:
if (hideableKey?.currentState != null) {
hideableKey.currentState
..placeholderSize = null
..isVisible = true;
}
_toggleHideable(hide: false);
break;
case AnimationStatus.completed:
hideableKey.currentState
..placeholderSize = null
..isVisible = false;
_toggleHideable(hide: true);
break;
case AnimationStatus.forward:
case AnimationStatus.reverse:
Expand All @@ -649,6 +643,25 @@ class _OpenContainerRoute<T> extends ModalRoute<T> {
return super.didPop(result);
}

@override
void dispose() {
if (hideableKey?.currentState?.isVisible == false) {
// This route may be disposed without dismissing its animation if it is
// removed by the navigator.
SchedulerBinding.instance
.addPostFrameCallback((Duration d) => _toggleHideable(hide: false));
}
super.dispose();
}

void _toggleHideable({bool hide}) {
if (hideableKey?.currentState != null) {
hideableKey.currentState
..placeholderSize = null
..isVisible = !hide;
}
}

void _takeMeasurements({
BuildContext navigatorContext,
bool delayForSourceRoute = false,
Expand Down
40 changes: 40 additions & 0 deletions packages/animations/test/open_container_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1733,6 +1733,46 @@ void main() {
expect(modalRoute.settings, routeSettings);
},
);

testWidgets(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment here to help with context:

Suggested change
testWidgets(
// Regression test for https://github.com/flutter/flutter/issues/72238.
testWidgets(

'OpenContainer works when the route is removed',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe:

Suggested change
'OpenContainer works when the route is removed',
'OpenContainer's source widget is visible in closed container route if open container '
'route is pushed from not using the OpenContainer itself',

I'm trying to make this description more helpful, but I think that the link to the issue does help enough

(WidgetTester tester) async {

final Widget openContainer = OpenContainer(
closedBuilder: (BuildContext context, VoidCallback action) {
return GestureDetector(
onTap: action,
child: const Text('Closed'),
);
},
openBuilder: (BuildContext context, VoidCallback action) {
return GestureDetector(
onTap: action,
child: const Text('Open'),
);
},
);
await tester.pumpWidget(_boilerplate(child: openContainer));
expect(_getOpacity(tester, 'Closed'), 1.0);

// Open the container
await tester.tap(find.text('Closed'));
await tester.pumpAndSettle();

final Element container = tester.element(find.byType(OpenContainer, skipOffstage: false));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
final Element container = tester.element(find.byType(OpenContainer, skipOffstage: false));
final Element container = tester.element(
find.byType(OpenContainer, skipOffstage: false),
);

// Replace the open container route.
Navigator.pushReplacement<void, void>(
container,
MaterialPageRoute<void>(builder: (_) => const Placeholder())
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
Navigator.pushReplacement<void, void>(
container,
MaterialPageRoute<void>(builder: (_) => const Placeholder())
);
Navigator.pushReplacement<void, void>(
container,
MaterialPageRoute<void>(builder: (_) => const Placeholder()),
);

await tester.pumpAndSettle();
// Go back to the main page and verify the closed builder is showed.
Navigator.pop(container);
await tester.pumpAndSettle();

expect(_getOpacity(tester, 'Closed'), 1.0);
},
);
}

Color _getScrimColor(WidgetTester tester) {
Expand Down