Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: replace deprecated WillPopScope with PopScope #120

Merged
merged 6 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .github/workflows/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
uses: VeryGoodOpenSource/very_good_workflows/.github/workflows/flutter_package.yml@v1
with:
flutter_channel: stable
flutter_version: 3.13.9
flutter_version: 3.19.5

pana:
uses: VeryGoodOpenSource/very_good_workflows/.github/workflows/pana.yml@v1
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# 0.1.0

- **BREAKING**: replace deprecated `WillPopScope` with `PopScope`
- refactor: update dart sdk constrant to `>=3.2.0`
- refactor: update flutter constraint to `>=3.16.0`

# 0.0.10

- feat: add optional `clipBehavior` ([#113](https://github.com/felangel/flow_builder/pull/113))
Expand Down
15 changes: 5 additions & 10 deletions example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,9 @@ class MyApp extends StatelessWidget {
}
}

class Home extends StatefulWidget {
class Home extends StatelessWidget {
const Home({super.key});

@override
State<Home> createState() => _HomeState();
}

class _HomeState extends State<Home> {
@override
Widget build(BuildContext context) {
return Scaffold(
Expand All @@ -44,7 +39,7 @@ class _HomeState extends State<Home> {
trailing: const Icon(Icons.chevron_right),
onTap: () async {
await Navigator.of(context).push(OnboardingFlow.route());
if (!mounted) return;
if (!context.mounted) return;
ScaffoldMessenger.of(context)
..hideCurrentSnackBar()
..showSnackBar(
Expand All @@ -62,7 +57,7 @@ class _HomeState extends State<Home> {
final profile = await Navigator.of(context).push(
ProfileFlow.route(),
);
if (!mounted) return;
if (!context.mounted) return;
ScaffoldMessenger.of(context)
..hideCurrentSnackBar()
..showSnackBar(
Expand All @@ -80,7 +75,7 @@ class _HomeState extends State<Home> {
final location = await Navigator.of(context).push(
LocationFlow.route(),
);
if (!mounted) return;
if (!context.mounted) return;
ScaffoldMessenger.of(context)
..hideCurrentSnackBar()
..showSnackBar(
Expand All @@ -98,7 +93,7 @@ class _HomeState extends State<Home> {
await Navigator.of(context).push<AuthenticationState>(
AuthenticationFlow.route(),
);
if (!mounted) return;
if (!context.mounted) return;
ScaffoldMessenger.of(context)
..hideCurrentSnackBar()
..showSnackBar(
Expand Down
3 changes: 2 additions & 1 deletion example/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ version: 1.0.0+1
publish_to: none

environment:
sdk: ">=2.17.0 <3.0.0"
sdk: ">=3.2.0 <4.0.0"
flutter: ">=3.16.0 <4.0.0"

dependencies:
equatable: ^2.0.0
Expand Down
17 changes: 6 additions & 11 deletions lib/flow_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ class _FlowBuilderState<T> extends State<FlowBuilder<T>> {
if (mounted) {
final popHandled = await _navigator?.maybePop() ?? false;
if (popHandled) return true;
if (mounted && !_canPop) return Navigator.of(context).maybePop();
if (mounted && !_canPop) _navigator?.pop();
return false;
}
return false;
Expand Down Expand Up @@ -176,12 +176,8 @@ class _FlowBuilderState<T> extends State<FlowBuilder<T>> {
Widget build(BuildContext context) {
return _InheritedFlowController(
controller: _controller,
child: _ConditionalWillPopScope(
child: _ConditionalPopScope(
condition: _canPop,
onWillPop: () async {
await _navigator?.maybePop();
return false;
},
child: Navigator(
key: _navigatorKey,
pages: _pages,
Expand All @@ -197,6 +193,7 @@ class _FlowBuilderState<T> extends State<FlowBuilder<T>> {
_pages.removeLast();
}
setState(() {});
route.onPopInvoked(true);
return route.didPop(result);
},
),
Expand Down Expand Up @@ -329,20 +326,18 @@ class FakeFlowController<T> extends FlowController<T> {
}
}

class _ConditionalWillPopScope extends StatelessWidget {
const _ConditionalWillPopScope({
class _ConditionalPopScope extends StatelessWidget {
const _ConditionalPopScope({
required this.condition,
required this.onWillPop,
required this.child,
});

final bool condition;
final Widget child;
final Future<bool> Function() onWillPop;

@override
Widget build(BuildContext context) {
return condition ? WillPopScope(onWillPop: onWillPop, child: child) : child;
return condition ? PopScope(canPop: false, child: child) : child;
}
}

Expand Down
5 changes: 3 additions & 2 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ repository: https://github.com/felangel/flow_builder
homepage: https://github.com/felangel/flow_builder
topics: [navigation, routing]

version: 0.0.10
version: 0.1.0

environment:
sdk: ">=2.17.0 <3.0.0"
sdk: ">=3.2.0 <4.0.0"
flutter: ">=3.16.0 <4.0.0"

dependencies:
flutter:
Expand Down
52 changes: 24 additions & 28 deletions test/flow_builder_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1063,7 +1063,7 @@ void main() {
expect(find.byKey(button2Key), findsOneWidget);
});

testWidgets('onWillPop pops top page when there are multiple',
testWidgets('Navigator.pop pops top page when there are multiple',
(tester) async {
const button1Key = Key('__button1__');
const button2Key = Key('__button2__');
Expand All @@ -1084,10 +1084,14 @@ void main() {
),
MaterialPage<void>(
child: Scaffold(
body: TextButton(
key: button2Key,
child: const Text('Button'),
onPressed: () {},
body: Builder(
builder: (context) {
return TextButton(
key: button2Key,
child: const Text('Button'),
onPressed: () => Navigator.of(context).pop(),
);
},
),
),
),
Expand All @@ -1100,19 +1104,14 @@ void main() {
expect(find.byKey(button1Key), findsNothing);
expect(find.byKey(button2Key), findsOneWidget);

final willPopScope = tester.widget<WillPopScope>(
find.byType(WillPopScope),
);
final result = await willPopScope.onWillPop!();
expect(result, isFalse);

await tester.tap(find.byKey(button2Key));
await tester.pumpAndSettle();

expect(find.byKey(button1Key), findsOneWidget);
expect(find.byKey(button2Key), findsNothing);
});

testWidgets('onWillPop does not exist for only one page', (tester) async {
testWidgets('PopScope does not exist for only one page', (tester) async {
const button1Key = Key('__button1__');
await tester.pumpWidget(
MaterialApp(
Expand All @@ -1136,7 +1135,7 @@ void main() {
);

expect(find.byKey(button1Key), findsOneWidget);
expect(find.byType(WillPopScope), findsNothing);
expect(find.byType(PopScope), findsNothing);
});

testWidgets('controller change triggers a rebuild with correct state',
Expand Down Expand Up @@ -1389,21 +1388,19 @@ void main() {
expect(navigators.last.observers, equals(observers));
});

testWidgets('SystemNavigator.pop respects when WillPopScope returns false',
testWidgets('SystemNavigator.pop respects PopScope(canPop: false)',
(tester) async {
const targetKey = Key('__target__');
var onWillPopCallCount = 0;
var onPopCallCount = 0;
final flow = FlowBuilder<int>(
state: 0,
onGeneratePages: (state, pages) {
return <Page<dynamic>>[
MaterialPage<void>(
child: Builder(
builder: (context) => WillPopScope(
onWillPop: () async {
onWillPopCallCount++;
return false;
},
builder: (context) => PopScope(
canPop: false,
onPopInvoked: (_) => onPopCallCount++,
child: TextButton(
key: targetKey,
onPressed: () {
Expand Down Expand Up @@ -1441,24 +1438,23 @@ void main() {
await tester.tap(find.byKey(targetKey));
await tester.pumpAndSettle();

expect(onWillPopCallCount, equals(1));
expect(onPopCallCount, equals(1));
expect(find.byKey(targetKey), findsOneWidget);
});

testWidgets('SystemNavigator.pop respects when WillPopScope returns true',
testWidgets('SystemNavigator.pop respects PopScope(canPop: true)',
(tester) async {
const targetKey = Key('__target__');
var onWillPopCallCount = 0;
var onPopCallCount = 0;
final flow = FlowBuilder<int>(
state: 0,
onGeneratePages: (state, pages) {
return <Page<dynamic>>[
MaterialPage<void>(
child: Builder(
builder: (context) => WillPopScope(
onWillPop: () async {
onWillPopCallCount++;
return true;
builder: (context) => PopScope(
onPopInvoked: (_) {
onPopCallCount++;
},
child: TextButton(
key: targetKey,
Expand Down Expand Up @@ -1497,7 +1493,7 @@ void main() {
await tester.tap(find.byKey(targetKey));
await tester.pumpAndSettle();

expect(onWillPopCallCount, equals(1));
expect(onPopCallCount, equals(1));
expect(find.byKey(targetKey), findsNothing);
});

Expand Down
Loading