[WEB-3422] fix: app sidebar fixes and improvements #6630
Conversation
WalkthroughThis pull request removes the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as Sidebar Container
participant L as SidebarProjectsListItem
participant N as ProjectNavigationRoot
participant PN as ProjectNavigation
U->>S: Open sidebar
S->>L: Render list item (with renderInExtendedSidebar flag)
L->>N: Pass isSidebarCollapsed state
N->>PN: Render navigation based on collapse state
PN-->>S: Display updated sidebar UI
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![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)
web/core/components/workspace/sidebar/projects-list.tsx (2)
247-247: Extract magic number as a constant.The number
7inslice(0, 7)should be extracted as a named constant at the top of the file for better maintainability and reusability.+const MAX_VISIBLE_PROJECTS = 7; // ... -{joinedProjects.slice(0, 7).map((projectId, index) => ( +{joinedProjects.slice(0, MAX_VISIBLE_PROJECTS).map((projectId, index) => (
274-274: Use translation for "More" text.The hardcoded "More" text should use the translation system for consistency with i18n.
-{!sidebarCollapsed && <span>More</span>} +{!sidebarCollapsed && <span>{t("more")}</span>}web/core/components/workspace/sidebar/project-navigation.tsx (1)
36-41: Consider memoizing the toggleSidebar callback.Since
toggleSidebaris used in an event handler, consider memoizing it withuseCallbackto prevent unnecessary re-renders.- const { toggleSidebar } = useAppTheme(); + const { toggleSidebar: rawToggleSidebar } = useAppTheme(); + const toggleSidebar = useCallback(() => rawToggleSidebar(), [rawToggleSidebar]);web/app/[workspaceSlug]/(projects)/extended-project-sidebar.tsx (1)
99-108: Consider adding ARIA attributes for better accessibility.The sidebar implementation could benefit from ARIA attributes to improve screen reader support.
<div ref={extendedProjectSidebarRef} + role="complementary" + aria-label="Extended Project Sidebar" className={cn(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apiserver/plane/db/models/workspace.py(0 hunks)packages/constants/src/workspace.ts(2 hunks)web/app/[workspaceSlug]/(projects)/extended-project-sidebar.tsx(4 hunks)web/app/[workspaceSlug]/(projects)/extended-sidebar.tsx(1 hunks)web/ce/components/sidebar/project-navigation-root.tsx(1 hunks)web/ce/components/workspace/sidebar/sidebar-item.tsx(1 hunks)web/core/components/workspace/sidebar/project-navigation.tsx(1 hunks)web/core/components/workspace/sidebar/projects-list-item.tsx(3 hunks)web/core/components/workspace/sidebar/projects-list.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- apiserver/plane/db/models/workspace.py
✅ Files skipped from review due to trivial changes (1)
- web/app/[workspaceSlug]/(projects)/extended-sidebar.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (11)
web/core/components/workspace/sidebar/projects-list.tsx (1)
246-278: LGTM! Clean and well-structured rendering logic.The changes improve the organization of the project list rendering by:
- Properly grouping related elements within a fragment
- Implementing responsive design for collapsed state
- Adding clear conditional logic for the "More" button
web/ce/components/sidebar/project-navigation-root.tsx (1)
7-18: LGTM! Clean implementation of the sidebar collapsed state.The addition of the
isSidebarCollapsedprop and its propagation through the component is well-structured and follows React best practices.web/ce/components/workspace/sidebar/sidebar-item.tsx (1)
45-45:Details
❓ Verification inconclusive
Verify navigation behavior with updated static items.
The change from "home" to "your-work" in static items could affect navigation paths. Please ensure that:
- All navigation paths are updated accordingly
- No broken links are introduced
- User experience remains consistent
🏁 Script executed:
#!/bin/bash # Search for any remaining references to "home" in navigation rg -A 5 "home.*href" # Search for "your-work" navigation implementation rg -A 5 "your-work.*href"Length of output: 1231
Ensure consistent navigation paths post static items update.
The current static items array now includes both "home" and "your-work", yet our search shows that the "home" link is still referenced (e.g., in web/app/workspace-invitations/page.tsx as “Continue to home” with href="/"). Please verify that all navigation components have been updated as intended—whether to retire "home" or reconcile its coexistence with "your-work"—so that no broken links or confusing navigation patterns exist. A manual review of the navigation paths across the codebase is recommended.
web/core/components/workspace/sidebar/project-navigation.tsx (1)
29-34: LGTM! Props interface updated correctly.The addition of
isSidebarCollapsedtoTProjectItemsPropsis well-documented and properly typed.web/app/[workspaceSlug]/(projects)/extended-project-sidebar.tsx (1)
138-152: LGTM! Improved scroll behavior implementation.The addition of overflow control and custom scrollbar styling enhances the user experience while maintaining functionality.
packages/constants/src/workspace.ts (2)
301-306: LGTM! The "your-work" item is properly configured in static navigation.The configuration includes:
- Correct key and translation key
- Appropriate href path
- Proper access roles limited to admin and member
323-323: LGTM! The "your-work" item is properly added to static navigation links.The item is correctly referenced from the static navigation items record.
web/core/components/workspace/sidebar/projects-list-item.tsx (4)
46-46: LGTM! The new prop is well-defined.The
renderInExtendedSidebaroptional boolean property is clearly named and its purpose is self-evident.
50-59: LGTM! Props destructuring is properly implemented.The destructuring includes:
- All required props
- Default value for
renderInExtendedSidebar- Clean formatting
101-101: LGTM! Sidebar collapse logic is properly updated.The condition
sidebarCollapsed && !renderInExtendedSidebarensures the sidebar only collapses when not rendering in extended mode.
404-408: LGTM! ProjectNavigationRoot props are properly updated.The component now receives:
- Correct workspaceSlug and projectId values
- Properly typed isSidebarCollapsed prop derived from the updated collapse logic
Description
This PR includes app sidebar fixes and improvements.
Type of Change
References
[WEB-3422]
Summary by CodeRabbit
New Features
Style