fix(user-settings): collapse sidebar to icon-only rail on mobile#15678
Open
carlh7777 wants to merge 3 commits into
Open
fix(user-settings): collapse sidebar to icon-only rail on mobile#15678carlh7777 wants to merge 3 commits into
carlh7777 wants to merge 3 commits into
Conversation
- Updated the sidebar component to utilize the `useIsMobile` hook for better responsiveness on mobile devices. - Adjusted CSS classes for the sidebar and main content area to improve layout and spacing across different screen sizes. - Ensured that user information and menu items adapt based on the device type, enhancing user experience on mobile. - Cleaned up unused code and comments for better readability.
dosubot
Bot
added
the
size:M
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe UserSetting page grid classes and inner wrapper were updated, and the SideBar was refactored for responsive behavior: mobile icon-only navigation, hidden labels/metadata on small screens, responsive sizing/ARIA attributes, and an icon-only logout on mobile. ChangesResponsive User Settings Layout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/src/pages/user-setting/index.tsx (1)
11-15: 💤 Low valueOptional: Simplify unnecessary
cn()wrapper.The
cn()call wraps a single static className string with no conditional logic. This can be simplified to a directclassNameprop.♻️ Simplification
- <div - className={cn( - 'pr-2 md:pr-6 pb-6 flex flex-1 min-w-0 rounded-lg overflow-hidden', - )} - > + <div className="pr-2 md:pr-6 pb-6 flex flex-1 min-w-0 rounded-lg overflow-hidden"> <Outlet /> </div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/pages/user-setting/index.tsx` around lines 11 - 15, The JSX div in user-setting/index.tsx currently wraps a single static class string with the cn() helper; remove the unnecessary cn() call and pass the literal class string directly to the className prop (replace className={cn('pr-2 md:pr-6 pb-6 flex flex-1 min-w-0 rounded-lg overflow-hidden')} with className="pr-2 md:pr-6 pb-6 flex flex-1 min-w-0 rounded-lg overflow-hidden") to simplify the code and avoid an unneeded dependency call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/src/pages/user-setting/sidebar/index.tsx`:
- Around line 75-80: The sidebar width is driven by JS state from useIsMobile
causing an initial mismatch with the parent grid; replace the JS conditional
class on the <aside> (and other places using isMobile) with Tailwind responsive
classes so the browser-applied media query and component classes match on first
render (e.g., use mobile-first classes like w-16 and larger-breakpoint classes
like md:w-[303px] to mirror grid-cols-[4rem_minmax(0,1fr)]), update className
usage in the same way where cn(...) checks isMobile (header, email visibility,
navigation list, buttons, footer), and remove useIsMobile if no other logic
depends on it.
---
Nitpick comments:
In `@web/src/pages/user-setting/index.tsx`:
- Around line 11-15: The JSX div in user-setting/index.tsx currently wraps a
single static class string with the cn() helper; remove the unnecessary cn()
call and pass the literal class string directly to the className prop (replace
className={cn('pr-2 md:pr-6 pb-6 flex flex-1 min-w-0 rounded-lg
overflow-hidden')} with className="pr-2 md:pr-6 pb-6 flex flex-1 min-w-0
rounded-lg overflow-hidden") to simplify the code and avoid an unneeded
dependency call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 339390b6-c2a1-4ea8-aabe-8b820dd4c920
📒 Files selected for processing (2)
web/src/pages/user-setting/index.tsxweb/src/pages/user-setting/sidebar/index.tsx
…esponsiveness - Removed the `useIsMobile` hook and adjusted CSS classes to streamline the sidebar's layout. - Enhanced responsiveness by ensuring user information and menu items adapt correctly across different screen sizes. - Cleaned up unused code and improved readability for better maintainability.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/src/pages/user-setting/sidebar/index.tsx`:
- Around line 106-109: Replace the invalid <section> used inside the Button
content with a phrasing element: wrap the icon and label in a <span> that
preserves the classes "flex items-center gap-2.5 max-md:gap-0" and keep the
{icon} and <span className="hidden md:inline">{label}</span> children unchanged;
update the JSX in web/src/pages/user-setting/sidebar/index.tsx where the Button
renders this block so the wrapper is a <span> instead of <section>.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ff9eb860-c651-4b1b-8600-74dae1c628f6
📒 Files selected for processing (1)
web/src/pages/user-setting/sidebar/index.tsx
- Changed the sidebar layout from a section to a span element for improved semantic structure. - Maintained existing styles and responsiveness while enhancing code clarity.
Author
|
@JinHai-CN, @dcc123456 could you please check this PR? |
Author
|
@KevinHuSh @yingfeng could you please check this pr? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Improves the responsiveness of the User Settings layout by converting the left navigation sidebar into a compact icon-only rail on mobile devices.
Previously, the sidebar retained its full desktop width on narrow viewports, reducing the available space for settings content and making pages such as Data Sources difficult to use on phones and smaller tablets.
With this change:
Type of Change
Screenshots
What Changed
Mobile Sidebar Optimization
useIsMobile()Layout Improvements
4rem(64px)303pxminmax(0, 1fr)for the content panel to prevent overflow and allow proper shrinkingImpact
Test Plan
Desktop (≥768px)
Mobile (<768px)
Verification
npm run build