-
Notifications
You must be signed in to change notification settings - Fork 0
Feat: landing page ssr + seo optimization #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR migrates the landing page to Next.js Server-Side Generation (SSG) for improved SEO and performance, and restructures internationalization from subdomain-based (fr.mobilitydatabase.org) to path-based (mobilitydatabase.org/fr) routing following Next.js conventions.
Changes:
- Landing page converted to server component with SSG, enabling pre-rendering and better SEO discoverability
- Internationalization restructured to use path prefixes (
/fr) instead of subdomains, following Next.js and Google SEO recommendations - Removed
react-helmet-asyncdependency in favor of Next.js native metadata API
Reviewed changes
Copilot reviewed 30 out of 32 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/proxy.ts |
Replaced subdomain-based locale detection with path-based locale routing using Next.js middleware |
src/i18n/routing.ts |
New centralized routing configuration defining available locales and path prefix behavior |
src/i18n/request.ts |
Updated to use requestLocale parameter from route segments instead of cookies |
src/i18n/navigation.ts |
New locale-aware navigation utilities for Next.js App Router |
src/i18n/config.ts |
Removed (replaced by routing.ts) |
src/app/[locale]/page.tsx |
New SSG landing page with metadata for SEO optimization |
src/app/[locale]/layout.tsx |
New locale-aware root layout replacing old layout.tsx |
src/app/[locale]/components/HomePage.tsx |
Server component version of Home screen with translations |
src/app/[locale]/components/SearchBox.tsx |
Extracted client-side search functionality from Home page |
src/app/screens/Home.tsx |
Removed (replaced by HomePage server component) |
src/app/context/ThemeProvider.tsx |
Fixed SSR hydration issues with theme initialization |
src/app/components/Header.tsx |
Added SSR safety checks and lazy loading for better performance |
src/app/components/ThemeToggle.tsx |
Removed direct localStorage access in favor of context state |
src/app/App.tsx |
Added locale-aware basename for BrowserRouter compatibility |
messages/en.json & messages/fr.json |
Added translation keys for home and about pages |
| xs: 2, | ||
| sm: 4, | ||
| }, | ||
| fontWeight: 700, |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fontSize property is missing from this section's styling, but the previous Typography elements specify font sizes. For consistency and proper semantic heading hierarchy, consider adding an appropriate font size (e.g., fontSize: 18 or fontSize: '1.125rem').
| fontWeight: 700, | |
| fontWeight: 700, | |
| fontSize: '1.125rem', |
| <br /> <br /> | ||
| In addition to our database, we develop and maintain other tools that | ||
| integrate with it such as |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 47-48 contains hardcoded English text that should be moved to the translation file. This text is not internationalized while the rest of the page uses t() for translations. Move this content to messages/en.json and messages/fr.json under the about namespace.
| the GBFS Validator. | ||
| {t('gbfsValidator')} | ||
| </Button> | ||
| Additional benefits of using the Mobility Database include |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line contains hardcoded English text that should be moved to the translation file. While the list items below use t('benefits.*'), the introductory text is not internationalized. Add this text to the translation files for proper i18n support.
| Additional benefits of using the Mobility Database include | |
| {t('additionalBenefitsIntro')} |
| // TODO: Revisit theme for best SSR practices | ||
|
|
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This TODO comment should be converted to a GitHub issue and linked here, or removed if the current implementation is considered acceptable. The current SSR theme handling may cause the "flash of unstyled content" mentioned in the PR description's follow-up tasks.
| // TODO: Revisit theme for best SSR practices | |
| // Theme is initialized from system preference and persisted via localStorage to | |
| // provide a consistent experience between server-rendered markup and client-side hydration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
c23457e to
2ab721c
Compare
|
*Lighthouse ran on https://mobilitydatabase-1fukewb84-mobility-data.vercel.app/ * (Desktop)
*Lighthouse ran on https://mobilitydatabase-1fukewb84-mobility-data.vercel.app/feeds * (Desktop)
*Lighthouse ran on https://mobilitydatabase-1fukewb84-mobility-data.vercel.app/feeds/gtfs/mdb-2126 * (Desktop)
*Lighthouse ran on https://mobilitydatabase-1fukewb84-mobility-data.vercel.app/feeds/gtfs_rt/mdb-2585 * (Desktop)
*Lighthouse ran on https://mobilitydatabase-1fukewb84-mobility-data.vercel.app/gbfs/gbfs-flamingo_porirua * (Desktop)
|
| React.useEffect(() => { | ||
| const auth = app.auth(); | ||
| const unsubscribe = auth.onAuthStateChanged(async (user) => { | ||
| if (user != null) { | ||
| setCurrentUser({ | ||
| email: user.email ?? '', | ||
| isAuthenticated: !user.isAnonymous, | ||
| }); | ||
| } else { | ||
| setCurrentUser(undefined); | ||
| } | ||
| }); | ||
| return () => { | ||
| unsubscribe(); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reason the Header gets authentication client side from auth.onAuthStateChanged is because on SSG pages (landing) we cannot get any dynamic data (ex: user session from cookie). The trade-off is that on the initial load, there will be a small flicker from 'not auth' to 'auth' state, but after that, normal navigation the header will be stable (through the use of next/link)
We also do not want to use the redux store as that is slower and requires more conditional logic to assure it's setup properly without delaying any other render
| @@ -26,6 +26,7 @@ export default function ConfirmModal({ | |||
| }: ConfirmModalProps): React.ReactElement { | |||
| const dispatch = useAppDispatch(); | |||
| const router = useRouter(); | |||
| const isRehydrated = useRehydrated(); | |||
| const confirmLogout = (): void => { | |||
| dispatch( | |||
| logout({ | |||
| @@ -65,6 +66,7 @@ export default function ConfirmModal({ | |||
| color='primary' | |||
| variant='contained' | |||
| data-cy='confirmSignOutButton' | |||
| disabled={!isRehydrated} | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the Rehydration pattern. Since this component can be shown to the user before the Redux store is ready, we add a check to assure the store is ready. Ideally we will have a different way to trigger logout to avoid redux all together
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 43 out of 45 changed files in this pull request and generated 8 comments.
| import { type AVAILABLE_LOCALES, routing } from '../../i18n/routing'; | ||
| import HomePage from './components/HomePage'; | ||
| import { type Metadata } from 'next'; | ||
|
|
||
| export const dynamic = 'force-static'; | ||
|
|
||
| export function generateStaticParams(): Array<{ | ||
| locale: (typeof AVAILABLE_LOCALES)[number]; | ||
| }> { | ||
| return routing.locales.map((locale) => ({ locale })); | ||
| } | ||
|
|
||
| interface PageProps { | ||
| params: Promise<{ locale: (typeof AVAILABLE_LOCALES)[number] }>; |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AVAILABLE_LOCALES is a value export (a const), but it’s imported as a type (type AVAILABLE_LOCALES). This will fail type-checking. Import AVAILABLE_LOCALES as a value (or avoid referencing it directly in the type and instead use Locale / typeof routing.locales[number]).
| import { type AVAILABLE_LOCALES, routing } from '../../i18n/routing'; | |
| import HomePage from './components/HomePage'; | |
| import { type Metadata } from 'next'; | |
| export const dynamic = 'force-static'; | |
| export function generateStaticParams(): Array<{ | |
| locale: (typeof AVAILABLE_LOCALES)[number]; | |
| }> { | |
| return routing.locales.map((locale) => ({ locale })); | |
| } | |
| interface PageProps { | |
| params: Promise<{ locale: (typeof AVAILABLE_LOCALES)[number] }>; | |
| import { routing } from '../../i18n/routing'; | |
| import HomePage from './components/HomePage'; | |
| import { type Metadata } from 'next'; | |
| export const dynamic = 'force-static'; | |
| export function generateStaticParams(): Array<{ | |
| locale: (typeof routing.locales)[number]; | |
| }> { | |
| return routing.locales.map((locale) => ({ locale })); | |
| } | |
| interface PageProps { | |
| params: Promise<{ locale: (typeof routing.locales)[number] }>; |
| export const metadata: Metadata = { | ||
| title: 'Mobility Database', | ||
| description: | ||
| 'Discover open public transit data worldwide. Mobility Database provides GTFS, GTFS-RT, and GBFS feeds to help developers, cities, and agencies build better mobility tools.', | ||
| applicationName: 'Mobility Database', | ||
|
|
||
| metadataBase: new URL('https://mobilitydatabase.org'), | ||
|
|
||
| alternates: { | ||
| canonical: '/', | ||
| }, | ||
| openGraph: { | ||
| type: 'website', | ||
| url: 'https://mobilitydatabase.org', | ||
| siteName: 'Mobility Database', | ||
| title: 'Mobility Database', | ||
| description: | ||
| 'Discover open public transit data worldwide. Find GTFS, GTFS-RT, and GBFS feeds to build better mobility applications.', | ||
| }, | ||
| robots: { | ||
| index: true, | ||
| follow: true, | ||
| googleBot: { | ||
| index: true, | ||
| follow: true, | ||
| 'max-image-preview': 'large', | ||
| 'max-snippet': -1, | ||
| 'max-video-preview': -1, | ||
| }, | ||
| }, | ||
| }; |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The landing page metadata is hard-coded to a single canonical (/) and OpenGraph URL for all locales, and it always enables indexing. For /fr, this can produce incorrect canonical/OG URLs and can also override the environment-based robots logic defined in src/app/[locale]/layout.tsx (potentially indexing non-prod builds). Consider switching to generateMetadata({ params }) and deriving canonical, openGraph.url, and robots based on the resolved locale and env (and optionally emitting alternates.languages for en/fr).
| export const metadata: Metadata = { | |
| title: 'Mobility Database', | |
| description: | |
| 'Discover open public transit data worldwide. Mobility Database provides GTFS, GTFS-RT, and GBFS feeds to help developers, cities, and agencies build better mobility tools.', | |
| applicationName: 'Mobility Database', | |
| metadataBase: new URL('https://mobilitydatabase.org'), | |
| alternates: { | |
| canonical: '/', | |
| }, | |
| openGraph: { | |
| type: 'website', | |
| url: 'https://mobilitydatabase.org', | |
| siteName: 'Mobility Database', | |
| title: 'Mobility Database', | |
| description: | |
| 'Discover open public transit data worldwide. Find GTFS, GTFS-RT, and GBFS feeds to build better mobility applications.', | |
| }, | |
| robots: { | |
| index: true, | |
| follow: true, | |
| googleBot: { | |
| index: true, | |
| follow: true, | |
| 'max-image-preview': 'large', | |
| 'max-snippet': -1, | |
| 'max-video-preview': -1, | |
| }, | |
| }, | |
| }; | |
| export async function generateMetadata({ | |
| params, | |
| }: { | |
| params: Promise<{ locale: (typeof AVAILABLE_LOCALES)[number] }>; | |
| }): Promise<Metadata> { | |
| const { locale } = await params; | |
| const metadataBase = new URL('https://mobilitydatabase.org'); | |
| const path = locale === 'fr' ? '/fr' : '/'; | |
| const fullUrl = new URL(path, metadataBase).toString(); | |
| return { | |
| title: 'Mobility Database', | |
| description: | |
| 'Discover open public transit data worldwide. Mobility Database provides GTFS, GTFS-RT, and GBFS feeds to help developers, cities, and agencies build better mobility tools.', | |
| applicationName: 'Mobility Database', | |
| metadataBase, | |
| alternates: { | |
| canonical: path, | |
| languages: { | |
| en: '/', | |
| fr: '/fr', | |
| }, | |
| }, | |
| openGraph: { | |
| type: 'website', | |
| url: fullUrl, | |
| siteName: 'Mobility Database', | |
| title: 'Mobility Database', | |
| description: | |
| 'Discover open public transit data worldwide. Find GTFS, GTFS-RT, and GBFS feeds to build better mobility applications.', | |
| }, | |
| }; | |
| } |
| function buildPathFromNextRouter( | ||
| pathname: string, | ||
| searchParams: URLSearchParams, | ||
| locale?: string, | ||
| ): string { | ||
| const cleanPath = | ||
| locale != null && locale !== 'en' | ||
| ? (pathname.replace(`/${locale}`, '') ?? '/') | ||
| : pathname; | ||
|
|
||
| const searchString = searchParams.toString(); | ||
| return searchString !== '' ? `${cleanPath}?${searchString}` : cleanPath; | ||
| } |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useSearchParams() from next/navigation returns ReadonlyURLSearchParams, not URLSearchParams, so this call is likely a TypeScript error. Also, pathname.replace(\/${locale}`, '')will replace the first occurrence anywhere in the string rather than only stripping a leading locale segment; using a prefix check (e.g.,pathname.startsWith(...)`) avoids accidental replacements.
| const pathname = usePathname(); | ||
| const searchParams = useSearchParams(); | ||
|
|
||
| const initialPath = buildPathFromNextRouter(pathname, searchParams, locale); |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useSearchParams() from next/navigation returns ReadonlyURLSearchParams, not URLSearchParams, so this call is likely a TypeScript error. Also, pathname.replace(\/${locale}`, '')will replace the first occurrence anywhere in the string rather than only stripping a leading locale segment; using a prefix check (e.g.,pathname.startsWith(...)`) avoids accidental replacements.
| export default function Page({ params }: PageProps): ReactNode { | ||
| const { locale } = use(params); | ||
| const pathKey = use(params).slug?.join('/') ?? '/'; |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The semicolon after <App ... /> is inside JSX and will cause a syntax/parsing error in TSX. Remove the trailing ;. While you’re here, avoid calling use(params) twice—resolve it once into a local variable and read both locale and slug from that object.
| return ( | ||
| <PersistGate loading={null} persistor={persistor}> | ||
| <App locale={locale} key={pathKey} />; | ||
| </PersistGate> | ||
| ); | ||
| } |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The semicolon after <App ... /> is inside JSX and will cause a syntax/parsing error in TSX. Remove the trailing ;. While you’re here, avoid calling use(params) twice—resolve it once into a local variable and read both locale and slug from that object.
| <Link | ||
| data-cy={ | ||
| 'header-' + item.title.toLowerCase().replace(/\s+/g, '-') | ||
| } | ||
| href={item.external === true ? item.target : '/' + item.target} | ||
| key={item.title} | ||
| target={item.external === true ? '_blank' : '_self'} | ||
| rel={item.external === true ? 'noopener noreferrer' : ''} | ||
| variant={'text'} | ||
| endIcon={item.external === true ? <OpenInNew /> : null} | ||
| className={ | ||
| activeTab.includes('/' + item.target) ? 'active' : '' | ||
| } | ||
| > | ||
| {item.title} | ||
| </Button> | ||
| <Button | ||
| sx={(theme) => ({ | ||
| ...animatedButtonStyling(theme), | ||
| color: theme.palette.text.primary, | ||
| })} | ||
| variant={'text'} | ||
| endIcon={item.external === true ? <OpenInNew /> : null} | ||
| className={ | ||
| activeTab.includes('/' + item.target) ? 'active' : '' | ||
| } | ||
| > | ||
| {item.title} | ||
| </Button> | ||
| </Link> |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This renders an anchor (Link → <a>) wrapping a <button> (Button default), which is invalid nested interactive content and can cause keyboard/screen-reader issues. Prefer rendering a single interactive element, e.g. using MUI Button with component={Link} (or component="a" with Link props) so the output is a single <a> element styled as a button.
| // Mock GET /v1/feeds/{id} - basic feed info | ||
| http.get(`*/v1/feeds/*/test-516`, () => { | ||
| return HttpResponse.json(feedJson); | ||
| }), |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MSW route pattern includes an extra path segment (/v1/feeds/*/test-516), which likely won’t match requests to /v1/feeds/test-516. Adjust the matcher to the actual endpoint shape (or use an MSW path param matcher) so the handler reliably intercepts the feed details request.
Summary:
closes #1554
Make the landing page a server component and apply best practices for SEO discoverability.
Why so many changes?
For the landing page to have it's best performance it needs to be SSG (Static Site Generated) which means that it will come from a CDN and not the Vercel Server. It also means that the CDN will server static files = great performance. For this to happen I needed to modify the way we do i18n so that Vercel is able to generate the static version of each landing page (en / fr). I also needed to remove the blocking resources of Redux from the root so it doesn't block static generation behind javascript
Landing Page Changes
i18n Changed
mobilitydatabase.org/frand notfr.mobilitydatabase.orgArchitecture Changes
Follow up tickets
next/linkthis is needed to prevent the root from re-rendering on each navigation (creates a flicker and bad UX)Expected behavior:
Explain and/or show screenshots for how you expect the pull request to work in your testing (in case other devices exhibit different behavior).
Important
In SSG (Static Site Generated) pages like: Landing Page, About Page, Contact (soon), FAQ (soon)
The Firebase Remote Configs are gotten at build time then are frozen for the entirety of the build
If the Firebase Remote Configs change, we will need to trigger a rebuild or republish
A rebuild can be done in the Vercel GUI or a Github Action
Note: This is a trade-off I took as the performance of SSG outweighs the amount of time we change firebase remote configs. If this is something we want to lock up, we can add a logic in the Header to fetch the remote configs. Right now the remote configs are fetched from the server and distributed through providers
Testing tips:
Provide tips, procedures and sample files on how to test the feature.
Testers are invited to follow the tips AND to try anything they deem relevant outside the bounds of the testing tips.
Please make sure these boxes are checked before submitting your pull request - thanks!
yarn testto make sure you didn't break anythingBuild struture

Screaming Spider SEO Auditing Tool
Current MobilityDatabase landing page

New Landing Page

Current Lighthouse Performance Mobilitydatabase (Bots crawl the mobile version)

New Lighthouse Performance (SSG)

Build Flow of SSG
