From da5cd74eb2723be34b3f2d8db723f1dfce69cafd Mon Sep 17 00:00:00 2001 From: Benjamin Belanger <99574524+BenjaminBelanger@users.noreply.github.com> Date: Wed, 30 Oct 2024 17:51:16 -0400 Subject: [PATCH] Bugfix/1026 Android back gesture should follow navigation history (#1056) * Add navigation history observer * Fix Android back gesture to navigate through route history * Implement route history navigation for landscape orientation * Update bottom bar tests to work with Android back gesture navigation bugfix * Fix typo and code formatting in navigation_service * [BOT] Applying version. --------- Co-authored-by: BenjaminBelanger --- .../navigation_history_observer.dart | 32 +++++++++++++++++++ .../app/navigation/navigation_service.dart | 30 ++++++++++++++++- lib/features/app/widgets/bottom_bar.dart | 10 +++--- lib/features/app/widgets/navigation_rail.dart | 10 +++--- lib/main.dart | 2 ++ pubspec.yaml | 2 +- .../features/app/widgets/bottom_bar_test.dart | 15 ++++----- 7 files changed, 80 insertions(+), 21 deletions(-) create mode 100644 lib/features/app/navigation/navigation_history_observer.dart diff --git a/lib/features/app/navigation/navigation_history_observer.dart b/lib/features/app/navigation/navigation_history_observer.dart new file mode 100644 index 000000000..bf79cee77 --- /dev/null +++ b/lib/features/app/navigation/navigation_history_observer.dart @@ -0,0 +1,32 @@ +import 'package:built_collection/built_collection.dart'; +import 'package:flutter/widgets.dart'; + +class NavigationHistoryObserver extends NavigatorObserver { + final List?> _history = ?>[]; + + /// Gets a clone of the navigation history as an immutable list. + BuiltList> get history => + BuiltList>.from(_history); + + /// Implements a singleton pattern for NavigationHistoryObserver. + static final NavigationHistoryObserver _singleton = NavigationHistoryObserver._internal(); + factory NavigationHistoryObserver() { + return _singleton; + } + NavigationHistoryObserver._internal(); + + @override + void didPop(Route route, Route? previousRoute) { + _history.removeLast(); + } + + @override + void didPush(Route route, Route? previousRoute) { + _history.add(route); + } + + @override + void didRemove(Route route, Route? previousRoute) { + _history.remove(route); + } +} diff --git a/lib/features/app/navigation/navigation_service.dart b/lib/features/app/navigation/navigation_service.dart index f3b2e75f2..8db288af6 100644 --- a/lib/features/app/navigation/navigation_service.dart +++ b/lib/features/app/navigation/navigation_service.dart @@ -4,6 +4,7 @@ import 'package:flutter/material.dart'; // Project imports: import 'package:notredame/features/app/analytics/analytics_service.dart'; import 'package:notredame/features/app/analytics/remote_config_service.dart'; +import 'package:notredame/features/app/navigation/navigation_history_observer.dart'; import 'package:notredame/features/app/navigation/router_paths.dart'; import 'package:notredame/utils/locator.dart'; @@ -24,6 +25,9 @@ class NavigationService { /// Will be used to report event and error. final AnalyticsService _analyticsService = locator(); + /// Will be used to remove duplicate routes. + final NavigationHistoryObserver _navigationHistoryObserver = NavigationHistoryObserver(); + GlobalKey get navigatorKey => _navigatorKey; /// Pop the last route of the navigator if possible @@ -37,7 +41,7 @@ class NavigationService { return false; } - /// Push a named route ([routeName] onto the navigator. + /// Push a named route [routeName] onto the navigator. Future pushNamed(String routeName, {dynamic arguments}) { final currentState = _navigatorKey.currentState; @@ -53,6 +57,30 @@ class NavigationService { return currentState.pushNamed(routeName, arguments: arguments); } + /// Push a named route [routeName] onto the navigator + /// and remove existing routes with the same [routeName] + Future pushNamedAndRemoveDuplicates(String routeName, {dynamic arguments}) { + final currentState = _navigatorKey.currentState; + + if (currentState == null) { + _analyticsService.logError(tag, "Navigator state is null"); + return Future.error("Navigator state is null"); + } + + if (remoteConfigService.outage) { + return currentState.pushNamedAndRemoveUntil( + RouterPaths.serviceOutage, (route) => false); + } + + final route = _navigationHistoryObserver.history + .where((r) => r.settings.name == routeName).firstOrNull; + + if (route != null) { + currentState.removeRoute(route); + } + return currentState.pushNamed(routeName, arguments: arguments); + } + /// Replace the current route of the navigator by pushing the route named /// [routeName] and then delete the stack of previous routes Future pushNamedAndRemoveUntil(String routeName, diff --git a/lib/features/app/widgets/bottom_bar.dart b/lib/features/app/widgets/bottom_bar.dart index 1db3b942a..7ea1e3752 100644 --- a/lib/features/app/widgets/bottom_bar.dart +++ b/lib/features/app/widgets/bottom_bar.dart @@ -73,19 +73,19 @@ class _BottomBarState extends State { switch (index) { case BottomBar.dashboardView: - _navigationService.pushNamedAndRemoveUntil(RouterPaths.dashboard); + _navigationService.pushNamedAndRemoveDuplicates(RouterPaths.dashboard); _analyticsService.logEvent("BottomBar", "DashboardView clicked"); case BottomBar.scheduleView: - _navigationService.pushNamedAndRemoveUntil(RouterPaths.schedule); + _navigationService.pushNamedAndRemoveDuplicates(RouterPaths.schedule); _analyticsService.logEvent("BottomBar", "ScheduleView clicked"); case BottomBar.studentView: - _navigationService.pushNamedAndRemoveUntil(RouterPaths.student); + _navigationService.pushNamedAndRemoveDuplicates(RouterPaths.student); _analyticsService.logEvent("BottomBar", "StudentView clicked"); case BottomBar.etsView: - _navigationService.pushNamedAndRemoveUntil(RouterPaths.ets); + _navigationService.pushNamedAndRemoveDuplicates(RouterPaths.ets); _analyticsService.logEvent("BottomBar", "EtsView clicked"); case BottomBar.moreView: - _navigationService.pushNamedAndRemoveUntil(RouterPaths.more); + _navigationService.pushNamedAndRemoveDuplicates(RouterPaths.more); _analyticsService.logEvent("BottomBar", "MoreView clicked"); } _currentView = index; diff --git a/lib/features/app/widgets/navigation_rail.dart b/lib/features/app/widgets/navigation_rail.dart index 285dfb443..4da67908d 100644 --- a/lib/features/app/widgets/navigation_rail.dart +++ b/lib/features/app/widgets/navigation_rail.dart @@ -72,19 +72,19 @@ class _NavRailState extends State { switch (index) { case NavRail.dashboardView: - _navigationService.pushNamedAndRemoveUntil(RouterPaths.dashboard); + _navigationService.pushNamedAndRemoveDuplicates(RouterPaths.dashboard); _analyticsService.logEvent("BottomBar", "DashboardView clicked"); case NavRail.scheduleView: - _navigationService.pushNamedAndRemoveUntil(RouterPaths.schedule); + _navigationService.pushNamedAndRemoveDuplicates(RouterPaths.schedule); _analyticsService.logEvent("BottomBar", "ScheduleView clicked"); case NavRail.studentView: - _navigationService.pushNamedAndRemoveUntil(RouterPaths.student); + _navigationService.pushNamedAndRemoveDuplicates(RouterPaths.student); _analyticsService.logEvent("BottomBar", "StudentView clicked"); case NavRail.etsView: - _navigationService.pushNamedAndRemoveUntil(RouterPaths.ets); + _navigationService.pushNamedAndRemoveDuplicates(RouterPaths.ets); _analyticsService.logEvent("BottomBar", "EtsView clicked"); case NavRail.moreView: - _navigationService.pushNamedAndRemoveUntil(RouterPaths.more); + _navigationService.pushNamedAndRemoveDuplicates(RouterPaths.more); _analyticsService.logEvent("BottomBar", "MoreView clicked"); } _currentView = index; diff --git a/lib/main.dart b/lib/main.dart index 64d4da7e9..4b9baa075 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -23,6 +23,7 @@ import 'package:notredame/features/app/error/outage/outage_view.dart'; import 'package:notredame/features/app/integration/firebase_options.dart'; import 'package:notredame/features/app/navigation/navigation_service.dart'; import 'package:notredame/features/app/navigation/router.dart'; +import 'package:notredame/features/app/navigation/navigation_history_observer.dart'; import 'package:notredame/features/app/startup/startup_view.dart'; import 'package:notredame/features/ets/events/api-client/hello_api_client.dart'; import 'package:notredame/features/more/feedback/models/custom_feedback_localization.dart'; @@ -108,6 +109,7 @@ class ETSMobile extends StatelessWidget { navigatorKey: locator().navigatorKey, navigatorObservers: [ locator().getAnalyticsObserver(), + NavigationHistoryObserver(), ], home: outage ? OutageView() : StartUpView(), onGenerateRoute: generateRoute, diff --git a/pubspec.yaml b/pubspec.yaml index e64757a41..92c915a26 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -100,4 +100,4 @@ flutter: - assets/ flutter_intl: - enabled: true + enabled: true \ No newline at end of file diff --git a/test/features/app/widgets/bottom_bar_test.dart b/test/features/app/widgets/bottom_bar_test.dart index 0a732902c..0c3cf8d9b 100644 --- a/test/features/app/widgets/bottom_bar_test.dart +++ b/test/features/app/widgets/bottom_bar_test.dart @@ -58,7 +58,7 @@ void main() { await tester.tap(find.byIcon(Icons.school_outlined)); await tester.tap(find.byIcon(Icons.school_outlined)); - verify(navigationServiceMock.pushNamedAndRemoveUntil(RouterPaths.student)) + verify(navigationServiceMock.pushNamedAndRemoveDuplicates(RouterPaths.student)) .called(1); }); @@ -71,8 +71,7 @@ void main() { await tester.tap(find.byIcon(Icons.schedule_outlined)); await tester.tap(find.byIcon(Icons.dashboard_outlined)); - verify(navigationServiceMock - .pushNamedAndRemoveUntil(RouterPaths.dashboard)); + verify(navigationServiceMock.pushNamedAndRemoveDuplicates(RouterPaths.dashboard)); }); testWidgets('schedule', (WidgetTester tester) async { @@ -82,8 +81,7 @@ void main() { await tester.tap(find.byIcon(Icons.schedule_outlined)); - verify(navigationServiceMock - .pushNamedAndRemoveUntil(RouterPaths.schedule)); + verify(navigationServiceMock.pushNamedAndRemoveDuplicates(RouterPaths.schedule)); }); testWidgets('student', (WidgetTester tester) async { @@ -93,8 +91,7 @@ void main() { await tester.tap(find.byIcon(Icons.school_outlined)); - verify( - navigationServiceMock.pushNamedAndRemoveUntil(RouterPaths.student)); + verify(navigationServiceMock.pushNamedAndRemoveDuplicates(RouterPaths.student)); }); testWidgets('ets', (WidgetTester tester) async { @@ -104,7 +101,7 @@ void main() { await tester.tap(find.byIcon(Icons.account_balance_outlined)); - verify(navigationServiceMock.pushNamedAndRemoveUntil(RouterPaths.ets)); + verify(navigationServiceMock.pushNamedAndRemoveDuplicates(RouterPaths.ets)); }); testWidgets('more', (WidgetTester tester) async { @@ -114,7 +111,7 @@ void main() { await tester.tap(find.byIcon(Icons.menu_outlined)); - verify(navigationServiceMock.pushNamedAndRemoveUntil(RouterPaths.more)); + verify(navigationServiceMock.pushNamedAndRemoveDuplicates(RouterPaths.more)); }); }); });