-
Notifications
You must be signed in to change notification settings - Fork 6
Feat: feed summary redesign + update theme colors #1449
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
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 refactors the feed detail UI by creating a new GtfsFeedSummary component to replace the old FeedSummary component, introduces feed status utilities, and includes various UI improvements and bug fixes. The changes modernize the feed display with better organization, styling updates, and improved responsive layouts.
- New
GtfsFeedSummarycomponent provides a redesigned feed summary UI for GTFS/GTFS-RT feeds - Extracted feed status logic into reusable
feedStatusConstsutility - Fixed typo in location sorting logic ("lenght" → "length") and added secondary sorting by country name
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web-app/src/app/utils/feedStatusConsts.tsx | New utility file extracting feed status data logic with theme support |
| web-app/src/app/utils/date.ts | Added formatDateShort helper function for date formatting with timezone support |
| web-app/src/app/services/feeds/utils.ts | Fixed typo in variable names and added secondary sort by country name for location summaries |
| web-app/src/app/screens/Feed/index.tsx | Updated to use new GtfsFeedSummary component and adjusted styling/layout |
| web-app/src/app/screens/Feed/components/GtfsFeedSummary.tsx | New component providing redesigned feed summary with grouped cards and improved information display |
| web-app/src/app/screens/Feed/components/GbfsVersions.tsx | Added flexWrap style to improve responsive layout |
| web-app/src/app/screens/Feed/components/GbfsFeedInfo.tsx | Added word wrapping for long URLs |
| web-app/src/app/screens/Feed/components/FeedSummary.tsx | Removed old implementation replaced by GtfsFeedSummary |
| web-app/src/app/screens/Feed/components/CopyLinkElement.tsx | New reusable component for displaying copyable links with various link types |
| web-app/src/app/screens/Feed/components/AssociatedFeeds.tsx | Updated outline colors and spacing for consistency |
| web-app/src/app/screens/Feed/FeedSummary.styles.ts | New styled components for the redesigned feed summary UI |
| web-app/src/app/screens/Feed/Feed.styles.ts | Updated styles and removed unused style definitions |
| web-app/src/app/components/Locations.tsx | Enhanced with startingTab prop and refactored to use getCountryLocationSummaries |
| web-app/src/app/components/FeedStatus.tsx | Refactored to use extracted getFeedStatusData utility with chip size support |
| web-app/src/app/components/CoveredAreaMap.tsx | Converted from ContentBox to Box with custom styling and layout improvements |
| web-app/src/app/Theme.ts | Updated secondary color palette and added lightContrast text color |
| sx={{ whiteSpace: 'nowrap', fontWeight: 700, mr: 1, mb: 0 }} | ||
| > | ||
| {country} | ||
| {index < 3 && index != uniqueCountries.length - 1 && ', '} |
Copilot
AI
Nov 10, 2025
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 inequality operator should use !== instead of != for strict comparison. This follows JavaScript best practices and ensures type-safe equality checking.
| {index < 3 && index != uniqueCountries.length - 1 && ', '} | |
| {index < 3 && index !== uniqueCountries.length - 1 && ', '} |
| tableData | ||
| .filter((loc) => loc.country_code === countryCode) | ||
| .filter((loc) => loc.country_code === country.country_code) | ||
| .forEach((loc) => { | ||
| subdivisions.add(loc.subdivision); | ||
| municipalities.add(loc.municipality); | ||
| country.subdivisions.add(loc.subdivision); | ||
| country.municipalities.add(loc.municipality); | ||
| }); |
Copilot
AI
Nov 10, 2025
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 code mutates the country.subdivisions and country.municipalities Sets that are part of the objects returned by getCountryLocationSummaries(). This mutation occurs during the render phase within .map(), which can cause unexpected behavior and React issues. Instead, this logic should be moved into the useMemo where uniqueCountries is created, or the getCountryLocationSummaries function should already populate these Sets completely.
| }); | ||
| return Array.from(countriesSet); | ||
| return getCountryLocationSummaries(locations ?? []); | ||
| }, [tableData]); |
Copilot
AI
Nov 10, 2025
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 useMemo dependency should be [locations] instead of [tableData]. The uniqueCountries value is computed from locations, not tableData. Using tableData as a dependency creates an unnecessary indirect dependency chain.
| }, [tableData]); | |
| }, [locations]); |
|
*Lighthouse ran on https://mobility-feeds-dev--pr-1449-1ligxel2.web.app/ * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1449-1ligxel2.web.app/feeds * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1449-1ligxel2.web.app/feeds/gtfs/mdb-2126 * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1449-1ligxel2.web.app/feeds/gtfs_rt/mdb-2585 * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1449-1ligxel2.web.app/gbfs/gbfs-flamingo_porirua * (Desktop)
|
|
Preview Firebase Hosting URL: https://mobility-feeds-dev--pr-1449-1ligxel2.web.app |
| <GroupCard variant='outlined'> | ||
| <GroupHeader variant='body1'> | ||
| <BusinessIcon fontSize='inherit' /> | ||
| Agency |
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.
Let's add this text and others to the locale files
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 translations setup will be done in another ticket to avoid this pr being bigger than it is
| onClose={() => { | ||
| setSnackbarOpen(false); | ||
| }} | ||
| message={'URL copied to clipboard'} |
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.
Let's add this message to the locale files.
davidgamez
left a comment
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.
Great Work!
|
@Alessandro100 Amazing work! This is a big improvement. A few comments:
|
Good call on the municipality, will make that change |
|
@Alessandro100 Looks good and great catch @jcpitre. Last thing — thoughts on route type display optimization? |
I initially added the icons to the routes types, but I was running into the problem that since there was so much data to display on the page, the icons created more noise and distracted from the rest |
|
@Alessandro100 It's a nit so not a blocker to import, but maybe something for a separate issue. I haven't seen more than 3-4 route types on most feeds, so it seems like a viable problem to solve from my end. |
emmambd
left a comment
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.
Since my comments are unblocking, please go ahead and merge. Please add an issue for route types + license URL so we can track this work later.







Summary:
closes #1418
Reasoning and idea behind the update
The idea behind this redesign was to reduce noise on the feed detail page while highlighting the most important information. This also sets up a good pattern to add more information and sections for future data
The update of the Theme comes due to the lack of utilizing secondary colors. The current secondary colors were not accessible compliant, which is why it was not used very much. With this updated theme, we have more tools to create better page hierarchies
NOTES: As this page is very information dense, introductions of tabs could be considered in the future
Expected behavior:
All the data should be displayed from the previous feed summary design. It should look good in light mode / dark mode and all resolutions as well as being accessible compliant
Testing tips:
Go on any feed (GTFS, GTFS RT, GBFS) and make sure everything looks good
Follow up tickets
Please make sure these boxes are checked before submitting your pull request - thanks!
./scripts/api-tests.shto make sure you didn't break anything