[WEB-2738] chore: app sidebar behaviour #5964
Conversation
WalkthroughThe changes in this pull request involve modifications to several components, primarily focusing on the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
🧹 Outside diff range and nitpick comments (2)
web/core/components/workspace/sidebar/favorites/favorite-items/common/favorite-item-title.tsx (2)
16-16: Consider adding ARIA attributes for better accessibility.While the click handler simplification is good, consider enhancing accessibility by adding appropriate ARIA attributes.
<Link href={href} className={isSidebarCollapsed ? collapsedClass : linkClass} draggable onClick={handleOnClick} + aria-label={`Navigate to ${title}`} + role="menuitem" >Also applies to: 27-29
Line range hint
22-23: Consider extracting CSS classes to a constants file.The CSS classes are quite long and could be reused across similar components. Consider extracting them to a shared constants file for better maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
web/core/components/workspace/sidebar/favorites/favorite-items/common/favorite-item-title.tsx(1 hunks)web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx(1 hunks)web/core/components/workspace/sidebar/projects-list-item.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- web/core/components/workspace/sidebar/projects-list-item.tsx
🔇 Additional comments (3)
web/core/components/workspace/sidebar/favorites/favorite-items/common/favorite-item-title.tsx (2)
8-13: LGTM! Props interface is now more focused.
The removal of the projectId prop has simplified the component's interface, making it more focused on its core responsibility of rendering a favorite item title.
8-13: Verify removal of projectId prop usage.
Let's ensure there are no remaining instances where projectId is being passed to this component.
✅ Verification successful
No remaining projectId prop usage found
Based on the search results, the FavoriteItemTitle component is only used in one place (web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx) and it's being called with the correct props (href, icon, title, isSidebarCollapsed) as defined in its type definition. There are no instances where projectId is being passed to this component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances where projectId is passed to FavoriteItemTitle
ast-grep --pattern 'FavoriteItemTitle $$$projectId=$$$'
Length of output: 57
Script:
#!/bin/bash
# Let's first find all files that might be using FavoriteItemTitle
rg "FavoriteItemTitle" -l
# Then let's see the actual usage contexts
rg "FavoriteItemTitle" -A 3 -B 3
Length of output: 2775
web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx (1)
94-94: LGTM! Props interface simplification.
The removal of the projectId prop simplifies the component interface while maintaining necessary functionality through the useFavoriteItemDetails hook.
Let's verify if the scroll functionality previously dependent on projectId is handled elsewhere:
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
web/core/components/workspace/sidebar/favorites/favorite-items/common/favorite-item-title.tsx (1)
Line range hint
31-37: Consider enhancing accessibility attributes.While the component structure is solid, we could improve accessibility by adding ARIA attributes for better screen reader support.
<Link href={href} className={isSidebarCollapsed ? collapsedClass : linkClass} draggable - onClick={handleOnClick}> + onClick={handleOnClick} + aria-label={`Navigate to ${title}`} + role="menuitem">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
web/core/components/workspace/sidebar/favorites/favorite-items/common/favorite-item-title.tsx(1 hunks)web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx(1 hunks)web/core/components/workspace/sidebar/projects-list-item.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- web/core/components/workspace/sidebar/projects-list-item.tsx
🔇 Additional comments (3)
web/core/components/workspace/sidebar/favorites/favorite-items/common/favorite-item-title.tsx (2)
7-13: LGTM! Props interface has been simplified.
The removal of projectId from Props makes the component more focused and easier to maintain.
Line range hint 26-28: Verify the impact of removing scroll behavior.
The click handler has been simplified to only handle mobile sidebar toggling. Let's verify that the removal of scroll behavior doesn't affect user experience in other parts of the application.
✅ Verification successful
Let me generate additional scripts to search for any scroll-related functionality specifically in the favorites section and the component's usage:
This will help us understand:
- If there are any scroll behaviors in the favorites section that might be affected
- How the FavoriteItemTitle component is being used across the codebase
Removal of scroll behavior is safe to proceed
The removal of scroll behavior from the FavoriteItemTitle component is safe as:
- The component is only used as a simple link renderer in the favorites section
- No other components in the favorites section rely on scroll behavior from this component
- Scroll functionality in the sidebar is properly handled at the container level (as seen in
projects-list.tsxand other sidebar components)
The click handler's simplification to only handle mobile sidebar toggling aligns with the component's current usage pattern and doesn't impact the user experience.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining scroll-to-element behaviors that might need to be updated
# or alternative implementations that should handle the scroll behavior
# Look for scroll-related code in sidebar components
rg -g '**/*sidebar*/**/*.{ts,tsx}' -A 3 'scroll|getElementById'
# Look for references to the removed projectId prop
rg -g '**/*.{ts,tsx}' 'FavoriteItemTitle.*projectId'
Length of output: 4778
Script:
#!/bin/bash
# Search for scroll behavior in favorites-related components
rg -g '**/favorites/**/*.{ts,tsx}' -A 5 'scroll|onClick'
# Search for FavoriteItemTitle component usage
ast-grep --pattern 'FavoriteItemTitle'
Length of output: 7644
web/core/components/workspace/sidebar/favorites/favorite-items/root.tsx (1)
94-94: LGTM! Verify navigation behavior after prop simplification.
The removal of projectId prop from FavoriteItemTitle creates a cleaner interface. The component now focuses solely on presentation and basic interaction concerns.
Let's verify that navigation still works correctly after removing the projectId prop:
✅ Verification successful
Removal of projectId and scroll behavior is intentional
The changes are part of a deliberate simplification where:
- The
projectIdprop and associated scroll-into-view behavior have been removed - Navigation now relies solely on the
hrefprop through Next.js Link component - Mobile sidebar toggle behavior is preserved
- Component maintains its core presentation and navigation responsibilities
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if FavoriteItemTitle's click handling is properly implemented
# Expected: The click handler should now be simplified and handle navigation through href
# Search for click handler implementation in FavoriteItemTitle
ast-grep --pattern 'const FavoriteItemTitle = $_($_) {
$$$
onClick($_) {
$$$
}
$$$
}'
# Check for any direct projectId references that might have been missed
rg 'projectId.*FavoriteItemTitle'
Length of output: 135
Script:
#!/bin/bash
# Let's try a different approach to find FavoriteItemTitle implementation
# First, find the file containing FavoriteItemTitle
fd "favorite-item-title" --type f
# Then let's examine its implementation and usage
ast-grep --pattern 'const FavoriteItemTitle = $_'
# Also check for any files importing FavoriteItemTitle
rg "import.*FavoriteItemTitle"
# Look for any test files related to FavoriteItemTitle
fd "favorite-item-title.test" --type f
Length of output: 251
Script:
#!/bin/bash
# Let's examine the FavoriteItemTitle component implementation
cat web/core/components/workspace/sidebar/favorites/favorite-items/common/favorite-item-title.tsx
# Also check for any changes in git history related to projectId
git log -p web/core/components/workspace/sidebar/favorites/favorite-items/common/favorite-item-title.tsx | grep -A 10 -B 10 "projectId"
Length of output: 6620
Summary:
This PR enhances the app sidebar by improving redirection and collapsible functionality.
Media:
Reference:
[WEB-2738]
Summary by CodeRabbit
Summary by CodeRabbit
New Features
FavoriteItemTitlecomponent by removing theprojectIdprop, focusing on essential properties.SidebarProjectsListItemwith improved drag-and-drop functionality and refined project list visibility management.Bug Fixes
Refactor