feat: expandable sidebar #3341
feat: expandable sidebar #3341
Conversation
WalkthroughThis PR introduces a collapsible menu feature to the ProjectMenu component. The expanded state is managed at the parent level and propagates down to child components via props, enabling menu items to conditionally display text labels and apply responsive styling based on the expansion state. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
webapp/src/views/projects/projectMenu/SideMenu.tsx (1)
6-12: Avoid leaking theexpandedstyling prop to DOM elementsThe styling logic and transition behavior look good, but
expandedis currently being passed through to the underlyingdivandmenuDOM nodes, which can trigger React “unknown prop” warnings.Consider filtering it out via
shouldForwardProp:-import { styled } from '@mui/material'; +import { styled } from '@mui/material'; -const StyledMenuWrapper = styled('div')<{ expanded: boolean }>` +const StyledMenuWrapper = styled('div', { + shouldForwardProp: (prop) => prop !== 'expanded', +})<{ expanded: boolean }>` min-width: ${({ expanded }) => expanded ? MENU_WIDTH_EXPANDED : MENU_WIDTH}px; transition: min-width 0.2s ease-in-out; `; -const StyledMenuFixed = styled('menu')<{ expanded: boolean }>` +const StyledMenuFixed = styled('menu', { + shouldForwardProp: (prop) => prop !== 'expanded', +})<{ expanded: boolean }>` position: fixed; ... width: ${({ expanded }) => (expanded ? MENU_WIDTH_EXPANDED : MENU_WIDTH)}px; ... `;Please double‑check the exact
styledAPI for your MUI version to confirmshouldForwardPropusage.Also applies to: 14-25
webapp/src/views/projects/projectMenu/SideMenuItem.tsx (1)
52-61: Prop API and rendering forexpandedare consistent and backward‑compatibleMaking
expandedoptional with a default offalsekeeps existing callers working, while still allowing ProjectMenu to control item expansion. The conditional text span inside theLinkis a straightforward way to reveal labels without affecting the tooltip behavior.One follow‑up: when you apply the
shouldForwardProppattern fromSideMenuto avoid leakingexpandedto the DOM, consider doing the same forStyledItemhere.Please confirm that adding
shouldForwardPropforexpandedtoStyledItemis compatible with your MUIstyledhelper, similar to:const StyledItem = styled('li', { shouldForwardProp: (prop) => prop !== 'expanded', })<{ expanded: boolean }>`...`;Also applies to: 68-76, 104-126
webapp/src/views/projects/projectMenu/ProjectMenu.tsx (2)
30-37: Consider using theme spacing for the toggle button offset
margin-bottom: 70px;works but hard‑codes a magic number. If you want this to be more consistent with the rest of the UI, you could tie it totheme.spacinginstead (e.g.theme.spacing(9)or similar).
49-50: Expanded state wiring is clear; add E2E and ARIA hooks to the toggleThe expanded state is cleanly threaded from
ProjectMenuintoSideMenuand eachSideMenuItem, and the chevron toggle logic is straightforward.Given your guidelines and for accessibility:
- Add a
data-cyattribute toStyledToggleButtonso E2E tests can target the control without relying on icon content.- Add appropriate ARIA attributes, e.g.
aria-labelandaria-expanded, so screen readers understand that this button expands/collapses the navigation.For example:
- <StyledToggleButton - onClick={() => setExpanded((prev) => !prev)} - size="small" - > + <StyledToggleButton + onClick={() => setExpanded((prev) => !prev)} + size="small" + data-cy="project-menu-expand-toggle" + aria-label={expanded ? 'Collapse project menu' : 'Expand project menu'} + aria-expanded={expanded} + >Also applies to: 173-199
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
webapp/src/views/projects/projectMenu/ProjectMenu.tsx(5 hunks)webapp/src/views/projects/projectMenu/SideMenu.tsx(1 hunks)webapp/src/views/projects/projectMenu/SideMenuItem.tsx(6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
webapp/**/*.{ts,tsx}: Use TypeScript path aliases (tg.component/, tg.service/, tg.hooks/, tg.views/, tg.globalContext/*) instead of relative imports
After backend API changes, regenerate TypeScript types by runningnpm run schema(andnpm run billing-schemaif applicable) in the webapp directory
Use typed React Query hooks fromuseQueryApi.tsfor API communication, not raw React Query
Use Tolgee-specific hooksuseReportEventanduseReportOncefrom 'tg.hooks/useReportEvent' for business event tracking and analytics
Files:
webapp/src/views/projects/projectMenu/SideMenu.tsxwebapp/src/views/projects/projectMenu/ProjectMenu.tsxwebapp/src/views/projects/projectMenu/SideMenuItem.tsx
webapp/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Always use
data-cyattributes for E2E test selectors, never rely on text content
Files:
webapp/src/views/projects/projectMenu/SideMenu.tsxwebapp/src/views/projects/projectMenu/ProjectMenu.tsxwebapp/src/views/projects/projectMenu/SideMenuItem.tsx
**/*.{ts,tsx,js,jsx,kt,kts}
⚙️ CodeRabbit configuration file
As part of review, please check if the file follows 'The Stepdown Rule': The most important, high-level concepts should be at the top (like a newspaper headline and opening paragraph). Details should increase as you read downward. Functions should be ordered so that a caller appears before the functions it calls. When working with JavaScript components, we allow the main component to live at the bottom of the file as an exception to the rule.
Files:
webapp/src/views/projects/projectMenu/SideMenu.tsxwebapp/src/views/projects/projectMenu/ProjectMenu.tsxwebapp/src/views/projects/projectMenu/SideMenuItem.tsx
🧬 Code graph analysis (1)
webapp/src/views/projects/projectMenu/ProjectMenu.tsx (1)
webapp/src/views/projects/projectMenu/SideMenu.tsx (1)
SideMenu(27-45)
🔇 Additional comments (2)
webapp/src/views/projects/projectMenu/SideMenuItem.tsx (1)
7-50: Expanded/collapsed item styling is coherent and preserves truncationThe conditional padding, alignment, width, and
.textoverflow handling nicely support both compact and expanded states while keeping labels ellipsized instead of wrapping. This fits the new sidebar behavior well.webapp/src/views/projects/projectMenu/SideMenu.tsx (1)
27-44: SideMenu now requiresexpanded: verify all call sites are updatedThe new
expanded: booleanprop is cleanly threaded into the layout. Ensure all usages ofSideMenuprovide theexpandedprop, not justProjectMenu. Check for missing prop warnings during compilation or testing.
|
Hey Aditya! That's slay! ❤️ @ZuzanaOdstrcilova, what do you think? |
2 participants
It is hard to recognize the items by their icons - I often hover over them for a second to know where they will redirect.
Added an expandable sidebar to quickly know where it would be navigating.
feat.mp4
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.