[WEB-2554] improvement: dashboard sidebar list items.#5970
[WEB-2554] improvement: dashboard sidebar list items.#5970SatishGandham merged 1 commit intopreviewfrom
Conversation
WalkthroughThe changes in this pull request involve the introduction of new type definitions and helper functions related to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (4)
web/ce/constants/dashboard.ts (2)
13-20: Consider enhancing type safety for function parameters.While the type definition is good, we can improve type safety further:
export type TSidebarUserMenuItems = { key: TSidebarUserMenuItemKeys; label: string; href: string; access: EUserPermissions[]; - highlight: (pathname: string, baseUrl: string, options?: TLinkOptions) => boolean; + highlight: { + (pathname: `/${string}`, baseUrl: `/${string}`, options?: TLinkOptions): boolean; + }; - Icon: React.FC<Props>; + Icon: React.FC<Props & { size?: number }>; };This change:
- Makes pathname and baseUrl more specific by ensuring they start with "/"
- Makes the Icon type more explicit about the size prop that icon components typically accept
Line range hint
22-58: Consider simplifying highlight functions.The highlight functions could be simplified to improve readability and reduce complexity.
export const SIDEBAR_USER_MENU_ITEMS: TSidebarUserMenuItems[] = [ { key: "home", label: "Home", href: ``, access: [EUserPermissions.ADMIN, EUserPermissions.MEMBER, EUserPermissions.GUEST], - highlight: (pathname: string, baseUrl: string) => pathname === `${baseUrl}/`, + highlight: (pathname, baseUrl) => pathname === baseUrl + "/", Icon: Home, }, // ... other items ... { key: "your-work", label: "Your work", href: "/profile", access: [EUserPermissions.ADMIN, EUserPermissions.MEMBER], - highlight: (pathname: string, baseUrl: string, options?: TLinkOptions) => - options?.userId ? pathname.includes(`${baseUrl}/profile/${options?.userId}`) : false, + highlight: (pathname, baseUrl, options) => + Boolean(options?.userId && pathname.includes(`${baseUrl}/profile/${options.userId}`)), Icon: UserActivityIcon, }, // ... remaining items ... ];These changes:
- Simplify string concatenation
- Make the boolean logic more explicit
- Remove redundant type annotations (they're inferred from the type definition)
web/core/components/workspace/sidebar/user-menu.tsx (2)
66-66: Consider enhancing the feature flag implementation.While the feature flag check is correctly implemented, consider these improvements:
- Add error handling for invalid feature keys
- Add debug logging to help troubleshoot feature flag issues in production
- if (!isUserFeatureEnabled(link.key)) return null; + try { + if (!isUserFeatureEnabled(link.key)) { + console.debug(`Feature ${link.key} is disabled for the current user`); + return null; + } + } catch (error) { + console.error(`Error checking feature flag for ${link.key}:`, error); + return null; + }
65-67: Consider optimizing conditional checks and component structure.The current implementation has multiple separate conditions for rendering menu items. Consider:
- Combining the draft check and feature flag check for better maintainability
- Extracting the menu item rendering logic into a separate component
Example refactor:
const shouldRenderMenuItem = (link: TSidebarUserMenuItems) => { if (link.key === "drafts" && draftIssueCount === 0) return false; return isUserFeatureEnabled(link.key); }; const SidebarMenuItem = ({ link, ...props }) => { if (!shouldRenderMenuItem(link)) return null; // ... rest of the rendering logic };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
web/ce/constants/dashboard.ts(1 hunks)web/ce/helpers/dashboard.helper.ts(1 hunks)web/ce/types/dashboard.ts(1 hunks)web/core/components/workspace/sidebar/user-menu.tsx(2 hunks)web/ee/helpers/dashboard.helper.ts(1 hunks)web/ee/types/dashboard.ts(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- web/ce/helpers/dashboard.helper.ts
- web/ce/types/dashboard.ts
- web/ee/helpers/dashboard.helper.ts
- web/ee/types/dashboard.ts
🔇 Additional comments (5)
web/ce/constants/dashboard.ts (3)
10-11: LGTM! Clean import organization.
The import of TSidebarUserMenuItemKeys is properly organized with other type imports.
22-22: LGTM! Type safety improvement.
The constant now uses the new TSidebarUserMenuItems type, improving type safety and maintainability.
Line range hint 13-58: Verify type consistency across the codebase.
Let's ensure the new type is used consistently and all menu item keys are valid.
✅ Verification successful
Let me generate another script to gather more information since the previous script had issues with the file type recognition.
Type and menu items are consistent across the codebase
The verification shows:
- The type
TSidebarUserMenuItemsis properly defined and used in the dashboard constants - All menu item keys match the
TSidebarUserMenuItemKeystype definition - The
SIDEBAR_USER_MENU_ITEMSis correctly imported and used in the sidebar user menu component - The menu items are rendered in a map operation in
web/core/components/workspace/sidebar/user-menu.tsx
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify type usage and key consistency
# Test 1: Check if TSidebarUserMenuItems is imported and used in other files
echo "Checking TSidebarUserMenuItems usage:"
rg "TSidebarUserMenuItems" --type ts --type tsx
# Test 2: Find all sidebar menu key references to ensure they match TSidebarUserMenuItemKeys
echo -e "\nChecking sidebar menu key references:"
rg '"(home|your-work|notifications|drafts)"' --type ts --type tsx
# Test 3: Verify TSidebarUserMenuItemKeys type definition
echo -e "\nChecking TSidebarUserMenuItemKeys definition:"
rg "type TSidebarUserMenuItemKeys" -A 10 --type ts
Length of output: 623
Script:
#!/bin/bash
# Search for type usage and menu references with correct file extensions
echo "Checking TSidebarUserMenuItems usage:"
rg "TSidebarUserMenuItems"
echo -e "\nChecking sidebar menu key references:"
rg "key: \"(home|your-work|notifications|drafts)\""
echo -e "\nChecking imports of dashboard constants:"
rg "from ['\"].*constants/dashboard"
echo -e "\nChecking usage of SIDEBAR_USER_MENU_ITEMS:"
rg "SIDEBAR_USER_MENU_ITEMS"
Length of output: 3211
web/core/components/workspace/sidebar/user-menu.tsx (2)
18-22: LGTM! Well-organized imports.
The new imports are properly organized and grouped with clear comments, maintaining consistency with the existing import structure.
18-22: Verify the integration with imported modules.
Let's verify the proper implementation of the imported modules.
✅ Verification successful
Integration with imported modules is properly implemented
The verification confirms that:
SIDEBAR_USER_MENU_ITEMSis correctly defined inweb/ce/constants/dashboard.tsisUserFeatureEnabledhelper is implemented inweb/ce/helpers/dashboard.helper.ts- Both imports are actively used in the user-menu component for rendering menu items and feature checks
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of imported modules
# Check the SIDEBAR_USER_MENU_ITEMS constant definition
echo "Checking SIDEBAR_USER_MENU_ITEMS constant..."
rg -t typescript "export const SIDEBAR_USER_MENU_ITEMS.*=.*" --context 3
# Check the isUserFeatureEnabled helper implementation
echo "Checking isUserFeatureEnabled helper..."
rg -t typescript "export .* isUserFeatureEnabled.*" --context 3
Length of output: 398
Script:
#!/bin/bash
# Let's try without typescript file type flag and use more specific paths
# Check the SIDEBAR_USER_MENU_ITEMS constant definition
echo "Checking SIDEBAR_USER_MENU_ITEMS constant..."
rg "export const SIDEBAR_USER_MENU_ITEMS" -g "*.ts" -g "*.tsx" --context 3
# Check the isUserFeatureEnabled helper implementation
echo "Checking isUserFeatureEnabled helper..."
rg "export .* isUserFeatureEnabled" -g "*.ts" -g "*.tsx" --context 3
# Also check for any usage of these imports
echo "Checking usage of imports..."
rg "SIDEBAR_USER_MENU_ITEMS|isUserFeatureEnabled" -g "*.ts" -g "*.tsx" --context 2
Length of output: 3170
2 participants
Summary by CodeRabbit