From 54c0d3625bb39f529dc700300ee3555da2991f6b Mon Sep 17 00:00:00 2001 From: Codefarmer Date: Sat, 29 Aug 2026 14:38:07 +0100 Subject: [PATCH 1/2] fix: complete the flow when popping its first screen --- packages/kaisel/CHANGELOG.md | 10 ++ packages/kaisel/lib/src/kaisel_scope.dart | 23 ++- .../test/kaisel_pop_flow_root_test.dart | 167 ++++++++++++++++++ 3 files changed, 197 insertions(+), 3 deletions(-) create mode 100644 packages/kaisel/test/kaisel_pop_flow_root_test.dart diff --git a/packages/kaisel/CHANGELOG.md b/packages/kaisel/CHANGELOG.md index 3996ab9..c37fe29 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..aa0b9ea 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 — the same thing the system + /// back button 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()]); + }); +} From c966a58f700fad036fac115670b698366007b69e Mon Sep 17 00:00:00 2001 From: Codefarmer Date: Sat, 29 Aug 2026 14:48:37 +0100 Subject: [PATCH 2/2] docs: drop dashes from the pop doc comment and changelog --- packages/kaisel/CHANGELOG.md | 2 +- packages/kaisel/lib/src/kaisel_scope.dart | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/kaisel/CHANGELOG.md b/packages/kaisel/CHANGELOG.md index c37fe29..2d5e991 100644 --- a/packages/kaisel/CHANGELOG.md +++ b/packages/kaisel/CHANGELOG.md @@ -5,7 +5,7 @@ - **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 + 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. diff --git a/packages/kaisel/lib/src/kaisel_scope.dart b/packages/kaisel/lib/src/kaisel_scope.dart index aa0b9ea..9587b8e 100644 --- a/packages/kaisel/lib/src/kaisel_scope.dart +++ b/packages/kaisel/lib/src/kaisel_scope.dart @@ -170,12 +170,12 @@ extension KaiselContextNavigation on BuildContext { /// 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 — the same thing the system - /// back button does there. That keeps a screen's contract identical whether + /// 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 + /// 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