-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WEB-3292] feat: workspace switcher redesign #6543
Conversation
WalkthroughThe changes update the workspace-related API and frontend components. In the backend, 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 (
|
…fication chore: user workspace api modification
…t-enchanced-workspace-switcher
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
apiserver/plane/app/serializers/workspace.py (1)
49-57
: Remove 'owner' from read_only_fields.The
owner
field has been removed from the serializer but is still listed inread_only_fields
. This should be removed for consistency.read_only_fields = [ "id", "created_by", "updated_by", "created_at", "updated_at", - "owner", "logo_url", ]
web/core/components/workspace/sidebar/dropdown.tsx (1)
45-45
: Remove hardcoded value for isUserInstanceAdmin.The
isUserInstanceAdmin
is hardcoded tofalse
, which could prevent instance admins from accessing their features. Consider using the actual value from the user store.- const isUserInstanceAdmin = false; + const { isUserInstanceAdmin } = useUser();
🧹 Nitpick comments (3)
web/core/components/workspace/sidebar/dropdown-item.tsx (1)
16-103
: Consider memoizing the component for performance optimization.The component is well-structured and follows React best practices. However, since it's used in a list rendering context, consider memoizing it with
React.memo
to prevent unnecessary re-renders when parent state changes.-const SidebarDropdownItem = (props: TProps) => { +const SidebarDropdownItem = React.memo((props: TProps) => { // ... component implementation -}; +});web/core/components/workspace/sidebar/dropdown.tsx (1)
84-84
: Address the TODO comment about workspace list scroll.The comment indicates a pending fix for workspace list scrolling. Consider implementing a virtualized list for better performance with large workspace lists.
Would you like me to provide an implementation using react-window or react-virtualized for efficient list rendering?
apiserver/plane/app/views/workspace/base.py (1)
178-182
: Fix line length and LGTM for the role annotation.The role annotation is well-implemented using an efficient subquery. However, line 179 exceeds the recommended line length of 88 characters.
Consider breaking down the long line:
- WorkspaceMember.objects.filter(workspace=OuterRef("id"), member=request.user, is_active=True) + WorkspaceMember.objects.filter( + workspace=OuterRef("id"), + member=request.user, + is_active=True + )🧰 Tools
🪛 Ruff (0.8.2)
179-179: Line too long (105 > 88)
(E501)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apiserver/plane/app/serializers/workspace.py
(1 hunks)apiserver/plane/app/views/workspace/base.py
(3 hunks)packages/types/src/workspace.d.ts
(2 hunks)web/core/components/workspace/sidebar/dropdown-item.tsx
(1 hunks)web/core/components/workspace/sidebar/dropdown.tsx
(4 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/views/workspace/base.py
179-179: Line too long (105 > 88)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-space
🔇 Additional comments (4)
web/core/components/workspace/sidebar/dropdown-item.tsx (1)
10-15
: LGTM! Props interface is well-defined.The props interface clearly defines the required properties with appropriate types.
apiserver/plane/app/views/workspace/base.py (3)
10-10
: LGTM! Improved code readability.The added blank lines help separate import groups, following PEP 8 style guidelines.
Also applies to: 14-14
194-194
: LGTM! Efficient annotation combination.Good optimization by combining both total_issues and role annotations in a single query.
200-200
: LGTM! Enhanced code structure.The added blank lines improve readability by clearly separating the query building from the serialization logic.
Also applies to: 206-206
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/core/components/workspace/sidebar/dropdown.tsx (2)
131-155
: Consider adding ARIA attributes for better accessibility.The workspace list implementation looks good, but could benefit from additional accessibility attributes.
Consider adding these ARIA attributes to improve accessibility:
- <div className="vertical-scrollbar scrollbar-sm flex max-h-96 flex-col items-start justify-start overflow-y-scroll"> + <div + className="vertical-scrollbar scrollbar-sm flex max-h-96 flex-col items-start justify-start overflow-y-scroll" + role="listbox" + aria-label="Workspaces" + >
163-168
: Simplify the Plus icon implementation.The current implementation of the Plus icon container seems overly complex with multiple nested divs and transform styles.
Consider simplifying the implementation:
- <div className="flex w-4 h-4 backdrop-invert rounded-full "> - <Plus - strokeWidth={3} - className="flex-shrink-0 invert h-[10px] w-[10px] text-custom-text-100 m-auto" - /> - </div> + <Plus + className="size-4 flex-shrink-0 text-custom-sidebar-text-200" + strokeWidth={2} + />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/components/workspace/sidebar/dropdown.tsx
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: lint-space
- GitHub Check: lint-admin
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
web/core/components/workspace/sidebar/dropdown.tsx (3)
3-25
: LGTM! Well-organized imports with clear categorization.The import statements are properly organized and the introduction of
SidebarDropdownItem
promotes better component composition.
27-81
: LGTM! Well-structured hooks and handlers.The component demonstrates good practices with:
- Clean organization of hooks
- Proper error handling in sign-out functionality
- Efficient workspace navigation handling
202-265
: LGTM! Well-implemented user profile menu.The implementation demonstrates good practices:
- Proper use of Headless UI components
- Smooth transitions
- Secure access control for admin features
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/core/components/workspace/sidebar/dropdown.tsx (1)
240-240
: Remove redundant boolean conversion.The expression
isUserInstanceAdmin || false
is unnecessary asisUserInstanceAdmin
is already a boolean.- <div className={`pt-2 ${isUserInstanceAdmin || false ? "pb-2" : ""}`}> + <div className={`pt-2 ${isUserInstanceAdmin ? "pb-2" : ""}`}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/core/components/workspace/sidebar/dropdown-item.tsx
(1 hunks)web/core/components/workspace/sidebar/dropdown.tsx
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/core/components/workspace/sidebar/dropdown-item.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
web/core/components/workspace/sidebar/dropdown.tsx (4)
3-25
: LGTM! Import changes align with the refactoring.The updated imports reflect the component's needs after refactoring, particularly the addition of the new
SidebarDropdownItem
component.
39-39
: Remove unusedallowPermissions
import.The
allowPermissions
hook is imported but not used in the component.This aligns with the previous review comment questioning the need for permission checks.
131-155
: LGTM! Improved workspace list implementation.The changes enhance the user experience with:
- Better scrolling behavior using
vertical-scrollbar
- Semantic HTML structure with sticky header
- Clean implementation using the new
SidebarDropdownItem
component
163-194
: LGTM! Enhanced action items with better accessibility.The changes improve the UI with:
- Better contrast for the Plus icon using backdrop inversion
- Well-structured workspace invites section
- Consistent styling across action items
…t-enchanced-workspace-switcher
I tested it locally and noted the following observations:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/core/components/workspace/sidebar/dropdown.tsx (2)
176-184
: Enhance hover state visibility for the Invitations button.The PR feedback mentions that hover states are not sufficiently visible. Consider enhancing the hover state to provide better visual feedback.
Apply this diff to enhance the hover state:
- className="flex items-center gap-2 rounded px-2 py-1 text-sm font-medium text-custom-sidebar-text-200 hover:bg-custom-sidebar-background-80" + className="flex items-center gap-2 rounded px-2 py-1 text-sm font-medium text-custom-sidebar-text-200 hover:bg-custom-sidebar-background-80 hover:text-custom-text-100 transition-colors duration-200"
231-241
: Maintain consistent text colors across menu items.The PR feedback highlights color inconsistencies. The text colors vary between different menu items, which affects visual coherence.
Apply this diff to maintain consistent colors:
- <span className="px-2 text-custom-sidebar-text-200">{currentUser?.email}</span> + <span className="px-2 text-custom-sidebar-text-400">{currentUser?.email}</span> <Link href="/profile"> <Menu.Item as="div"> - <span className="flex w-full items-center gap-2 rounded px-2 py-1 hover:bg-custom-sidebar-background-80"> + <span className="flex w-full items-center gap-2 rounded px-2 py-1 text-custom-sidebar-text-200 hover:bg-custom-sidebar-background-80 hover:text-custom-text-100">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/core/components/workspace/sidebar/dropdown-item.tsx
(1 hunks)web/core/components/workspace/sidebar/dropdown.tsx
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/core/components/workspace/sidebar/dropdown-item.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
web/core/components/workspace/sidebar/dropdown.tsx (2)
3-25
: LGTM! Clean import organization.The imports are well-organized and the new
SidebarDropdownItem
component is properly imported.
80-80
: Address the TODO comment regarding workspace list scroll.This TODO aligns with the reported scrollbar issues in the PR feedback. Consider implementing a proper scrolling solution to prevent the appearance of scrollbars in non-scrollable sections.
Would you like me to propose a solution for the workspace list scrolling implementation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
web/core/components/workspace/sidebar/dropdown.tsx (1)
41-41
: Verify the hardcoded admin flag.The
isUserInstanceAdmin
is hardcoded tofalse
, which could lead to permission-related issues. This should be derived from the user's actual permissions.- const isUserInstanceAdmin = false; + const { isUserInstanceAdmin } = useUser();
🧹 Nitpick comments (2)
web/core/components/workspace/sidebar/dropdown.tsx (2)
173-181
: Add hover state styles for the invites link.The PR feedback mentions missing hover states for the invites option. While there is a hover background color, the text color should also change on hover for better visual feedback.
- className="flex items-center gap-2 rounded px-2 py-1 text-sm font-medium text-custom-sidebar-text-200 hover:bg-custom-sidebar-background-80" + className="flex items-center gap-2 rounded px-2 py-1 text-sm font-medium text-custom-sidebar-text-200 hover:bg-custom-sidebar-background-80 hover:text-custom-sidebar-text-100"
229-229
: Add email truncation in the avatar menu.Similar to the main dropdown, the email in the avatar menu should also be truncated to prevent layout issues with long email addresses.
- <span className="px-2 text-custom-sidebar-text-200">{currentUser?.email}</span> + <span className="px-2 text-custom-sidebar-text-200 truncate block w-full">{currentUser?.email}</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/components/workspace/sidebar/dropdown.tsx
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (1)
web/core/components/workspace/sidebar/dropdown.tsx (1)
142-148
: Verify dropdown item styling in SidebarDropdownItem component.The PR feedback mentions issues with dropdown size and color inconsistencies. While this file handles the container layout, please ensure that the
SidebarDropdownItem
component properly handles:
- Consistent sizing of dropdown elements
- Consistent text and icon colors
- Proper truncation of workspace names
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check the implementation of SidebarDropdownItem for styling issues # Find and display the SidebarDropdownItem implementation ast-grep --pattern 'export const SidebarDropdownItem = $$$'Length of output: 61
I'm not getting enough information from the previous script. I'll run additional searches using rg to locate the definition of SidebarDropdownItem with different patterns. This should help us verify its implementation details regarding styling issues.
🏁 Script executed:
#!/bin/bash # Search for SidebarDropdownItem definitions using multiple patterns in TSX files echo "Searching for function component definition:" rg "function SidebarDropdownItem" -ttsx -A 10 echo "Searching for class component definition:" rg "class SidebarDropdownItem" -ttsx -A 10 echo "Searching for export statement containing SidebarDropdownItem:" rg "export.*SidebarDropdownItem" -ttsx -A 10Length of output: 550
I'm not getting enough information from the previous search. Let's run additional shell scripts to locate the SidebarDropdownItem component by file name and search within TSX files for its definition. This should help us inspect its styling implementation.
🏁 Script executed:
#!/bin/bash # Step 1: Locate any files that contain "SidebarDropdownItem" in the filename echo "Searching for files with 'SidebarDropdownItem' in their name:" fd -HI "SidebarDropdownItem.*\.tsx$" --type file # Step 2: Search inside TSX files for the SidebarDropdownItem definition # We add tsx file type for rg with a glob pattern. echo "Searching within TSX files for 'export' statements related to SidebarDropdownItem:" rg --type-add "tsx:*.tsx" -g "*.tsx" "export .*SidebarDropdownItem" -A 10Length of output: 538
Let's inspect the SidebarDropdownItem component's implementation in detail to verify its styling. We need to review the file’s content – particularly for any styling rules (CSS classes, inline styles, or truncation logic) that address dropdown size, text/icon colors, and name truncation.
🏁 Script executed:
#!/bin/bash echo "Displaying first 150 lines of web/core/components/workspace/sidebar/dropdown-item.tsx" head -n 150 web/core/components/workspace/sidebar/dropdown-item.tsx echo "Displaying last 150 lines of web/core/components/workspace/sidebar/dropdown-item.tsx" tail -n 150 web/core/components/workspace/sidebar/dropdown-item.tsx echo "Searching for occurrences of className attribute for styling clues:" rg "className=" web/core/components/workspace/sidebar/dropdown-item.tsxLength of output: 10876
Dropdown item styling verified.
The SidebarDropdownItem component correctly applies Tailwind classes that ensure:• A full-width clickable item with a defined sub-container width (w-full on Link and w-[80%] for workspace info) for consistent sizing.
• Conditional text and icon color styling—using classes like text-custom-sidebar-text-100, text-custom-text-200, and text-custom-text-300—to manage active and inactive states.
• Proper truncation of workspace names via "truncate text-ellipsis".Overall, the styling implementation appears to address the review feedback regarding dropdown size, color consistency, and text truncation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
web/core/components/workspace/sidebar/dropdown.tsx (1)
41-41
: Fix hardcoded admin flag.The
isUserInstanceAdmin
flag is hardcoded tofalse
, which could prevent legitimate instance admins from accessing admin features.Use the commented-out
isUserInstanceAdmin
from theuseUser
hook:- const isUserInstanceAdmin = false; + const { isUserInstanceAdmin } = useUser();
🧹 Nitpick comments (1)
web/core/components/workspace/sidebar/dropdown.tsx (1)
229-229
: Add email truncation to profile menu.The email display in the profile menu lacks truncation, which could cause layout issues with long email addresses.
Apply this diff to add email truncation:
- <span className="px-2 text-custom-sidebar-text-200">{currentUser?.email}</span> + <span className="px-2 text-custom-sidebar-text-200 truncate">{currentUser?.email}</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/components/workspace/sidebar/dropdown.tsx
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
web/core/components/workspace/sidebar/dropdown.tsx (3)
129-129
: Fix the workspace list scroll issue.The TODO comment indicates a known issue with the workspace list scroll. The current implementation uses
overflow-y-scroll
which shows the scrollbar even when not needed, as reported in the PR feedback.
130-132
: LGTM! Email truncation implemented.The email display now includes proper truncation with the
truncate
class, addressing the reported issue in the PR feedback.
173-181
: LGTM! Hover states implemented correctly.The menu items now have proper hover states using the
hover:bg-custom-sidebar-background-80
class, addressing the reported issue in the PR feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
web/core/components/workspace/sidebar/dropdown.tsx (3)
130-132
: Improve email truncation implementation.While truncation is added, the layout could be improved to prevent potential overflow issues.
- <span className="rounded-md px-4 sticky top-0 z-[21] h-full w-full bg-custom-sidebar-background-100 pb-1 pt-3 text-sm font-medium text-custom-text-400 truncate flex-shrink-0"> + <span className="rounded-md px-4 sticky top-0 z-[21] w-full bg-custom-sidebar-background-100 pb-1 pt-3 text-sm font-medium text-custom-text-400"> + <span className="block truncate">{currentUser?.email}</span> + </span>
135-141
: Optimize workspace list sorting.The current implementation creates a new array on each render. Consider memoizing the sorted list.
+ const sortedWorkspacesList = useMemo(() => + activeWorkspace + ? [activeWorkspace, ...workspacesList.filter((workspace) => workspace.id !== activeWorkspace?.id)] + : workspacesList, + [activeWorkspace, workspacesList] + ); + {(activeWorkspace - ? [ - activeWorkspace, - ...workspacesList.filter((workspace) => workspace.id !== activeWorkspace?.id), - ] - : workspacesList + sortedWorkspacesList ).map((workspace) => (
173-181
: Enhance hover state consistency.As reported in PR feedback, the hover states for menu items should be more consistent.
- <Link href="/invitations" className="w-full" onClick={handleItemClick}> + <Link + href="/invitations" + className="w-full transition-colors duration-300" + onClick={handleItemClick} + > <Menu.Item as="div" - className="flex items-center gap-2 rounded px-2 py-1 text-sm font-medium text-custom-sidebar-text-200 hover:bg-custom-sidebar-background-80" + className="flex items-center gap-2 rounded px-2 py-1 text-sm font-medium text-custom-sidebar-text-200 hover:bg-custom-sidebar-background-80 transition-colors duration-300" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/core/components/workspace/sidebar/dropdown-item.tsx
(1 hunks)web/core/components/workspace/sidebar/dropdown.tsx
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/core/components/workspace/sidebar/dropdown-item.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
web/core/components/workspace/sidebar/dropdown.tsx (2)
129-129
: Fix workspace list scroll behavior.The workspace list still uses
overflow-y-scroll
which shows scrollbar even when not needed, as reported in PR feedback.
41-41
: Verify the hardcodedisUserInstanceAdmin
flag.The
isUserInstanceAdmin
flag is hardcoded tofalse
, which might prevent instance admins from accessing admin features.
Description
This PR implements the new workspace switcher
References
[WEB-3292]
Summary by CodeRabbit
Summary by CodeRabbit
New Features
SubscriptionPill
component for future subscription-related features.Refactor
SidebarDropdown
component to utilize the newSidebarDropdownItem
for better structure and navigation.