Skip to content

fix(toolbar): make the top menu bar horizontally scrollable on tablets #879

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.

Sign up for GitHub

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

Merged
giswqs merged 2 commits into from
Jun 25, 2026
Merged

fix(toolbar): make the top menu bar horizontally scrollable on tablets #879

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
948b69e
fix(toolbar): make the top menu bar horizontally scrollable on tablet…
giswqs Jun 25, 2026
e3009bd
Address Claude review feedback
giswqs Jun 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion apps/geolibre-desktop/src/components/layout/TopToolbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,8 @@ export function TopToolbar({
"flex min-h-11 min-w-0 shrink-0 items-center gap-1 border-b bg-card py-1",
compact
? "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",

@github-actions github-actions Bot Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@giswqs giswqs Jun 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions github-actions Bot Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Suggested change
"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.

@giswqs giswqs Jun 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

)}
>
<span className="mr-1 flex shrink-0 items-center gap-1.5 text-sm font-semibold text-primary md:mr-2">
Expand Down
Loading