fix(toolbar): make the top menu bar horizontally scrollable on tablets#879
Conversation
#871) At md and above the toolbar switched to a single non-wrapping row with no overflow handling, so on iPad and landscape phones the menus that did not fit (Settings, Help, the theme toggle, the project name) ran off the right edge with no way to reach them. Add md:overflow-x-auto so the row scrolls horizontally while keeping the wrapping behavior below md and the unchanged desktop layout when everything fits.
✅ Deploy Preview for geolibre-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
More reviews will be available in 6 minutes and 34 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe top toolbar header now allows horizontal scrolling on medium and larger screens while preserving its non-wrapping layout. The change updates the responsive class list in the desktop toolbar component. ChangesTop toolbar overflow handling
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⚡ Cloudflare Pages preview
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // above we switch to a single non-wrapping row and let it scroll | ||
| // horizontally so tablets/landscape phones can still reach the | ||
| // menus that overflow the viewport (e.g. Help, Settings) (#871). | ||
| "flex-wrap px-2 md:flex-nowrap md:overflow-x-auto", |
There was a problem hiding this comment.
UX / discoverability concern (medium confidence): overflow-x: auto on a flex container pushes the ml-auto spacer to the far right of the scroll content, not the visible viewport edge. On a tablet at ~800 px the theme-toggle button and project-name input (the ml-auto div at roughly line 1127) will only be visible after the user scrolls all the way right — they are not pinned to the visible right edge.
This is better than the previous fully-inaccessible behaviour, and the compact path already has the same pattern, so it is internally consistent. But on touch devices the horizontal scrollbar is auto-hidden, so there is no visual affordance that the bar is scrollable. Worth considering a subtle scroll-shadow or the scrollbar-width: thin token (already used elsewhere in index.css) so users on touch screens can discover the overflow content.
Not a blocker — just worth noting for a follow-up if discoverability proves to be a problem in practice.
There was a problem hiding this comment.
Thanks, noted. Leaving this open as a possible follow-up. The right-side controls becoming reachable only after scrolling is the same pattern the compact/embed path already uses, so the behavior stays internally consistent, and it is strictly better than the previous fully-inaccessible state. A scroll-shadow or thin-scrollbar affordance can be added later if discoverability proves to be a problem in practice.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Code reviewThe change is a single Tailwind class addition ( CLAUDE.md (must-fix)
Quality / UX (informational)
Bugs / Security / PerformanceNothing found. The change is CSS-class-only with no logic, data-handling, or external calls. |
- Collapse the 4-line toolbar comment to a single short line per the project's one-line comment rule (#871).
| ? "flex-nowrap overflow-x-auto px-1.5" | ||
| : "flex-wrap px-2 md:flex-nowrap", | ||
| : // Wrap below md; scroll a single row at md+ so tablets reach every menu (#871). | ||
| "flex-wrap px-2 md:flex-nowrap md:overflow-x-auto", |
There was a problem hiding this comment.
The fix is correct and mirrors what the compact path already does. One mild concern worth tracking: iOS and iPadOS hide horizontal scrollbars by default (they only appear mid-swipe and fade out), so on the main target device (tablet) there is no persistent visual affordance telling the user the toolbar is scrollable. If a menu item is just outside the viewport the user will not know to swipe.
This is a pre-existing gap in the compact path too, so it is out of scope for this PR, but a follow-up could add a right-edge fade/shadow on overflow—something like:
| "flex-wrap px-2 md:flex-nowrap md:overflow-x-auto", | |
| "flex-wrap px-2 md:flex-nowrap md:overflow-x-auto md:[mask-image:linear-gradient(to_right,black_calc(100%-2rem),transparent)]", |
(or a JS-measured approach if the mask-image approach is too blunt). Not a blocker; just flagging so the issue thread has the note.
There was a problem hiding this comment.
Agreed, and thanks for the concrete suggestion. As you note, this is out of scope for this PR and pre-exists in the compact path, so I'll leave it as a follow-up rather than add a mask-fade here (a right-edge mask would also visually dim the project-name input at the end of the row). Tracking it for a separate discoverability pass across both the compact and md+ scroll paths. Leaving this open for the maintainer to weigh in.
Code reviewReviewed the diff (2 additions / 1 deletion in BugsNothing found. The fix is correct: adding SecurityNothing found. PerformanceNothing found. Pure CSS-class change; zero runtime cost. Quality
CLAUDE.mdNo violations. The change is Tailwind-class-only, touches no i18n strings, adds no new external hosts, and does not directly modify Overall: The fix is minimal, correct, and consistent with the existing compact-mode pattern. One inline note filed for discoverability; nothing blocking. |
1 participant
Summary
On iPad and landscape phones the top menu bar ran off the right edge of the screen with no way to reach the menus that overflowed (Settings, Help, the theme toggle, the project name). At the
mdbreakpoint and above the toolbar switched to a single non-wrapping row (md:flex-nowrap) with no overflow handling, so any items past the viewport edge were simply clipped and unreachable.This adds
md:overflow-x-autoso the toolbar scrolls horizontally when its items do not fit, keeping every menu reachable. Behavior is unchanged elsewhere:mdthe toolbar still wraps to multiple rows as before.The branding (map icon + "GeoLibre" text) is kept per the maintainer's decision in the issue thread; this change keeps it while ensuring the rest of the menus stay accessible on small screens.
Verification
Drove the real app in a browser with Playwright in both light and dark themes:
scrollWidth1168 >clientWidth800) and Settings, Help, the theme toggle, and the project name are all reachable.scrollWidth == clientWidth, not scrollable) and the desktop layout is unchanged.pre-commit run --files(including the full npm build) passes.Fixes #871
Summary by CodeRabbit