refactor: favorites sidebar implementation #6716
Conversation
WalkthroughThis pull request introduces new functionality related to sidebar favorites, including new files and export statements in both the CE and EE directories. It defines constants and hooks for managing favorite item icons and links. The helper functions for retrieving icons and generating links have been refactored to utilize dynamic mappings, and hooks have been updated to provide additional details when processing favorite items. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Component
participant DetailsHook as useFavoriteItemDetails
participant AddDetails as useAdditionalFavoriteItemDetails
participant Helper as Helper Functions
UI->>DetailsHook: Request favorite item details
DetailsHook->>AddDetails: Call getAdditionalFavoriteItemDetails(workspaceSlug, favorite)
AddDetails->>Helper: Retrieve icon via getFavoriteItemIcon
Helper-->>AddDetails: Return matching icon (or default)
AddDetails-->>DetailsHook: Return additional details (icon, title)
DetailsHook-->>UI: Provide enhanced favorite item details (including link)
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (4)
✨ Finishing Touches
🪧 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: 1
🧹 Nitpick comments (4)
web/ce/hooks/use-additional-favorite-item-details.ts (2)
6-26: Hook implementation could be enhancedThe hook creates a new function
getAdditionalFavoriteItemDetailsthat currently only handles the default case with no specific cases in the switch statement. This suggests it's designed for extensibility, but the current implementation seems incomplete.Consider either:
- Adding specific cases to the switch statement for different entity types, or
- Adding a comment explaining that specific cases will be handled in the future or by EE implementation.
Additionally, since the function doesn't currently use the
_workspaceSlugparameter (note the underscore prefix indicating it's unused), consider removing it until needed or documenting why it's there for future use.
14-19: Empty switch statement could be simplifiedThe switch statement currently only has a default case with no specific cases for different entity types.
Since there are no specific cases handled, you could simplify by removing the switch statement:
- switch (favoriteItemEntityType) { - default: - itemTitle = favoriteItemName; - itemIcon = getFavoriteItemIcon(favoriteItemEntityType); - break; - } + itemTitle = favoriteItemName; + itemIcon = getFavoriteItemIcon(favoriteItemEntityType);Or add a comment indicating that specific entity type handling will be added in the future.
web/ce/constants/sidebar-favorites.ts (1)
15-41: FAVORITE_ITEM_LINKS implementation looks good but could be improvedThe
FAVORITE_ITEM_LINKSconstant provides a mapping for generating links to favorite items based on their type. This is a good abstraction that supports the code separation objective.A few suggestions for improvement:
- The
projectitem'sgetLinkfunction doesn't use thefavoriteparameter, unlike the others. Consider unifying the pattern:project: { itemLevel: "project", - getLink: () => `issues`, + getLink: (_favorite) => `issues`, },
- Consider adding typing to the function parameters for consistency and better type checking:
- getLink: (favorite) => `cycles/${favorite.entity_identifier}`, + getLink: (favorite: IFavorite) => `cycles/${favorite.entity_identifier}`,
- If
folderexists inFAVORITE_ITEM_ICONS, consider adding it toFAVORITE_ITEM_LINKSas well for completeness.web/core/components/workspace/sidebar/favorites/favorite-items/common/helper.tsx (1)
14-27: Consider consolidating repeated icon usage to reduce duplication.You render the same
<Icon>component in both the hover-visible and hover-hidden blocks. Although this is a valid pattern for showing different states on hover, you might consider encapsulating the shared<Icon>rendering into a helper function or component to reduce redundancy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
web/ce/constants/index.ts(1 hunks)web/ce/constants/sidebar-favorites.ts(1 hunks)web/ce/hooks/use-additional-favorite-item-details.ts(1 hunks)web/core/components/workspace/sidebar/favorites/favorite-items/common/helper.tsx(1 hunks)web/core/hooks/use-favorite-item-details.tsx(3 hunks)web/ee/constants/index.ts(1 hunks)web/ee/constants/sidebar-favorites.ts(1 hunks)web/ee/hooks/use-additional-favorite-item-details.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- web/ee/constants/sidebar-favorites.ts
- web/ee/constants/index.ts
- web/ee/hooks/use-additional-favorite-item-details.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (10)
web/ce/constants/index.ts (1)
5-5: New export for sidebar-favorites constantsThis change adds an export for the newly created sidebar-favorites constants, making them accessible throughout the application. The addition aligns with the PR objective of separating code related to user favorites.
web/ce/hooks/use-additional-favorite-item-details.ts (1)
1-5: Imports look goodThe imports are properly organized with clear separation between Plane types and components.
web/ce/constants/sidebar-favorites.ts (2)
1-5: Imports are well-organizedGood job on organizing imports with clear separation between third-party libraries and internal Plane imports.
6-13: Constants for favorite item icons defined clearlyThe
FAVORITE_ITEM_ICONSconstant provides a clear mapping between entity types and their respective icons, which supports the code separation objective.web/core/hooks/use-favorite-item-details.tsx (4)
14-14: New import for additional favorite item detailsGood addition of the import for the new hook that provides additional favorite item details.
17-21: Improved object destructuringThe change from individual assignments to object destructuring improves code readability and maintainability.
28-29: Additional details hook integrationGood addition of the hook that provides additional favorite item details, enhancing the modularity of the codebase.
65-69: Refactored default case with additional details hookThe default case now uses the
getAdditionalFavoriteItemDetailsfunction from the new hook, which is a good abstraction that supports the code separation objective.web/core/components/workspace/sidebar/favorites/favorite-items/common/helper.tsx (2)
11-12: Great fallback for undefined icon types.Using
FAVORITE_ITEM_ICONS[type] || FileTextensures there’s always a valid icon even if the type isn’t recognized. Nicely done!
33-39:Details
❓ Verification inconclusive
Verify that
favorite.project_idis defined for project-level entities.When
entityLinkDetails.itemLevelis "project", the code appendsfavorite.project_idin the URL. Consider verifying thatfavorite.project_idis indeed set for all project-level items:
🏁 Script executed:
#!/bin/bash # Description: Search for all references to `favorite.project_id` # to check if the code guarantees it is never null or undefined for project-level items rg -A 5 "favorite.project_id"Length of output: 1051
Action Required: Confirm
favorite.project_idPresence for Project-Level Entities
- In
web/core/components/workspace/sidebar/favorites/favorite-items/common/helper.tsx, the URL for project-level items is built usingfavorite.project_idwithout an explicit fallback.- However, in
web/core/hooks/use-favorite-item-details.tsxa fallback (favorite.project_id ?? "") is used when invokinggetProjectById, suggesting that there may be cases whenfavorite.project_idis undefined.- Please verify that for every project-level entity the
favorite.project_idis reliably defined. If there’s any possibility it might be undefined, consider applying a consistent fallback or an explicit validation in the URL generation logic.
web/core/components/workspace/sidebar/favorites/favorite-items/common/helper.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/ce/constants/sidebar-favorites.ts (1)
6-13: Consider adding type safety for the keys in FAVORITE_ITEM_ICONSThe record definition could benefit from using a union type for the keys to ensure only valid types are used, making the code more robust against typos and future modifications.
-export const FAVORITE_ITEM_ICONS: Record<string, React.FC<ISvgIcons> | LucideIcon> = { +type FavoriteItemType = "page" | "project" | "view" | "module" | "cycle" | "folder"; +export const FAVORITE_ITEM_ICONS: Record<FavoriteItemType, React.FC<ISvgIcons> | LucideIcon> = { page: FileText, project: Briefcase, view: Layers, module: DiceIcon, cycle: ContrastIcon, folder: FavoriteFolderIcon, };web/ce/hooks/use-additional-favorite-item-details.ts (1)
7-21: Simplify the switch statement and clarify unused parameterThe switch statement only has a default case, which makes it unnecessarily complex. Also, the
_workspaceSlugparameter is prefixed with an underscore suggesting it's not used, but there's no indication of its intended purpose.- const getAdditionalFavoriteItemDetails = (_workspaceSlug: string, favorite: IFavorite) => { + const getAdditionalFavoriteItemDetails = (workspaceSlug: string, favorite: IFavorite) => { const { entity_type: favoriteItemEntityType } = favorite; const favoriteItemName = favorite?.entity_data?.name || favorite?.name; let itemIcon; let itemTitle; - switch (favoriteItemEntityType) { - default: - itemTitle = favoriteItemName; - itemIcon = getFavoriteItemIcon(favoriteItemEntityType); - break; - } + // Set default values (can be expanded with specific cases in the future) + itemTitle = favoriteItemName; + itemIcon = getFavoriteItemIcon(favoriteItemEntityType); + return { itemIcon, itemTitle }; };The structure is likely in place to allow for future expansion with specific entity type handling, but for now, it can be simplified.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
web/ce/constants/index.ts(1 hunks)web/ce/constants/sidebar-favorites.ts(1 hunks)web/ce/hooks/use-additional-favorite-item-details.ts(1 hunks)web/core/components/workspace/sidebar/favorites/favorite-items/common/helper.tsx(1 hunks)web/core/hooks/use-favorite-item-details.tsx(3 hunks)web/ee/constants/index.ts(1 hunks)web/ee/constants/sidebar-favorites.ts(1 hunks)web/ee/hooks/use-additional-favorite-item-details.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- web/ee/constants/index.ts
- web/ee/hooks/use-additional-favorite-item-details.ts
- web/ee/constants/sidebar-favorites.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (8)
web/ce/constants/index.ts (1)
5-5: Clean export addition. Looks good!The code adds a new export statement for the sidebar-favorites module, making the associated constants available through the main index file. This follows the established pattern of the other exports in the file.
web/core/hooks/use-favorite-item-details.tsx (3)
17-21: Good refactoring to use destructuringThe change to use destructuring for extracting properties from the favorite object makes the code cleaner and more maintainable. This is a good improvement.
28-29: Clean integration of the additional details hookAdding the hook to handle additional favorite item details follows good separation of concerns and makes the code more extensible.
65-69: Well-structured default caseThe default case is now properly enclosed in braces and uses the new hook to get additional details. This provides a clear extension point for handling new favorite item types in the future.
web/core/components/workspace/sidebar/favorites/favorite-items/common/helper.tsx (4)
2-9: Good job on improved import organization and code separationThe separation of constants into dedicated imports from
@/plane-web/constantsaligns perfectly with the PR objective of enhancing code scalability through separation of favorite-related code.
11-13: Well-implemented dynamic icon resolutionGood implementation of dynamic icon lookup with a fallback. Using
FAVORITE_ITEM_ICONS[type] || FileTextprovides both flexibility and safety when handling unknown types.
14-27: Clean UI implementation with hover statesThe component structure with conditional group-hover states creates a nice UI experience. The conditional logo rendering based on
logo.in_useis also well implemented.
30-39: Improved link generation with clear level-based structureThe refactored link generation using the
itemLevelproperty significantly improves code readability and maintainability. The three distinct paths (workspace, project, and default) are clearly handled with appropriate routing patterns.
sriramveeraghanta
changed the title
* chore: code separation for favorites * chore: error handling
Description
This PR separates user favorites code for scalability.
Type of Change
Summary by CodeRabbit
New Features
Refactor