Improve app navigation.

[amaury901130]

Description

• Refine and streamline overall app navigation.
• Add auth-aware routing: guard protected screens, redirect unauthenticated users to Login, and persist/restore the intended destination after sign-in.

[Copilot]

Pull Request Overview

This PR implements auth-aware routing with protected route guards and automatic redirection for unauthenticated users. The changes introduce a new Routes enum-based navigation system and refactor the routing architecture to handle authentication state changes.

• Adds isLoggedIn() method to AuthCubit for checking authentication status
• Implements nested route structure with auth and app route groups
• Moves authentication state listening from app-level to route-level within the root '/' route

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
modules/domain/lib/bloc/auth/auth_cubit.dart	Adds isLoggedIn() helper method to check if user is authenticated
app/pubspec.yaml	Upgrades go_router from v7 to v17, adds flutter_dotenv and flutter_web_plugins dependencies
app/lib/presentation/ui/pages/splash/splash_page.dart	Adds instant parameter to control splash screen delay duration
app/lib/presentation/ui/pages/login/login_page.dart	Removes unused app_themes import
app/lib/presentation/ui/pages/home/home_view.dart	Adds title and removes back button from home page AppBar
app/lib/presentation/navigation/routers.dart	Complete rewrite introducing Routes enum and nested route structure with auth guards
app/lib/main/init.dart	Replaces url_strategy package with flutter_web_plugins for URL strategy
app/lib/main/app.dart	Moves BlocListener from app level to root route and simplifies builder fallback

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The nested route structure has a critical issue: the auth route uses path: Routes.auth.path which evaluates to '/auth', but since it's a child route of the root '/', the full path becomes '//auth' (double slash). Child routes in go_router should use relative paths without the leading slash.

Change path: Routes.auth.path to path: Routes.auth.subPath (or just 'auth') to fix the path construction.

Suggested change

	path: Routes.auth.path,
	path: Routes.auth.subPath,

[Copilot]

The builder for the /app route returns const SplashPage() without the instant parameter, which defaults to true based on line 7 of splash_page.dart. However, this causes the splash screen to call _authCubit.onValidate() immediately, which will trigger navigation back through the BlocListener at the root route. This creates a circular navigation pattern and unnecessary re-validation.

Consider removing this builder entirely or using a different widget, as the app route should only serve as a container for its child routes, not display a splash screen.

Suggested change

	builder: (context, state) => const SplashPage(),
	builder: (context, state) => const SizedBox.shrink(),

[Copilot]

The usePathUrlStrategy() call should be placed after WidgetsFlutterBinding.ensureInitialized() but before any other initialization. However, it's currently placed after the binding initialization but on a separate line without a comment. While functionally correct, consider adding a comment explaining why path-based URLs are being used instead of hash-based URLs (e.g., for better SEO, cleaner URLs).

Suggested change

	WidgetsFlutterBinding.ensureInitialized();
	WidgetsFlutterBinding.ensureInitialized();
	// Use path-based URLs instead of hash-based URLs for better SEO and cleaner URLs.

[Copilot]

The Routes enum lacks documentation explaining its purpose and how it should be used. Given its central role in the navigation architecture, consider adding documentation:

• What each route represents
• When to use path vs subPath
• How the nav() method should be used
• The relationship with the Routers class

This is especially important since this appears to be a new architectural pattern for the project.

[Copilot]

The redirect logic has a critical issue: when accessing /auth route directly, it will be constructed with path '/$name' which evaluates to '/auth'. However, the redirect at line 62 returns '${Routes.app.path}${Routes.home.path}' which evaluates to '/app/home'. This creates inconsistent path construction - the auth route uses path while the redirect uses nested path concatenation. This may cause navigation issues.

Consider making the path construction consistent: either use Routes.app.path + '/' + Routes.home.subPath or restructure to avoid manual path concatenation.

Suggested change

	return '${Routes.app.path}${Routes.home.path}';
	return '${Routes.app.path}/${Routes.home.subPath}';

[Copilot]

The root route ('/') should not have a builder that displays a page. In go_router, when a route has child routes, the parent route should typically not have a builder, or it should use a ShellRoute pattern. The current implementation displays SplashPage at the root while also having nested auth and app routes, which creates ambiguous navigation behavior.

Consider either:

1. Removing the builder and using redirect logic to navigate to the appropriate child route, or
2. Using a ShellRoute if you need persistent UI across child routes.

Suggested change

	builder: (context, state) {
	return BlocListener<AuthCubit, Resource>(
	listenWhen: (previous, current) => current is RSuccess,
	listener: (_, state) {
	if (state is RSuccess) {
	switch (state.data) {
	case AuthStateAuthenticated _:
	Routes.app.nav();
	break;
	case AuthStateUnauthenticated _:
	Routes.auth.nav();
	break;
	case _:
	}
	}
	},
	child: const SplashPage(instant: true),
	);
	redirect: (context, state) {
	final authCubit = context.read<AuthCubit>();
	if (authCubit.isLoggedIn()) {
	return '${Routes.app.path}${Routes.home.path}';
	} else {
	return '${Routes.auth.path}${Routes.login.path}';
	}

[Copilot]

The redirect callback accesses context.read<AuthCubit>().isLoggedIn() synchronously, but the auth state might be in transition during navigation. If the AuthCubit state changes between the redirect check and when the BlocListener at the root responds (lines 38-52), it could cause race conditions leading to incorrect navigation. Consider adding state checks to ensure the auth state is stable (not in loading) before making redirect decisions.

[Copilot]

The redirect callback at the /app route has the same concurrency issue as the /auth route. The auth state might change during navigation, creating race conditions between this redirect and the BlocListener at the root (lines 38-52).

[Copilot]

The /auth route lacks a builder but has child routes with builders. In go_router, when a parent route doesn't have a builder, navigating directly to the parent path (e.g., /auth) will result in a blank screen or error. Either add a redirect to a default child route (e.g., redirect to /auth/login when no child is specified), or add a builder with appropriate content.

[Copilot]

The isLoggedIn() method performs runtime type checking with state as RSuccess without first verifying the cast will succeed. While the first condition state is RSuccess protects against null/wrong type, this pattern could be simplified. Consider using pattern matching or extracting the data check:

bool isLoggedIn() { return state case RSuccess(data: AuthStateAuthenticated()) => true, _ => false; }

Or using a safer approach:

bool isLoggedIn() { if (state is! RSuccess) return false; return (state as RSuccess).data is AuthStateAuthenticated; }

Suggested change

	bool isLoggedIn() =>
	state is RSuccess && (state as RSuccess).data is AuthStateAuthenticated;
	// Use pattern matching for safer and more concise type checking.
	bool isLoggedIn() {
	return state case RSuccess(data: AuthStateAuthenticated()) => true, _ => false;
	}