Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
*Lighthouse ran on https://mobilitydatabase-6mtlxtv9v-mobility-data.vercel.app/ * (Desktop)
*Lighthouse ran on https://mobilitydatabase-6mtlxtv9v-mobility-data.vercel.app/feeds * (Desktop)
*Lighthouse ran on https://mobilitydatabase-6mtlxtv9v-mobility-data.vercel.app/feeds/gtfs/mdb-2126 * (Desktop)
*Lighthouse ran on https://mobilitydatabase-6mtlxtv9v-mobility-data.vercel.app/feeds/gtfs_rt/mdb-2585 * (Desktop)
*Lighthouse ran on https://mobilitydatabase-6mtlxtv9v-mobility-data.vercel.app/feeds/gbfs/gbfs-flamingo_porirua * (Desktop)
|
There was a problem hiding this comment.
Pull request overview
Migrates multiple authentication/account-related routes from the legacy React Router setup into the Next.js App Router (under src/app/[locale]/...), while preserving existing Redux/PersistGate-dependent behavior via wrapper components and removing dead/legacy artifacts.
Changes:
- Adds App Router pages for sign-in, sign-up, account, change-password, complete-registration, and verify-email (with Redux + auth-guard wrappers).
- Moves cross-tab auth BroadcastChannel registration out of the legacy router and into the App Router provider tree.
- Removes legacy-only routes/components and replaces legacy Account CSS with MUI
sxstyling.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/app/styles/Account.css | Removes legacy CSS now replaced by sx styles. |
| src/app/screens/ContactInformation.tsx | Deletes dead legacy page. |
| src/app/router/Router.tsx | Removes migrated routes + legacy auth channel wiring from React Router. |
| src/app/router/ProtectedRoute.tsx | Removes legacy React Router route-guard component. |
| src/app/providers.tsx | Registers the new App Router-level BroadcastChannel sync component. |
| src/app/components/ReduxGateWrapper.tsx | Adds a client wrapper to provide PersistGate + Suspense for migrated pages. |
| src/app/components/ProtectedPageWrapper.tsx | Adds a client auth-guard wrapper for App Router pages. |
| src/app/components/AuthBroadcastChannelSync.tsx | Adds App Router-level cross-tab login/logout channel registration. |
| src/app/[locale]/sign-in/page.tsx | Adds App Router sign-in page entrypoint using ReduxGateWrapper. |
| src/app/[locale]/sign-in/SignIn.tsx | Migrates sign-in screen into App Router locale route. |
| src/app/[locale]/sign-up/page.tsx | Adds App Router sign-up page entrypoint using ReduxGateWrapper. |
| src/app/[locale]/sign-up/SignUp.tsx | Migrates sign-up screen into App Router locale route. |
| src/app/[locale]/complete-registration/page.tsx | Adds guarded App Router complete-registration page. |
| src/app/[locale]/complete-registration/CompleteRegistration.tsx | Migrates complete-registration screen into App Router locale route. |
| src/app/[locale]/change-password/page.tsx | Adds guarded App Router change-password page. |
| src/app/[locale]/change-password/ChangePassword.tsx | Migrates change-password screen into App Router locale route. |
| src/app/[locale]/account/page.tsx | Adds guarded App Router account page. |
| src/app/[locale]/account/Account.tsx | Updates account screen imports + replaces CSS usage with sx styles. |
| src/app/[locale]/account/Account.styles.ts | Introduces sx style objects replacing removed CSS. |
| src/app/[locale]/verify-email/page.tsx | Adds guarded App Router verify-email page. |
| src/app/[locale]/verify-email/PostRegistration.tsx | Migrates verify-email screen into App Router locale route and updates imports. |
Skill applied: custom
Comments suppressed due to low confidence (2)
src/app/[locale]/sign-in/SignIn.tsx:86
- Remove the console.log here: it logs the submitted credentials (including password) to the browser console, which is a security risk and can leak sensitive data in production logs/screenshares.
src/app/[locale]/change-password/ChangePassword.tsx:35 - Avoid importing useSelector from the internal react-redux path ("react-redux/es/hooks/useSelector"). This is not part of the public API and can break bundling/tree-shaking across build targets. Import useSelector from "react-redux" instead (consistent with other files).
| useEffect(() => { | ||
| app.auth(); | ||
|
|
||
| // Mirrors onAuthStateChanged logic from the legacy ProtectedRoute | ||
| const unsubscribe = app.auth().onAuthStateChanged((user) => { | ||
| if (user != null) { | ||
| dispatch(refreshAppSuccess()); | ||
| } | ||
| }); | ||
|
|
||
| if (app.auth().currentUser == null) { | ||
| dispatch(refreshApp()); | ||
| } |
There was a problem hiding this comment.
This wrapper registers its own Firebase auth listener (onAuthStateChanged) and dispatches refreshApp/refreshAppSuccess. The codebase already provides a single global auth listener via AuthSessionProvider, and its doc comment explicitly says to use it instead of registering your own auth state listener (src/app/components/AuthSessionProvider.tsx:31-34). Duplicating listeners here can cause redundant subscriptions/dispatches and makes auth behavior harder to reason about; consider using useAuthSession() (or otherwise relying on AuthSessionProvider) to gate redirects without adding another Firebase listener.
Summary:
closes #24
This PR moves the following pages out of the legacy router into the new one
Deleted pages that were dead weight: 'contact-info' (not contact us)
Architectural decision about accessing the Redux store
A lot of the pages above rely on the Redux store to dispatch actions. To not have to do a complete refactor, these pages are wrapped behind the PersistGate using a wrapper component. At the end of this migration, these wrapper components will be route groups as it's a better NextJs pattern
Expected behavior:
All these pages and URLs should have the exact same behaviour as they previously did
Testing tips:
Flows to test out
Please make sure these boxes are checked before submitting your pull request - thanks!
yarn testto make sure you didn't break anything