Organise icons and misc tidy up #279
Conversation
…ucide icons also fix some minor issues with the mobile menu, removed some rarely/never used components
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis pull request introduces significant changes across multiple components, primarily focusing on the restructuring of icon imports and the introduction of new styled icon components. Key modifications include the removal of several existing icon components and their replacement with new styled counterparts sourced from the Changes
Possibly related PRs
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: 1
🧹 Outside diff range and nitpick comments (11)
web/src/components/ui/icons/Bookmark.tsx (1)
5-8: Consider adding TypeScript types and documentation.The styled components are well-implemented and follow a consistent pattern. The
fill: "current"onBookmarkSavedIconensures proper color inheritance.Consider adding:
- TypeScript interface for component props
- JSDoc comments explaining the usage and differences between the two components
+/** BookmarkSavedIcon is a filled variant of the bookmark icon that inherits its color */ export const BookmarkSavedIcon = styled(Bookmark, { base: { fill: "current" }, }); + +/** BookmarkIcon is the outline variant of the bookmark icon */ export const BookmarkIcon = styled(Bookmark);web/src/components/site/Navigation/Anchors/Admin.tsx (1)
1-1: LGTM! Icon import aligns with consolidation strategy.The migration to a centralized icon import from
@/components/ui/iconsaligns well with the PR's objective of consolidating icons and improving maintainability.web/src/components/site/Navigation/LibraryNavigationTree/LibraryNavigationTree.tsx (1)
6-6: Consider using barrel files to simplify deep imports.The new import path uses multiple parent directory traversals (
../../../), which can be fragile and harder to maintain. Consider creating an index.ts barrel file in the library directory to re-export components, allowing for shorter and more maintainable imports.Example barrel file structure:
// web/src/components/library/index.ts export { CreatePageAction } from './CreatePage'; // This file import { CreatePageAction } from '../../../library';web/src/components/notifications/NotificationsTrigger.tsx (1)
3-3: LGTM! Import changes align with icon consolidation objective.The migration from @heroicons/react to local components follows the PR's goal of consolidating icons. However, the MenuItem import appears unused after the removal of NotificationsMenuItem.
Consider removing the unused MenuItem import:
-import { AnchorProps, MenuItem } from "../site/Navigation/Anchors/Anchor"; +import { AnchorProps } from "../site/Navigation/Anchors/Anchor";Also applies to: 6-6
web/src/components/role/PermissionList.tsx (1)
40-48: Consider enhancing the empty state message and stylingThe addition of empty state handling is a good improvement to the UX. However, consider these enhancements:
- Make the empty state message more informative
- Use styled-system components consistently instead of HTML
ptag- <Box pl="4"> - <p>No permissions.</p> - </Box> + <Box pl="4" color="gray.500"> + <Box as="span">No permissions assigned to this role yet.</Box> + </Box>web/src/components/site/Onboarding/Checklist/ChecklistItem.tsx (1)
44-49: Consider adjusting icon size to match container.The icon's dimensions (width="8", height="8") are larger than its parent Circle component's size="7". This might cause the icon to overflow its container.
Consider adjusting the icon size to match its container:
<CheckCircleIcon - width="8" - height="8" + width="7" + height="7" color="green.10" fill="green.5" />web/src/components/role/RoleEdit/RoleEditModal.tsx (1)
93-95: Consider i18n and component standardizationThe IconButton implementation looks good, but consider these improvements:
- The "Create role" text should be internationalized for better localization support
- Consider moving the padding configuration (
px="2") to the IconButton component if this is a common pattern- <IconButton px="2" variant="subtle" size="xs" onClick={handleCreate}> - <CreateIcon /> Create role + <IconButton variant="subtle" size="xs" onClick={handleCreate}> + <CreateIcon /> {t('actions.createRole')}web/src/components/site/Navigation/MobileCommandBar/MobileCommandBar.tsx (2)
7-7: Remove unused importMemberBadgeThe
MemberBadgecomponent is imported but not used anywhere in this file.-import { MemberBadge } from "@/components/member/MemberBadge/MemberBadge";
103-114: Consider moving ExpandTrigger to a separate fileWhile the implementation is good, moving this component to a dedicated file (e.g.,
ExpandTrigger.tsx) would improve maintainability and follow the project's component organization pattern.// New file: web/src/components/site/Navigation/MobileCommandBar/ExpandTrigger.tsx +import { ButtonProps } from "@/components/ui/button"; +import { IconButton } from "@/components/ui/icon-button"; +import { MenuIcon } from "@/components/ui/icons/Menu"; + +export function ExpandTrigger(props: ButtonProps) { + return ( + <IconButton + title="Main navigation menu" + variant="ghost" + size="sm" + {...props} + > + <MenuIcon /> + </IconButton> + ); +}web/src/components/library/Breadcrumbs.tsx (1)
Line range hint
1-124: Consider extracting breadcrumb item into a separate component.While reviewing the import changes, I noticed the component could benefit from extracting the repeated breadcrumb item logic into a separate component. This would improve maintainability and reduce code duplication.
Consider creating a
BreadcrumbItemcomponent:type BreadcrumbItemProps = { path: string; isCurrent: boolean; visibility?: Visibility; paths: string[]; }; const BreadcrumbItem = ({ path, isCurrent, visibility, paths }: BreadcrumbItemProps) => ( <LinkButton size="xs" variant="subtle" flexShrink="0" minW="min" borderColor={ isCurrent && visibility === "published" ? "white" : visibility === "draft" ? "accent" : "blue.8" } borderStyle={isCurrent && visibility !== "published" ? "dashed" : "none"} borderWidth={isCurrent && visibility !== "published" ? "thin" : "none"} href={`/l/${joinLibraryPath(paths, path)}`} > {path}{" "} {isCurrent && visibility && visibility !== "published" && ( <span>({visibility})</span> )} </LinkButton> );web/src/components/library/LibraryPageTree/LibraryPageTree.tsx (1)
Line range hint
214-244: Consider migrating custom SVG icons to Lucide.Since this PR aims to migrate to Lucide icons, consider replacing the custom SVG implementations of
ChevronRightIconandBulletIconwith their Lucide equivalents:
ChevronRightIcon→import { ChevronRight } from "lucide-react"BulletIcon→import { Circle } from "lucide-react"-const ChevronRightIcon = () => ( - <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"> - <title>Chevron Right Icon</title> - <path - fill="none" - stroke="currentColor" - strokeLinecap="round" - strokeLinejoin="round" - strokeWidth="2" - d="m9 18l6-6l-6-6" - /> - </svg> -); - -const BulletIcon = () => ( - <svg - xmlns="http://www.w3.org/2000/svg" - width="16" - height="16" - viewBox="0 0 24 24" - fill={token("colors.fg.muted")} - > - <circle cx="12.1" cy="12.1" r="2.5" /> - </svg> -); +import { ChevronRight, Circle } from "lucide-react"; + +const ChevronRightIcon = ChevronRight; +const BulletIcon = () => <Circle size={16} fill={token("colors.fg.muted")} />;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (70)
web/src/components/form/ColourInput/ColourInput.tsx(2 hunks)web/src/components/graphics/CheckCircle.tsx(0 hunks)web/src/components/graphics/CrownIcon.tsx(0 hunks)web/src/components/graphics/DragHandleIcon.tsx(0 hunks)web/src/components/graphics/LoginIcon.tsx(0 hunks)web/src/components/graphics/LogoutIcon.tsx(0 hunks)web/src/components/graphics/Mailbox.tsx(0 hunks)web/src/components/graphics/SendIcon.tsx(0 hunks)web/src/components/graphics/SpeechPlusIcon.tsx(0 hunks)web/src/components/library/Breadcrumbs.tsx(1 hunks)web/src/components/library/CreatePage.tsx(2 hunks)web/src/components/library/Header.tsx(0 hunks)web/src/components/library/LibraryPageTree/LibraryPageTree.tsx(1 hunks)web/src/components/notifications/NotificationsMenu.tsx(2 hunks)web/src/components/notifications/NotificationsTrigger.tsx(1 hunks)web/src/components/role/PermissionList.tsx(2 hunks)web/src/components/role/RoleEdit/RoleEditModal.tsx(2 hunks)web/src/components/site/Action/Add.tsx(0 hunks)web/src/components/site/Action/Back.tsx(0 hunks)web/src/components/site/Action/Bookmark.tsx(1 hunks)web/src/components/site/Action/Cancel.tsx(2 hunks)web/src/components/site/Action/Close.tsx(1 hunks)web/src/components/site/Action/Edit.tsx(1 hunks)web/src/components/site/Action/More.tsx(1 hunks)web/src/components/site/Action/Save.tsx(1 hunks)web/src/components/site/Action/Send.tsx(0 hunks)web/src/components/site/Avatar/Avatar.tsx(0 hunks)web/src/components/site/Avatar/useAvatar.ts(0 hunks)web/src/components/site/InfoTip.tsx(1 hunks)web/src/components/site/Navigation/AccountMenu/AccountMenu.tsx(2 hunks)web/src/components/site/Navigation/Actions/Menu.tsx(0 hunks)web/src/components/site/Navigation/Anchors/Admin.tsx(3 hunks)web/src/components/site/Navigation/Anchors/Compose.tsx(1 hunks)web/src/components/site/Navigation/Anchors/Drafts.tsx(1 hunks)web/src/components/site/Navigation/Anchors/Home.tsx(1 hunks)web/src/components/site/Navigation/Anchors/Library.tsx(2 hunks)web/src/components/site/Navigation/Anchors/Logout.tsx(3 hunks)web/src/components/site/Navigation/Anchors/Members.tsx(2 hunks)web/src/components/site/Navigation/Anchors/Notifications.tsx(1 hunks)web/src/components/site/Navigation/Anchors/Profile.tsx(3 hunks)web/src/components/site/Navigation/Anchors/Queue.tsx(2 hunks)web/src/components/site/Navigation/Anchors/Settings.tsx(3 hunks)web/src/components/site/Navigation/LibraryNavigationTree/LibraryNavigationTree.tsx(1 hunks)web/src/components/site/Navigation/MobileCommandBar/MobileCommandBar.tsx(2 hunks)web/src/components/site/Onboarding/Checklist/ChecklistItem.tsx(2 hunks)web/src/components/site/ProfilePill/Handle.tsx(0 hunks)web/src/components/site/ProfilePill/ProfilePill.tsx(0 hunks)web/src/components/site/Toolpill/Toolpill.tsx(1 hunks)web/src/components/thread/ReplyBox/ReplyBox.tsx(2 hunks)web/src/components/ui/icons/Admin.tsx(1 hunks)web/src/components/ui/icons/Bookmark.tsx(1 hunks)web/src/components/ui/icons/Cancel.tsx(1 hunks)web/src/components/ui/icons/CheckCircle.tsx(1 hunks)web/src/components/ui/icons/Close.tsx(1 hunks)web/src/components/ui/icons/Create.tsx(1 hunks)web/src/components/ui/icons/Draft.tsx(1 hunks)web/src/components/ui/icons/DragHandle.tsx(1 hunks)web/src/components/ui/icons/Edit.tsx(1 hunks)web/src/components/ui/icons/Home.tsx(1 hunks)web/src/components/ui/icons/Info.tsx(1 hunks)web/src/components/ui/icons/Library.tsx(1 hunks)web/src/components/ui/icons/Logout.tsx(1 hunks)web/src/components/ui/icons/Members.tsx(1 hunks)web/src/components/ui/icons/Menu.tsx(1 hunks)web/src/components/ui/icons/More.tsx(1 hunks)web/src/components/ui/icons/Notification.tsx(1 hunks)web/src/components/ui/icons/Profile.tsx(1 hunks)web/src/components/ui/icons/Queue.tsx(1 hunks)web/src/components/ui/icons/Save.tsx(1 hunks)web/src/components/ui/icons/Settings.tsx(1 hunks)
💤 Files with no reviewable changes (17)
- web/src/components/graphics/CheckCircle.tsx
- web/src/components/graphics/CrownIcon.tsx
- web/src/components/graphics/DragHandleIcon.tsx
- web/src/components/graphics/LoginIcon.tsx
- web/src/components/graphics/LogoutIcon.tsx
- web/src/components/graphics/Mailbox.tsx
- web/src/components/graphics/SendIcon.tsx
- web/src/components/graphics/SpeechPlusIcon.tsx
- web/src/components/library/Header.tsx
- web/src/components/site/Action/Add.tsx
- web/src/components/site/Action/Back.tsx
- web/src/components/site/Action/Send.tsx
- web/src/components/site/Avatar/Avatar.tsx
- web/src/components/site/Avatar/useAvatar.ts
- web/src/components/site/Navigation/Actions/Menu.tsx
- web/src/components/site/ProfilePill/Handle.tsx
- web/src/components/site/ProfilePill/ProfilePill.tsx
✅ Files skipped from review due to trivial changes (22)
- web/src/components/site/Navigation/Anchors/Compose.tsx
- web/src/components/site/Toolpill/Toolpill.tsx
- web/src/components/ui/icons/Admin.tsx
- web/src/components/ui/icons/Cancel.tsx
- web/src/components/ui/icons/CheckCircle.tsx
- web/src/components/ui/icons/Close.tsx
- web/src/components/ui/icons/Create.tsx
- web/src/components/ui/icons/Draft.tsx
- web/src/components/ui/icons/DragHandle.tsx
- web/src/components/ui/icons/Edit.tsx
- web/src/components/ui/icons/Home.tsx
- web/src/components/ui/icons/Info.tsx
- web/src/components/ui/icons/Library.tsx
- web/src/components/ui/icons/Logout.tsx
- web/src/components/ui/icons/Members.tsx
- web/src/components/ui/icons/Menu.tsx
- web/src/components/ui/icons/More.tsx
- web/src/components/ui/icons/Notification.tsx
- web/src/components/ui/icons/Profile.tsx
- web/src/components/ui/icons/Queue.tsx
- web/src/components/ui/icons/Save.tsx
- web/src/components/ui/icons/Settings.tsx
🔇 Additional comments (47)
web/src/components/ui/icons/Bookmark.tsx (1)
1-3: LGTM! Imports are well-organized.
The imports follow good practices with external packages first, followed by internal imports. The use of Lucide icons aligns with the PR's objective of consolidating icon usage.
web/src/components/site/Action/More.tsx (1)
3-3: LGTM! Icon migration looks good.
The change aligns with the PR objective of consolidating icons and migrating to centralized icon management.
Let's verify this is part of the broader icon consolidation effort:
Also applies to: 8-8
web/src/components/site/Action/Close.tsx (2)
1-3: LGTM! Clean and well-organized imports.
The imports are properly scoped and follow a consistent pattern, aligning with the PR's objective of consolidating icons into re-exports.
7-9: LGTM! Semantically improved implementation.
The switch to IconButton with ghost variant is more appropriate for a close action. The component maintains its flexibility through props spreading while providing better semantic meaning.
Let's verify the consistent usage of this component across the codebase:
✅ Verification successful
Component usage is consistent across the codebase
The verification shows:
- No remaining
@heroicons/XMarkIconimports found - All
CloseActionusages are consistent:- Used in mobile command bar with
size="sm"prop - Used in modal drawer with default size
- Props are correctly passed through in all instances
- Used in mobile command bar with
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of CloseAction component
# and ensure no old XMarkIcon imports remain
# Check for any remaining @heroicons XMarkIcon imports
echo "Checking for any remaining heroicons XMarkIcon imports..."
rg -l "@heroicons.*XMarkIcon"
# Find all usages of CloseAction to verify implementation
echo "Finding CloseAction component usage..."
rg -A 2 "<CloseAction"
Length of output: 1129
web/src/components/site/Action/Save.tsx (2)
12-12: LGTM! Clean icon replacement.
The icon replacement maintains the same component structure while using the new standardized icon.
4-4: Verify the new SaveIcon implementation.
The import change aligns with the PR's objective of consolidating icons.
Let's verify the new SaveIcon implementation:
✅ Verification successful
SaveIcon implementation verified and properly configured
The SaveIcon is correctly implemented as a styled wrapper around the Lucide Save icon component, which aligns with the PR's icon consolidation objectives. The implementation:
- Uses the official Lucide React package
- Applies styled-system styling capabilities
- Maintains a clean and consistent approach to icon usage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the SaveIcon implementation exists and uses Lucide
# Expected: Should find the SaveIcon component using Lucide icons
# Check the SaveIcon implementation
ast-grep --pattern 'import { $_ } from "lucide-react"' web/src/components/ui/icons/Save.tsx
Length of output: 169
Script:
#!/bin/bash
# Let's verify the complete implementation of SaveIcon to ensure it's properly wrapped
ast-grep --pattern 'export function SaveIcon() {
$$$
}'
Length of output: 59
Script:
#!/bin/bash
# Let's check the entire content of the Save.tsx icon file
cat web/src/components/ui/icons/Save.tsx
Length of output: 165
web/src/components/site/Action/Edit.tsx (2)
12-12: Verify visual consistency of the new icon dimensions.
The change from 0.5em to 4 (Tailwind spacing) might affect the visual appearance. Please ensure this has been tested across different viewport sizes and contexts where this component is used.
#!/bin/bash
# Description: Find all usages of EditAction to verify visual impact
# Expected: List of components that need visual verification
rg -l "EditAction" --type tsx4-4: Verify icon re-export implementation.
The import change aligns with the PR's objective of consolidating icons. Let's verify the implementation of the re-exported icon.
✅ Verification successful
Icon implementation verified and correctly exported
The implementation in web/src/components/ui/icons/Edit.tsx shows that the EditIcon is properly exported as a styled component wrapping the Lucide React's Edit icon. This aligns with the PR's icon consolidation effort and maintains the connection to Lucide icons while providing styling capabilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the EditIcon implementation in the new location
# Expected: Should find the icon re-export implementation
# Check the implementation of the Edit icon
ast-grep --pattern 'export { EditIcon } from "lucide-react"' web/src/components/ui/icons/Edit.tsx
Length of output: 240
Script:
#!/bin/bash
# Let's first check if the Edit.tsx file exists and see its content
fd "Edit.tsx" --type f
# Check the content of the file if found
fd "Edit.tsx" --type f --exec cat {}
# Let's also search for any EditIcon exports or imports across the codebase
rg "EditIcon" --type ts --type tsx
Length of output: 1583
web/src/components/site/Action/Cancel.tsx (2)
4-4: LGTM! Import change aligns with icon consolidation goal.
The change to use a local styled icon component follows the PR's objective of consolidating icons and maintaining consistent styling.
19-19: LGTM! Icon replacement maintains component functionality.
The CancelIcon replacement is clean and maintains the component's visual feedback for cancellation actions.
web/src/components/site/Action/Bookmark.tsx (2)
14-14: LGTM! Clean implementation.
The conditional rendering is well-implemented and maintains the component's functionality while using the new icon components.
3-6: LGTM! Verify new icon imports exist.
The migration to local icon components aligns with the PR's objective of consolidating icons.
✅ Verification successful
Icon imports are correctly implemented and available
The verification confirms that:
- Both
BookmarkIconandBookmarkSavedIconcomponents are properly exported from the icons directory - The components are styled variants of the base Lucide
Bookmarkicon - The implementation matches the import statement in the Bookmark.tsx component
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the new icon components exist
fd --type f "Bookmark.(tsx|ts)" "web/src/components/ui/icons"
Length of output: 102
Script:
#!/bin/bash
# Let's verify the content of the Bookmark icons file to ensure it exports the required components
cat web/src/components/ui/icons/Bookmark.tsx
Length of output: 266
web/src/components/site/Navigation/Anchors/Queue.tsx (1)
1-1: LGTM! Changes align well with icon consolidation objectives.
The migration to the centralized QueueIcon and its consistent usage in both components follows good React practices and improves code organization.
Also applies to: 16-16, 28-28
web/src/components/site/Navigation/Anchors/Home.tsx (2)
1-1: LGTM! Icon import aligned with new standardization.
The change from @heroicons/react to local components aligns with the PR's objective of consolidating icons and migrating to Lucide icons.
16-16: LGTM! Consistent icon usage across components.
The HomeIcon component is consistently used in both HomeAnchor and HomeMenuItem components, maintaining a uniform approach to icon rendering.
Also applies to: 28-28
web/src/components/site/Navigation/Anchors/Admin.tsx (1)
18-18: LGTM! Consistent icon implementation.
The AdminIcon is consistently implemented across both AdminAnchor and AdminMenuItem components, maintaining a uniform approach to icon rendering.
Also applies to: 30-30
web/src/components/site/Navigation/Anchors/Library.tsx (2)
1-1: LGTM! Icon consolidation aligns with PR objectives.
The migration from @heroicons to a centralized local icon component follows the PR's goal of consolidating icons and improving maintainability.
16-16: LGTM! Consistent icon implementation.
The direct usage of <LibraryIcon /> in both components follows React best practices and maintains consistency across the codebase.
Also applies to: 28-28
web/src/components/site/Navigation/Anchors/Members.tsx (1)
1-1: LGTM! Icon migration looks consistent.
The changes align well with the PR objective of consolidating icons and migrating to a centralized icon system.
Let's verify there are no circular dependencies:
Also applies to: 16-16, 28-28
✅ Verification successful
No circular dependencies found - implementation looks clean
The icon implementation follows a clean, unidirectional dependency flow:
Members.tsxin Navigation/Anchors imports the icon from ui/icons/Members- The icon component is a simple styled wrapper around Lucide's UsersIcon
- No circular references exist between these files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Members.tsx is importing any files that import it back
rg -l "from.*Members" $(fd Members.tsx) | xargs -I {} rg -l "from.*{}" $(fd Members.tsx)
Length of output: 282
Script:
#!/bin/bash
# Let's check the content of both Members files to understand the dependency structure
echo "=== Content of Navigation/Anchors/Members.tsx ==="
cat web/src/components/site/Navigation/Anchors/Members.tsx
echo -e "\n=== Content of icons/Members.tsx ==="
cat web/src/components/ui/icons/Members.tsx
Length of output: 1152
web/src/components/site/Navigation/Anchors/Drafts.tsx (1)
1-1: LGTM! Icon migration looks good.
The changes align well with the PR's objective of consolidating icons and migrating to a more organized structure. The implementation maintains the component's functionality while improving the overall icon management.
Also applies to: 9-9
web/src/components/site/Navigation/Anchors/Settings.tsx (2)
1-1: LGTM! Icon import aligned with new standardization.
The change to import from local components aligns with the PR's objective to consolidate icons and standardize their usage.
18-18: LGTM! Consistent icon usage pattern.
The icon usage follows the new standardized pattern using JSX syntax, maintaining consistency across the codebase.
Also applies to: 30-30
web/src/components/site/Navigation/Anchors/Profile.tsx (1)
1-1: LGTM! Icon migration successfully implemented.
The changes effectively implement the icon consolidation strategy outlined in the PR objectives. The consistent usage of the new ProfileIcon across both ProfileAnchor and ProfileMenuItem components maintains UI consistency while following the project's icon standardization efforts.
Also applies to: 19-19, 31-31
web/src/components/site/Navigation/Anchors/Notifications.tsx (1)
1-1: LGTM! Icon migration aligns with PR objectives.
The change from HeroIcons to the new consolidated icon system is implemented correctly and consistently with the PR's goal of organizing icons and standardizing on Lucide.
Let's verify that the notification icon is consistently updated across the codebase:
Also applies to: 9-9
✅ Verification successful
Icon migration is correctly implemented and consistent
The verification shows that:
NotificationIconis a styled wrapper around Lucide'sBellIcon- All usages of the notification icon consistently use the new
NotificationIconcomponent - No legacy or inconsistent icon implementations were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to BellIcon
# to ensure complete migration to NotificationIcon
# Search for any remaining BellIcon imports or usage
rg "BellIcon" --type ts --type tsx
# Search for NotificationIcon usage for context
rg "NotificationIcon" --type ts --type tsx
Length of output: 133
Script:
#!/bin/bash
# Let's try without file type restrictions since rg doesn't recognize tsx
# Search for any remaining BellIcon references
rg "BellIcon"
# Search for NotificationIcon usage
rg "NotificationIcon"
# Also search for potential icon file names
fd -e tsx -e ts "Bell.*Icon\.tsx?"
fd -e tsx -e ts "Notification.*Icon\.tsx?"
Length of output: 883
web/src/components/site/Navigation/LibraryNavigationTree/LibraryNavigationTree.tsx (1)
6-6: LGTM! Import path updated as part of component reorganization.
The change aligns with the PR's objective of organizing components by moving CreatePageAction to a more appropriate location in the library directory.
web/src/components/site/Navigation/Anchors/Logout.tsx (2)
1-1: LGTM! Import path update aligns with icon consolidation.
The updated import path follows the new icon organization structure, using the @ alias for better path resolution.
21-21: LGTM! Proper component rendering syntax.
The updated syntax {<LogoutIcon />} correctly renders the icon as a React component, which is the proper way to use components in JSX.
Also applies to: 35-35
web/src/components/notifications/NotificationsTrigger.tsx (2)
11-11: LGTM! Icon update matches PR objectives.
The switch to NotificationIcon aligns with the migration to consolidated icon components.
Line range hint 17-40: LGTM! Component rename improves clarity.
The rename from NotificationAction to NotificationsTrigger better describes the component's purpose as a trigger for notifications.
Let's verify the safe removal of NotificationsMenuItem:
web/src/components/library/CreatePage.tsx (3)
4-4: LGTM: Icon import change aligns with consolidation effort
The import change follows the PR's objective of consolidating icons and maintaining consistent module structure.
Line range hint 18-71: LGTM: Components handle the icon change gracefully
Both CreatePageAction and CreatePageMenuItem components maintain consistent icon usage and proper label handling.
15-15: Verify usage of CreatePageIcon in dependent components
The icon change looks good, but since this is an exported constant, let's verify its usage across the codebase.
✅ Verification successful
Icon change is self-contained within CreatePage component
The CreatePageIcon is only used internally within the same file (CreatePage.tsx) by the CreatePageAction component. The icon change is localized and doesn't affect any external components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all files that import CreatePageIcon
# This helps ensure the icon change doesn't cause unexpected visual regressions
rg -l "CreatePageIcon.*from.*CreatePage"
Length of output: 42
Script:
#!/bin/bash
# Let's try a broader search for CreatePageIcon without the import pattern
rg "CreatePageIcon" -A 2
# Also search for potential usage in different case formats
rg -i "createpageicon" -A 2
Length of output: 1300
web/src/components/role/PermissionList.tsx (1)
4-4: LGTM: Import statement properly includes required components
The import statement correctly adds both the new Box component needed for the empty state and includes LStack which was already in use.
web/src/components/site/Onboarding/Checklist/ChecklistItem.tsx (1)
9-9: LGTM: Import change aligns with icon consolidation objective.
The updated import path follows the new icon organization structure and uses the alias path convention correctly.
web/src/components/site/Navigation/AccountMenu/AccountMenu.tsx (2)
18-18: LGTM! Well-structured type definition.
The optional size prop with a union type of "sm" | "md" is properly defined and follows TypeScript best practices.
21-21: LGTM! Clean implementation of size-based adjustments.
The changes effectively implement size-based styling:
- Default value maintains backward compatibility
- Shift calculation adapts menu positioning based on size
- Size prop is correctly forwarded to MemberAvatar
Let's verify that the size prop is consistently used across all parent components:
Also applies to: 31-31, 35-35
✅ Verification successful
Note: Trying different approaches to locate AccountMenu usage since the initial tsx file type wasn't recognized. This will help us verify the consistency of size prop usage across the codebase.
Size prop usage is consistent across AccountMenu implementations
The verification shows that the size prop is being used consistently across the codebase:
- Default
mdsize is used inMemberActions.tsx - Explicit
smsize is used inMobileCommandBar.tsxfor mobile-specific layouts - No conflicting or inconsistent usage patterns were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistency in AccountMenu size prop usage
# Expected: All AccountMenu instances should either specify a size or rely on default
# Search for AccountMenu usage
rg -A 2 '<AccountMenu' --type tsx
Length of output: 63
Script:
#!/bin/bash
# Search for AccountMenu usage with different file extensions
rg -t js -t jsx -t ts -t tsx '<AccountMenu' -A 2
Length of output: 78
Script:
#!/bin/bash
# Alternative search using ripgrep without file type filtering
rg '<AccountMenu' -A 2 web/src/
Length of output: 1004
Script:
#!/bin/bash
# Search using find and grep as fallback
find web/src -type f -exec grep -l "AccountMenu" {} \;
Length of output: 246
web/src/components/role/RoleEdit/RoleEditModal.tsx (1)
11-12: LGTM: Import changes align with icon consolidation goals
The new imports for IconButton and CreateIcon are properly organized and follow the project's module aliasing conventions.
web/src/components/thread/ReplyBox/ReplyBox.tsx (2)
8-8: LGTM: Import change aligns with component migration.
The change from ProfilePill to MemberIdent is consistent with the PR's objective of consolidating components.
42-43: Verify consistent usage of MemberIdent across the codebase.
The migration from ProfilePill to MemberIdent looks good. The explicit text wrapping control and avatar hiding improve the UI consistency.
Let's verify the consistent usage of MemberIdent props across the codebase:
✅ Verification successful
MemberIdent usage is consistent with component's API
The verification shows that MemberIdent's usage in ReplyBox is correct and consistent with its API:
- The
nameprop accepts "handle" as one of its valid values ("hidden" | "handle" | "full-horizontal" | "full-vertical") - The
avatarprop correctly accepts "hidden" as specified - All instances of MemberIdent across the codebase use props within the defined types
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of MemberIdent props
# Expected: All MemberIdent usages should follow similar pattern with name="handle"
# Search for MemberIdent usage patterns
rg -A 2 '<MemberIdent'
# Search for any remaining ProfilePill usages that need migration
rg -l 'ProfilePill'
Length of output: 1555
Script:
#!/bin/bash
# Let's examine the MemberIdent component definition to understand its props
ast-grep --pattern 'interface $_ {
$$$
}' web/src/components/member/MemberIdent/MemberIdent.tsx
# Also check for any type definitions related to name prop
rg -A 2 'name.*=.*"handle"' web/src/components/member/MemberIdent/MemberIdent.tsx
Length of output: 378
Script:
#!/bin/bash
# Let's first find the correct location of MemberIdent component
fd MemberIdent
# Then examine its implementation
rg -A 5 'interface.*MemberIdent'
rg -A 5 'type.*MemberIdent'
# Look for any name prop type definitions
rg -A 2 'name.*=.*"handle"'
rg -A 2 'name:.*string'
Length of output: 7434
Script:
#!/bin/bash
# Let's examine the MemberIdent implementation since we found its location
rg -A 10 'export.*MemberIdent' web/src/components/member/MemberBadge/MemberIdent.tsx
# Also check for prop types or interfaces in this file
rg -A 5 'interface|type' web/src/components/member/MemberBadge/MemberIdent.tsx
# Look for all name prop usages in MemberIdent
rg 'name=' web/src/components/member/MemberBadge/MemberIdent.tsx
Length of output: 710
web/src/components/form/ColourInput/ColourInput.tsx (2)
5-5: LGTM! Import path update aligns with icon consolidation.
The updated import path follows the new icon organization structure and uses the @ alias effectively.
89-89: LGTM! Consistent icon styling.
The addition of the semantic color token fg.muted follows good theming practices and maintains consistency with the new icon styling conventions.
web/src/components/site/Navigation/MobileCommandBar/MobileCommandBar.tsx (1)
65-81: Well-structured layout improvements!
The changes demonstrate good improvements:
- Consistent use of "sm" size prop across components
- Better alignment with WStack
- Clear separation between expanded and collapsed states
Also applies to: 83-97
web/src/components/library/Breadcrumbs.tsx (1)
14-14: LGTM! Import path change aligns with component organization.
The simplified import path indicates better component organization, which aligns with the PR's objective of tidying up the codebase.
web/src/components/notifications/NotificationsMenu.tsx (3)
14-14: LGTM: Clean import replacement
The replacement of NotificationAction with NotificationsTrigger aligns with the PR's consolidation objectives.
Line range hint 22-27: LGTM: Proper loading state handling
The NotificationsTrigger implementation correctly handles the loading state with appropriate props and error handling.
39-39: Verify unread indicator behavior
The NotificationsTrigger implementation looks correct, but let's ensure the unread indicator behaves as expected with the new component.
✅ Verification successful
NotificationsTrigger unread indicator implementation is correct
The component correctly implements the unread indicator as a small red dot positioned at the top-right corner when the unread prop is true. The implementation in NotificationsMenu.tsx uses this prop appropriately.
- The unread indicator is rendered conditionally based on the
unreadprop - The styling uses appropriate positioning and visual properties for a notification dot
- The component composition in NotificationsMenu.tsx follows the expected pattern
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for tests covering the unread indicator functionality
rg -l "unread.*NotificationsTrigger" --type=test
Length of output: 79
Script:
#!/bin/bash
# Search for test files containing NotificationsTrigger
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts | xargs rg "NotificationsTrigger.*unread"
# Search for the NotificationsTrigger component implementation
rg -A 5 "export.*NotificationsTrigger"
Length of output: 736
Script:
#!/bin/bash
# Look for the implementation details of NotificationsTrigger to verify unread prop handling
rg -A 15 "export function NotificationsTrigger" web/src/components/notifications/NotificationsTrigger.tsx
# Search for any stories or examples showing unread state
fd -e stories.tsx | xargs rg "NotificationsTrigger.*unread"
Length of output: 626
Script:
#!/bin/bash
# Get the complete implementation of NotificationsTrigger to see the unread indicator styling
rg -A 25 "export function NotificationsTrigger" web/src/components/notifications/NotificationsTrigger.tsx
# Look for any tests in the same directory
rg -l "test|describe|it\(" web/src/components/notifications/
Length of output: 690
web/src/components/library/LibraryPageTree/LibraryPageTree.tsx (1)
11-11: LGTM: Import path update aligns with component reorganization.
The change to import CreatePageAction from the library directory instead of site/Navigation makes the component organization more logical and maintainable.
| import { IconButton } from "@/components/ui/icon-button"; | ||
| import { InfoIcon } from "@/components/ui/icons/Info"; |
There was a problem hiding this comment.
💡 Codebase verification
Direct lucide-react import found in EditAvatarScreen.tsx needs migration
The verification revealed an inconsistency in icon imports:
web/src/components/member/EditAvatar/EditAvatarScreen.tsxstill importsInfoIcondirectly fromlucide-react- The new consolidated
InfoIconcomponent exists atweb/src/components/ui/icons/Info.tsx
Please update the import in EditAvatarScreen.tsx to use the consolidated icon:
import { InfoIcon } from "@/components/ui/icons/Info";🔗 Analysis chain
LGTM! Import changes align with icon consolidation strategy.
The updated import paths follow the new pattern of using local icon components instead of direct lucide-react imports, which aligns well with the PR's objective of consolidating icons.
Let's verify the consistency of icon import patterns across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining direct lucide-react icon imports
# that should be migrated to local components
# Test: Search for direct lucide-react icon imports
# Expect: No direct imports of InfoIcon from lucide-react
rg "import.*InfoIcon.*from ['\"]lucide-react['\"]"
# Test: Verify the new InfoIcon component exists
# Expect: Should find the new InfoIcon component file
fd "Info.tsx" "web/src/components/ui/icons"
Length of output: 234
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (10)
web/src/components/ui/icons/Breadcrumb.tsx (1)
1-4: Consider renaming the file to be more specificSince this file only contains the
BreadcrumbIconcomponent, consider renaming it toBreadcrumbIcon.tsxto better reflect its single responsibility.web/src/components/site/DeleteConfirmationButton.tsx (1)
50-52: Enhance accessibility for the confirmation dialogConsider improving the accessibility of the confirmation UI:
<IconButton type="button" variant="ghost" title="Cancel delete" + aria-label="Cancel deletion" onClick={handleCancelAction} > <CancelIcon /> </IconButton>web/src/components/site/Unready.tsx (1)
58-61: Consider adding aria-label to the warning icon.While the icon usage is appropriate, adding an aria-label would improve accessibility for screen readers.
- <WarningIcon /> + <WarningIcon aria-label="Warning indicator" />web/src/screens/auth/LoginScreen/LoginForm.tsx (1)
64-64: Consider enhancing button accessibility.The icon replacement looks good, but consider adding an aria-label to the button to improve accessibility for screen readers.
<Button w="full" variant="ghost" size="sm" type="button" + aria-label="Login with biometric authentication" onClick={handleWebauthn} >web/src/components/library/links/LinkView.tsx (1)
32-32: Consider adding aria-label for accessibilityThe icon implementation looks good, but consider adding an aria-label to improve accessibility for screen readers.
- <ExternalLinkIcon w="4" /> + <ExternalLinkIcon w="4" aria-label="Opens in new tab" />web/src/components/thread/Breadcrumbs.tsx (1)
67-67: Consider adding aria-hidden to decorative iconThe icon implementation looks good, but since this is a decorative separator between breadcrumbs, consider adding
aria-hidden="true"to improve screen reader experience.- <BreadcrumbIcon /> + <BreadcrumbIcon aria-hidden="true" />web/src/components/notifications/NotificationCardList.tsx (1)
44-44: Remove commented out codeInstead of leaving commented out code, it should be removed if it's no longer needed. This helps maintain cleaner and more maintainable code.
- // controls={}web/src/components/collection/CollectionMenu/CollectionMenu.tsx (1)
Line range hint
67-106: Remove commented-out delete confirmation codeLarge blocks of commented-out code can lead to maintenance issues and code clutter. If this delete confirmation logic is no longer needed, it should be removed. If it's intended for future use, consider tracking it in a separate issue.
- {/* {isDeletingEnabled && - (isConfirmingDelete ? ( - <HStack gap="0"> - <Menu.Item - className={menuItemColorPalette({ colorPalette: "red" })} - value="delete" - w="full" - closeOnSelect={false} - > - Are you sure? - </Menu.Item> - - <Menu.Item - value="delete-cancel" - closeOnSelect={false} - asChild - > - <CancelAction - borderRadius="md" - onClick={handlers.handleCancelDelete} - /> - </Menu.Item> - </HStack> - ) : ( - <Menu.Item - className={menuItemColorPalette({ colorPalette: "red" })} - value="delete" - closeOnSelect={false} - onClick={handlers.handleConfirmDelete} - > - <HStack gap="1"> - <TrashIcon width="1.4em" /> Delete - </HStack> - </Menu.Item> - ))} */}web/src/components/notifications/NotificationsMenu.tsx (1)
108-108: Consider documenting icon usage patternsThe icon replacements look good and align with the new icon system. Consider adding a comment explaining the SettingsIcon usage as a fallback avatar for system notifications, as this might not be immediately obvious to other developers.
- return <SettingsIcon />; + // Use SettingsIcon as fallback avatar for system-generated notifications + return <SettingsIcon />;Also applies to: 127-127
web/src/screens/profile/ProfileScreen.tsx (1)
131-131: Consider adding aria-label for accessibility.The icon implementation looks good and uses the correct design system spacing. Consider adding an aria-label to improve accessibility.
- <LikeIcon w="4" /> + <LikeIcon w="4" aria-label="likes count" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (47)
web/src/components/category/CategoryCreate/CategoryCreateTrigger.tsx(2 hunks)web/src/components/collection/CollectionMenu/CollectionMenu.tsx(2 hunks)web/src/components/content/CollectionCreate/CollectionCreateTrigger.tsx(2 hunks)web/src/components/content/CollectionMenu/CollectionMenu.tsx(2 hunks)web/src/components/feed/ThreadItem.tsx(2 hunks)web/src/components/library/Breadcrumbs.tsx(3 hunks)web/src/components/library/LibraryPageAssetList/LibraryPageAssetList.tsx(2 hunks)web/src/components/library/links/LinkIndexView/LinkIndexView.tsx(2 hunks)web/src/components/library/links/LinkView.tsx(2 hunks)web/src/components/member/MemberRoleMenu/MemberRoleMenu.tsx(2 hunks)web/src/components/member/MemberSuspension/useMemberSuspension.ts(0 hunks)web/src/components/notifications/NotificationCardList.tsx(3 hunks)web/src/components/notifications/NotificationsMenu.tsx(4 hunks)web/src/components/site/DeleteConfirmationButton.tsx(3 hunks)web/src/components/site/DeleteConfirmationMenuItem.tsx(2 hunks)web/src/components/site/Empty.tsx(1 hunks)web/src/components/site/Navigation/Search/Search.tsx(3 hunks)web/src/components/site/PaginatedSearch/PaginatedSearch.tsx(2 hunks)web/src/components/site/Unready.tsx(3 hunks)web/src/components/thread/Breadcrumbs.tsx(2 hunks)web/src/components/thread/ReplyBox/ReplyBox.tsx(3 hunks)web/src/components/thread/ReplyMenu/ReplyMenu.tsx(2 hunks)web/src/components/thread/ThreadIndexView/ThreadIndexView.tsx(2 hunks)web/src/components/thread/ThreadMenu/ThreadMenu.tsx(4 hunks)web/src/components/ui/checkbox.tsx(2 hunks)web/src/components/ui/icons/Add.tsx(1 hunks)web/src/components/ui/icons/Archive.tsx(1 hunks)web/src/components/ui/icons/Biometric.tsx(1 hunks)web/src/components/ui/icons/Breadcrumb.tsx(1 hunks)web/src/components/ui/icons/Check.tsx(1 hunks)web/src/components/ui/icons/CreateFolder.tsx(1 hunks)web/src/components/ui/icons/Delete.tsx(1 hunks)web/src/components/ui/icons/Discussion.tsx(1 hunks)web/src/components/ui/icons/ExternalLink.tsx(1 hunks)web/src/components/ui/icons/Inbox.tsx(1 hunks)web/src/components/ui/icons/Like.tsx(1 hunks)web/src/components/ui/icons/Link.tsx(1 hunks)web/src/components/ui/icons/Remove.tsx(1 hunks)web/src/components/ui/icons/Search.tsx(1 hunks)web/src/components/ui/icons/Share.tsx(1 hunks)web/src/components/ui/icons/Submenu.tsx(1 hunks)web/src/components/ui/icons/Warning.tsx(1 hunks)web/src/recipes/rich-card.ts(1 hunks)web/src/screens/auth/LoginScreen/LoginForm.tsx(2 hunks)web/src/screens/auth/RegisterScreen/RegisterForm.tsx(2 hunks)web/src/screens/profile/ProfileScreen.tsx(2 hunks)web/src/screens/thread/ThreadScreen/ThreadScreen.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- web/src/components/member/MemberSuspension/useMemberSuspension.ts
✅ Files skipped from review due to trivial changes (17)
- web/src/components/library/links/LinkIndexView/LinkIndexView.tsx
- web/src/components/ui/icons/Add.tsx
- web/src/components/ui/icons/Archive.tsx
- web/src/components/ui/icons/Biometric.tsx
- web/src/components/ui/icons/Check.tsx
- web/src/components/ui/icons/CreateFolder.tsx
- web/src/components/ui/icons/Delete.tsx
- web/src/components/ui/icons/Discussion.tsx
- web/src/components/ui/icons/ExternalLink.tsx
- web/src/components/ui/icons/Inbox.tsx
- web/src/components/ui/icons/Like.tsx
- web/src/components/ui/icons/Link.tsx
- web/src/components/ui/icons/Remove.tsx
- web/src/components/ui/icons/Search.tsx
- web/src/components/ui/icons/Share.tsx
- web/src/components/ui/icons/Submenu.tsx
- web/src/components/ui/icons/Warning.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/components/library/Breadcrumbs.tsx
- web/src/components/thread/ReplyBox/ReplyBox.tsx
🔇 Additional comments (51)
web/src/components/ui/icons/Breadcrumb.tsx (1)
5-9: Verify the height token and consider aspect ratio
The height is set to "4", which appears to be using your design token system. However:
- Consider documenting the actual size this represents for maintainability
- Consider setting both height and width to maintain the icon's aspect ratio
export const BreadcrumbIcon = styled(ChevronRight, {
base: {
height: "4",
+ width: "4", // Maintain aspect ratio
},
});Let me verify the design token system usage:
✅ Verification successful
Let me search for other icon usages to understand the design system's patterns:
Let me fix the regex and try again:
Height "4" is consistent with other icon implementations
The height value of "4" is correctly used in the design system:
- The
Emptyicon also uses size "4" for width - Most icons use the default styled component without explicit sizing, suggesting "4" is a standard token size
- The tokens file shows "4" is a valid size token in the design system
The aspect ratio concern is not warranted since the ChevronRight icon has an inherent aspect ratio that should be preserved by setting only the height.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other height: "4" usage in styled components
rg -l 'height: "4",' $(fd -e tsx -e ts)
# Search for design tokens documentation
fd -g '*tokens*.{ts,js,json}' -X cat
Length of output: 189452
Script:
#!/bin/bash
# Search for other icon components to understand sizing patterns
rg -l 'styled\([^,]+,\s*{\s*base:\s*{[^}]*height:' web/src/components/ui/icons/
# Search for specific icon usage with height "4"
rg -A 5 'styled\([^,]+,\s*{\s*base:\s*{[^}]*height:\s*"4"' web/src/components/ui/icons/
Length of output: 463
Script:
#!/bin/bash
# Search for other icon components to understand sizing patterns
rg 'styled\(' web/src/components/ui/icons/ -A 5
# Also check for any icon-related tokens in the design system
rg '"4"' web/src/components/ui/icons/
Length of output: 4730
web/src/components/site/Empty.tsx (2)
5-5: LGTM: Clean import restructuring
The change to import EmptyIcon from a local module aligns well with the PR's objective of consolidating icons and improves maintainability through centralized icon management.
10-10: LGTM: Simplified icon implementation
The replacement of CubeTransparentIcon with EmptyIcon eliminates the need for external styling while maintaining the component's functionality.
web/src/components/content/CollectionCreate/CollectionCreateTrigger.tsx (2)
6-6: LGTM! Icon import change aligns with project objectives.
The replacement of Heroicons with local icon components follows the PR's goal of consolidating icons.
33-33: Verify icon sizing consistency.
The width prop has been removed from the icon. Let's verify that the new CreateFolderIcon maintains consistent sizing with other icons in the application.
✅ Verification successful
Let me gather more information about the icon implementation and styling.
Let me check one more thing to understand how the icons are styled in the system.
Icon sizing is consistently managed through styled-system
The removal of the explicit width prop is correct. All icons in the application, including CreateFolderIcon, HomeIcon, and AddIcon, use the same consistent styling pattern through the styled-system. They are all styled variants of Lucide icons using the same styled() wrapper, which ensures consistent sizing and styling across the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if other icon components have explicit width props or use a consistent approach
# This helps ensure visual consistency across the application
# Search for icon components with width props
rg -l 'width=\{[0-9]+\}' 'web/src/components/**/*Icon*.tsx'
# Search for CreateFolderIcon usage patterns
rg -A 2 'CreateFolderIcon' 'web/src/components'
Length of output: 937
Script:
#!/bin/bash
# Check the CreateFolder icon implementation and styling
cat web/src/components/ui/icons/CreateFolder.tsx
# Look for other styled icons to compare the pattern
rg -l "styled\(" web/src/components/ui/icons/
# Check if there's a common icon configuration
fd "icon" web/src/components/ui/ --type f --exec cat {}
Length of output: 3122
Script:
#!/bin/bash
# Check the icon recipe configuration
fd "icon" web/src/styled-system/ --type f --exec cat {}
# Check a few other styled icons to compare implementation
cat web/src/components/ui/icons/Home.tsx web/src/components/ui/icons/Add.tsx
Length of output: 499
web/src/components/site/DeleteConfirmationMenuItem.tsx (1)
4-4: LGTM! Icon import aligns with project standardization.
The change from Heroicons to local icon components aligns with the PR's objective to consolidate icons into re-exports.
web/src/components/category/CategoryCreate/CategoryCreateTrigger.tsx (2)
5-5: LGTM! Import change aligns with icon consolidation objective.
The new import follows proper module resolution and naming conventions.
17-17: LGTM! Icon replacement maintains consistency.
The CreateIcon replacement is properly implemented and maintains consistent usage across both trigger and menu item components.
Let's verify this icon migration pattern across the codebase:
✅ Verification successful
The verification results show that:
- No remaining HeroIcon plus icons were found in the codebase
- CreateIcon is consistently imported from "@/components/ui/icons/Create" across multiple components
Let me verify the actual usage pattern of CreateIcon in these files to ensure the migration is complete and consistent.
Let me try again with the correct file type flag:
Let me try one final verification with a different approach:
Icon migration is correctly implemented and consistent
The verification confirms that:
- CreateIcon is properly defined as a styled PlusIcon from lucide-react
- It's consistently used across multiple components with the same pattern
- Other PlusIcon imports are for different purposes (ImagePlus, SmilePlus) and don't require migration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining HeroIcon plus icons that might need migration
# and verify the consistency of CreateIcon usage
# Check for any remaining HeroIcon plus icons
echo "Checking for remaining HeroIcon plus icons..."
rg -l "PlusIcon.*heroicons"
# Verify CreateIcon usage pattern
echo "Verifying CreateIcon usage pattern..."
rg -A 2 "CreateIcon.*components/ui/icons/Create"
Length of output: 1510
Script:
#!/bin/bash
# Check how CreateIcon is used in the components
echo "Checking CreateIcon usage patterns..."
rg -A 5 "CreateIcon" --type tsx
Length of output: 144
Script:
#!/bin/bash
# Check how CreateIcon is used in the components
echo "Checking CreateIcon usage patterns..."
rg -A 5 "CreateIcon" -t tsx
# Also check the actual icon component definition
echo -e "\nChecking CreateIcon component definition..."
fd "Create.tsx" --exec cat {}
Length of output: 408
Script:
#!/bin/bash
# Check CreateIcon usage without file type flag
echo "Checking CreateIcon usage patterns..."
rg -A 5 "export.*CreateIcon|CreateIcon.*=|<CreateIcon"
# Check if there are any other plus icon imports that might need migration
echo -e "\nChecking other plus icon imports..."
rg "import.*Plus.*Icon"
Length of output: 2877
web/src/components/site/Navigation/Search/Search.tsx (2)
4-5: LGTM: Icon imports aligned with new standardization.
The migration from @heroicons to local icon components aligns with the PR's objective of consolidating icon usage.
47-47: LGTM: Icon replacements maintain functionality.
The replacement of XMarkIcon with CancelIcon and MagnifyingGlassIcon with SearchIcon maintains the same visual and functional behavior while adhering to the new icon standardization.
Also applies to: 60-60
web/src/components/site/DeleteConfirmationButton.tsx (2)
4-6: LGTM: Clean icon import organization
The new icon imports are well-organized and follow the project's conventions for module imports.
65-67: LGTM: Clear delete button implementation
The combination of icon and text provides good visual clarity while maintaining accessibility.
web/src/components/thread/ThreadIndexView/ThreadIndexView.tsx (2)
6-6: LGTM! Clean import statement.
The import follows the project's conventions with proper aliasing and grouping.
42-42: LGTM! Appropriate icon replacement.
The CancelIcon is semantically appropriate for the reset action and aligns with the PR's objective of consolidating icons to Lucide.
web/src/components/site/Unready.tsx (2)
14-14: LGTM: Icon import aligns with PR objectives.
The import of WarningIcon from a dedicated icons directory follows good organization practices and aligns with the PR's goal of consolidating icons.
35-35: LGTM: Appropriate icon usage for error state.
The WarningIcon is semantically correct for error states and properly contained within a fixed-width box for consistent layout.
web/src/screens/auth/LoginScreen/LoginForm.tsx (1)
4-4: LGTM: Icon import aligned with standardization goals.
The replacement of the external HeroIcon with a local BiometricIcon component follows the PR's objective of consolidating icons and standardizing the icon system.
web/src/screens/auth/RegisterScreen/RegisterForm.tsx (2)
4-4: LGTM: Icon import aligned with new standardization.
The change from HeroIcons to local components aligns with the PR's objective of consolidating icon imports.
64-64: LGTM: Consistent icon replacement.
The BiometricIcon replacement maintains the WebAuthn functionality while following the new icon standardization pattern, matching similar changes in the LoginForm component.
web/src/components/member/MemberRoleMenu/MemberRoleMenu.tsx (4)
7-7: LGTM: Import changes align with icon consolidation objective.
The new imports properly organize icons into a centralized location and introduce the badge color utility, which aligns with the PR's goal of consolidating icons and improving maintainability.
Also applies to: 9-11
36-36: LGTM: Consistent icon replacement.
The SubmenuIcon replacement maintains the same semantic meaning while adhering to the new icon system.
43-54: LGTM: Improved color handling approach.
The new approach using badgeColourCSS and CSS variables is more maintainable and performant than direct color manipulation.
56-56: LGTM: Consistent icon styling.
The RemoveCircleIcon implementation maintains consistency with CheckCircleIcon's dimensions.
web/src/components/library/links/LinkView.tsx (1)
4-4: LGTM: Icon import aligns with consolidation effort
The import of ExternalLinkIcon from the consolidated icons directory aligns with the PR's objective of organizing icons.
web/src/components/feed/ThreadItem.tsx (2)
12-15: LGTM! Verify icon source implementation.
The new icon imports align with the PR's objective of consolidating icons.
Let's verify the implementation of these new icon components:
#!/bin/bash
# Description: Check the implementation of the Discussion icons to ensure they use Lucide
# Test: Expect to find Lucide icon usage in the implementation
rg -l "lucide-react" "$(fd -t f Discussion.tsx)"67-69: Consider enhancing accessibility for interactive icons.
While the icon implementation looks good, consider adding aria-label attributes to improve accessibility.
Apply this diff to enhance accessibility:
- <DiscussionParticipatingIcon width="4" />
+ <DiscussionParticipatingIcon width="4" aria-label="Thread with your replies" />
- <DiscussionIcon width="4" />
+ <DiscussionIcon width="4" aria-label="Thread discussion" />Let's verify the width prop usage across the codebase:
web/src/components/ui/checkbox.tsx (2)
11-12: LGTM! Icon imports align with PR objectives.
The migration to dedicated icon components from ./icons directory aligns with the PR's goal of consolidating icons and improving maintainability.
70-70: LGTM! Consistent icon usage for indeterminate state.
The replacement of MinusIcon with RemoveIcon maintains the checkbox's functionality while adhering to the new icon standardization.
web/src/components/thread/Breadcrumbs.tsx (1)
9-9: LGTM: Icon import aligns with consolidation effort
The import of BreadcrumbIcon follows the project's conventions and supports the PR's goal of consolidating icons.
web/src/components/thread/ReplyMenu/ReplyMenu.tsx (2)
8-11: LGTM: Icon imports are well-organized
The icon imports are consistently sourced from the local components directory, which aligns with the PR's objective of consolidating icons.
47-47: LGTM: Consistent icon implementation
The icons are well-integrated into the menu items with consistent spacing and standardized sizing. The implementation follows a clean, uniform pattern across all actions.
Also applies to: 54-54, 62-62, 70-70
web/src/components/notifications/NotificationCardList.tsx (3)
3-4: LGTM: Icon imports aligned with PR objectives
The replacement of Heroicons with local icon components aligns well with the PR's goal of consolidating icons and migrating to Lucide icons. The import structure is clean and well-organized.
Also applies to: 6-6, 11-11
46-49: LGTM: Improved layout structure
The addition of WStack provides better layout organization for the notification source and controls.
88-104: Verify mobile usability with reduced button size
The button size has been reduced from 'sm' to 'xs'. While this might improve the visual aesthetics, please verify that the smaller touch target size doesn't impact mobile usability.
Consider implementing touch target size testing or following platform-specific guidelines:
- iOS Human Interface Guidelines recommend a minimum touch target size of 44x44 points
- Material Design recommends touch targets be at least 48x48 dp
Would you like me to help create a mobile usability testing plan?
web/src/components/site/PaginatedSearch/PaginatedSearch.tsx (1)
12-12: LGTM! Icon standardization looks good.
The replacement of the icon aligns well with the PR's objective to consolidate icons and migrate to Lucide icons. The functionality remains unchanged while improving consistency across the application.
Also applies to: 99-99
web/src/components/content/CollectionMenu/CollectionMenu.tsx (2)
8-9: LGTM: Icon imports aligned with PR objectives
The replacement of heroicons with local icon components aligns with the PR's goal of consolidating icons and migrating to Lucide icons.
98-98: LGTM: Icon implementation maintains functionality
The replacement of MinusIcon/PlusIcon with RemoveIcon/AddIcon maintains the same conditional logic while adopting the new icon system. The Center wrapper ensures proper alignment and consistent sizing.
web/src/components/library/LibraryPageAssetList/LibraryPageAssetList.tsx (1)
10-10: LGTM! Icon replacement aligns with project's icon consolidation effort.
The replacement of XMarkIcon with DeleteIcon is semantically more appropriate for the delete action and aligns with the PR's objective of consolidating icons and migrating to Lucide-based components.
Also applies to: 100-100
web/src/components/collection/CollectionMenu/CollectionMenu.tsx (2)
9-10: LGTM: Icon imports aligned with PR objectives
The replacement of heroicons with local icon components aligns with the PR's goal of consolidating icons and migrating to Lucide.
48-48: LGTM: Clean and consistent icon usage
The implementation maintains good readability with consistent spacing and layout in the menu items.
Also applies to: 55-55
web/src/components/thread/ThreadMenu/ThreadMenu.tsx (3)
9-12: LGTM: Icon imports are well-organized
The consolidation of icons into centralized components aligns with the PR objectives and improves maintainability.
31-36: Verify menu positioning across different viewport sizes
The shift value of 32 might affect the menu's positioning differently across various screen sizes and device types.
Let's check for any responsive design considerations:
✅ Verification successful
Menu positioning shift value is consistent with other menu implementations
Based on the search results, the shift value of 32 in ThreadMenu is appropriate because:
- Other menus in the codebase use similar positioning configurations:
- AccountMenu uses
positioningwith viewport fitting and slide - LibraryPageMenu uses
positioningwith specific placement and gutter - MemberRoleMenu uses
positioningwith placement and gutter values - CollectionMenu uses
positioningwith slide property
- AccountMenu uses
The shift value helps align the menu with its trigger while maintaining consistent spacing across the UI, and there's no evidence of viewport-related issues in the implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar Menu.Root components with positioning props
ast-grep --pattern 'Menu.Root {
positioning: {
$$$
}
$$$
}'
# Look for any responsive design patterns or viewport-specific styles
rg -l "positioning.*shift" --type tsx
Length of output: 19570
Script:
#!/bin/bash
# Search for Menu.Root components with positioning props
rg -A 5 "Menu.Root.*positioning" --type tsx
# Look for any menu-related styling or positioning patterns
rg -A 3 "menu.*positioning|positioning.*menu" --type tsx --type css
Length of output: 171
Script:
#!/bin/bash
# Search for Menu.Root components with positioning props
rg -A 5 "Menu.Root.*positioning"
# Look for any menu-related styling or positioning patterns
rg -A 3 "menu.*positioning|positioning.*menu"
# Search for Menu.Root usage patterns
rg -A 5 "Menu.Root"
Length of output: 11447
61-61: Verify icon sizing consistency
The icons no longer have explicit width attributes. Please ensure they maintain consistent sizing across all menu items.
Let's check how these icons are styled:
Also applies to: 68-68, 76-76, 112-112
web/src/components/notifications/NotificationsMenu.tsx (3)
4-14: LGTM: Import organization and icon consolidation
The import changes align well with the PR's objective of consolidating icons and using Lucide-based components. The imports are well-organized by their categories.
39-39: LGTM: Consistent NotificationsTrigger implementation
The NotificationsTrigger is properly integrated within the Menu.Trigger context with consistent prop usage.
Line range hint 22-28: Verify NotificationsTrigger props interface
The NotificationsTrigger implementation looks correct, but let's verify that all required props are properly typed.
✅ Verification successful
Let me gather more specific information about the NotificationsTrigger component.
Let me try a different search strategy to locate the NotificationsTrigger component and its type definitions.
NotificationsTrigger props are correctly implemented and used
The verification shows that the NotificationsTrigger component accepts all the props being passed:
- It extends both
AnchorPropsandButtonPropswhich coversize,variant,disabled, andtitle - It has its own
Propsinterface withunreadandhideLabelproperties - The usage in NotificationsMenu.tsx matches the component's prop interface perfectly
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify NotificationsTrigger component props
ast-grep --pattern 'interface Props {
$$$
}'
Length of output: 278
Script:
#!/bin/bash
# Search for NotificationsTrigger component definition and its props
ast-grep --pattern 'interface NotificationsTriggerProps {
$$$
}'
# Also try a broader search for the component
ast-grep --pattern 'const NotificationsTrigger = $$$'
# Search for any type definitions related to NotificationsTrigger
rg "NotificationsTrigger.*Props" -A 5
Length of output: 164
Script:
#!/bin/bash
# Find the file containing NotificationsTrigger
fd "NotificationsTrigger" --type f
# Search for any imports of NotificationsTrigger
rg "import.*NotificationsTrigger"
# Look for type definitions or component declarations
rg "NotificationsTrigger" -A 5 -B 5
Length of output: 3764
web/src/screens/profile/ProfileScreen.tsx (1)
16-16: LGTM! Icon import aligns with the new organization.
The import of LikeIcon from the centralized icons directory follows the project's new icon organization strategy.
web/src/screens/thread/ThreadScreen/ThreadScreen.tsx (2)
17-17: LGTM: Import changes align with PR objectives.
The changes successfully consolidate icon imports and remove unused imports, improving code organization.
Also applies to: 21-24
185-186: Verify the width property value.
The icon changes improve semantic meaning, but please verify that width="4" is a valid value in your styling system. Consider using a more explicit unit like "1rem" or confirming that "4" maps to the intended size in your design tokens.
Also applies to: 190-191
web/src/recipes/rich-card.ts (2)
120-122: LGTM: Container width property ensures consistent layout.
The addition of width: "full" to the container ensures the rich card takes up the full width of its parent container, providing consistent layout behavior.
Line range hint 183-184: LGTM: Well-documented backdrop gradient implementation.
The addition of the maskImage property with a horizontal gradient creates a smooth fade effect for the backdrop. The implementation is clear and well-documented, explaining the purpose of the gradient in preventing the backdrop from being too attention-seeking.
| @@ -45,7 +44,7 @@ export function DeleteWithConfirmationMenuItem(props: Props) { | |||
| onClick={handleConfirmAction} | |||
| > | |||
| <HStack gap="1"> | |||
| <TrashIcon width="1.4em" /> Delete | |||
| <DeleteIcon /> Delete | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent icon usage detected across the codebase
The verification reveals inconsistencies in delete icon implementation:
- Most components correctly use the standardized
DeleteIconfrom@/components/ui/icons/Delete - However, some components still use the old
TrashIconfromlucide-react:CollectionMenu/CollectionMenu.tsxFeedItemMenu/FeedItemMenu.tsx
These components need to be updated to use the standardized DeleteIcon component for consistency.
🔗 Analysis chain
LGTM! Icon implementation is consistent.
The DeleteIcon implementation maintains the component's functionality while following the new icon standardization pattern.
Let's verify the DeleteIcon implementation across the codebase for consistency:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check DeleteIcon usage pattern across the codebase
# Expected: Consistent usage of DeleteIcon from local components
# Search for DeleteIcon imports and usage
rg -A 2 "DeleteIcon"
# Verify no remaining TrashIcon usage
rg "TrashIcon"
Length of output: 3400
| @@ -1,12 +1,14 @@ | |||
| "use client"; | |||
|
|
|||
| import { Portal } from "@ark-ui/react"; | |||
| import { ChevronRightIcon, MinusCircleIcon } from "@heroicons/react/24/outline"; | |||
| import { CheckCircleIcon } from "@heroicons/react/24/solid"; | |||
| import chroma from "chroma-js"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused chroma-js import.
The chroma-js library is imported but no longer used after switching to badgeColourCSS.
-import chroma from "chroma-js";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import chroma from "chroma-js"; |
| import { DeleteIcon } from "@/components/ui/icons/Delete"; | ||
| import { EditIcon } from "@/components/ui/icons/Edit"; | ||
| import { LinkIcon } from "@/components/ui/icons/Link"; | ||
| import { ShareIcon } from "@/components/ui/icons/Share"; |
There was a problem hiding this comment.
💡 Codebase verification
Icon usage inconsistency detected in RoleOptionsMenu
There's an inconsistency in the icon imports. While most components use the new icon components from @/components/ui/icons/, the RoleOptionsMenu component is still using EditIcon from lucide-react directly:
web/src/components/role/RoleOptionsMenu/RoleOptionsMenu.tsximportsEditIconfrom "lucide-react"
Additionally, there's a mix of icon imports in FeedItemMenu:
web/src/components/feed/FeedItemMenu/FeedItemMenu.tsximports{ LinkIcon, ShareIcon, TrashIcon }from "lucide-react"
These components should be updated to use the new icon components for consistency.
🔗 Analysis chain
Verify consistent icon usage across the codebase
Let's ensure these icons are being used consistently across other components that might have similar functionality.
Let me gather more specific information about the icon usage patterns to ensure consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining heroicons imports and verify consistent icon usage
# Check for any remaining heroicons imports
echo "Checking for remaining heroicons imports..."
rg -l "@heroicons"
# Check usage patterns of the new icons
echo "\nChecking usage of new icons..."
rg -l "import.*from.*@/components/ui/icons/(Delete|Edit|Link|Share)"
Length of output: 907
Script:
#!/bin/bash
# Check specific usage patterns of each icon to verify consistency
echo "Checking DeleteIcon usage..."
rg "DeleteIcon" -B 2 -A 2
echo "\nChecking EditIcon usage..."
rg "EditIcon" -B 2 -A 2
echo "\nChecking LinkIcon usage..."
rg "LinkIcon" -B 2 -A 2
echo "\nChecking ShareIcon usage..."
rg "ShareIcon" -B 2 -A 2
# Check if there are any old icon patterns still in use
echo "\nChecking for potential old icon patterns..."
rg "(delete|edit|link|share).*icon" -i --type ts --type tsx
Length of output: 18819
consolidate most action icons into re-exports and migrate mostly to Lucide icons
also fix some minor issues with the mobile menu, removed some rarely/never used components