diff --git a/packages/kaisel/CHANGELOG.md b/packages/kaisel/CHANGELOG.md index 3996ab9..2d5e991 100644 --- a/packages/kaisel/CHANGELOG.md +++ b/packages/kaisel/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## Unreleased + +- **Behaviour change:** `context.pop()` on the first screen of a modal flow + now completes that flow with the result, instead of returning `false` and + doing nothing. The system back button already dismissed the flow there, so + the two now agree, and a screen keeps the same contract whether it was + pushed with `pushForResult` or opened with `run` + ([#86](https://github.com/Mastersam07/kaisel/issues/86)). `pop` still + returns `false` at the root of the main stack. + ## 1.1.0 Additive only — no breaking changes. diff --git a/packages/kaisel/lib/src/kaisel_scope.dart b/packages/kaisel/lib/src/kaisel_scope.dart index b6e62c5..9587b8e 100644 --- a/packages/kaisel/lib/src/kaisel_scope.dart +++ b/packages/kaisel/lib/src/kaisel_scope.dart @@ -167,8 +167,16 @@ extension KaiselContextNavigation on BuildContext { _scopeForRoute(route).pushRouteForResult(route); /// Pop the nearest kaisel router, optionally returning [result] to a - /// matching [pushForResult] awaiter. Returns `false` if it was already at - /// root. + /// matching [pushForResult] awaiter. + /// + /// On the **first screen of a modal flow** there is nothing left to pop, so + /// this completes the flow with [result] instead. That is what the system + /// back button already does there. That keeps a screen's contract identical whether + /// it was pushed with [pushForResult] or opened with [KaiselRouter.run], so + /// one widget can serve both without knowing which it is in. + /// + /// Returns `false` only when nothing could be popped or completed: the root + /// of the main stack. /// /// When called from inside an overlay you pushed imperatively /// ([showModalBottomSheet], [showDialog], …), that overlay is closed instead @@ -190,10 +198,19 @@ extension KaiselContextNavigation on BuildContext { // that overlay's route, not the kaisel screen beneath it. final navigator? when boundary is RouterScope && navigator.canPop() => navigator.maybePop(result), - _ => _nearestRouterScope().router.pop(result), + _ => _popOrCompleteFlow(result), }; } + Future _popOrCompleteFlow(Object? result) async { + if (await _nearestRouterScope().router.pop(result)) return true; + if (FlowScope.maybeOf(this) case final flow?) { + flow._complete(result); + return true; + } + return false; + } + /// Pop the way the system back button does: ask the [Navigator] first, and /// only fall through to the kaisel stack when it has nothing to pop. /// diff --git a/packages/kaisel/test/kaisel_pop_flow_root_test.dart b/packages/kaisel/test/kaisel_pop_flow_root_test.dart new file mode 100644 index 0000000..0293cb3 --- /dev/null +++ b/packages/kaisel/test/kaisel_pop_flow_root_test.dart @@ -0,0 +1,167 @@ +import 'dart:async'; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:kaisel/kaisel.dart'; + +sealed class _R extends KaiselRoute { + const _R(); +} + +final class _Home extends _R { + const _Home(); +} + +// The reporter's case: one screen usable as a pushed page and as a flow root. +final class _Editor extends _R implements KaiselModalRoute { + const _Editor(); +} + +final class _Step2 extends _R { + const _Step2(); +} + +final class _Inner extends _R implements KaiselModalRoute { + const _Inner(); +} + +Widget _screen(String label, VoidCallback onBack) => Scaffold( + body: Center( + child: ElevatedButton(onPressed: onBack, child: Text(label)), + ), +); + +KaiselRouterDelegate<_R> _delegateFor(KaiselRouter<_R> router) => + KaiselRouterDelegate<_R>( + router: router, + modalBuilder: (context, route, child) => Center(child: child), + builder: (context, route) => switch (route) { + _Home() => const Scaffold(body: Text('home')), + _Editor() => Builder( + builder: (context) => _screen('editor-back', () { + context.pop('saved'); + }), + ), + _Step2() => Builder( + builder: (context) => _screen('step2-back', context.pop), + ), + _Inner() => Builder( + builder: (context) => _screen('inner-back', () { + context.pop('inner-done'); + }), + ), + }, + ); + +void main() { + testWidgets('pop on a flow root completes the flow with the result', ( + tester, + ) async { + final router = KaiselRouter<_R>(initial: const _Home()); + String? result; + await tester.pumpWidget( + MaterialApp.router(routerDelegate: _delegateFor(router)), + ); + + unawaited(router.run(const _Editor()).then((v) => result = v)); + await tester.pumpAndSettle(); + + await tester.tap(find.text('editor-back')); + await tester.pumpAndSettle(); + + expect(router.hasActiveFlow, isFalse); + expect(result, 'saved'); + expect(find.text('home'), findsOneWidget); + }); + + testWidgets('the same screen still pops normally when pushed as a page', ( + tester, + ) async { + final router = KaiselRouter<_R>(initial: const _Home()); + String? result; + await tester.pumpWidget( + MaterialApp.router(routerDelegate: _delegateFor(router)), + ); + + unawaited( + router.pushForResult(const _Editor()).then((v) => result = v), + ); + await tester.pumpAndSettle(); + + await tester.tap(find.text('editor-back')); + await tester.pumpAndSettle(); + + expect(router.stack, const [_Home()]); + expect(result, 'saved'); + }); + + testWidgets('inside a flow sub-stack it pops the step, not the flow', ( + tester, + ) async { + final router = KaiselRouter<_R>(initial: const _Home()); + await tester.pumpWidget( + MaterialApp.router(routerDelegate: _delegateFor(router)), + ); + + unawaited(router.run(const _Editor())); + await tester.pumpAndSettle(); + final flowRouter = router.activeFlows.first.router; + await flowRouter.push(const _Step2()); + await tester.pumpAndSettle(); + + await tester.tap(find.text('step2-back')); + await tester.pumpAndSettle(); + + expect(router.hasActiveFlow, isTrue); + expect(flowRouter.stack, const [_Editor()]); + expect(find.text('editor-back'), findsOneWidget); + }); + + testWidgets('a nested flow root completes the inner flow only', ( + tester, + ) async { + final router = KaiselRouter<_R>(initial: const _Home()); + String? outer; + String? inner; + await tester.pumpWidget( + MaterialApp.router(routerDelegate: _delegateFor(router)), + ); + + unawaited(router.run(const _Editor()).then((v) => outer = v)); + await tester.pumpAndSettle(); + unawaited( + router.activeFlows.first.router + .run(const _Inner()) + .then((v) => inner = v), + ); + await tester.pumpAndSettle(); + expect(router.activeFlows.length, 2); + + await tester.tap(find.text('inner-back')); + await tester.pumpAndSettle(); + + expect(inner, 'inner-done'); + expect(outer, isNull); + expect(router.activeFlows.length, 1); + }); + + testWidgets('at the root of the main stack it still reports false', ( + tester, + ) async { + final router = KaiselRouter<_R>(initial: const _Home()); + late BuildContext homeContext; + final delegate = KaiselRouterDelegate<_R>( + router: router, + builder: (context, route) => Builder( + builder: (context) { + homeContext = context; + return const Scaffold(body: Text('home')); + }, + ), + ); + await tester.pumpWidget(MaterialApp.router(routerDelegate: delegate)); + + expect(await homeContext.pop(), isFalse); + expect(router.stack, const [_Home()]); + }); +}