Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Modernizes the app’s top-level navigation chrome by restructuring the Header (search + avatar-based account access + hover menus) and redesigning the Footer to centralize secondary links and standardize navigation. (custom: vercel-react-best-practices guidance applied)
Changes:
- Header: adds a new search bar, updates account UI to an avatar + dropdown, and switches dropdown discovery to hover interactions.
- Footer: replaces the prior minimal footer with a multi-column footer containing key site links and new i18n strings.
- Removes
mui-nested-menuand updates layout min-height calculations to match the new footer height.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Removes mui-nested-menu lock entry. |
| src/app/screens/Feeds/Feeds.styles.ts | Adjusts sticky header offset for md breakpoint. |
| src/app/constants/Navigation.ts | Removes less-critical nav links from header navigation items. |
| src/app/components/HeaderSearchBar.tsx | Adds new header search UI + routing to /feeds?q=.... |
| src/app/components/HeaderMobileDrawer.tsx | Adds search field and ThemeToggle to the mobile drawer. |
| src/app/components/Header.tsx | Major header redesign: hover menus, avatar account entry, search overlay, i18n labels. |
| src/app/components/Header.style.ts | Tweaks shared header button spacing. |
| src/app/components/FooterElements.tsx | Adds reusable footer link/title components. |
| src/app/components/Footer.tsx | Replaces footer with multi-column layout + social links + i18n. |
| src/app/[locale]/layout.tsx | Updates main container min-height calc to accommodate new footer size. |
| src/app/App.css | Updates client-side min-height calc to accommodate new footer size. |
| package.json | Removes mui-nested-menu dependency. |
| messages/fr.json | Adds new common.* keys and new footer.* namespace. |
| messages/en.json | Adds new common.* keys and new footer.* namespace. |
| onClick={() => { | ||
| handleNavigation(navigationAccountItem); | ||
| }} |
There was a problem hiding this comment.
The account avatar button advertises a menu via aria-controls/aria-haspopup, but its onClick navigates to the account page and the menu only opens on hover. This prevents keyboard/touch users from accessing Sign Out / Metrics items. Suggest making click open the menu (and provide a distinct menu item for Account Details), and add focus/keyboard support to match the ARIA semantics.
| onClick={() => { | |
| handleNavigation(navigationAccountItem); | |
| }} | |
| onClick={handleAccountOpen} |
| <Box | ||
| component='span' | ||
| onMouseEnter={handleValidatorOpen} | ||
| onMouseLeave={handleValidatorClose} | ||
| sx={{ display: 'inline-flex' }} | ||
| > | ||
| <Button | ||
| aria-controls='validator-menu' | ||
| aria-haspopup='true' | ||
| aria-expanded={validatorAnchorEl !== null} | ||
| endIcon={<ArrowDropDownIcon />} |
There was a problem hiding this comment.
The validators dropdown is now only triggered via onMouseEnter/onMouseLeave on the wrapper and the button has no click/focus handler. This makes the menu inaccessible for keyboard users and unreliable on touch devices (no hover). Consider keeping hover behavior, but also open the menu on click (and/or onFocus/onKeyDown for Enter/Space) so the dropdown is operable without a mouse.
|
*Lighthouse ran on https://mobilitydatabase-f89m28zyp-mobility-data.vercel.app/ * (Desktop)
*Lighthouse ran on https://mobilitydatabase-f89m28zyp-mobility-data.vercel.app/feeds * (Desktop)
*Lighthouse ran on https://mobilitydatabase-f89m28zyp-mobility-data.vercel.app/feeds/gtfs/mdb-2126 * (Desktop)
*Lighthouse ran on https://mobilitydatabase-f89m28zyp-mobility-data.vercel.app/feeds/gtfs_rt/mdb-2585 * (Desktop)
*Lighthouse ran on https://mobilitydatabase-f89m28zyp-mobility-data.vercel.app/feeds/gbfs/gbfs-flamingo_porirua * (Desktop)
|
| <FooterLink href='/gbfs-validator'> | ||
| {t('links.gbfsValidator')}{' '} | ||
| </FooterLink> |
There was a problem hiding this comment.
This must be hidden behind the feature flag or pointing to the current GBFS validator.
| @@ -264,186 +312,212 @@ export default function DrawerAppBar(): React.ReactElement { | |||
| </Link> | |||
| ))} | |||
| {config.gbfsValidator && ( | |||
There was a problem hiding this comment.
This feature flag should be applied only to the GBFS validator, not to all validators' links.
There was a problem hiding this comment.
this is the same logic we have in the Header, do we want to start exposing it in both? Should we even allow the gbfs validator link to start with the external link?
There was a problem hiding this comment.
As the new GBFS UI validator is not there yet, let's expose the current GBFS validator and the others. This can be done in a different issue, as it's not part of the current scope.
Summary:
closes #90
Updates the styling and functionality of the Header and Footer of the page
Expected behavior:
Header
Footer
Testing tips:
Simply play around with the header / footer, test the linking
Please make sure these boxes are checked before submitting your pull request - thanks!
yarn testto make sure you didn't break anything