diff --git a/.github/actions/setup/action.yaml b/.github/actions/setup/action.yaml index f8d8a23..cc2abc8 100644 --- a/.github/actions/setup/action.yaml +++ b/.github/actions/setup/action.yaml @@ -17,31 +17,30 @@ runs: - name: 📦 Checkout the repo uses: actions/checkout@v4 - - name: 🔢 Set up version from tags - id: set-version - if: startsWith(github.ref, 'refs/tags') - shell: bash - run: | - BASE_VERSION="${GITHUB_REF#refs/tags/v}" - UNIXTIME=$(date +%s) - VERSION="${BASE_VERSION}+${UNIXTIME}" - echo "VERSION=$VERSION" >> $GITHUB_ENV - sed -i "s/^version: .*/version: ${VERSION}/" pubspec.yaml - echo "Version set to $VERSION" - - name: 🚂 Setup Flutter uses: subosito/flutter-action@v2 with: flutter-version: "${{ inputs.flutter-version }}" channel: "stable" + # Without a lockfile the pubspecs alone do not pin the resolved versions: + # the week in the key refreshes the cache regularly, and the restore keys + # start from the newest cache of the same SDK when a pubspec changes. + - name: 🗓️ Compute the cache week + id: cache-week + shell: bash + run: echo "week=$(date -u +%G-W%V)" >> "$GITHUB_OUTPUT" + - name: 📤 Restore Pub modules id: cache-pub-restore uses: actions/cache/restore@v4 with: path: | /home/runner/.pub-cache - key: ${{ runner.os }}-pub-${{ inputs.pub-cache }}-${{ hashFiles('pubspec.lock') }} + key: ${{ runner.os }}-pub-${{ inputs.pub-cache }}-flutter-${{ inputs.flutter-version }}-${{ steps.cache-week.outputs.week }}-${{ hashFiles('pubspec.yaml', 'example/pubspec.yaml') }} + restore-keys: | + ${{ runner.os }}-pub-${{ inputs.pub-cache }}-flutter-${{ inputs.flutter-version }}-${{ steps.cache-week.outputs.week }}- + ${{ runner.os }}-pub-${{ inputs.pub-cache }}-flutter-${{ inputs.flutter-version }}- - name: 👷 Install Dependencies shell: bash @@ -49,6 +48,7 @@ runs: echo /home/runner/.pub-cache/bin >> $GITHUB_PATH flutter config --no-cli-animations --no-analytics flutter pub get + flutter pub get --directory example #- name: ⏲️ Run build runner # shell: bash diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 1110c84..8b5b815 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -1,6 +1,9 @@ name: Deploy on: + # pub.dev rejects a publish started by workflow_dispatch unless + # "Enable publishing from workflow_dispatch events" is checked in the + # automated publishing settings of the package admin page on pub.dev. workflow_dispatch: push: tags: @@ -39,9 +42,14 @@ jobs: flutter pub get flutter pub get --directory example + # Also guards a manual run: it must be started from the tag of the + # version, never from a branch. - name: 🔢 Check that the tag matches the version - if: startsWith(github.ref, 'refs/tags/') run: | + if [ "${GITHUB_REF_TYPE}" != "tag" ]; then + echo "::error::Publish from a tag, not from ${GITHUB_REF}" + exit 1 + fi TAG="${GITHUB_REF_NAME}" VERSION="$(awk '/^version:/ {print $2; exit}' pubspec.yaml)" if [ "$TAG" != "$VERSION" ]; then diff --git a/.pubignore b/.pubignore new file mode 100644 index 0000000..fa6b6aa --- /dev/null +++ b/.pubignore @@ -0,0 +1,67 @@ +# pub ignores .gitignore when this file exists, so it repeats it. + +# Miscellaneous +*.class +*.log +*.pyc +*.swp +.DS_Store +.atom/ +.buildlog/ +.history +.svn/ +migrate_working_dir/ + +# IntelliJ related +*.iml +*.ipr +*.iws +.idea/ + +# The .vscode folder contains launch configuration and tasks you configure in +# VS Code which you may wish to be included in version control, so this line +# is commented out by default. +#.vscode/ + +# Flutter/Dart/Pub related +# Libraries should not include pubspec.lock, per https://dart.dev/guides/libraries/private-files#pubspeclock. +/pubspec.lock +.dart_tool +build/ +**/doc/api/ +**/ios/Flutter/.last_build_id +.flutter-plugins +.flutter-plugins-dependencies +.packages +.pub-cache/ + +# Pana +log.pana.json + +# Test +.coverage/ +coverage/ +doc/api/ +/test/**/*.json +/test/.test_coverage.dart +reports/ + +# Temp +/tmp +/temp + +# FVM +.fvm/flutter_sdk + +# Generated files +*.*.dart + +# LLMS +.claude/ +# Example +/example/pubspec.lock + +# Development only +Makefile +.vscode/ +.github/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a3f70a..34df33a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,86 @@ All notable changes to this project will be documented in this file. -## Unreleased +## 0.1.1 +- **Fixed**: on Android with the predictive back gesture, the back button + no longer closes the application while the framework can handle the + press. The top level views report one state for the whole application: + a route to close, a visible nested view that can go back, a `PopScope` + blocking the visible route, a previous tab of a + `NavigationTabsController` or an `onBackButtonPressed` of the view. A + nested view reports again when its route becomes current again, an + inactive tab no longer speaks for the visible one, a view with + `interceptBackButton: false` no longer claims the press, and removing + the last view withdraws its claim. +- **Fixed**: a `NavigationTabsController` reports the back button when it + is created around mounted views and when it is disposed, so no stale + claim of a previous tab remains. +- **Fixed**: a page built from the context of the view (the locale, the + theme, an inherited scope read in `NavigationRoute.page`) is built again + when that context changes instead of being taken from the cache. A guard + reading the context, e.g. the size of the window, no longer rebuilds the + pages. +- **Fixed**: the default `key` of a route escapes `%`, `&`, `=` and `?` in + the name and in the arguments and writes a `null` argument without a + value, so different routes, e.g. `{'q': 'a&b=c'}` and + `{'q': 'a', 'b': 'c'}`, `null` and `'null'`, or the name `'a?b'` and the + name `'a'` with `{'b': null}`, no longer produce the same key. +- **Fixed**: observers are notified at the end of the frame, together with + the listeners, when a `ContextGuard` changes the stack during the build, + so an observer calling `setState` no longer throws. The events keep + their order when an observer changes the stack while they are delivered, + and a change made by a listener at that moment no longer notifies the + listeners once more on the next frame. +- **Fixed**: an exception in one callback of an observer no longer swallows + the remaining events of the same transition for that observer. +- **Fixed**: a guard returning an empty stack is reported to `FlutterError` + instead of throwing an assertion, which dropped the changes queued after + it in a debug build. +- **Fixed**: `pop` and `removeKey` called from a guard, an observer or a + listener are postponed instead of being checked against the stack that is + about to change, e.g. `push(route)` followed by `remove(route)`. + `maybePop` called from there decides against the stack it applies to and + no longer reports a press as handled when nothing is popped. +- **Fixed**: the result of a `pop` cancelled by a guard is dropped as soon + as the guard cancels it, instead of waiting for the end of the queued + changes. +- **Fixed**: a guard can dispose the controller: the remaining guards do + not run, the observers are not notified, and a controller disposed during + its initial validation does not subscribe to `revalidate`. Observers are + also not notified when a listener disposes the controller. +- **Docs**: a guard keeping a route closed by the user brings back a new + instance of it, use a `PopScope` to prevent the closing. +- **Docs**: the name of a class route is minified in an obfuscated build, + routes compare by instance in a plain `List`, overridden `tags` must keep + `kModalTag`, calling `pushForResult` again with the same key completes + the previous waiter with `null`, `controller.stack` while the guards run, + changes made from a guard are queued, observer events may be delivered + at the end of the frame, what the default `key` guarantees. +- **CI**: a manual publication must start from the tag of the version and + be enabled on pub.dev, the pub cache depends on the pubspecs, the Flutter + version and the week and covers the example, a `.pubignore` keeps the + development files out of the package. + +## 0.1.0 + +- Initial release. +- `NavigationRoute` — a route is an immutable value: an `enum` value when it + has no parameters, a `sealed class` when it has. +- `NavigationController` — the owner of the stack, usable without any widget, + with guards, observers and revalidation built in. +- `NavigationView` — renders a controller with a `Navigator`, reports the + routes closed by the user and handles the system back button. +- `DialogRouteMixin` and `BottomSheetRouteMixin` — dialogs and bottom sheets + as ordinary entries of the navigation stack. +- `NavigationGuard` with the built-in `PriorityGuard`, `RootGuard`, + `LimitGuard` and `GateGuard`. +- `ContextGuard` — the rules that depend on the widget tree (the size of the + window, the theme, an inherited scope) re-run by themselves whenever what + they read changes. +- `NavigationObserver` and `NavigationLogger`. +- `NavigationTabsController` — several stacks side by side, cross tab + navigation and automatic routing of the back button. - **Docs**: "Helpers of your application" — an extension type over `BuildContext` with application specific shortcuts and `popModals`, which closes the declarative and the imperative popups at once. @@ -40,23 +118,3 @@ All notable changes to this project will be documented in this file. `opened()`, the same way a closed gate falls back to `closed()`. - **Fixed**: replacing the controller of a `NavigationView` runs its guards with the context of the view, so a `ContextGuard` sees it immediately. - -## 0.1.0 - -- Initial release. -- `NavigationRoute` — a route is an immutable value: an `enum` value when it - has no parameters, a `sealed class` when it has. -- `NavigationController` — the owner of the stack, usable without any widget, - with guards, observers and revalidation built in. -- `NavigationView` — renders a controller with a `Navigator`, reports the - routes closed by the user and handles the system back button. -- `DialogRouteMixin` and `BottomSheetRouteMixin` — dialogs and bottom sheets - as ordinary entries of the navigation stack. -- `NavigationGuard` with the built-in `PriorityGuard`, `RootGuard`, - `LimitGuard` and `GateGuard`. -- `ContextGuard` — the rules that depend on the widget tree (the size of the - window, the theme, an inherited scope) re-run by themselves whenever what - they read changes. -- `NavigationObserver` and `NavigationLogger`. -- `NavigationTabsController` — several stacks side by side, cross tab - navigation and automatic routing of the back button. diff --git a/lib/src/controller.dart b/lib/src/controller.dart index a7315e9..50e3429 100644 --- a/lib/src/controller.dart +++ b/lib/src/controller.dart @@ -30,6 +30,10 @@ abstract interface class NavigationAttachment { /// Navigator driven by the controller, `null` before the first build. NavigatorState? get navigator; + + /// Called when the group of the controller has changed, e.g. another tab + /// has been selected, so the widget may handle the back button differently. + void didChangeGroup(); } /// A group of controllers where only one of them is active at a time. @@ -45,6 +49,9 @@ abstract interface class NavigationGroup { /// Returns `true` when the group switched back to the previously /// selected member. bool popGroupMember(); + + /// Whether [popGroupMember] would switch to another member. + bool get canPopGroupMember; } /// {@template squid.controller} @@ -99,6 +106,9 @@ class NavigationController // reading `controller.stack` sees the stack it is validating. _stack = UnmodifiableListView(candidate); final next = _dedupe(_runGuards(candidate)); + // A guard may dispose the controller, which must not subscribe it to + // the revalidation afterwards. + if (_disposed) return; if (next.isEmpty) { throw ArgumentError.value( initial, @@ -128,7 +138,18 @@ class NavigationController >{}; final Map _results = {}; + /// Results of the [maybePop] calls postponed until the current change is + /// committed, completed with `false` when the queue is dropped. + final List> _pendingPops = >[]; + + /// The [maybePop] calls waiting for the current change to be committed. + final List> _commitWaiters = >[]; + /// Maximum number of changes requested from within a single transition. + /// + /// The changes over the limit are reported to `FlutterError` and dropped: + /// use [pushAll] or a single [change] instead of many separate calls from + /// a listener. static const int _maxQueuedChanges = 64; NavigationStack _stack; @@ -142,6 +163,15 @@ class NavigationController bool _deferNotifications = false; bool _notificationScheduled = false; + /// Whether a change committed during the build waits for the listeners to + /// be notified at the end of the frame. + bool _listenersPending = false; + + /// Transitions waiting to be reported to the [observers] at the end of the + /// frame, see [revalidateDuringBuild]. + final List<(NavigationStack, NavigationStack)> _deferredObserverEvents = + <(NavigationStack, NavigationStack)>[]; + /* #region State */ /// Current navigation stack, an unmodifiable list ordered from the root @@ -248,6 +278,14 @@ class NavigationController _queue = null; _processing = false; _settleResults(); + _settlePendingPops(); + if (_commitWaiters.isNotEmpty) { + final waiters = List>.of(_commitWaiters); + _commitWaiters.clear(); + for (final completer in waiters) { + completer.complete(); + } + } } } @@ -292,6 +330,11 @@ class NavigationController /// `null` when the route is closed in any other way: a swipe back, a tap on /// the barrier of a dialog, the system back button or a guard. /// + /// Only one caller can wait for a route: calling [pushForResult] again with + /// the same [NavigationRoute.key] completes the previous future with `null` + /// right away, even though the route stays on the screen. A plain [push] of + /// that route keeps the previous future waiting. + /// /// ```dart /// final confirmed = /// await controller.pushForResult(const ConfirmDialog()); @@ -316,8 +359,13 @@ class NavigationController /// /// Does nothing and returns `false` when there is only one route left, /// because an empty stack is not allowed. + /// + /// When called from a guard, an observer or a listener, the pop is + /// postponed until the current change is committed and `true` only means + /// that it has been scheduled: the stack it applies to is not known yet, + /// and the pop does nothing if that stack has a single route. bool pop([Object? result]) { - if (_stack.length < 2) return false; + if (!_processing && _stack.length < 2) return false; // The result is bound to the route that is actually removed, which is // not necessarily the current top when the call is postponed. change((stack) { @@ -336,20 +384,52 @@ class NavigationController /// first. When the stack cannot be popped and the controller belongs to a /// group of tabs, the group switches back to the previously selected tab. /// - /// Returns `true` when the back navigation was handled. + /// Returns `true` when the back navigation was handled. When called from a + /// guard, an observer or a listener, the decision is made against the + /// stack the pop applies to, once the current change is committed (and, + /// with a mounted [NavigationView], once its pages are rebuilt on the + /// next frame). Future maybePop() async { if (_disposed) return false; + if (_processing && this.navigator != null) { + // The navigator shows the stack being committed only once the view + // has been rebuilt: ask it after the next frame. + await _whenCommitted(); + await SchedulerBinding.instance.endOfFrame; + return maybePop(); + } final group = _group; if (group != null && !group.isActiveMember(this)) return false; final navigator = this.navigator; if (navigator != null) { if (await navigator.maybePop()) return true; - } else if (pop()) { + } else if (_processing ? await _popPending() : pop()) { return true; } return group?.popGroupMember() ?? false; } + /// Completes once the current change and the changes queued after it + /// are committed. + Future _whenCommitted() { + final completer = Completer(); + _commitWaiters.add(completer); + return completer.future; + } + + /// Queues a pop after the current change and completes with whether it + /// has found a route to remove. + Future _popPending() { + final completer = Completer(); + _pendingPops.add(completer); + change((stack) { + final popped = stack.length > 1; + if (!completer.isCompleted) completer.complete(popped); + return popped ? (stack..removeLast()) : stack; + }); + return completer.future; + } + /// Removes the routes from the top of the stack until [predicate] returns /// `true` for the visible route. /// @@ -370,8 +450,14 @@ class NavigationController /// Removes the route with the given [key] and completes its /// [pushForResult] future with [result]. + /// + /// Returns `false` when the stack has no such route. When called from a + /// guard, an observer or a listener, the removal is postponed until the + /// current change is committed and `true` only means that it has been + /// scheduled: the removal does nothing if the stack it applies to has no + /// such route. bool removeKey(LocalKey key, [Object? result]) { - if (!containsKey(key)) return false; + if (!_processing && !containsKey(key)) return false; change((stack) { final length = stack.length; stack.removeWhere((route) => route.key == key); @@ -416,6 +502,19 @@ class NavigationController @internal bool get isActiveInGroup => _group?.isActiveMember(this) ?? true; + /// Whether the group of this controller handles a back button press that + /// the controller itself cannot handle. + @internal + bool get canPopGroupMember => _group?.canPopGroupMember ?? false; + + /// Tells the widgets rendering this controller that its group has changed. + @internal + void didChangeGroup() { + for (final attachment in List.of(_attachments)) { + attachment.didChangeGroup(); + } + } + /// Joins a group of controllers, e.g. a set of tabs. @internal // ignore: use_setters_to_change_properties @@ -429,36 +528,50 @@ class NavigationController /// Applies a single change: mutation, guards and commit. void _transition(NavigationChange fn) { - final previous = _stack; - NavigationStack next; try { - next = fn(List.of(previous)); - } on Object catch (error, stackTrace) { - _reportError(error, stackTrace, 'changing the navigation stack'); - return; - } - next = _dedupe(_runGuards(_dedupe(next))); - if (next.isEmpty) { - assert( - false, - 'The navigation stack of $this cannot be empty, ' - 'the change has been rejected', + final previous = _stack; + NavigationStack next; + try { + next = fn(List.of(previous)); + } on Object catch (error, stackTrace) { + _reportError(error, stackTrace, 'changing the navigation stack'); + return; + } + next = _dedupe(_runGuards(_dedupe(next))); + // A guard may dispose the controller, e.g. on sign out. + if (_disposed) return; + if (next.isEmpty) { + // Reported instead of asserted: an exception here would drop the + // changes queued after this one. + _reportError( + StateError('The navigation stack cannot be empty'), + StackTrace.current, + 'committing a change rejected by the guards', + ); + return; + } + if (_sameStack(previous, next)) return; + _stack = UnmodifiableListView( + List.of(next), ); - return; + _completeRemoved(previous, next); + _notifyListeners(); + // A listener may dispose the controller while it is being notified. + if (_disposed) return; + _notifyObservers(previous, next); + } finally { + // A result belongs to the change that set it: the result of a pop + // cancelled by a guard must not leak into a later removal. + _results.clear(); } - if (_sameStack(previous, next)) return; - _stack = UnmodifiableListView( - List.of(next), - ); - _completeRemoved(previous, next); - _notifyListeners(); - _notifyObservers(previous, next); } /// Runs all the guards, skipping the failed ones. NavigationStack _runGuards(NavigationStack candidate) { var next = candidate; for (final guard in guards) { + // A guard may dispose the controller, e.g. on sign out. + if (_disposed) break; try { next = guard(this, List.of(next)); } on Object catch (error, stackTrace) { @@ -518,22 +631,50 @@ class NavigationController void _notifyListeners() { if (!_deferNotifications) return notifyListeners(); + _listenersPending = true; + _scheduleNotifications(); + } + + /// Notifies the listeners and the observers at the end of the frame. + void _scheduleNotifications() { if (_notificationScheduled) return; _notificationScheduled = true; SchedulerBinding.instance ..addPostFrameCallback((_) { - _notificationScheduled = false; - if (!_disposed) notifyListeners(); + if (_disposed) return; + // The flag stays set while the listeners and the observers are + // notified: the observer event of a change they make is appended to + // the queue drained below instead of scheduling another callback, + // which would notify the listeners once more for nothing. + try { + _listenersPending = false; + notifyListeners(); + // Every event stays in the queue until it is dispatched, so the + // change made by an observer is reported after the pending ones + // instead of in the middle of them, see [_notifyObservers]. + while (!_disposed && _deferredObserverEvents.isNotEmpty) { + final (previous, next) = _deferredObserverEvents.first; + _dispatchObservers(previous, next); + if (_disposed) return; + _deferredObserverEvents.removeAt(0); + } + } finally { + _notificationScheduled = false; + // Only a change deferred by the dispatch itself needs another + // callback: everything else has been reported by this one. + if (!_disposed && _listenersPending) _scheduleNotifications(); + } }, debugLabel: 'NavigationController.notifyListeners') ..ensureVisualUpdate(); } /// Completes the futures of the routes that are not in the stack once all /// the queued changes have been committed, e.g. a [pushForResult] requested - /// from a listener and then rejected by a guard, and drops the results that - /// were not consumed by any removal. + /// from a listener and then rejected by a guard. + /// + /// The results that were not consumed by any removal are already dropped + /// by [_transition]. void _settleResults() { - _results.clear(); if (_completers.isEmpty) return; final keys = {for (final route in _stack) route.key}; final orphans = >[]; @@ -547,27 +688,56 @@ class NavigationController } } + /// Completes the [maybePop] calls whose pop has been dropped with the + /// queue, e.g. because the controller has been disposed. + void _settlePendingPops() { + if (_pendingPops.isEmpty) return; + final pending = List>.of(_pendingPops); + _pendingPops.clear(); + for (final completer in pending) { + if (!completer.isCompleted) completer.complete(false); + } + } + void _notifyObservers(NavigationStack previous, NavigationStack next) { if (observers.isEmpty) return; + if (!_deferNotifications && _deferredObserverEvents.isEmpty) { + return _dispatchObservers(previous, next); + } + // An observer may mark widgets dirty, which is not allowed during the + // build: report the transition at the end of the frame, in order. While + // older transitions wait in the queue, this one waits behind them, even + // when it is not made during the build. + _deferredObserverEvents.add((previous, next)); + _scheduleNotifications(); + } + + void _dispatchObservers(NavigationStack previous, NavigationStack next) { final before = {for (final route in previous) route.key}; final after = {for (final route in next) route.key}; for (final observer in observers) { - try { - observer.onChange(this, previous, next); - for (final route in next) { - if (before.contains(route.key)) continue; - observer.onAdd(this, route); - } - for (final route in previous) { - if (after.contains(route.key)) continue; - observer.onRemove(this, route); - } - } on Object catch (error, stackTrace) { - _reportError(error, stackTrace, 'notifying the observer $observer'); + // Every callback is isolated, so a failed one does not swallow the + // remaining events of the same observer. + _notifyObserver(observer, () => observer.onChange(this, previous, next)); + for (final route in next) { + if (before.contains(route.key)) continue; + _notifyObserver(observer, () => observer.onAdd(this, route)); + } + for (final route in previous) { + if (after.contains(route.key)) continue; + _notifyObserver(observer, () => observer.onRemove(this, route)); } } } + void _notifyObserver(NavigationObserver observer, void Function() fn) { + try { + fn(); + } on Object catch (error, stackTrace) { + _reportError(error, stackTrace, 'notifying the observer $observer'); + } + } + void _reportError(Object error, StackTrace stackTrace, String context) => FlutterError.reportError( FlutterErrorDetails( @@ -591,9 +761,37 @@ class NavigationController } _completers.clear(); _results.clear(); + _deferredObserverEvents.clear(); + // A listener may dispose the controller while it is being notified, + // which `ChangeNotifier` does not allow: finish once it is done. + if (_notifyingListeners > 0) { + _disposePending = true; + return; + } super.dispose(); } + /// The depth of the [notifyListeners] calls in progress. + int _notifyingListeners = 0; + + /// Whether [dispose] has been called by a listener and waits for the + /// notification to end. + bool _disposePending = false; + + @override + void notifyListeners() { + _notifyingListeners++; + try { + super.notifyListeners(); + } finally { + _notifyingListeners--; + if (_notifyingListeners == 0 && _disposePending) { + _disposePending = false; + super.dispose(); + } + } + } + @override String toString() { final buffer = StringBuffer('NavigationController('); diff --git a/lib/src/guard.dart b/lib/src/guard.dart index 5dd61e0..0f4046c 100644 --- a/lib/src/guard.dart +++ b/lib/src/guard.dart @@ -38,6 +38,22 @@ typedef NavigationGuardCallback = /// * a different stack — the navigation is redirected; /// * `controller.stack` — the change is cancelled. /// +/// A route closed by the user — a swipe back, a tap on the barrier of a +/// dialog — has already left the navigator when the guards see its removal. +/// A guard keeping it brings back a new instance of the route, with a new +/// state. To prevent the user from closing a route, use a [PopScope] or the +/// `canPop` of its page instead. +/// +/// While a change is validated, `controller.stack` is the stack committed +/// before it. The only exception is the validation of the initial stack in +/// the constructor, where `controller.stack` is already the requested stack. +/// +/// A guard should express a redirect through the stack it returns. A +/// `controller.pop`, `controller.removeKey` or `controller.change` called +/// from a guard does not touch the stack being validated: it is queued and +/// applied to the stack committed after the current change, and is validated +/// by the guards in turn. +/// /// Guards are applied in the order they are declared, each one receiving the /// result of the previous one. An exception thrown by a guard is reported /// to `FlutterError` and the guard is skipped, so a broken rule cannot break diff --git a/lib/src/observer.dart b/lib/src/observer.dart index ccd9af8..28cff41 100644 --- a/lib/src/observer.dart +++ b/lib/src/observer.dart @@ -9,6 +9,15 @@ import 'package:squid/src/route.dart'; /// new stack has been accepted and is meant for the side effects — analytics, /// logging, the title of the application window and so on. /// +/// The callbacks are usually invoked synchronously, right after the commit. +/// A change made during the build — e.g. by a `ContextGuard` reacting to a +/// new size of the window — is reported at the end of the frame instead, +/// because an observer may mark widgets dirty. In that case all the +/// listeners of the controller are notified before the observer events, +/// the events are delivered in the order of the commits, and +/// `controller.stack` may already be newer than the `next` stack of the +/// event. +/// /// ```dart /// class AnalyticsObserver with NavigationObserver { /// AnalyticsObserver(this._analytics); diff --git a/lib/src/route.dart b/lib/src/route.dart index d344dd1..05ae39e 100644 --- a/lib/src/route.dart +++ b/lib/src/route.dart @@ -6,6 +6,13 @@ import 'package:flutter/material.dart'; /// This is a plain [List], so any operation you already know works here: /// `[...stack, route]`, `stack.where(...)`, `stack.sublist(...)` and so on. /// +/// [NavigationRoute] does not override `==`, so unless a route class +/// overrides it, `stack.contains(route)`, `stack.indexOf(route)` and +/// `stack.remove(route)` compare instances, while the controller always +/// identifies a route by its [NavigationRoute.key]: prefer +/// `controller.contains(route)`, `stack.containsKey(route.key)` and +/// `stack.without(route)` for the routes that are not constants. +/// /// {@template squid.stack} /// The last element is the visible one, the first element is the root of /// the navigation. An empty stack is not allowed and will be rejected @@ -61,6 +68,10 @@ mixin NavigationRoute { /// Used for debugging, analytics and as a part of the default [key]. /// Defaults to the enum value name for enums and to the runtime type /// for classes. + /// + /// The runtime type is minified in an obfuscated build (`--obfuscate`, a + /// release build for the web), so a class used for analytics, for logs or + /// with `findName` must override this getter with a constant string. String get name => switch (this) { Enum e => e.name, _ => runtimeType.toString(), @@ -73,7 +84,18 @@ mixin NavigationRoute { /// Keys must be unique within a single stack. /// /// The default value is built from [name] and [arguments], which is enough - /// for enums and for parameterless routes. **Routes that carry their + /// for enums and for parameterless routes: `name` without arguments, + /// `name?a=1&b=2` with them, in the order of the entries of the map, and a + /// `null` value written as a bare `name?a`. The name, the argument names + /// and the values are escaped (`%` as `%25`, `&` as `%26`, `=` as `%3D`, + /// `?` as `%3F`), so a `?`, `&` or `=` inside any of them never collides + /// with the separators: `a?b` gives `a%3Fb`, unlike `a` with `{'b': null}`. + /// + /// Two default keys are equal exactly when the names are equal and the + /// arguments have the same names, in the same order, with the same `null` + /// values and the same `toString()` of the other values. So + /// `{'a': 1, 'b': 2}` and `{'b': 2, 'a': 1}` are different keys, while + /// `{'x': 1}` and `{'x': '1'}` are the same one. **Routes that carry their /// parameters as fields must override this getter**, otherwise all of them /// will collide: /// @@ -83,23 +105,35 @@ mixin NavigationRoute { /// ``` LocalKey get key { final args = arguments; - if (args.isEmpty) return ValueKey(name); - final buffer = StringBuffer(name)..write('?'); + final escapedName = _escape(name); + if (args.isEmpty) return ValueKey(escapedName); + final buffer = StringBuffer(escapedName)..write('?'); var first = true; - for (final entry in args.entries) { + for (final MapEntry(key: argument, :value) in args.entries) { if (first) { first = false; } else { buffer.write('&'); } - buffer - ..write(entry.key) - ..write('=') - ..write(entry.value); + buffer.write(_escape(argument)); + if (value != null) { + buffer + ..write('=') + ..write(_escape(value.toString())); + } } return ValueKey(buffer.toString()); } + /// Escapes the separators of the default [key], so that a separator inside + /// the name, an argument name or a value never produces the key of another + /// route. `%` goes first, so an already escaped sequence stays distinct. + static String _escape(String value) => value + .replaceAll('%', '%25') + .replaceAll('&', '%26') + .replaceAll('=', '%3D') + .replaceAll('?', '%3F'); + /// Arbitrary marks used to address groups of routes declaratively. /// /// Tags are the cheapest way to write a guard or a bulk operation without @@ -110,6 +144,11 @@ mixin NavigationRoute { /// controller.change((stack) => /// stack.where((route) => !route.tags.contains('modal')).toList()); /// ``` + /// + /// [DialogRouteMixin] and [BottomSheetRouteMixin] mark their routes with + /// [kModalTag]: a route overriding the tags must keep it, + /// `{...super.tags, 'paywall'}`, otherwise `removeTag(kModalTag)` will not + /// close it. Set get tags => const {}; /// Ordering hint for the [PriorityGuard]. diff --git a/lib/src/tabs.dart b/lib/src/tabs.dart index 553d305..2f26778 100644 --- a/lib/src/tabs.dart +++ b/lib/src/tabs.dart @@ -61,6 +61,9 @@ class NavigationTabsController ..joinGroup(this) ..addListener(notifyListeners); } + // The views of the controllers may already be mounted: an inactive tab + // must stop claiming the back button. + _didChangeGroup(); } /// The controllers of all the tabs, in their declaration order. @@ -120,6 +123,7 @@ class NavigationTabsController ..add(_active); _active = tab; notifyListeners(); + _didChangeGroup(); } /// Returns to the previously selected tab. @@ -129,9 +133,18 @@ class NavigationTabsController if (_history.isEmpty) return false; _active = _history.removeLast(); notifyListeners(); + _didChangeGroup(); return true; } + /// Lets the views of the tabs report to the platform whether the back + /// button is handled, e.g. by returning to the previous tab. + void _didChangeGroup() { + for (final controller in tabs.values) { + controller.didChangeGroup(); + } + } + /// Tries to close the visible route of the active tab, and returns to the /// previously selected tab when it cannot be closed. /// @@ -145,15 +158,20 @@ class NavigationTabsController @override bool popGroupMember() => back(); + @override + bool get canPopGroupMember => _history.isNotEmpty; + @override void dispose() { + _history.clear(); for (final controller in tabs.values) { + // A view outliving the tabs must stop returning to the previous tab. controller ..removeListener(notifyListeners) - ..leaveGroup(this); + ..leaveGroup(this) + ..didChangeGroup(); if (disposeTabs) controller.dispose(); } - _history.clear(); super.dispose(); } diff --git a/lib/src/view.dart b/lib/src/view.dart index fc8a2ea..639df07 100644 --- a/lib/src/view.dart +++ b/lib/src/view.dart @@ -1,4 +1,5 @@ import 'package:flutter/foundation.dart'; +import 'package:flutter/scheduler.dart'; import 'package:flutter/widgets.dart'; import 'package:squid/src/controller.dart'; import 'package:squid/src/route.dart'; @@ -60,6 +61,10 @@ class NavigationView extends StatefulWidget { /// press reach the view that contains it. A nested view is visible while /// the route that contains it is the current route of the outer /// navigator. Disabling this flag disables the nested views as well. + /// + /// A disabled view does not tell the platform that it could handle the + /// press either, so on Android the predictive back gesture is not claimed + /// for its routes. final bool interceptBackButton; /// Overrides the reaction to the system back button. @@ -115,10 +120,18 @@ class _NavigationViewState extends State /// Results passed to `Navigator.pop`, waiting for [_handleDidRemovePage]. final Map _popResults = {}; + /// The top level views, which receive the back button from the + /// [WidgetsBinding]. + static final Set<_NavigationViewState> _roots = <_NavigationViewState>{}; + /// The closest [NavigationView] above this one, `null` for a top level /// view, which receives the back button from the [WidgetsBinding]. _NavigationViewState? _parent; + /// The navigator a top level view is shown in, e.g. the one of the + /// `MaterialApp`, which reports the back button once no view is left. + NavigatorState? _hostNavigator; + /// The nested views, in the order they were mounted. final List<_NavigationViewState> _children = <_NavigationViewState>[]; @@ -135,6 +148,8 @@ class _NavigationViewState extends State /// about a stack this view has already built. NavigationStack? _built; + bool _reportScheduled = false; + @override NavigatorState? get navigator => _spy.navigator; @@ -154,11 +169,16 @@ class _NavigationViewState extends State void activate() { super.activate(); _register(); + _reportBackButton(); } @override void deactivate() { + final parent = _parent; _unregister(); + // The views left behind may now be the ones handling the back button. + (parent ?? _roots.firstOrNull)?._reportBackButton(); + if (parent == null && _roots.isEmpty) _reportNoViewLeft(); super.deactivate(); } @@ -171,13 +191,31 @@ class _NavigationViewState extends State if (parent is _NavigationViewMarker) { _parent = parent.state.._children.add(this); } else { + _hostNavigator = Navigator.maybeOf(context); + _roots.add(this); WidgetsBinding.instance.addObserver(this); } } + /// Withdraws the claim of the last top level view: nothing else would + /// replace it, so the platform would keep sending the back button to a + /// framework that no longer handles it. + void _reportNoViewLeft() { + final host = _hostNavigator; + if (host == null) return; + // After the frame: the view may only be moved with a global key. + SchedulerBinding.instance.addPostFrameCallback((_) { + if (!host.mounted || _roots.isNotEmpty) return; + NavigationNotification( + canHandlePop: host.canPop(), + ).dispatch(host.context); + }, debugLabel: 'NavigationView.reportNoViewLeft'); + } + void _unregister() { final parent = _parent; if (parent == null) { + _roots.remove(this); WidgetsBinding.instance.removeObserver(this); } else { parent._children.remove(this); @@ -196,6 +234,10 @@ class _NavigationViewState extends State // // When no guard reads anything, this element has no dependencies at all // and the callback is invoked exactly once, right after the mount. + // + // The pages are built with the context of [_PagesBuilder] instead, so a + // guard reading e.g. the size of the window on every frame of the + // keyboard animation does not build all the pages again. widget.controller.revalidateDuringBuild(); } @@ -205,6 +247,12 @@ class _NavigationViewState extends State if (!identical(widget.observers, oldWidget.observers)) { _observers = [_spy, ...widget.observers]; } + if (!identical(widget.controller, oldWidget.controller) || + widget.interceptBackButton != oldWidget.interceptBackButton || + (widget.onBackButtonPressed == null) != + (oldWidget.onBackButtonPressed == null)) { + _reportBackButton(); + } if (identical(widget.controller, oldWidget.controller)) return; oldWidget.controller ..removeListener(_handleStackChanged) @@ -242,13 +290,15 @@ class _NavigationViewState extends State /// navigator that contains it is not covered by anything. bool get _visible => mounted && (_route?.isCurrent ?? true); + /// Whether this view reacts to the back button at all: it is not disabled + /// and its controller is not an inactive tab. + bool get _enabled => + widget.interceptBackButton && widget.controller.isActiveInGroup; + /// Offers the back button to the visible nested views first, then handles /// it with the controller of this view. Future _handleBackButton() async { - final controller = widget.controller; - if (!widget.interceptBackButton || !controller.isActiveInGroup) { - return false; - } + if (!_enabled) return false; for (final child in _children.reversed.toList(growable: false)) { if (!child._visible) continue; if (await child._handleBackButton()) return true; @@ -260,6 +310,80 @@ class _NavigationViewState extends State return widget.controller.maybePop(); } + /// Whether a press reaching this view would be handled, decided the same + /// way as [_handleBackButton] does it. + /// + /// It is handled by a visible nested view that can go back, by closing a + /// route, by a [PopScope] blocking the visible route, by a handler of the + /// view or by returning to the previous tab. + bool get _canHandlePop { + if (!mounted || !_enabled) return false; + for (final child in _children) { + if (child._visible && child._canHandlePop) return true; + } + final navigator = this.navigator; + if (navigator != null && + (navigator.canPop() || + _spy.top?.popDisposition == RoutePopDisposition.doNotPop)) { + return true; + } + return widget.onBackButtonPressed != null || + widget.controller.canPopGroupMember; + } + + /// Replaces the notifications of the navigator, of its routes and of the + /// nested views with the state of the whole application, see + /// [_reportBackButton]. + /// + /// The navigator alone gets it wrong: it reports `canHandlePop: false` + /// with a single route even when the tabs return to the previous one, and + /// `true` with several routes even when this view is disabled or is an + /// inactive tab kept alive by an `IndexedStack`. + bool _handleNavigationNotification(NavigationNotification notification) { + _reportBackButton(); + return true; + } + + /// Tells the platform whether the framework handles the back button. + /// + /// Without it, on Android with the predictive back gesture the press + /// would close the application without ever reaching [didPopRoute]. + /// + /// Every report describes all the top level views at once, so that two of + /// them, e.g. the tabs of an `IndexedStack`, never overwrite each other + /// whatever the order of their reports is. + void _reportBackButton() { + // The navigator dispatches its notifications from the post frame + // callbacks: answer in the same frame. + if (SchedulerBinding.instance.schedulerPhase == + SchedulerPhase.postFrameCallbacks) { + _dispatchBackButton(); + return; + } + if (_reportScheduled) return; + _reportScheduled = true; + SchedulerBinding.instance + ..addPostFrameCallback((_) { + _reportScheduled = false; + _dispatchBackButton(); + }, debugLabel: 'NavigationView.reportBackButton') + ..ensureVisualUpdate(); + } + + void _dispatchBackButton() { + if (!mounted) return; + final parent = _parent; + if (parent != null) return parent._dispatchBackButton(); + // Deactivated, e.g. while it is moved with a global key. + if (!_roots.contains(this)) return; + NavigationNotification( + canHandlePop: _roots.any((view) => view._visible && view._canHandlePop), + ).dispatch(context); + } + + @override + void didChangeGroup() => _reportBackButton(); + /// Called by the [Navigator] when a route has been closed by the user /// or by the framework: a swipe back, a tap on the barrier of a dialog, /// the button of an [AppBar], a `Navigator.pop(context, result)`. @@ -274,6 +398,9 @@ class _NavigationViewState extends State } /// Converts the routes into pages, reusing the unchanged ones. + /// + /// [context] belongs to [_PagesBuilder], which clears the cache when an + /// inherited widget read by a page changes. List> _buildPages(BuildContext context) { final stack = _built = widget.controller.stack; final keys = {}; @@ -300,28 +427,72 @@ class _NavigationViewState extends State return pages; } + void _handleRouteChanged(ModalRoute? route) { + _route = route; + // The route may have been covered or uncovered: a nested view starts or + // stops speaking for the back button. + _reportBackButton(); + } + @override - Widget build(BuildContext context) => _NavigationViewMarker( - state: this, - child: _RouteProbe( - onRoute: (route) => _route = route, - child: NavigationScope( - controller: widget.controller, - child: Navigator( - pages: _buildPages(context), - onDidRemovePage: _handleDidRemovePage, - transitionDelegate: widget.transitionDelegate, - observers: _observers, - restorationScopeId: widget.restorationScopeId, - requestFocus: widget.requestFocus, - clipBehavior: widget.clipBehavior, - reportsRouteUpdateToEngine: widget.reportsRouteUpdateToEngine, + Widget build(BuildContext context) => _PagesBuilder( + onDependenciesChanged: _cache.clear, + builder: (context) => _NavigationViewMarker( + state: this, + child: _RouteProbe( + onRoute: _handleRouteChanged, + child: NavigationScope( + controller: widget.controller, + child: NotificationListener( + onNotification: _handleNavigationNotification, + child: Navigator( + pages: _buildPages(context), + onDidRemovePage: _handleDidRemovePage, + transitionDelegate: widget.transitionDelegate, + observers: _observers, + restorationScopeId: widget.restorationScopeId, + requestFocus: widget.requestFocus, + clipBehavior: widget.clipBehavior, + reportsRouteUpdateToEngine: widget.reportsRouteUpdateToEngine, + ), + ), ), ), ), ); } +/// Builds the pages of a [NavigationView] with a context of its own. +/// +/// A separate element, so that the inherited widgets read by the guards and +/// by the pages are tracked separately: a page reading the locale or the +/// theme is built again when it changes, but not when only a guard depends +/// on the changed widget. +class _PagesBuilder extends StatefulWidget { + const _PagesBuilder({ + required this.onDependenciesChanged, + required this.builder, + }); + + final VoidCallback onDependenciesChanged; + + final WidgetBuilder builder; + + @override + State<_PagesBuilder> createState() => _PagesBuilderState(); +} + +class _PagesBuilderState extends State<_PagesBuilder> { + @override + void didChangeDependencies() { + super.didChangeDependencies(); + widget.onDependenciesChanged(); + } + + @override + Widget build(BuildContext context) => widget.builder(context); +} + /// Links a [NavigationView] with the views nested into its routes. class _NavigationViewMarker extends InheritedWidget { const _NavigationViewMarker({required this.state, required super.child}); @@ -366,6 +537,16 @@ class _NavigationViewObserver extends NavigatorObserver { final void Function(LocalKey key, Object? result) _onPop; + /// The visible route of the navigator, to see whether a [PopScope] + /// blocks the back button. + Route? top; + + @override + void didChangeTop( + Route topRoute, + Route? previousTopRoute, + ) => top = topRoute; + @override void didPush(Route route, Route? previousRoute) => _track(route); diff --git a/pubspec.yaml b/pubspec.yaml index ad15d8f..48fd6cd 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,7 +1,7 @@ name: squid description: "A simple, reliable and extensible declarative navigator for Flutter. The whole navigation is a plain list of routes you can rewrite at any moment." -version: 0.1.0 +version: 0.1.1 homepage: https://github.com/PlugFox/squid diff --git a/test/back_button_test.dart b/test/back_button_test.dart new file mode 100644 index 0000000..75bdc81 --- /dev/null +++ b/test/back_button_test.dart @@ -0,0 +1,466 @@ +import 'dart:async'; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:squid/squid.dart'; + +import 'src/routes.dart'; + +enum _Tab { feed, cart } + +/// What the application last told the platform about the back button. +/// +/// On Android with the predictive back gesture the system closes the +/// application on its own unless the framework claims the press. +class _Platform { + bool? handlesBack; +} + +void main() => group('back button', () { + Future<_Platform> pumpApp(WidgetTester tester, Widget home) async { + final platform = _Platform(); + await tester.pumpWidget( + MaterialApp( + onNavigationNotification: (notification) { + platform.handlesBack = notification.canHandlePop; + return true; + }, + home: home, + ), + ); + await tester.pumpAndSettle(); + return platform; + } + + /// A shell rebuilt on every change of [tabs], one view per tab. + Widget shell(NavigationTabsController<_Tab> tabs) => ListenableBuilder( + listenable: tabs, + builder: (context, _) => IndexedStack( + index: tabs.activeIndex, + children: [ + for (final controller in tabs.tabs.values) + NavigationView(controller: controller), + ], + ), + ); + + testWidgets('a nested view is reported again when its route is on top', ( + tester, + ) async { + final inner = NavigationController([ + Routes.home, + Routes.catalog, + ]); + addTearDown(inner.dispose); + final outer = NavigationController([_ShellRoute(inner)]); + addTearDown(outer.dispose); + final platform = await pumpApp(tester, NavigationView(controller: outer)); + expect(platform.handlesBack, isTrue, reason: 'the nested view can pop'); + + outer.push(Routes.settings); + await tester.pumpAndSettle(); + expect(platform.handlesBack, isTrue); + + outer.pop(); + await tester.pumpAndSettle(); + expect(platform.handlesBack, isTrue, reason: 'the nested view is back'); + + // The press goes to the nested view. + expect(await tester.binding.handlePopRoute(), isTrue); + await tester.pumpAndSettle(); + expect(inner.stack, equals([Routes.home])); + expect(outer.length, equals(1)); + expect(platform.handlesBack, isFalse, reason: 'nothing left to pop'); + + // Nothing to pop: the application may be closed. + expect(await tester.binding.handlePopRoute(), isFalse); + }); + + testWidgets('a tab is reported with the nested view it hosts', ( + tester, + ) async { + final inner = NavigationController([ + Routes.home, + Routes.catalog, + ]); + addTearDown(inner.dispose); + final tabs = NavigationTabsController<_Tab>( + tabs: <_Tab, NavigationController>{ + _Tab.feed: NavigationController([_ShellRoute(inner)]), + _Tab.cart: NavigationController([Routes.catalog]), + }, + ); + addTearDown(tabs.dispose); + final platform = await pumpApp(tester, shell(tabs)); + expect(platform.handlesBack, isTrue); + + tabs.select(_Tab.cart); + await tester.pumpAndSettle(); + expect(platform.handlesBack, isTrue, reason: 'returns to the feed'); + + tabs.back(); + await tester.pumpAndSettle(); + expect(platform.handlesBack, isTrue, reason: 'the nested view can pop'); + + expect(await tester.binding.handlePopRoute(), isTrue); + await tester.pumpAndSettle(); + expect(inner.stack, equals([Routes.home])); + expect(tabs.active, equals(_Tab.feed)); + expect(platform.handlesBack, isFalse); + + expect(await tester.binding.handlePopRoute(), isFalse); + }); + + testWidgets('a PopScope on the root of a tab keeps the back button', ( + tester, + ) async { + var blocked = 0; + final tabs = NavigationTabsController<_Tab>( + tabs: <_Tab, NavigationController>{ + _Tab.feed: NavigationController([ + _GuardedRoute(() => blocked++), + ]), + _Tab.cart: NavigationController([Routes.catalog]), + }, + ); + addTearDown(tabs.dispose); + // The views are created once: nothing rebuilds them on a switch. + final views = [ + for (final controller in tabs.tabs.values) + NavigationView(controller: controller), + ]; + final platform = await pumpApp( + tester, + ListenableBuilder( + listenable: tabs, + builder: (context, _) => + IndexedStack(index: tabs.activeIndex, children: views), + ), + ); + expect(platform.handlesBack, isTrue, reason: 'the PopScope blocks it'); + + tabs.select(_Tab.cart); + await tester.pumpAndSettle(); + expect(platform.handlesBack, isTrue); + + tabs.back(); + await tester.pumpAndSettle(); + expect(platform.handlesBack, isTrue, reason: 'the PopScope blocks it'); + + // The press is handled by the PopScope, nothing is closed. + expect(await tester.binding.handlePopRoute(), isTrue); + await tester.pumpAndSettle(); + expect(blocked, equals(1)); + expect(tabs.active, equals(_Tab.feed)); + expect(find.text('guarded'), findsOneWidget); + }); + + testWidgets('a disabled view does not claim the back button', (tester) async { + final controller = NavigationController([ + Routes.home, + Routes.catalog, + ]); + addTearDown(controller.dispose); + final platform = await pumpApp( + tester, + NavigationView(controller: controller, interceptBackButton: false), + ); + expect(platform.handlesBack, isFalse); + + expect(await tester.binding.handlePopRoute(), isFalse); + expect(controller.length, equals(2)); + }); + + testWidgets('disabling the back button is reported', (tester) async { + final controller = NavigationController([ + Routes.home, + Routes.catalog, + ]); + addTearDown(controller.dispose); + final enabled = ValueNotifier(true); + addTearDown(enabled.dispose); + final platform = await pumpApp( + tester, + ValueListenableBuilder( + valueListenable: enabled, + builder: (context, value, _) => + NavigationView(controller: controller, interceptBackButton: value), + ), + ); + expect(platform.handlesBack, isTrue); + + enabled.value = false; + await tester.pumpAndSettle(); + expect(platform.handlesBack, isFalse); + expect(await tester.binding.handlePopRoute(), isFalse); + + enabled.value = true; + await tester.pumpAndSettle(); + expect(platform.handlesBack, isTrue); + expect(await tester.binding.handlePopRoute(), isTrue); + await tester.pumpAndSettle(); + expect(controller.length, equals(1)); + expect(platform.handlesBack, isFalse); + }); + + testWidgets('hand-made tabs disabled with interceptBackButton', ( + tester, + ) async { + final first = NavigationController([ + Routes.home, + Routes.catalog, + ]); + addTearDown(first.dispose); + final second = NavigationController([Routes.settings]); + addTearDown(second.dispose); + final index = ValueNotifier(0); + addTearDown(index.dispose); + final platform = await pumpApp( + tester, + ValueListenableBuilder( + valueListenable: index, + builder: (context, value, _) => IndexedStack( + index: value, + children: [ + NavigationView(controller: first, interceptBackButton: value == 0), + NavigationView(controller: second, interceptBackButton: value == 1), + ], + ), + ), + ); + expect(platform.handlesBack, isTrue); + + index.value = 1; + await tester.pumpAndSettle(); + expect(platform.handlesBack, isFalse, reason: 'a single visible route'); + expect(await tester.binding.handlePopRoute(), isFalse); + expect(first.length, equals(2), reason: 'the hidden tab is untouched'); + + index.value = 0; + await tester.pumpAndSettle(); + expect(platform.handlesBack, isTrue); + expect(await tester.binding.handlePopRoute(), isTrue); + await tester.pumpAndSettle(); + expect(first.length, equals(1)); + expect(platform.handlesBack, isFalse); + }); + + testWidgets('disposing the tabs drops the claim of the previous tab', ( + tester, + ) async { + final feed = NavigationController([Routes.home]); + addTearDown(feed.dispose); + final cart = NavigationController([Routes.catalog]); + addTearDown(cart.dispose); + final tabs = NavigationTabsController<_Tab>( + tabs: <_Tab, NavigationController>{_Tab.feed: feed, _Tab.cart: cart}, + disposeTabs: false, + ); + final platform = await pumpApp(tester, NavigationView(controller: cart)); + expect(platform.handlesBack, isFalse, reason: 'an inactive tab is silent'); + + tabs.select(_Tab.cart); + await tester.pumpAndSettle(); + expect(platform.handlesBack, isTrue, reason: 'returns to the feed'); + + tabs.dispose(); + await tester.pumpAndSettle(); + expect(platform.handlesBack, isFalse, reason: 'no tabs to return to'); + expect(await tester.binding.handlePopRoute(), isFalse); + }); + + testWidgets('tabs created around a mounted view are reported', ( + tester, + ) async { + final feed = NavigationController([Routes.home]); + addTearDown(feed.dispose); + final cart = NavigationController([ + Routes.catalog, + Routes.settings, + ]); + addTearDown(cart.dispose); + final platform = await pumpApp(tester, NavigationView(controller: cart)); + expect(platform.handlesBack, isTrue); + + final tabs = NavigationTabsController<_Tab>( + tabs: <_Tab, NavigationController>{_Tab.feed: feed, _Tab.cart: cart}, + disposeTabs: false, + ); + await tester.pumpAndSettle(); + expect(platform.handlesBack, isFalse, reason: 'an inactive tab'); + expect(await tester.binding.handlePopRoute(), isFalse); + expect(cart.length, equals(2)); + + tabs.dispose(); + await tester.pumpAndSettle(); + expect(platform.handlesBack, isTrue, reason: 'out of the tabs again'); + }); + + testWidgets('removing the last view is reported', (tester) async { + final controller = NavigationController([ + Routes.home, + Routes.catalog, + ]); + addTearDown(controller.dispose); + final visible = ValueNotifier(true); + addTearDown(visible.dispose); + final platform = await pumpApp( + tester, + ValueListenableBuilder( + valueListenable: visible, + builder: (context, value, _) => value + ? NavigationView(controller: controller) + : const Text('no navigation'), + ), + ); + expect(platform.handlesBack, isTrue); + + visible.value = false; + await tester.pumpAndSettle(); + expect(find.text('no navigation'), findsOneWidget); + expect(platform.handlesBack, isFalse, reason: 'no view is left'); + expect(await tester.binding.handlePopRoute(), isFalse); + }); + + testWidgets('maybePop from a listener pops the pages of the new stack', ( + tester, + ) async { + final controller = NavigationController([Routes.home]); + addTearDown(controller.dispose); + await pumpApp(tester, NavigationView(controller: controller)); + + Future? popped; + controller + ..addListener(() => popped ??= controller.maybePop()) + ..pushAll([Routes.catalog, Routes.settings]); + await tester.pumpAndSettle(); + + expect(await popped, isTrue); + expect( + controller.stack, + equals([Routes.home, Routes.catalog]), + ); + }); + + testWidgets('a plain route pushed above a view is reported', (tester) async { + final controller = NavigationController([Routes.home]); + addTearDown(controller.dispose); + final platform = await pumpApp( + tester, + NavigationView(controller: controller), + ); + expect(platform.handlesBack, isFalse, reason: 'nothing to pop'); + + final app = tester.state(find.byType(Navigator).first); + unawaited( + app.push(MaterialPageRoute(builder: (_) => const Text('plain'))), + ); + await tester.pumpAndSettle(); + expect(find.text('plain'), findsOneWidget); + expect(platform.handlesBack, isTrue, reason: 'the app can pop the route'); + + // The hidden view reports again. + controller.replaceTop(Routes.catalog); + await tester.pumpAndSettle(); + expect(platform.handlesBack, isTrue, reason: 'the app can pop the route'); + + expect(await tester.binding.handlePopRoute(), isTrue); + await tester.pumpAndSettle(); + expect(find.text('plain'), findsNothing); + expect(platform.handlesBack, isFalse); + }); + + testWidgets('a guard reading the context does not rebuild the pages', ( + tester, + ) async { + var builds = 0; + final controller = NavigationController( + [_CountedRoute(() => builds++)], + guards: [const _SizeGuard()], + ); + addTearDown(controller.dispose); + final width = ValueNotifier(400); + addTearDown(width.dispose); + await tester.pumpWidget( + MaterialApp( + home: ValueListenableBuilder( + valueListenable: width, + builder: (context, value, child) => MediaQuery( + data: MediaQuery.of(context).copyWith(size: Size(value, 600)), + child: child!, + ), + child: NavigationView(controller: controller), + ), + ), + ); + await tester.pumpAndSettle(); + final initial = builds; + expect(initial, isPositive); + + width.value = 300; + await tester.pumpAndSettle(); + width.value = 200; + await tester.pumpAndSettle(); + expect(builds, equals(initial), reason: 'the routes have not changed'); + expect(find.text('counted'), findsOneWidget); + }); +}); + +final class _ShellRoute with NavigationRoute { + const _ShellRoute(this.controller); + + final NavigationController controller; + + @override + String get name => 'shell'; + + @override + Widget build(BuildContext context) => NavigationView(controller: controller); +} + +final class _GuardedRoute with NavigationRoute { + const _GuardedRoute(this.onBlocked); + + final VoidCallback onBlocked; + + @override + String get name => 'guarded'; + + @override + Widget build(BuildContext context) => PopScope( + canPop: false, + onPopInvokedWithResult: (didPop, _) { + if (!didPop) onBlocked(); + }, + child: const Text('guarded'), + ); +} + +final class _CountedRoute with NavigationRoute { + const _CountedRoute(this.onPage); + + final VoidCallback onPage; + + @override + String get name => 'counted'; + + @override + Page page(BuildContext context) { + onPage(); + return MaterialPage(key: key, name: name, child: build(context)); + } + + @override + Widget build(BuildContext context) => const Text('counted'); +} + +/// Subscribes the view to the size of the window, keeps the stack. +class _SizeGuard extends ContextGuard { + const _SizeGuard(); + + @override + NavigationStack guard(BuildContext context, NavigationStack stack) { + MediaQuery.sizeOf(context); + return stack; + } +} diff --git a/test/context_test.dart b/test/context_test.dart index f320672..5840870 100644 --- a/test/context_test.dart +++ b/test/context_test.dart @@ -230,6 +230,80 @@ void main() => group('context', () { ); } + testWidgets('an observer marking an ancestor dirty during a build time ' + 'change is notified at the end of the frame', (tester) async { + final current = ValueNotifier(''); + addTearDown(current.dispose); + final controller = NavigationController( + [Routes.home], + guards: [const _AdaptiveGuard()], + observers: [_TopObserver(current)], + ); + addTearDown(controller.dispose); + + Widget app(double width) => MaterialApp( + home: MediaQuery( + data: MediaQueryData(size: Size(width, 600)), + child: ValueListenableBuilder( + valueListenable: current, + builder: (context, top, _) => Column( + children: [ + Text('observed:$top'), + Expanded(child: NavigationView(controller: controller)), + ], + ), + ), + ), + ); + + await tester.pumpWidget(app(800)); + await tester.pumpAndSettle(); + expect(tester.takeException(), isNull); + expect(find.text('observed:_SidePaneRoute'), findsOneWidget); + + await tester.pumpWidget(app(300)); + await tester.pumpAndSettle(); + expect(tester.takeException(), isNull); + expect(find.text('observed:home'), findsOneWidget); + }); + + testWidgets('observers see the build time changes in order when one of ' + 'them changes the stack', (tester) async { + final first = _TranscriptObserver(pushOnFirst: Routes.settings); + final second = _TranscriptObserver(); + final controller = NavigationController( + [Routes.home], + guards: [const _AdaptiveGuard()], + observers: [first, second], + ); + addTearDown(controller.dispose); + + var notifications = 0; + controller.addListener(() => notifications++); + + await tester.pumpWidget( + MaterialApp( + home: MediaQuery( + data: const MediaQueryData(size: Size(800, 600)), + child: NavigationView(controller: controller), + ), + ), + ); + await tester.pumpAndSettle(); + expect(tester.takeException(), isNull); + + // The guard adds the side pane while the view is built, the observers + // hear about it at the end of the frame, and the first one reacts. + const transcript = [ + 'home -> home>_SidePaneRoute', + 'home>_SidePaneRoute -> home>_SidePaneRoute>settings', + ]; + expect(first.transcript, equals(transcript)); + expect(second.transcript, equals(transcript)); + expect(notifications, equals(2), reason: 'one per transition'); + expect(find.text('screen:settings'), findsOneWidget); + }); + testWidgets('a page reading the stack follows a build time change ' 'in the same frame', (tester) async { final controller = NavigationController( @@ -262,3 +336,40 @@ final class _StackLengthRoute with NavigationRoute { Widget build(BuildContext context) => Text('length:${NavigationScope.stackOf(context).length}'); } + +class _TopObserver with NavigationObserver { + _TopObserver(this.current); + + final ValueNotifier current; + + @override + void onChange( + NavigationController controller, + NavigationStack previous, + NavigationStack next, + ) => current.value = next.last.name; +} + +/// Records every transition as `previous -> next`, and pushes [pushOnFirst] +/// on the first one. +class _TranscriptObserver with NavigationObserver { + _TranscriptObserver({this.pushOnFirst}); + + final NavigationRoute? pushOnFirst; + + final List transcript = []; + + static String _names(NavigationStack stack) => + stack.map((route) => route.name).join('>'); + + @override + void onChange( + NavigationController controller, + NavigationStack previous, + NavigationStack next, + ) { + transcript.add('${_names(previous)} -> ${_names(next)}'); + final route = pushOnFirst; + if (route != null && transcript.length == 1) controller.push(route); + } +} diff --git a/test/controller_test.dart b/test/controller_test.dart index 567e192..2ae4bc0 100644 --- a/test/controller_test.dart +++ b/test/controller_test.dart @@ -504,8 +504,370 @@ void main() => group('controller', () { expect(controller.isDisposed, isTrue); expect(controller.top, equals(Routes.catalog)); }); + + test('a guard returning an empty stack is reported, the queue goes on', () { + final errors = []; + final previous = FlutterError.onError; + FlutterError.onError = (details) => errors.add(details.exception); + addTearDown(() => FlutterError.onError = previous); + + final controller = NavigationController( + [Routes.home], + guards: [ + NavigationGuard( + (controller, stack) => stack.containsKey(Routes.signIn.key) + ? [] + : stack, + ), + ], + ); + addTearDown(controller.dispose); + + var queued = false; + controller + ..addListener(() { + if (queued) return; + queued = true; + controller + ..push(Routes.signIn) + ..push(Routes.settings); + }) + ..push(Routes.catalog); + + expect(errors, hasLength(1)); + expect(errors.single, isStateError); + expect( + controller.stack, + equals([Routes.home, Routes.catalog, Routes.settings]), + ); + }); + + test('pop and remove from a listener see the pending changes', () { + final controller = NavigationController([Routes.home]); + addTearDown(controller.dispose); + + var queued = false; + controller + ..addListener(() { + if (queued) return; + queued = true; + controller.push(const ProductRoute(1)); + expect(controller.remove(const ProductRoute(1)), isTrue); + controller.push(Routes.settings); + }) + ..push(Routes.catalog); + + expect( + controller.stack, + equals([Routes.home, Routes.catalog, Routes.settings]), + ); + }); + + test('a guard can dispose the controller', () async { + final errors = []; + final previous = FlutterError.onError; + FlutterError.onError = (details) => errors.add(details.exception); + addTearDown(() => FlutterError.onError = previous); + + var later = 0; + final controller = NavigationController( + [Routes.home], + guards: [ + NavigationGuard((controller, stack) { + // E.g. a sign out tearing down the whole navigation. + if (stack.contains(Routes.settings)) controller.dispose(); + return stack; + }), + NavigationGuard((controller, stack) { + later++; + return stack; + }), + ], + ); + expect(later, equals(1), reason: 'the initial validation'); + + final result = controller.pushForResult(Routes.settings); + expect(controller.isDisposed, isTrue); + expect(later, equals(1), reason: 'no guard runs after the dispose'); + expect(await result, isNull); + expect(errors, isEmpty); + expect(controller.stack, equals([Routes.home])); + }); + + test('a guard disposing the controller during the initial validation ' + 'does not subscribe it to revalidate', () { + final flag = _CountingListenable(); + final controller = NavigationController( + [Routes.home], + revalidate: flag, + guards: [ + NavigationGuard((controller, stack) { + controller.dispose(); + return stack; + }), + ], + ); + expect(controller.isDisposed, isTrue); + expect(flag.listeners, isEmpty); + }); + + test('observers are not notified about a change after a listener has ' + 'disposed the controller', () { + final errors = []; + final previous = FlutterError.onError; + FlutterError.onError = (details) => errors.add(details.exception); + addTearDown(() => FlutterError.onError = previous); + + final observer = _RecordingObserver(); + final controller = NavigationController( + [Routes.home], + observers: [observer], + ); + controller + ..addListener(controller.dispose) + ..push(Routes.catalog); + expect(errors, isEmpty); + expect(controller.isDisposed, isTrue); + expect(observer.changes, isZero); + expect(observer.added, isEmpty); + }); + + group('maybePop from a listener', () { + test('returns false when the pending stack has a single route', () async { + final controller = NavigationController([ + Routes.home, + Routes.catalog, + ]); + addTearDown(controller.dispose); + + Future? popped; + controller.addListener(() => popped ??= controller.maybePop()); + expect(controller.pop(), isTrue); + + expect(controller.stack, equals([Routes.home])); + expect(await popped, isFalse); + expect(controller.stack, equals([Routes.home])); + }); + + test('pops the route left by the pending changes', () async { + final controller = NavigationController([Routes.home]); + addTearDown(controller.dispose); + + Future? popped; + controller + ..addListener(() => popped ??= controller.maybePop()) + ..pushAll([Routes.catalog, Routes.settings]); + + expect(await popped, isTrue); + expect( + controller.stack, + equals([Routes.home, Routes.catalog]), + ); + }); + + test('completes with false when the queue is dropped', () async { + final errors = []; + final previous = FlutterError.onError; + FlutterError.onError = (details) => errors.add(details.exception); + addTearDown(() => FlutterError.onError = previous); + + late final NavigationController controller; + controller = NavigationController( + [Routes.home], + observers: [_DisposingObserver()], + ); + Future? popped; + controller + ..addListener(() => popped ??= controller.maybePop()) + ..push(Routes.catalog); + + expect(controller.isDisposed, isTrue); + expect(await popped, isFalse); + expect(errors, isEmpty); + }); + }); + + group('notifications deferred to the end of the frame', () { + testWidgets('are delivered in order when an observer changes the stack', ( + tester, + ) async { + var redirect = false; + final a = _ChainingObserver(); + final b = _TranscriptObserver(); + final controller = NavigationController( + [Routes.home], + guards: [ + NavigationGuard( + (controller, stack) => redirect && !stack.contains(Routes.catalog) + ? [...stack, Routes.catalog] + : stack, + ), + ], + observers: [a, b], + ); + addTearDown(controller.dispose); + + redirect = true; + controller.revalidateDuringBuild(); + expect(b.transcript, isEmpty, reason: 'deferred to the end of the frame'); + await tester.pump(); + + expect( + b.transcript, + equals([ + 'home -> home>catalog', + 'home>catalog -> home>catalog>settings', + ]), + ); + expect(a.transcript, equals(b.transcript)); + }); + + testWidgets('notify every listener once per frame', (tester) async { + var redirect = false; + final controller = NavigationController( + [Routes.home], + guards: [ + NavigationGuard( + (controller, stack) => redirect && !stack.contains(Routes.catalog) + ? [...stack, Routes.catalog] + : stack, + ), + ], + observers: [_TranscriptObserver()], + ); + addTearDown(controller.dispose); + + var notifications = 0; + controller.addListener(() { + // The first notification is the deferred one: the listener reacts + // with a change of its own, notified synchronously. + if (notifications++ == 0) controller.push(Routes.settings); + }); + + redirect = true; + controller.revalidateDuringBuild(); + expect(notifications, isZero); + await tester.pump(); + expect(notifications, equals(2)); + expect( + controller.stack, + equals([Routes.home, Routes.catalog, Routes.settings]), + ); + + // Nothing has changed since: no spurious notification next frame. + tester.binding.scheduleFrame(); + await tester.pump(); + await tester.pump(); + expect(notifications, equals(2)); + }); + + testWidgets('a change deferred by a listener is notified next frame', ( + tester, + ) async { + final redirects = {}; + final controller = NavigationController( + [Routes.home], + guards: [ + NavigationGuard( + (controller, stack) => [ + ...stack, + for (final route in redirects) + if (!stack.contains(route)) route, + ], + ), + ], + observers: [_TranscriptObserver()], + ); + addTearDown(controller.dispose); + + final seen = []; + controller.addListener(() { + seen.add(controller.top); + if (seen.length > 1) return; + redirects.add(Routes.settings); + controller.revalidateDuringBuild(); + }); + + redirects.add(Routes.catalog); + controller.revalidateDuringBuild(); + await tester.pump(); + expect(seen, equals([Routes.catalog])); + await tester.pump(); + expect(seen, equals([Routes.catalog, Routes.settings])); + }); + }); + + test('a stale result of a cancelled pop is not used by a queued removal', () { + final locked = Flag(value: true); + final controller = NavigationController( + [Routes.home], + guards: [ + NavigationGuard( + (controller, stack) => + locked.value && + !stack.containsTag(kModalTag) && + controller.stack.containsTag(kModalTag) + ? controller.stack + : stack, + ), + ], + ); + addTearDown(controller.dispose); + final result = controller.pushForResult(const ConfirmDialogRoute()); + + // The pop is cancelled, then a removal without a result is queued. + controller.change((stack) { + controller + ..pop(true) + ..change((stack) { + locked.value = false; + return stack.withoutTag(kModalTag); + }); + return stack; + }); + + expect(controller.top, equals(Routes.home)); + expect(result, completion(isNull)); + }); + + test('a failed callback does not swallow the other events', () { + final errors = []; + final previous = FlutterError.onError; + FlutterError.onError = (details) => errors.add(details.exception); + addTearDown(() => FlutterError.onError = previous); + + final observer = _ThrowingOnAddObserver(); + final controller = NavigationController( + [Routes.home, Routes.catalog], + observers: [observer], + ); + addTearDown(controller.dispose); + + controller.change( + (stack) => [Routes.home, Routes.settings, Routes.signIn], + ); + + expect(errors, hasLength(2)); + expect(observer.added, equals(['settings', 'signIn'])); + expect(observer.removed, equals(['catalog'])); + }); }); +class _ThrowingOnAddObserver with NavigationObserver { + final List added = []; + final List removed = []; + + @override + void onAdd(NavigationController controller, NavigationRoute route) { + added.add(route.name); + throw StateError('onAdd'); + } + + @override + void onRemove(NavigationController controller, NavigationRoute route) => + removed.add(route.name); +} + class _RecordingObserver with NavigationObserver { final List added = []; final List removed = []; @@ -535,3 +897,43 @@ class _DisposingObserver with NavigationObserver { NavigationStack next, ) => controller.dispose(); } + +/// Records every transition as `previous -> next`, and pushes the settings +/// on the first one. +class _ChainingObserver extends _TranscriptObserver { + @override + void onChange( + NavigationController controller, + NavigationStack previous, + NavigationStack next, + ) { + super.onChange(controller, previous, next); + if (transcript.length == 1) controller.push(Routes.settings); + } +} + +/// Records every transition as `previous -> next`. +class _TranscriptObserver with NavigationObserver { + final List transcript = []; + + static String _names(NavigationStack stack) => + stack.map((route) => route.name).join('>'); + + @override + void onChange( + NavigationController controller, + NavigationStack previous, + NavigationStack next, + ) => transcript.add('${_names(previous)} -> ${_names(next)}'); +} + +/// A listenable that exposes its listeners, to check for the leaks. +class _CountingListenable implements Listenable { + final List listeners = []; + + @override + void addListener(VoidCallback listener) => listeners.add(listener); + + @override + void removeListener(VoidCallback listener) => listeners.remove(listener); +} diff --git a/test/observer_edge_test.dart b/test/observer_edge_test.dart index fe6e73e..165b28c 100644 --- a/test/observer_edge_test.dart +++ b/test/observer_edge_test.dart @@ -179,8 +179,8 @@ void main() => group('observer edge cases', () { ); expect( first.events, - equals(['change']), - reason: 'its own onAdd is skipped after the throw', + equals(['change', 'add:catalog']), + reason: 'its own onAdd still runs after the throw', ); expect(second.events, equals(['change', 'add:catalog'])); expect(third.events, equals(['change', 'add:catalog'])); @@ -199,8 +199,7 @@ void main() => group('observer edge cases', () { expect(errors, hasLength(3), reason: 'one per throwing callback'); }); - test('a throw from onAdd skips the remaining routes of that observer ' - 'only', () { + test('a throw from onAdd does not skip the remaining routes', () { final broken = _ThrowingObserver(_Stage.add); final recording = _RecordingObserver(); final controller = NavigationController( @@ -210,12 +209,15 @@ void main() => group('observer edge cases', () { addTearDown(controller.dispose); controller.pushAll([Routes.catalog, Routes.settings]); - expect(broken.events, equals(['change', 'add:catalog'])); + expect( + broken.events, + equals(['change', 'add:catalog', 'add:settings']), + ); expect( recording.events, equals(['change', 'add:catalog', 'add:settings']), ); - expect(errors, hasLength(1)); + expect(errors, hasLength(2), reason: 'one per throwing callback'); expect(controller.length, equals(3)); }); }); diff --git a/test/routes_edge_test.dart b/test/routes_edge_test.dart index 79f3f76..6fca3d4 100644 --- a/test/routes_edge_test.dart +++ b/test/routes_edge_test.dart @@ -38,6 +38,20 @@ final class _ArgumentsRoute with NavigationRoute { Widget build(BuildContext context) => const SizedBox.shrink(); } +/// A route with the name and the arguments given from the outside. +final class _NamedRoute with NavigationRoute { + const _NamedRoute(this.name, [this.arguments = const {}]); + + @override + final String name; + + @override + final Map arguments; + + @override + Widget build(BuildContext context) => const SizedBox.shrink(); +} + /// A generic route: its runtime type includes the type argument. final class _Generic with NavigationRoute { const _Generic(); @@ -77,6 +91,95 @@ void main() => group('routes edge cases', () { ); }); + test('the separators of the arguments are escaped', () { + const joined = _ArgumentsRoute({'q': 'a&b=c'}); + const split = _ArgumentsRoute({'q': 'a', 'b': 'c'}); + expect( + joined.key, + equals(const ValueKey('_ArgumentsRoute?q=a%26b%3Dc')), + ); + expect(joined.key, isNot(equals(split.key))); + for (final (escaped, raw) in const <(String, String)>[ + ('%26', '&'), + ('%3D', '='), + ('%3F', '?'), + ('%25', '%'), + ]) { + final percent = _ArgumentsRoute({'q': escaped}); + final separator = _ArgumentsRoute({'q': raw}); + expect(percent.key, isNot(equals(separator.key)), reason: escaped); + final percentName = _ArgumentsRoute({escaped: 1}); + final separatorName = _ArgumentsRoute({raw: 1}); + expect( + percentName.key, + isNot(equals(separatorName.key)), + reason: escaped, + ); + } + const nothing = _ArgumentsRoute({'id': null}); + const text = _ArgumentsRoute({'id': 'null'}); + expect(nothing.key, isNot(equals(text.key))); + }); + + test('a "?" in the name does not collide with the arguments', () { + expect( + const _NamedRoute('a?b', {'c': 1}).key, + isNot(equals(const _NamedRoute('a', {'b?c': 1}).key)), + ); + }); + + test('a name without arguments does not collide with the arguments', () { + expect( + const _NamedRoute('a?b=c').key, + isNot(equals(const _NamedRoute('a', {'b': 'c'}).key)), + ); + expect( + const _NamedRoute('a?b').key, + isNot(equals(const _NamedRoute('a', {'b': null}).key)), + ); + }); + + test('the separators in the name do not collide with the arguments', () { + expect( + const _NamedRoute('a?b&c').key, + isNot( + equals( + const _NamedRoute('a', {'b': null, 'c': null}).key, + ), + ), + ); + }); + + test('the default key escapes the name and the arguments', () { + expect( + const _NamedRoute('catalog', {'id': 1}).key, + equals(const ValueKey('catalog?id=1')), + ); + expect( + const _NamedRoute('a?b').key, + equals(const ValueKey('a%3Fb')), + ); + expect( + const _NamedRoute('a?b=c&d%', { + 'k?': 'v?', + 'e': null, + }).key, + equals(const ValueKey('a%3Fb%3Dc%26d%25?k%3F=v%3F&e')), + ); + expect( + const _NamedRoute('a%3Fb').key, + isNot(equals(const _NamedRoute('a?b').key)), + ); + }); + + test('the values are compared by their string representation', () { + // By design: the default key writes every value with toString(). + expect( + const _ArgumentsRoute({'x': 1}).key, + equals(const _ArgumentsRoute({'x': '1'}).key), + ); + }); + test('the arguments are written in their declaration order', () { const ab = _ArgumentsRoute({'a': 1, 'b': 2}); const ba = _ArgumentsRoute({'b': 2, 'a': 1}); @@ -85,7 +188,7 @@ void main() => group('routes edge cases', () { expect(ab.key, isNot(equals(ba.key))); }); - test('null, boolean and collection arguments are written as strings', () { + test('a null argument has no value, the others are written as strings', () { const route = _ArgumentsRoute({ 'id': null, 'flag': true, @@ -94,9 +197,7 @@ void main() => group('routes edge cases', () { expect( route.key, equals( - const ValueKey( - '_ArgumentsRoute?id=null&flag=true&ids=[1, 2]', - ), + const ValueKey('_ArgumentsRoute?id&flag=true&ids=[1, 2]'), ), ); }); diff --git a/test/squid_test.dart b/test/squid_test.dart index b36848c..4e36e51 100644 --- a/test/squid_test.dart +++ b/test/squid_test.dart @@ -1,3 +1,4 @@ +import 'back_button_test.dart' as back_button_test; import 'context_test.dart' as context_test; import 'controller_test.dart' as controller_test; import 'extensions_test.dart' as extensions_test; @@ -26,4 +27,5 @@ void main() { pages_edge_test.main(); routes_edge_test.main(); tabs_edge_test.main(); + back_button_test.main(); } diff --git a/test/tabs_test.dart b/test/tabs_test.dart index b0e7bd3..0b2bf12 100644 --- a/test/tabs_test.dart +++ b/test/tabs_test.dart @@ -155,6 +155,79 @@ void main() => group('tabs', () { expect(tabs.active, equals(_Tab.feed), reason: 'back to the previous tab'); }); + testWidgets('the platform is told when the tabs handle the back button', ( + tester, + ) async { + // On Android the predictive back gesture closes the application on its + // own unless the framework claims the press. + bool? handlesBack; + await tester.pumpWidget( + MaterialApp( + onNavigationNotification: (notification) { + handlesBack = notification.canHandlePop; + return true; + }, + home: ListenableBuilder( + listenable: tabs, + builder: (context, _) => IndexedStack( + index: tabs.activeIndex, + children: [ + for (final controller in tabs.tabs.values) + NavigationView(controller: controller), + ], + ), + ), + ), + ); + await tester.pumpAndSettle(); + expect(handlesBack, isFalse, reason: 'a single route, no previous tab'); + + tabs.select(_Tab.cart); + await tester.pumpAndSettle(); + expect(handlesBack, isTrue, reason: 'returns to the previous tab'); + + tabs[_Tab.cart].push(const ProductRoute(1)); + await tester.pumpAndSettle(); + expect(handlesBack, isTrue); + + tabs[_Tab.cart].pop(); + await tester.pumpAndSettle(); + expect(handlesBack, isTrue, reason: 'still returns to the feed'); + + expect(await tester.binding.handlePopRoute(), isTrue); + await tester.pumpAndSettle(); + expect(tabs.active, equals(_Tab.feed)); + expect(handlesBack, isFalse, reason: 'nothing left to go back to'); + }); + + testWidgets('a view with its own back handler claims the back button', ( + tester, + ) async { + final controller = NavigationController([Routes.home]); + addTearDown(controller.dispose); + bool? handlesBack; + var presses = 0; + await tester.pumpWidget( + MaterialApp( + onNavigationNotification: (notification) { + handlesBack = notification.canHandlePop; + return true; + }, + home: NavigationView( + controller: controller, + onBackButtonPressed: (controller) async { + presses++; + return true; + }, + ), + ), + ); + await tester.pumpAndSettle(); + expect(handlesBack, isTrue); + expect(await tester.binding.handlePopRoute(), isTrue); + expect(presses, equals(1)); + }); + testWidgets('switching tabs keeps the stack of every tab', (tester) async { await tester.pumpWidget( MaterialApp( diff --git a/test/view_test.dart b/test/view_test.dart index 0deb853..5bf843d 100644 --- a/test/view_test.dart +++ b/test/view_test.dart @@ -7,6 +7,29 @@ import 'package:squid/squid.dart'; import 'src/routes.dart'; void main() => group('view', () { + testWidgets('a page reading the context is built again when it changes', ( + tester, + ) async { + final controller = NavigationController([ + const _TitledRoute(), + ]); + addTearDown(controller.dispose); + + Widget app(String title) => MaterialApp( + home: _Title( + title: title, + child: NavigationView(controller: controller), + ), + ); + + await tester.pumpWidget(app('first')); + expect(find.text('title:first'), findsOneWidget); + + await tester.pumpWidget(app('second')); + await tester.pumpAndSettle(); + expect(find.text('title:second'), findsOneWidget); + }); + Future pumpView(WidgetTester tester, NavigationController controller) => tester.pumpWidget( MaterialApp(home: NavigationView(controller: controller)), @@ -614,3 +637,32 @@ class _WidthGuard extends ContextGuard { ? stack.withRoute(Routes.settings) : stack; } + +class _Title extends InheritedWidget { + const _Title({required this.title, required super.child}); + + final String title; + + @override + bool updateShouldNotify(_Title oldWidget) => oldWidget.title != title; +} + +final class _TitledRoute with NavigationRoute { + const _TitledRoute(); + + @override + String get name => 'titled'; + + @override + Page page(BuildContext context) { + final title = context.dependOnInheritedWidgetOfExactType<_Title>()!.title; + return MaterialPage( + key: key, + name: name, + child: Text('title:$title'), + ); + } + + @override + Widget build(BuildContext context) => const SizedBox.shrink(); +}