Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves UX and continues the App Router migration by moving the Feeds search + several static pages off the legacy React Router setup, adding SWR-based client caching, and introducing better loading states for feeds navigation and detail pages.
Changes:
- Migrates
/feedssearch to App Router and replaces Redux-based fetching with SWR (30-minute client cache). - Adds new statically generated App Router routes for FAQ/contact/policy/validator pages and App Router redirects for legacy
/feeds/{type}URLs. - Improves UX with skeleton/loading indicators, “page generated at” display, and fixes auth unsubscribe/broadcast edge cases.
Reviewed changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Adds SWR dependency lock entry. |
| package.json | Adds swr dependency. |
| src/app/styles/PageLayout.style.ts | Makes container position: relative (supports overlay/progress positioning). |
| src/app/store/saga/auth-saga.ts | Fixes logout flow (cookie clearing + broadcast guard + correct failure action). |
| src/app/screens/TermsAndConditions.tsx | Marks screen as client component. |
| src/app/screens/PrivacyPolicy.tsx | Marks screen as client component. |
| src/app/screens/GbfsValidator/index.tsx | Marks screen as client component. |
| src/app/screens/Feeds/SearchTable.tsx | Adds delayed full-screen loading overlay + transition-based navigation. |
| src/app/screens/Feeds/SearchTable.spec.tsx | Mocks locale-aware router for updated SearchTable behavior. |
| src/app/screens/Feeds/FeedsScreenSkeleton.tsx | Adds Suspense fallback skeleton for /feeds. |
| src/app/screens/Feeds/AdvancedSearchTable.tsx | Adds delayed loading overlay + transition-based navigation; avoids invalid hook usage by passing theme. |
| src/app/screens/FeedSubmissionFAQ.tsx | Marks screen as client component. |
| src/app/screens/Feed/components/ScrollToTop.tsx | Adds scroll-to-top on feed detail mount. |
| src/app/screens/Feed/components/GbfsVersions.tsx | Replaces ContentBox wrapper with Box. |
| src/app/screens/Feed/FeedView.tsx | Moves “generated at” into header area; adds ScrollToTop. |
| src/app/screens/FAQ.tsx | Marks screen as client component. |
| src/app/screens/ContactUs.tsx | Updates Contact link to new repo; marks screen as client component. |
| src/app/router/Router.tsx | Removes legacy React Router routes for pages migrated to App Router. |
| src/app/context/GbfsAuthProvider.tsx | Marks provider as client component. |
| src/app/components/HeaderMobileDrawer.tsx | Fixes mobile logo rendering per theme + spacing tweaks. |
| src/app/[locale]/terms-and-conditions/page.tsx | Adds SSG App Router page for terms. |
| src/app/[locale]/privacy-policy/page.tsx | Adds SSG App Router page for privacy policy. |
| src/app/[locale]/gbfs-validator/page.tsx | Adds SSG App Router page for GBFS validator (wrapped in provider). |
| src/app/[locale]/feeds/page.tsx | Adds App Router /feeds page with Suspense fallback. |
| src/app/[locale]/feeds/lib/useFeedsSearch.ts | Introduces SWR-powered search hook + URL-derived state helpers. |
| src/app/[locale]/feeds/gtfs_rt/page.tsx | Adds App Router redirect for /feeds/gtfs_rt. |
| src/app/[locale]/feeds/gtfs/page.tsx | Adds App Router redirect for /feeds/gtfs. |
| src/app/[locale]/feeds/gbfs/page.tsx | Adds App Router redirect for /feeds/gbfs. |
| src/app/[locale]/feeds/components/FeedsScreen.tsx | Replaces Redux fetching with URL-driven SWR hook + progress UX. |
| src/app/[locale]/feeds/[feedDataType]/[feedId]/static/loading.tsx | Adds loading skeleton for static feed detail route. |
| src/app/[locale]/feeds/[feedDataType]/[feedId]/static/layout.tsx | Removes layout-level data fetching in favor of page-level fetch. |
| src/app/[locale]/feeds/[feedDataType]/[feedId]/authed/map/page.tsx | Uses notFound() instead of rendering “Feed not found”. |
| src/app/[locale]/feeds/[feedDataType]/[feedId]/authed/loading.tsx | Adds loading skeleton for authed feed detail route. |
| src/app/[locale]/feeds/[feedDataType]/[feedId]/authed/layout.tsx | Removes layout-level data fetching in favor of page-level fetch. |
| src/app/[locale]/faq/page.tsx | Adds SSG App Router page for FAQ. |
| src/app/[locale]/contribute-faq/page.tsx | Adds SSG App Router page for contribute FAQ. |
| src/app/[locale]/contact-us/page.tsx | Adds SSG App Router page for contact us. |
| src/app/App.tsx | Fixes Firebase auth listener leak by unsubscribing on unmount. |
Applied skill guidance: custom
Comments suppressed due to low confidence (8)
src/app/components/HeaderMobileDrawer.tsx:66
- Inside
<picture>,<source>should usesrcSet(notsrc). Withsrc, the<source>element is ignored and only the<img>fallback is used, which makes the<source>tags misleading. Switch tosrcSet(or remove the<source>entirely if it’s not needed).
{theme.palette.mode === 'light' ? (
<picture style={{ display: 'flex' }}>
<source
src='/assets/MOBILTYDATA_logo_light_blue_M.png'
type='image/png'
height={40}
width={40}
/>
<img
src/app/[locale]/feeds/[feedDataType]/[feedId]/authed/loading.tsx:6
- Minor typo in the doc comment: duplicated word "automatically".
* Loading page streams content as it becomes available, providing a faster time-to-interactive and better user experience.
* NextJs automatically automatically detects user agents to choose between blocking and streaming behavior.
* Since streaming is server-rendered, it does not impact SEO
src/app/[locale]/feeds/lib/useFeedsSearch.ts:73
pageis parsed withNumber(...)directly. Ifois missing/invalid (e.g.?o=abc), this becomesNaN, which then breaks offset calculation and can propagate into the Paginationpageprop. Consider sanitizing/clamping to a positive integer (fallback to 1 whenNumber.isFinitefails).
return {
searchQuery: searchParams.get('q') ?? '',
page: searchParams.get('o') !== null ? Number(searchParams.get('o')) : 1,
feedTypes,
src/app/[locale]/feeds/lib/useFeedsSearch.ts:235
- When
authReadyis false, the SWR key isnull, soisLoadingbecomes false andfeedsDatastaysundefined. InFeedsScreenthis results in no skeleton/loading state being shown while anonymous Firebase auth is initializing (blank results area). Consider treating!authReadyas a loading state (or returnauthReadyfrom the hook) so the UI can render the existing skeleton/progress until SWR can fetch.
const authReady = useFirebaseAuthReady();
const { cache } = useSWRConfig();
const derivedSearchParams = deriveSearchParams(searchParams);
const key = authReady ? buildSwrKey(derivedSearchParams) : null;
const cachedState = key !== null ? cache.get(key) : undefined;
const hasCachedDataForKey =
cachedState !== undefined &&
typeof cachedState === 'object' &&
cachedState !== null &&
'data' in cachedState &&
cachedState.data !== undefined;
const {
data,
error,
isLoading,
isValidating: swrIsValidating,
} = useSWR<AllFeedsType | undefined>(
key,
async () => await feedsFetcher(derivedSearchParams),
{
// Keep previous data visible while revalidating (no flash to skeleton)
keepPreviousData: true,
// Don't refetch on window focus for search results
revalidateOnFocus: false,
// Deduplicate identical requests within 2 seconds
dedupingInterval: 2000,
},
);
return {
feedsData: data,
// True only on first load (no cached data yet)
isLoading: isLoading && data === undefined,
// True when SWR is fetching and this key has no cached data yet.
// This avoids showing loading UI when navigating back/forward to a cached search.
isValidating: swrIsValidating && !hasCachedDataForKey,
isError: error !== undefined,
searchLimit: SEARCH_LIMIT,
};
src/app/screens/Feed/components/ScrollToTop.tsx:13
window.scrollToonly supportsbehavior: 'auto' | 'smooth'. Using'instant'is non-standard and may be ignored by browsers, leading to inconsistent scroll restoration. Usebehavior: 'auto'(or omitbehavior) for an immediate jump to top.
export default function ScrollToTop(): null {
useEffect(() => {
window.scrollTo({ top: 0, behavior: 'instant' });
}, []);
src/app/screens/Feeds/SearchTable.tsx:188
- New navigation behavior was added (preventDefault +
startTransition+ locale-awarerouter.push) but there are no unit tests asserting it. Since this is easy to regress (e.g. modifier clicks should not be prevented, push should be called with the right href), consider adding tests around the click handler behavior.
src/app/screens/Feeds/AdvancedSearchTable.tsx:255 - This component now intercepts navigation (preventDefault +
startTransition+router.push) and changes UI state (showLoading/ opacity). There are currently no unit tests forAdvancedSearchTable, so regressions here may go unnoticed. Consider adding a basic test that verifies the click handler callsrouter.pushand doesn’t intercept modifier/middle clicks.
sx={{ p: 1, opacity: showLoading || isLoadingFeeds ? 0.7 : 1 }}
component={NextLinkComposed}
href={`/feeds/${feed.data_type}/${feed.id}`}
prefetch={false}
onClick={(e) => {
// Navigation to Feed Detail Page can have a delay
// Show loading state to ease transition
// This will be further reviewed
if (e.metaKey || e.ctrlKey || e.shiftKey || e.button === 1)
return;
e.preventDefault();
startTransition(() => {
router.push(`/feeds/${feed.data_type}/${feed.id}`);
});
}}
src/app/[locale]/feeds/[feedDataType]/[feedId]/static/loading.tsx:6
- Minor typo in the doc comment: duplicated word "automatically".
* Loading page streams content as it becomes available, providing a faster time-to-interactive and better user experience.
* NextJs automatically automatically detects user agents to choose between blocking and streaming behavior.
* Since streaming is server-rendered, it does not impact SEO
|
*Lighthouse ran on https://mobilitydatabase-a1reuktux-mobility-data.vercel.app/ * (Desktop)
*Lighthouse ran on https://mobilitydatabase-a1reuktux-mobility-data.vercel.app/feeds * (Desktop)
*Lighthouse ran on https://mobilitydatabase-a1reuktux-mobility-data.vercel.app/feeds/gtfs/mdb-2126 * (Desktop)
*Lighthouse ran on https://mobilitydatabase-a1reuktux-mobility-data.vercel.app/feeds/gtfs_rt/mdb-2585 * (Desktop)
*Lighthouse ran on https://mobilitydatabase-a1reuktux-mobility-data.vercel.app/gbfs/gbfs-flamingo_porirua * (Desktop)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #23 #40 and #42
Summary:
Feed Search Page
Feed Detail Page
New Static Site Generated pages
General
Expected behavior:
Testing tips:
Use the feed search and play around with it (for extra verification set network to 4g slow)
Visit the affected urls to assure they display and behave correctly
Login - logout - login (test the login system)
Follow up tickets (not MVP)
Please make sure these boxes are checked before submitting your pull request - thanks!
yarn testto make sure you didn't break anythingFeeds loading states
On filter change
On navigation
Page generated
