-
Notifications
You must be signed in to change notification settings - Fork 489
fix: enable sidebar expand and project switching on mobile #403
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
- Sidebar now uses overlay pattern on mobile (fixed position when open) - Added backdrop overlay that dismisses sidebar on tap - Made collapse toggle button visible on all screen sizes - Made project options menu visible on all screen sizes Previously the sidebar was forced to collapsed width (w-16) on mobile even when sidebarOpen was true, and the toggle/options buttons were hidden with `hidden lg:flex`.
Summary of ChangesHello @adelin-b, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience on mobile devices by addressing critical usability issues related to the sidebar. Previously, mobile users were unable to expand the sidebar or switch between projects due to hidden controls and a fixed collapsed width. The changes introduce a responsive overlay design for the sidebar, complete with a dismissable backdrop, and ensure that essential navigation and project management controls are accessible regardless of screen size. This makes the application fully functional and intuitive for users on smaller viewports. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request successfully implements an overlay pattern for the sidebar on mobile devices, allowing it to be expanded and enabling project switching. The changes to make the collapse and project options buttons visible on all screen sizes are also correct. However, I've identified a significant accessibility issue with the new overlay implementation: focus is not trapped within the open sidebar on mobile, and it cannot be dismissed with the Escape key. Addressing this will greatly improve the user experience for keyboard-only users.
| <> | ||
| {/* Mobile backdrop overlay */} | ||
| {sidebarOpen && ( | ||
| <div | ||
| className="fixed inset-0 bg-black/50 z-20 lg:hidden" | ||
| onClick={toggleSidebar} | ||
| data-testid="sidebar-backdrop" | ||
| /> | ||
| )} |
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.
While the new overlay pattern for mobile works visually, it introduces some accessibility issues that should be addressed:
- Focus is not trapped: When the sidebar is open on mobile, users can still use the Tab key to navigate to and interact with elements behind the overlay. This can be confusing for keyboard users as the focused elements are not visible.
- No
Escapekey dismissal: The sidebar overlay cannot be closed by pressing theEscapekey, which is a standard and expected behavior for modals and overlays.
To improve accessibility, I recommend implementing focus trapping to keep focus within the sidebar when it's open as an overlay, and adding a keyboard event listener to close it when the Escape key is pressed. You could use a library like focus-trap-react or implement this logic within a useEffect hook that runs when the sidebar is open on mobile.
📝 WalkthroughWalkthroughThis PR restructures the sidebar component to support mobile responsive design by introducing a mobile backdrop overlay and reorganizing nested dialogs/modals. UI elements previously hidden on mobile (collapse toggle, project selector buttons) are now visible across all screen sizes, with responsive positioning handled through layout classes rather than visibility toggles. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
|
Please get this in! |
|
I think this has been fixed, let me know if not |
Reason
I am using tailscale from my phone/tablet and in both it was impossible to switch projects.
Summary
Problem
Previously the sidebar was forced to collapsed width (
w-16) on mobile even whensidebarOpenwas true, and the toggle/options buttons were hidden withhidden lg:flex. This made it impossible to:Solution
fixed inset-y-0 left-0 w-72on mobile when open (overlay pattern)lg:hidden)hidden lg:prefix from collapse toggle buttonhidden lg:prefix from project options menu buttonTest plan
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.