feat(styling): improve sidebar #1344
Conversation
WalkthroughThis set of changes primarily restructures the sidebar navigation system and related UI components in the deploy-web app. It introduces new sidebar menu popover and hovered group menu components, updates sidebar item types, simplifies the theme toggle, and refactors the paginator logic. Additional minor UI and logic adjustments are applied across several components, including removal of the footer from the home page, GPU form control enhancements, and state initialization improvements in the layout. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar
participant SidebarGroupMenu
participant SidebarGroupMenuPopover
participant SidebarHoveredGroupMenu
User->>Sidebar: Clicks or hovers on sidebar menu
Sidebar->>SidebarGroupMenu: Render group menu
SidebarGroupMenu->>SidebarGroupMenuPopover: If route has hoveredRoutes, render popover
SidebarGroupMenuPopover->>SidebarHoveredGroupMenu: Show hovered group menu in popover
SidebarHoveredGroupMenu-->>User: Display submenu items
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (2)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (23.28%) is below the target coverage (30.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1344 +/- ##
========================================
Coverage 33.42% 33.43%
========================================
Files 789 791 +2
Lines 19267 19297 +30
Branches 3569 3589 +20
========================================
+ Hits 6440 6451 +11
+ Misses 12616 12237 -379
- Partials 211 609 +398
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
apps/deploy-web/src/components/layout/SidebarHoveredGroupMenu.tsx (1)
21-37: Consider adding more aria attributes for better accessibilityWhile the component uses semantic HTML and role attributes, you could enhance accessibility by adding aria-labelledby to connect the nav element with the group title.
- <nav className={cn("flex flex-1 flex-col", { ["items-center"]: !isNavOpen })} aria-label="Sidebar"> + <nav + className={cn("flex flex-1 flex-col", { ["items-center"]: !isNavOpen })} + aria-label={`Sidebar ${group.title || 'Menu'}`} + aria-labelledby={group.title ? `sidebar-group-${group.title}` : undefined} + > <ul role="list" className="w-full space-y-1"> {!!group.title && isNavOpen && ( <li> - <span className="text-sm font-light">{group.title}</span> + <span id={`sidebar-group-${group.title}`} className="text-sm font-light">{group.title}</span> </li> )}apps/deploy-web/src/components/layout/SidebarGroupMenuPopover.tsx (1)
50-57: Shadowed variable & unstable React keyUsing the same identifier
routein themapcallback shadows the outer prop and makes the JSX harder to read.
More importantly, the key relies onroute.title, which is optional inISidebarRoute; if two routes omittitleyou’ll get duplicate keys and React warnings.{route.hoveredRoutes?.map((grp, idx) => ( - <SidebarHoveredGroupMenu key={route.title} group={route} … /> + <SidebarHoveredGroupMenu key={grp.title ?? idx} group={grp} … /> ))}apps/deploy-web/src/components/layout/SidebarRouteButton.tsx (1)
55-56: Anchor rendered even for “non-link” routes
route.url && useNextLinkTag ? <Link …>is fine, but the fallback always renders an<a>even whenroute.urlis falsy. If the item is supposed to be a plain button (e.g. triggers state changes), preferbuttonto avoid confusing semantics and screen-reader behaviour.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (12)
apps/deploy-web/src/components/home/HomeContainer.tsx(0 hunks)apps/deploy-web/src/components/layout/ModeToggle.tsx(2 hunks)apps/deploy-web/src/components/layout/NodeStatusBar.tsx(3 hunks)apps/deploy-web/src/components/layout/Sidebar.tsx(6 hunks)apps/deploy-web/src/components/layout/SidebarGroupMenu.tsx(2 hunks)apps/deploy-web/src/components/layout/SidebarGroupMenuPopover.tsx(1 hunks)apps/deploy-web/src/components/layout/SidebarHoveredGroupMenu.tsx(1 hunks)apps/deploy-web/src/components/layout/SidebarRouteButton.tsx(3 hunks)apps/deploy-web/src/components/sdl/GpuFormControl.tsx(1 hunks)apps/deploy-web/src/types/index.ts(1 hunks)apps/deploy-web/src/utils/sdl/data.ts(2 hunks)packages/ui/components/pagninator.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- apps/deploy-web/src/components/home/HomeContainer.tsx
🧰 Additional context used
🧬 Code Graph Analysis (4)
apps/deploy-web/src/components/layout/SidebarGroupMenu.tsx (2)
apps/deploy-web/src/components/layout/SidebarRouteButton.tsx (1)
SidebarRouteButton(21-72)apps/deploy-web/src/components/layout/SidebarGroupMenuPopover.tsx (1)
SidebarGroupMenuPopover(17-62)
apps/deploy-web/src/components/layout/SidebarRouteButton.tsx (2)
apps/provider-console/src/utils/styleUtils.ts (1)
cn(4-6)packages/ui/components/tooltip.tsx (3)
Tooltip(76-76)TooltipTrigger(76-76)TooltipContent(76-76)
apps/deploy-web/src/components/layout/SidebarHoveredGroupMenu.tsx (3)
apps/deploy-web/src/types/index.ts (1)
ISidebarGroupMenu(17-21)apps/provider-console/src/utils/styleUtils.ts (1)
cn(4-6)apps/deploy-web/src/components/layout/SidebarRouteButton.tsx (1)
SidebarRouteButton(21-72)
apps/deploy-web/src/components/layout/NodeStatusBar.tsx (2)
packages/network-store/src/network.store.ts (1)
selectedNetwork(66-68)apps/provider-console/src/components/shared/LinearLoadingSkeleton.tsx (1)
LinearLoadingSkeleton(8-10)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test-stats-web-build
- GitHub Check: test-deploy-web-build
- GitHub Check: test-provider-console-build
🔇 Additional comments (18)
apps/deploy-web/src/components/layout/ModeToggle.tsx (3)
4-4: Appropriate dependency management.The import of the Button component from the organization's UI library ensures consistent styling and behavior across the application.
27-41: UI improvement with better accessibility and user experience.The change from a dropdown to discrete buttons provides several benefits:
- Immediate visibility of all theme options
- Direct interaction without extra clicks
- Clear visual indication of the active theme via the "text-primary" class
- Proper accessibility with screen reader text for each button
This implementation aligns with modern UI practices for theme toggles.
36-40: Creative solution for system theme representation.The system theme button cleverly uses absolute positioning to overlay both light and dark theme icons, visually communicating that this option adapts to system preferences.
packages/ui/components/pagninator.tsx (5)
46-82: Great refactoring of the pagination logic!The new implementation is much more straightforward and maintainable. By using a consistent pattern of showing:
- Always the first page
- A conditional starting ellipsis
- A sliding window of pages around the current page
- A conditional ending ellipsis
- Always the last page
This approach creates a more intuitive pagination experience while simplifying the code. The sliding window calculation with
Math.maxandMath.minis elegant and handles edge cases well.
44-44: Good early return pattern.The early return for simpler cases improves code readability and performance by exiting the function early when pagination is straightforward.
59-61: Well-implemented sliding window logic.The calculation of
startPageandendPageusingMath.maxandMath.minis an elegant way to handle boundary conditions while maintaining a consistent window size around the current page.
55-57: Proper ellipsis condition for start range.The condition
currentPage > 4correctly determines when to show the starting ellipsis, which ensures we don't show an ellipsis when it's not necessary (when the current page is close to the beginning).
71-73: Proper ellipsis condition for end range.The condition
currentPage < totalPages - 3correctly determines when to show the ending ellipsis, which ensures we don't show an ellipsis when it's not necessary (when the current page is close to the end).apps/deploy-web/src/utils/sdl/data.ts (1)
163-163: Consistent pricing change applied to defaultRentGpuServiceThe same pricing amount increase (from 10,000 to 100,000 uakt) has been applied consistently to the GPU service configuration, maintaining parity between service types.
apps/deploy-web/src/components/sdl/GpuFormControl.tsx (1)
117-119: Improved UX with automatic GPU count initializationThis enhancement ensures a valid initial state by automatically setting the GPU count to 1 when a user enables GPU support with a zero count. This prevents inconsistent configurations and improves the user experience.
apps/deploy-web/src/types/index.ts (1)
24-24: Clean type updates to support new sidebar functionalityThe changes to
ISidebarRoutetype properly support the new sidebar navigation system by making previously required properties optional and adding new properties for hover menus, dividers, and custom components.These type changes align well with the component implementations in
SidebarGroupMenu,SidebarHoveredGroupMenu, and others.Also applies to: 26-27, 32-34
apps/deploy-web/src/components/layout/SidebarGroupMenu.tsx (2)
7-7: LGTM: New import for SidebarGroupMenuPopoverThe import is correctly added to support the conditional rendering pattern implemented below.
31-35: Good implementation of conditional menu renderingThe conditional rendering logic is clean and effectively determines whether to show a standard route button or a popover menu based on the presence of
hoveredRoutes.apps/deploy-web/src/components/layout/SidebarHoveredGroupMenu.tsx (1)
1-40: Well-structured new component for hovered sidebar menusThis component is well-organized and follows good React practices:
- Clear prop types with appropriate optional flags
- Good use of conditional rendering for the divider and group title
- Proper handling of custom components vs. standard route buttons
- Semantic HTML structure with nav and list roles for accessibility
The component correctly passes
isHoveredto theSidebarRouteButtonto ensure proper display within hover menus.apps/deploy-web/src/components/layout/NodeStatusBar.tsx (4)
2-2: Updated import to include Separator componentAdded Separator to support the new layout with a divider at the bottom of the component.
20-22: Improved layout for network title displayThe updated flex layout provides better alignment and spacing for the network title.
26-28: Enhanced button styling for better visual hierarchyGood styling updates to the node status button:
- Changed to "secondary" variant for visual consistency with the sidebar
- Added flex layout with justify-between for better alignment of the node ID and status
44-44: Good addition of visual separatorThe Separator component with margin creates a clear visual distinction between the node status and following content.
6ca90a9 to
2ed6000
Compare
#553
#1326
#1325
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Removals
Chores