Feat/#123 new sidebar #133
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces several new React components for the dashboard, including a collapsible sidebar, logo switcher, navigation menu, and a navigation bar. It replaces an older sidebar component with a more modular and responsive design integrated into the dashboard layout. Additionally, new icon components and tooltip implementations are added, along with a custom mobile hook and comprehensive sidebar context in the UI package. Various dependency and configuration updates are applied, and Git hook scripts along with minor middleware and Next.js configuration changes further adjust control flows and build processes. Changes
Sequence Diagram(s)sequenceDiagram
participant DP as DashboardPage
participant DL as DashboardLayout
participant SP as SidebarProvider
participant AS as AppSidebar
participant LS as LogoIconSwitcher
participant NM as NavMain
participant ND as NavbarDashboard
DP->>DL: Render layout
DL->>SP: Wrap children components
SP->>AS: Render AppSidebar (with useSidebar)
AS->>LS: Render LogoIconSwitcher (with logo and animation)
AS->>NM: Render NavMain (populate navigation items)
DL->>ND: Render NavbarDashboard
sequenceDiagram
participant D as Developer
participant PR as pre-rebase Script
D->>PR: Initiate rebase
PR->>PR: Fetch latest changes from main
PR->>PR: Count commits ahead of origin/main
alt More than one commit
PR->>D: Abort rebase - instruct squashing commits
else One or zero commit
PR->>D: Proceed with rebase
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (22)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 14
🧹 Outside diff range and nitpick comments (21)
apps/core/app/dashboard/_compnents/logo-icon-switcher.tsx (2)
15-22: Enhance accessibility for the logo sectionWhile the implementation is clean, let's improve accessibility by adding proper ARIA attributes and semantic HTML.
- <SidebarMenuItem className="flex items-center space-x-2"> + <SidebarMenuItem className="flex items-center space-x-2" role="banner"> <div className="flex aspect-square size-9 items-center justify-center rounded-lg text-sidebar-primary-foreground"> - <PixelIcon size={46} color="currentColor" /> + <PixelIcon size={46} color="currentColor" aria-hidden="true" /> </div> - <div className="grid flex-1 text-left text-lg leading-tight"> + <div className="grid flex-1 text-left text-lg leading-tight" aria-label="PixelGenius Dashboard"> <span className="truncate font-semibold">PixelGenius</span> </div>
12-25: Consider adding click handler for logo navigationThe logo typically serves as a navigation element to the home/dashboard page. Consider adding a click handler or wrapping it in a link.
Example implementation:
import { Link } from 'next/link'; export function LogoIconSwitcher() { return ( <SidebarMenu> <Link href="/dashboard"> <SidebarMenuItem className="flex items-center space-x-2"> {/* ... existing content ... */} </SidebarMenuItem> </Link> </SidebarMenu> ); }packages/ui/src/globals.css (1)
Line range hint
162-171: Consider accessibility implications of hidden scrollbarsThe
.no-scrollbarutility class is well-implemented with proper cross-browser support. However, please ensure that:
- The sidebar maintains clear visual indicators of scrollable content
- Touch and keyboard navigation are properly supported
- The implementation includes appropriate aria attributes for screen readers
🧰 Tools
🪛 Biome (1.9.4)
[error] 54-54: Unexpected value or character.
Expected one of:
(parse)
packages/icons/src/components/menu.tsx (1)
3-4: Fix the spread operator variable nameThe spread operator variable name
resPropsshould follow the common naming convention ofrestPropsfor better readability and consistency.-const { size = 24, color = "currentColor", ...resProps } = props; +const { size = 24, color = "currentColor", ...restProps } = props;packages/ui/src/components/ui/sidebar.tsx (2)
93-97: Remove unnecessaryreturnstatement intoggleSidebarfunctionThe
returnstatement is unnecessary sincesetOpenMobileandsetOpendo not return any value.Suggested change:
- const toggleSidebar = React.useCallback(() => { - return isMobile - ? setOpenMobile((open) => !open) - : setOpen((open) => !open); - }, [isMobile, setOpen, setOpenMobile]); + const toggleSidebar = React.useCallback(() => { + if (isMobile) { + setOpenMobile((open) => !open); + } else { + setOpen((open) => !open); + } + }, [isMobile, setOpen, setOpenMobile]);
552-586: Avoid mutating thetooltipprop inSidebarMenuButtonMutating props directly can lead to unexpected behavior. Instead, create a new variable to store the modified tooltip properties.
Suggested change:
function SidebarMenuButton(...) { ... - if (typeof tooltip === "string") { - tooltip = { - children: tooltip, - }; - } + const tooltipProps = typeof tooltip === "string" ? { children: tooltip } : tooltip; return ( <Tooltip> <TooltipTrigger asChild>{button}</TooltipTrigger> <TooltipContent side="right" align="center" hidden={state !== "collapsed" || isMobile} - {...tooltip} + {...tooltipProps} /> </Tooltip> ); }packages/ui/src/hooks/use-mobile.tsx (1)
1-4: Consider moving MOBILE_BREAKPOINT to a shared constants fileThe breakpoint value could be reused across components. Consider moving it to a shared constants file for better maintainability.
apps/core/app/dashboard/_compnents/nav-main.tsx (1)
9-17: Consider using TypeScript interface for propsExtract the items type into a separate interface for better reusability and documentation.
+interface NavItem { + title: string; + url: string; + icon: React.ElementType; +} + -export function NavMain({ - items, -}: { - items: { - title: string; - url: string; - icon: React.ElementType; - }[]; -}) { +export function NavMain({ items }: { items: NavItem[] }) {packages/ui/src/components/ui/tooltip.tsx (1)
21-24: Consider extracting animation classesThe long className string could be moved to a constant or utility function for better maintainability.
+const tooltipAnimationClasses = + "z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm " + + "text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 " + + "data-[state=closed]:animate-out data-[state=closed]:fade-out-0 " + + "data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 " + + "data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 " + + "data-[side=top]:slide-in-from-bottom-2" className={cn( - "z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", + tooltipAnimationClasses, className )}apps/core/app/dashboard/_compnents/app-sidebar.tsx (1)
16-21: Consider using TypeScript interfaces for navigation itemsThe navigation data structure would benefit from type safety. Consider defining an interface for the navigation items.
interface NavItem { title: string; url: string; icon: React.ComponentType; } const data: { navMain: NavItem[] } = { navMain: [ // ... existing items ], };packages/ui/tailwind.config.ts (1)
71-85: Inconsistent naming convention in color paletteThe
Graycolor object uses PascalCase while other color objects use camelCase. Consider maintaining consistency.- Gray: { + gray: {apps/core/app/dashboard/page.tsx (7)
Line range hint
1-8: Consider restructuring icon imports for better maintainabilityThe relative paths (
../../../../packages/icons/src/components/) for icon imports are brittle and could break during refactoring. Consider using aliased imports from the@repo/iconspackage instead.-import Serchinicon from "../../../../packages/icons/src/components/serach"; -import Bellicon from "../../../../packages/icons/src/components/bell"; -import Chevrondownicon from "../../../../packages/icons/src/components/chevron-down"; -import Infosquareroundedicon from "../../../../packages/icons/src/components/info-square-rounded"; +import { SearchIcon, BellIcon, ChevronDownIcon, InfoSquareRoundedIcon } from "@repo/icons";
Line range hint
131-137: Fix tab default value mismatchThe
defaultValue="account"doesn't match any of the available tab values ("General", "Images", "file", "Admin Chat"). This could cause the tabs to render without a selected tab initially.- <Tabs defaultValue="account" className="w-full"> + <Tabs defaultValue="General" className="w-full">
Line range hint
144-152: Improve form field labels and placeholdersThe duplicate product name fields and generic "Search" placeholders reduce form clarity:
- The second product name field appears to be a duplicate
- The placeholder "Search" doesn't match the input's purpose
<div className="flex gap-2 w-full"> <div className="w-1/2"> <Label className="text-xs font-normal">Product Name</Label> - <Input placeholder="Search" className="" /> + <Input placeholder="Enter product name" className="" /> </div> <div className="w-1/2"> - <Label>Product Name</Label> - <Input placeholder="Search" className="" /> + <Label>Product SKU</Label> + <Input placeholder="Enter product SKU" className="" /> </div> </div>
Line range hint
186-188: Add price input validationThe price input lacks validation to ensure valid numeric values and prevent negative prices.
<div className="pb-4"> <Label className="text-xs font-normal text-gray-500">Price</Label> - <Input placeholder="0" /> + <Input + type="number" + min="0" + step="0.01" + placeholder="0.00" + pattern="^\d*\.?\d*$" + /> </div>
Line range hint
264-284: Extract duplicate upload UI into reusable componentThe file upload UI is duplicated across three sections with minimal differences. Consider extracting this into a reusable component.
type UploadSectionProps = { title: string; description: string; acceptedFormats?: string; maxSize?: string; required?: boolean; }; const FileUploadSection = ({ title, description, acceptedFormats = "JPEG", maxSize = "3MB", required = true }: UploadSectionProps) => ( <div> <div className="flex items-center justify-between pb-2"> <p className="text-xl font-bold">{title}</p> {required && ( <div className="flex gap-1 text-gray-500"> <Infocircleicon height={18} width={18} color="white" /> <p className="text-sm font-light">Required</p> </div> )} </div> {/* Rest of the upload UI */} </div> );Also applies to: 285-305, 306-326
Line range hint
391-402: Use theme variables for button stylingThe submit button uses a hardcoded color class (
bg-primary-600). Consider using the variant system consistently for both buttons.<div className="flex gap-2"> <div> <Button variant="outline" className="px-4 py-2"> Save as draft </Button> </div> <div> - <Button variant="secondary" className="px-4 py-2 bg-primary-600"> + <Button variant="primary" className="px-4 py-2"> Submit for review </Button> </div> </div>
Line range hint
27-130: Remove commented out codeLarge blocks of commented code should be removed as they can be retrieved from version control if needed. This improves code readability and maintenance.
apps/core/app/dashboard/_compnents/navbar-dashboard.tsx (3)
5-5: Consider using responsive height utilitiesUsing a fixed height might cause issues on different screen sizes. Consider using responsive height utilities or min-height instead.
- <nav className="h-[76px] bg-sidebar flex items-center justify-between px-2"> + <nav className="min-h-[76px] h-auto bg-sidebar flex items-center px-2">
6-8: Remove unnecessary div wrapperThe div wrapper around SidebarTrigger is redundant since it doesn't provide any styling or functionality.
- <div> <SidebarTrigger /> - </div>
3-3: Add TypeScript interface for component propsEven though the component doesn't currently accept props, it's good practice to explicitly define the interface for future maintainability.
+interface NavbarDashboardProps {} + -const NavbarDashboard = () => { +const NavbarDashboard: React.FC<NavbarDashboardProps> = () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
apps/core/public/images/clip-path-group-sidebar.svgis excluded by!**/*.svgbun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (15)
apps/core/app/dashboard/_compnents/app-sidebar.tsx(1 hunks)apps/core/app/dashboard/_compnents/logo-icon-switcher.tsx(1 hunks)apps/core/app/dashboard/_compnents/nav-main.tsx(1 hunks)apps/core/app/dashboard/_compnents/navbar-dashboard.tsx(1 hunks)apps/core/app/dashboard/_compnents/sidebar.tsx(0 hunks)apps/core/app/dashboard/layout.tsx(1 hunks)apps/core/app/dashboard/page.tsx(1 hunks)packages/icons/src/components/menu.tsx(1 hunks)packages/ui/package.json(3 hunks)packages/ui/src/components/ui/sidebar.tsx(1 hunks)packages/ui/src/components/ui/tooltip.tsx(1 hunks)packages/ui/src/globals.css(3 hunks)packages/ui/src/hooks/use-mobile.tsx(1 hunks)packages/ui/tailwind.config.ts(1 hunks)packages/ui/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/core/app/dashboard/_compnents/sidebar.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ui/src/globals.css
[error] 54-54: Unexpected value or character.
Expected one of:
(parse)
🔇 Additional comments (6)
packages/ui/src/globals.css (2)
102-109: Well-structured sidebar theming variables
Good implementation of sidebar-specific CSS variables:
- Consistent naming convention
- HSL color format for better color manipulation
- Complete set of variables for all necessary UI elements
- Proper contrast ratios maintained
137-144: Verify dark theme contrast ratios
The dark theme sidebar variables look good with appropriate color adjustments. Please verify that all color combinations meet WCAG contrast requirements, especially:
- Text on primary/accent backgrounds
- Primary/accent elements on the sidebar background
✅ Verification successful
Let me verify the color contrast ratios by converting the HSL values to hex and checking them against WCAG standards.
Based on the HSL values from the dark theme:
- Sidebar background (240, 11%, 14%) to foreground (240, 4.8%, 95.9%): This creates a high contrast between dark background and light text
- Primary color (224.3, 76.3%, 48%) to white foreground (0, 0%, 100%): Blue primary with white text maintains good contrast
- Accent (240, 10%, 11%) to accent foreground (240, 4.8%, 95.9%): Very high contrast between the darker accent and light text
Dark theme contrast ratios meet accessibility standards
The color combinations in the dark theme maintain excellent contrast ratios:
- The main text contrast is achieved through a very dark background (14% lightness) with very light text (95.9% lightness)
- The primary interactive elements use white text (100% lightness) on a medium-bright blue background (48% lightness)
- The accent sections use the same high-contrast approach as the main theme with 11% vs 95.9% lightness
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Note: This is a reminder to verify contrast ratios
# You can use tools like https://webaim.org/resources/contrastchecker/
# with the following color pairs:
# Background (#1c1c24) with Foreground (#f4f4f5)
# Primary (#4B6BFB) with Primary Foreground (#ffffff)
# Accent (#1a1a21) with Accent Foreground (#f4f4f5)
echo "Please verify contrast ratios for the dark theme color combinations"
Length of output: 144
Script:
#!/bin/bash
# Extract and convert HSL values to hex using a simple grep
echo "Sidebar Background (--sidebar-background: 240 11% 14%):"
echo "Sidebar Foreground (--sidebar-foreground: 240 4.8% 95.9%):"
echo "Sidebar Primary (--sidebar-primary: 224.3 76.3% 48%):"
echo "Sidebar Primary Foreground (--sidebar-primary-foreground: 0 0% 100%):"
echo "Sidebar Accent (--sidebar-accent: 240 10% 11%):"
echo "Sidebar Accent Foreground (--sidebar-accent-foreground: 240 4.8% 95.9%):"
# Let's also check if there are any other theme-related color definitions
rg "color|background|foreground" packages/ui/src/globals.css
Length of output: 1930
packages/icons/src/components/menu.tsx (1)
3-31: Add ARIA attributes for accessibility
Consider adding appropriate ARIA attributes to make the icon more accessible, especially since it's used in a navigation context.
const MenuIcon = (props: IconProps) => {
- const { size = 24, color = "currentColor", ...resProps } = props;
+ const { size = 24, color = "currentColor", title = "Menu", ...resProps } = props;
return (
<svg
width={size}
height={size}
viewBox="0 0 24 24"
fill="none"
xmlns="http://www.w3.org/2000/svg"
+ role="img"
+ aria-labelledby="menuIconTitle"
{...resProps}
>
+ <title id="menuIconTitle">{title}</title>Let's verify the icon's usage across the application:
packages/ui/package.json (1)
33-33: Verify compatibility with updated dependencies
Several dependencies have been updated:
- @radix-ui/react-dialog: ^1.1.1 → ^1.1.2
- @radix-ui/react-slot: ^1.0.2 → ^1.1.0
- @radix-ui/react-tooltip: ^1.1.4 (new)
- class-variance-authority: ^0.7.0 → ^0.7.1
- lucide-react: ^0.395.0 → ^0.468.0
Please ensure these updates don't introduce breaking changes.
Also applies to: 46-46, 49-49, 52-52, 59-59
packages/ui/tailwind.config.ts (1)
143-152: Verify CSS variable definitions for sidebar theme
The new sidebar color system relies on CSS variables. Ensure these variables are defined in your CSS files.
✅ Verification successful
CSS variables for sidebar theme are properly defined
All required CSS variables for the sidebar theme are correctly defined in packages/ui/src/globals.css with both light and dark theme values:
- Light theme variables (first set)
- Dark theme variables (second set)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for CSS variable definitions
rg --type css "(?:--sidebar-(?:background|foreground|primary|primary-foreground|accent|accent-foreground|border|ring))"
Length of output: 1214
apps/core/app/dashboard/_compnents/navbar-dashboard.tsx (1)
5-9: Verify mobile responsiveness
Since this is a dashboard navigation component, we should ensure it works well on mobile devices.
✅ Verification successful
Let me check the content of app-sidebar.tsx to understand the mobile responsiveness implementation and its relationship with the navbar.
Let me check the Sidebar component from the UI package to understand the mobile responsiveness implementation since it's being used in both navbar and sidebar.
Based on the findings, I can now provide a final response:
Mobile responsiveness implementation looks adequate
The navbar implementation is properly integrated with the mobile-responsive sidebar system:
- Uses
SidebarTriggercomponent which is connected to the sidebar's mobile state - The sidebar itself (
AppSidebar) has mobile-specific handling throughuseSidebarhook'sisMobilestate - Navbar has a flexible layout with
flexandjustify-betweenclasses that adapt well to different screen sizes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for mobile-specific styles or components
rg -l "sm:|md:|lg:|xl:|2xl:" apps/core/app/dashboard/
# Check for any mobile-related hooks or utilities
rg -l "useMedia|useMobile|isMobile" apps/core/app/dashboard/
Length of output: 169
Script:
#!/bin/bash
# Check the content of app-sidebar.tsx to understand mobile implementation
cat apps/core/app/dashboard/_compnents/app-sidebar.tsx
# Also check for any responsive utility components or hooks in the project
rg -l "useBreakpoint|useResponsive|useWindowSize" apps/core/
Length of output: 2300
Script:
#!/bin/bash
# Check the Sidebar component implementation from UI package
fd -e tsx -e ts "sidebar" packages/ui/components/ --exec cat {}
# Also check if there are any mobile-specific styles in the navbar file
cat apps/core/app/dashboard/_compnents/navbar-dashboard.tsx
Length of output: 529
| SidebarMenuItem, | ||
| } from "@repo/ui/components/sidebar"; | ||
| //logo | ||
| import PixelIcon from "@repo/icons/pxiel"; |
There was a problem hiding this comment.
Fix typo in import path
There's a typo in the import path: @repo/icons/pxiel should be @repo/icons/pixel.
-import PixelIcon from "@repo/icons/pxiel";
+import PixelIcon from "@repo/icons/pixel";📝 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 PixelIcon from "@repo/icons/pxiel"; | |
| import PixelIcon from "@repo/icons/pixel"; |
| import * as React from "react"; | ||
|
|
||
| //ui | ||
| import { | ||
| SidebarMenu, | ||
| SidebarMenuButton, | ||
| SidebarMenuItem, | ||
| } from "@repo/ui/components/sidebar"; | ||
| //logo | ||
| import PixelIcon from "@repo/icons/pxiel"; | ||
|
|
||
| export function LogoIconSwitcher() { | ||
| return ( | ||
| <SidebarMenu> | ||
| <SidebarMenuItem className="flex items-center space-x-2"> | ||
| <div className="flex aspect-square size-9 items-center justify-center rounded-lg text-sidebar-primary-foreground"> | ||
| <PixelIcon size={46} color="currentColor" /> | ||
| </div> | ||
| <div className="grid flex-1 text-left text-lg leading-tight"> | ||
| <span className="truncate font-semibold">PixelGenius</span> | ||
| </div> | ||
| </SidebarMenuItem> | ||
| </SidebarMenu> | ||
| ); | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Rename the "_compnents" directory to "_components" to fix the typo
The directory name "_compnents" is indeed a typo as:
- It's the only instance of this naming pattern in the dashboard directory
- The standard Next.js convention is to use "_components" for component folders
- The misspelling could cause confusion and maintainability issues
🔗 Analysis chain
Verify the folder structure
The component is located in a folder named "_compnents" which appears to be a typo of "_components".
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if this is a consistent pattern or a typo in the folder structure
fd -t d "_compnents|_components" apps/core/app/dashboard/
Length of output: 95
packages/ui/src/globals.css
Outdated
|
|
||
| /* Success */ | ||
| --success-50: #f0fdf4; | ||
| --gray-950: #282828 /* Success */ --success-50: #f0fdf4; |
There was a problem hiding this comment.
Fix CSS syntax error
There's a syntax error where the Success comment and variable declaration are on the same line. This needs to be split into separate lines.
Apply this fix:
- --gray-950: #282828 /* Success */ --success-50: #f0fdf4;
+ --gray-950: #282828; /* Success */
+ --success-50: #f0fdf4;📝 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.
| --gray-950: #282828 /* Success */ --success-50: #f0fdf4; | |
| --gray-950: #282828; /* Success */ | |
| --success-50: #f0fdf4; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 54-54: Unexpected value or character.
Expected one of:
(parse)
| <g clip-path="url(#clip0_3569_11211)"> | ||
| <path | ||
| d="M4 6H20M0 12H16M4 18H20" | ||
| stroke="white" | ||
| stroke-width="2" | ||
| stroke-linecap="round" | ||
| stroke-linejoin="round" | ||
| /> |
There was a problem hiding this comment.
Fix SVG path attributes and color handling
- The stroke color is hardcoded to "white", ignoring the
colorprop - SVG attributes should use camelCase in React
- The path coordinates might cause clipping issues
- <g clip-path="url(#clip0_3569_11211)">
+ <g clipPath="url(#clip0_3569_11211)">
<path
- d="M4 6H20M0 12H16M4 18H20"
+ d="M4 6H20M4 12H20M4 18H20"
stroke="white"
- stroke-width="2"
- stroke-linecap="round"
- stroke-linejoin="round"
+ stroke={color}
+ strokeWidth="2"
+ strokeLinecap="round"
+ strokeLinejoin="round"
/>📝 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.
| <g clip-path="url(#clip0_3569_11211)"> | |
| <path | |
| d="M4 6H20M0 12H16M4 18H20" | |
| stroke="white" | |
| stroke-width="2" | |
| stroke-linecap="round" | |
| stroke-linejoin="round" | |
| /> | |
| <g clipPath="url(#clip0_3569_11211)"> | |
| <path | |
| d="M4 6H20M4 12H20M4 18H20" | |
| stroke={color} | |
| strokeWidth="2" | |
| strokeLinecap="round" | |
| strokeLinejoin="round" | |
| /> |
There was a problem hiding this comment.
@mrbadri Here’s the updated code snippet with the camelCase attribute names and the stroke color bound to the color prop:
import { IconProps } from "../types/types";
const MenuIcon = (props: IconProps) => {
const { size = 24, color = "currentColor", ...resProps } = props;
return (
<svg
width={size}
height={size}
viewBox="0 0 24 24"
fill="none"
xmlns="http://www.w3.org/2000/svg"
{...resProps}
>
<g clipPath="url(#clip0_3569_11211)">
<path
d="M4 6H20M4 12H20M4 18H20"
stroke={color}
strokeWidth="2"
strokeLinecap="round"
strokeLinejoin="round"
/>
</g>
</svg>
);
};
export default MenuIcon;| >(({ className, showIcon = false, ...props }, ref) => { | ||
| // Random width between 50 to 90%. | ||
| const width = React.useMemo(() => { | ||
| return `${Math.floor(Math.random() * 40) + 50}%`; | ||
| }, []); |
There was a problem hiding this comment.
Avoid using random values in render to prevent hydration mismatches
Generating random values during render can cause inconsistencies between server and client rendering. Consider using a ref or state to store the value.
Suggested change:
const SidebarMenuSkeleton = React.forwardRef(..., ({ ...props }, ref) => {
- // Random width between 50 to 90%.
- const width = React.useMemo(() => {
- return `${Math.floor(Math.random() * 40) + 50}%`;
- }, []);
+ // Use a ref to store the width to ensure consistency between renders.
+ const widthRef = React.useRef(`${Math.floor(Math.random() * 40) + 50}%`);
+ const width = widthRef.current;
return (
...
<Skeleton
className="h-4 flex-1 max-w-[--skeleton-width]"
data-sidebar="menu-skeleton-text"
style={
{
"--skeleton-width": width,
} as React.CSSProperties
}
/>
...
);
});📝 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.
| >(({ className, showIcon = false, ...props }, ref) => { | |
| // Random width between 50 to 90%. | |
| const width = React.useMemo(() => { | |
| return `${Math.floor(Math.random() * 40) + 50}%`; | |
| }, []); | |
| >(({ className, showIcon = false, ...props }, ref) => { | |
| // Use a ref to store the width to ensure consistency between renders. | |
| const widthRef = React.useRef(`${Math.floor(Math.random() * 40) + 50}%`); | |
| const width = widthRef.current; |
| <SidebarMenuItem | ||
| className={`${pathname === item.url ? "bg-sidebar-accent text-secondary-100 rounded-md text-[#6751D6]" : ""}`} | ||
| key={item.title} | ||
| > | ||
| <SidebarMenuButton asChild> | ||
| <a className="py-6" href={item.url}> | ||
| <item.icon /> | ||
| <span>{item.title}</span> | ||
| </a> | ||
| </SidebarMenuButton> |
There was a problem hiding this comment.
Enhance accessibility and UX
The navigation items need accessibility improvements and better active state handling.
Apply these changes:
<SidebarMenuItem
- className={`${pathname === item.url ? "bg-sidebar-accent text-secondary-100 rounded-md text-[#6751D6]" : ""}`}
+ className={cn(
+ "transition-colors duration-200",
+ pathname === item.url && "bg-sidebar-accent text-secondary-100 rounded-md text-[#6751D6]"
+ )}
key={item.title}
>
<SidebarMenuButton asChild>
- <a className="py-6" href={item.url}>
+ <a
+ className="py-6"
+ href={item.url}
+ aria-current={pathname === item.url ? 'page' : undefined}
+ role="menuitem"
+ >
<item.icon />
- <span>{item.title}</span>
+ <span className="sr-only md:not-sr-only">{item.title}</span>
</a>
</SidebarMenuButton>
</SidebarMenuItem>📝 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.
| <SidebarMenuItem | |
| className={`${pathname === item.url ? "bg-sidebar-accent text-secondary-100 rounded-md text-[#6751D6]" : ""}`} | |
| key={item.title} | |
| > | |
| <SidebarMenuButton asChild> | |
| <a className="py-6" href={item.url}> | |
| <item.icon /> | |
| <span>{item.title}</span> | |
| </a> | |
| </SidebarMenuButton> | |
| <SidebarMenuItem | |
| className={cn( | |
| "transition-colors duration-200", | |
| pathname === item.url && "bg-sidebar-accent text-secondary-100 rounded-md text-[#6751D6]" | |
| )} | |
| key={item.title} | |
| > | |
| <SidebarMenuButton asChild> | |
| <a | |
| className="py-6" | |
| href={item.url} | |
| aria-current={pathname === item.url ? 'page' : undefined} | |
| role="menuitem" | |
| > | |
| <item.icon /> | |
| <span className="sr-only md:not-sr-only">{item.title}</span> | |
| </a> | |
| </SidebarMenuButton> |
There was a problem hiding this comment.
please used cn function
link => next js
There was a problem hiding this comment.
@mrbadri, here’s an updated snippet that applies the cn function and adds Next.js-friendly accessibility attributes:
TypeScript JSX code:
import { cn } from "@/lib/utils"; // or wherever your cn helper is defined
<SidebarMenuItem
className={cn(
"transition-colors duration-200",
pathname === item.url && "bg-sidebar-accent text-secondary-100 rounded-md text-[#6751D6]"
)}
key={item.title}
{item.title}
| const TooltipContent = React.forwardRef< | ||
| React.ElementRef<typeof TooltipPrimitive.Content>, | ||
| React.ComponentPropsWithoutRef<typeof TooltipPrimitive.Content> | ||
| >(({ className, sideOffset = 4, ...props }, ref) => ( | ||
| <TooltipPrimitive.Content | ||
| ref={ref} | ||
| sideOffset={sideOffset} | ||
| className={cn( | ||
| "z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| )) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding default accessibility props
The tooltip implementation could benefit from some default accessibility properties.
Apply these changes:
const TooltipContent = React.forwardRef<
React.ElementRef<typeof TooltipPrimitive.Content>,
React.ComponentPropsWithoutRef<typeof TooltipPrimitive.Content>
->(({ className, sideOffset = 4, ...props }, ref) => (
+>(({
+ className,
+ sideOffset = 4,
+ delayDuration = 200,
+ skipDelayDuration = 300,
+ ...props
+ }, ref) => (
<TooltipPrimitive.Content
ref={ref}
sideOffset={sideOffset}
+ delayDuration={delayDuration}
+ skipDelayDuration={skipDelayDuration}
className={cn(
"z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2",
className
)}
+ aria-live="polite"
{...props}
/>
))📝 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.
| const TooltipContent = React.forwardRef< | |
| React.ElementRef<typeof TooltipPrimitive.Content>, | |
| React.ComponentPropsWithoutRef<typeof TooltipPrimitive.Content> | |
| >(({ className, sideOffset = 4, ...props }, ref) => ( | |
| <TooltipPrimitive.Content | |
| ref={ref} | |
| sideOffset={sideOffset} | |
| className={cn( | |
| "z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", | |
| className | |
| )} | |
| {...props} | |
| /> | |
| )) | |
| const TooltipContent = React.forwardRef< | |
| React.ElementRef<typeof TooltipPrimitive.Content>, | |
| React.ComponentPropsWithoutRef<typeof TooltipPrimitive.Content> | |
| >(({ | |
| className, | |
| sideOffset = 4, | |
| delayDuration = 200, | |
| skipDelayDuration = 300, | |
| ...props | |
| }, ref) => ( | |
| <TooltipPrimitive.Content | |
| ref={ref} | |
| sideOffset={sideOffset} | |
| delayDuration={delayDuration} | |
| skipDelayDuration={skipDelayDuration} | |
| className={cn( | |
| "z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", | |
| className | |
| )} | |
| aria-live="polite" | |
| {...props} | |
| /> | |
| )) |
| {isMobile ? ( | ||
| <Image | ||
| src="images/clip-path-group.svg" | ||
| width={290} | ||
| height={117} | ||
| alt="Picture of space" | ||
| /> | ||
| ) : open ? ( | ||
| <Image | ||
| src="images/clip-path-group.svg" | ||
| width={280} | ||
| height={117} | ||
| alt="Picture of space" | ||
| /> | ||
| ) : ( | ||
| <Image | ||
| src="images/clip-path-group-sidebar.svg" | ||
| width={49} | ||
| height={90} | ||
| alt="Picture of space" | ||
| /> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize image handling for better performance and maintainability
The current implementation duplicates the image logic with different dimensions. Consider refactoring to reduce duplication and improve maintainability.
- {isMobile ? (
- <Image
- src="images/clip-path-group.svg"
- width={290}
- height={117}
- alt="Picture of space"
- />
- ) : open ? (
- <Image
- src="images/clip-path-group.svg"
- width={280}
- height={117}
- alt="Picture of space"
- />
- ) : (
- <Image
- src="images/clip-path-group-sidebar.svg"
- width={49}
- height={90}
- alt="Picture of space"
- />
- )}
+ <Image
+ src={!open && !isMobile ? "images/clip-path-group-sidebar.svg" : "images/clip-path-group.svg"}
+ width={!open && !isMobile ? 49 : isMobile ? 290 : 280}
+ height={!open && !isMobile ? 90 : 117}
+ alt="Sidebar decoration"
+ priority
+ />📝 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.
| {isMobile ? ( | |
| <Image | |
| src="images/clip-path-group.svg" | |
| width={290} | |
| height={117} | |
| alt="Picture of space" | |
| /> | |
| ) : open ? ( | |
| <Image | |
| src="images/clip-path-group.svg" | |
| width={280} | |
| height={117} | |
| alt="Picture of space" | |
| /> | |
| ) : ( | |
| <Image | |
| src="images/clip-path-group-sidebar.svg" | |
| width={49} | |
| height={90} | |
| alt="Picture of space" | |
| /> | |
| )} | |
| <Image | |
| src={!open && !isMobile ? "images/clip-path-group-sidebar.svg" : "images/clip-path-group.svg"} | |
| width={!open && !isMobile ? 49 : isMobile ? 290 : 280} | |
| height={!open && !isMobile ? 90 : 117} | |
| alt="Sidebar decoration" | |
| priority | |
| /> |
| import LayoutDashboard from "../../../../../packages/icons/src/components/layout-dashboard"; | ||
| import BoxPackage from "../../../../../packages/icons/src/components/box-package"; | ||
| import ChartBarPopular from "../../../../../packages/icons/src/components/chart-bar-popular"; | ||
| import ReportMoney from "../../../../../packages/icons/src/components/report-money"; |
There was a problem hiding this comment.
💡 Codebase verification
Use @repo/icons path alias instead of relative paths
The codebase consistently uses the @repo/icons path alias for importing icons across multiple files. Update the imports to use this established pattern:
import LayoutDashboard from "@repo/icons/layout-dashboard";
import BoxPackage from "@repo/icons/box-package";
import ChartBarPopular from "@repo/icons/chart-bar-popular";
import ReportMoney from "@repo/icons/report-money";🔗 Analysis chain
Verify icon import paths
The relative imports for icons seem to traverse multiple directory levels. Consider using path aliases for better maintainability.
Let me check if there's a path alias configuration in the project that could be used instead of relative paths.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the icon files exist at the specified paths
for icon in "layout-dashboard" "box-package" "chart-bar-popular" "report-money"; do
if ! fd -t f "${icon}\.(tsx|ts)$" "packages/icons/src/components"; then
echo "Warning: Icon file ${icon} not found"
fi
done
Length of output: 795
Script:
#!/bin/bash
# Check for path alias configurations in tsconfig files
fd -t f "tsconfig.*json$" | xargs cat
# Also check for any existing usage of path aliases for icons
rg -g "*.{ts,tsx}" "@.*icons.*" -A 2
Length of output: 7789
There was a problem hiding this comment.
@mrbadri, here's the updated code with the path alias:
"use client";
import * as React from "react";
//components ui
import {
Sidebar,
SidebarContent,
SidebarFooter,
SidebarHeader,
SidebarRail,
useSidebar,
} from "@repo/ui/components/sidebar";
import Image from "next/image";
//icons
import LayoutDashboard from "@repo/icons/layout-dashboard";
import BoxPackage from "@repo/icons/box-package";
import ChartBarPopular from "@repo/icons/chart-bar-popular";
import ReportMoney from "@repo/icons/report-money";
// ... rest of the component code✏️ Learnings added
Learnt from: mrbadri
PR: pixel-genius/pixel-client#133
File: apps/core/app/dashboard/_compnents/app-sidebar.tsx:17-20
Timestamp: 2024-12-22T16:57:42.045Z
Learning: We always use '@repo/icons' path alias instead of relative paths for icons across the codebase for maintainability.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
|
||
| const NavbarDashboard = () => { | ||
| return ( | ||
| <nav className="h-[76px] bg-sidebar flex items-center justify-between px-2"> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add ARIA label for navigation accessibility
The navigation element should have an aria-label to improve accessibility for screen readers.
- <nav className="h-[76px] bg-sidebar flex items-center justify-between px-2">
+ <nav className="h-[76px] bg-sidebar flex items-center justify-between px-2" aria-label="Dashboard navigation">📝 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.
| <nav className="h-[76px] bg-sidebar flex items-center justify-between px-2"> | |
| <nav className="h-[76px] bg-sidebar flex items-center justify-between px-2" aria-label="Dashboard navigation"> |
There was a problem hiding this comment.
@mrbadri, I noticed your message seems incomplete. Can you clarify what specifically you’d like to add or change? Let me know, and I’ll be happy to assist!
There was a problem hiding this comment.
Actionable comments posted: 22
🧹 Nitpick comments (26)
apps/storybook/tsconfig.app.json (3)
3-8: Consider browser compatibility and build cache location.Two suggestions for consideration:
- The ES2020 target might be too modern for some browsers. Consider adding a browserslist configuration if you need to support older browsers.
- Storing build info in
node_modules/.tmpmight cause issues with clean builds. Consider moving it to a dedicated build or cache directory.
18-23: Consider additional type-safety rules.While the current linting configuration is good, consider adding these additional compiler options for enhanced type safety:
{ "compilerOptions": { // ... existing options ... "strict": true, + "strictNullChecks": true, + "strictFunctionTypes": true, + "strictPropertyInitialization": true, + "noImplicitAny": true, // ... remaining options ... } }🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: JSON standard does not allow comments.
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 22-22: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 22-22: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 22-22: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 22-22: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
25-25: Consider more specific include/exclude patterns.The current include pattern is very broad. Consider adding more specific patterns and exclusions:
{ // ... compiler options ... "include": ["src"], + "exclude": [ + "src/**/*.spec.ts", + "src/**/*.test.ts", + "src/**/__tests__/*" + ] }🧰 Tools
🪛 Biome (1.9.4)
[error] 25-25: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 25-25: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 25-25: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 25-26: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
turbo.json (1)
8-8: Consider updating related configuration files.Since you're caching Storybook builds, ensure your
.gitignoreand other configuration files are consistent with this setup.Consider these related configurations:
- Add
storybook-staticto.gitignoreif not already present- Update any CI/CD caching configurations to account for the Storybook build artifacts
- Consider adding a
cleanscript to remove storybook-static directory between buildsapps/storybook/README.md (1)
32-50: Add Storybook-specific ESLint rulesThe ESLint configuration example should include Storybook-specific rules and plugins.
// eslint.config.js import react from 'eslint-plugin-react' +import storybookPlugin from 'eslint-plugin-storybook' export default tseslint.config({ // Set the react version settings: { react: { version: '18.3' } }, plugins: { // Add the react plugin react, + storybook: storybookPlugin, }, rules: { // other rules... // Enable its recommended rules ...react.configs.recommended.rules, ...react.configs['jsx-runtime'].rules, + ...storybookPlugin.configs.recommended.rules, }, })apps/storybook/tsconfig.node.json (2)
3-3: Consider relocating the build info fileStoring build info in
node_modules/.tmpmight cause issues in CI/CD environments wherenode_modulesis often cached or cleaned. Consider moving it to a dedicated build or cache directory.- "tsBuildInfoFile": "./node_modules/.tmp/tsconfig.node.tsbuildinfo", + "tsBuildInfoFile": "./.cache/tsconfig.node.tsbuildinfo",
16-22: Consider adding Storybook-specific compiler optionsWhile the current linting rules are good, consider adding these Storybook-specific options:
"noUnusedLocals": true, "noUnusedParameters": true, "noFallthroughCasesInSwitch": true, - "noUncheckedSideEffectImports": true + "noUncheckedSideEffectImports": true, + "jsx": "react-jsx", + "types": ["node"]🧰 Tools
🪛 Biome (1.9.4)
[error] 16-16: JSON standard does not allow comments.
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 17-17: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 17-17: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 17-17: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 18-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 18-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 18-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 18-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 22-22: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
apps/storybook/eslint.config.js (1)
7-10: Consider adding more common ignore patternsWhile ignoring the
distdirectory is good, consider adding other common build and cache directories to the ignore patterns.export default tseslint.config( - { ignores: ['dist'] }, + { ignores: ['dist', 'node_modules', '.next', 'build', 'coverage'] }, { extends: [js.configs.recommended, ...tseslint.configs.recommended],apps/storybook/src/main.tsx (1)
7-9: Consider adding error boundaries for better error handling.To prevent the entire app from crashing due to component errors, consider wrapping the App component with an error boundary.
Example implementation:
// ErrorBoundary.tsx import { Component, ErrorInfo, ReactNode } from 'react'; interface Props { children: ReactNode; } interface State { hasError: boolean; } class ErrorBoundary extends Component<Props, State> { public state: State = { hasError: false }; public static getDerivedStateFromError(): State { return { hasError: true }; } public componentDidCatch(error: Error, errorInfo: ErrorInfo) { console.error('Uncaught error:', error, errorInfo); } public render() { if (this.state.hasError) { return <h1>Sorry.. there was an error</h1>; } return this.props.children; } } export default ErrorBoundary;Then use it in main.tsx:
root.render( <StrictMode> + <ErrorBoundary> <App /> + </ErrorBoundary> </StrictMode>, )apps/storybook/.storybook/preview.ts (1)
1-14: Consider adding theme configuration for consistent styling.For consistent styling across stories, especially for the sidebar component, consider adding theme configuration.
Example implementation:
// .storybook/ThemeDecorator.tsx import { Theme } from '../src/types/theme'; const themes: Record<Theme, object> = { light: { background: '#ffffff', text: '#000000', // Add other theme variables }, dark: { background: '#1a1a1a', text: '#ffffff', // Add other theme variables }, }; export const withTheme = (Story, context) => { const theme = context.parameters.theme || context.globals.theme; return ( <div style={themes[theme]}> <Story /> </div> ); };Then use it in preview.ts:
const preview: Preview = { parameters: { + themes: { + default: 'light', + list: [ + { name: 'light', class: 'light-theme', color: '#fff' }, + { name: 'dark', class: 'dark-theme', color: '#000' } + ] + }, // ... other parameters }, decorators: [ + withTheme, // ... other decorators ], };apps/storybook/.storybook/main.ts (1)
1-16: LGTM! Consider adding documentation addon.The Storybook configuration is well-structured with essential addons. Consider adding
@storybook/addon-docsfor enhanced documentation capabilities.addons: [ "@storybook/addon-onboarding", "@storybook/addon-essentials", "@chromatic-com/storybook", "@storybook/addon-interactions", + "@storybook/addon-docs", ],apps/storybook/src/stories/Header.stories.ts (2)
15-19: Consider adding more test scenarios for callback functions.While the basic callbacks are mocked, consider adding test scenarios to verify their behavior.
args: { - onLogin: fn(), - onLogout: fn(), - onCreateAccount: fn(), + onLogin: fn(() => console.log('Login clicked')), + onLogout: fn(() => console.log('Logout clicked')), + onCreateAccount: fn(() => console.log('Create account clicked')), },
25-31: Add more user scenarios.Consider adding stories for error states and different user roles.
export const ErrorState: Story = { args: { error: 'Authentication failed' }, }; export const AdminUser: Story = { args: { user: { name: 'Admin User', role: 'admin' }, }, };apps/storybook/src/stories/Page.stories.ts (2)
22-31: Enhance interaction test robustnessThe interaction test could be more comprehensive and include error handling:
play: async ({ canvasElement }) => { - const canvas = within(canvasElement); - const loginButton = canvas.getByRole('button', { name: /Log in/i }); - await expect(loginButton).toBeInTheDocument(); - await userEvent.click(loginButton); - await expect(loginButton).not.toBeInTheDocument(); - - const logoutButton = canvas.getByRole('button', { name: /Log out/i }); - await expect(logoutButton).toBeInTheDocument(); + try { + const canvas = within(canvasElement); + + // Test initial state + const loginButton = await canvas.findByRole('button', { name: /Log in/i }); + expect(loginButton).toBeInTheDocument(); + expect(canvas.queryByRole('button', { name: /Log out/i })).not.toBeInTheDocument(); + + // Test login interaction + await userEvent.click(loginButton); + expect(loginButton).not.toBeInTheDocument(); + + // Test logged-in state + const logoutButton = await canvas.findByRole('button', { name: /Log out/i }); + expect(logoutButton).toBeInTheDocument(); + + // Test logout interaction (optional) + await userEvent.click(logoutButton); + expect(await canvas.findByRole('button', { name: /Log in/i })).toBeInTheDocument(); + } catch (error) { + throw new Error(`Interaction test failed: ${error}`); + } },
6-13: Document story parametersAdd JSDoc comments to document the story configuration and parameters:
+/** + * Page component stories + * @description Demonstrates the Page component in different states + */ const meta = { title: 'Example/Page', component: Page, parameters: { - // More on how to position stories at: https://storybook.js.org/docs/configure/story-layout + /** Uses fullscreen layout to properly demonstrate the page component */ layout: 'fullscreen', }, } satisfies Meta<typeof Page>;apps/storybook/package.json (1)
41-45: Enhance ESLint configurationThe ESLint configuration could be more comprehensive for a TypeScript React project:
"eslintConfig": { "extends": [ - "plugin:storybook/recommended" + "plugin:storybook/recommended", + "plugin:@typescript-eslint/recommended", + "plugin:react/recommended", + "plugin:react-hooks/recommended" ] }apps/storybook/src/index.css (2)
1-14: Consider using CSS variables for better maintainabilityExtract colors and other repeated values into CSS variables for better maintainability and theme consistency.
:root { font-family: Inter, system-ui, Avenir, Helvetica, Arial, sans-serif; line-height: 1.5; font-weight: 400; + + /* Colors */ + --color-primary: #646cff; + --color-primary-hover: #535bf2; + --color-text: rgba(255, 255, 255, 0.87); + --color-background: #242424; + --color-button-bg: #1a1a1a; color-scheme: light dark; - color: rgba(255, 255, 255, 0.87); - background-color: #242424; + color: var(--color-text); + background-color: var(--color-background);
38-55: Enhance button transitions for smoother interactionsThe button's transition only affects border-color. Consider including other properties for smoother state changes.
button { /* ... existing properties ... */ - transition: border-color 0.25s; + transition: border-color 0.25s, background-color 0.25s, transform 0.25s; } button:hover { border-color: #646cff; + transform: translateY(-1px); }apps/storybook/src/stories/Button.stories.ts (1)
17-19: Enhance component documentation with comprehensive argTypesThe current argTypes configuration only includes backgroundColor. Consider adding documentation for all props, especially size options.
argTypes: { backgroundColor: { control: 'color' }, + size: { + control: 'select', + options: ['small', 'medium', 'large'], + description: 'Size variant of the button', + table: { + defaultValue: { summary: 'medium' }, + }, + }, + primary: { + control: 'boolean', + description: 'Primary variant of the button', + table: { + defaultValue: { summary: false }, + }, + }, },apps/storybook/src/stories/Header.tsx (2)
40-53: Add loading states for authentication actionsThe buttons should reflect loading states during authentication operations to improve user experience.
{user ? ( <> <span className="welcome"> Welcome, <b>{user.name}</b>! </span> - <Button size="small" onClick={onLogout} label="Log out" /> + <Button + size="small" + onClick={async () => { + try { + setIsLoading(true); + await onLogout(); + } finally { + setIsLoading(false); + } + }} + label={isLoading ? "Logging out..." : "Log out"} + disabled={isLoading} + /> </> ) : ( // Similar changes for login and signup buttons )}
37-37: Externalize hard-coded stringsConsider moving hard-coded strings to a localization system for better maintainability and internationalization support.
- <h1>Acme</h1> + <h1>{t('header.title')}</h1>apps/storybook/src/stories/Page.tsx (1)
6-8: Consider using a more comprehensive User typeThe
Usertype is quite minimal with only anameproperty. Consider expanding it to include other common user properties that might be needed in the future.type User = { name: string; + id?: string; + email?: string; + avatar?: string; };apps/storybook/src/stories/Configure.mdx (4)
18-32: Optimize RightArrow component implementationThe inline styles in the SVG component could be moved to CSS for better maintainability.
+const arrowStyles = { + marginLeft: '4px', + display: 'inline-block', + shapeRendering: 'inherit', + verticalAlign: 'middle', + fill: 'currentColor', + 'path fill': 'currentColor' +}; -export const RightArrow = () => <svg +export const RightArrow = () => <svg viewBox="0 0 14 14" width="8px" height="14px" - style={{ - marginLeft: '4px', - display: 'inline-block', - shapeRendering: 'inherit', - verticalAlign: 'middle', - fill: 'currentColor', - 'path fill': 'currentColor' - }} + style={arrowStyles}
199-199: Fix grammatical error in textThere's a grammatical error in the tutorials description.
- <p>Follow guided walkthroughs on for key workflows.</p> + <p>Follow guided walkthroughs for key workflows.</p>
208-364: Consider extracting styles to a separate CSS fileThe inline styles are quite extensive and would be better maintained in a separate CSS file. This would improve readability and allow for better CSS organization.
Consider moving these styles to a new file
configure.cssand importing it at the top of the MDX file.
1-16: Consider using path aliases for importsThe large number of asset imports could be simplified using path aliases.
Consider configuring path aliases in your build setup:
-import Github from "./assets/github.svg"; -import Discord from "./assets/discord.svg"; +import Github from "@assets/github.svg"; +import Discord from "@assets/discord.svg";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (26)
apps/storybook/%SystemDrive%/ProgramData/Microsoft/Windows/Caches/cversions.2.dbis excluded by!**/*.dbapps/storybook/%SystemDrive%/ProgramData/Microsoft/Windows/Caches/{6AF0698E-D558-4F6E-9B3C-3716689AF493}.2.ver0x0000000000000001.dbis excluded by!**/*.dbapps/storybook/%SystemDrive%/ProgramData/Microsoft/Windows/Caches/{7D350FE0-5F96-4752-A63A-05699029D904}.2.ver0x0000000000000001.dbis excluded by!**/*.dbapps/storybook/%SystemDrive%/ProgramData/Microsoft/Windows/Caches/{DDF571F2-BE98-426D-8288-1A9A39C3FDA2}.2.ver0x0000000000000001.dbis excluded by!**/*.dbapps/storybook/.cache/storybook/default/dev-server/storybook-0c9aba2732f8798ca2ae104c09855accis excluded by!**/.cache/**apps/storybook/.cache/storybook/default/dev-server/storybook-21d6f40cfb511982e4424e0e250a9557is excluded by!**/.cache/**apps/storybook/.cache/storybook/default/dev-server/storybook-e35710dcf9c6d4f837d5dae15e56122eis excluded by!**/.cache/**apps/storybook/package-lock.jsonis excluded by!**/package-lock.jsonapps/storybook/public/vite.svgis excluded by!**/*.svgapps/storybook/src/assets/react.svgis excluded by!**/*.svgapps/storybook/src/stories/assets/accessibility.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/accessibility.svgis excluded by!**/*.svgapps/storybook/src/stories/assets/addon-library.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/assets.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/context.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/discord.svgis excluded by!**/*.svgapps/storybook/src/stories/assets/docs.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/figma-plugin.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/github.svgis excluded by!**/*.svgapps/storybook/src/stories/assets/share.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/styling.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/testing.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/theming.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/tutorials.svgis excluded by!**/*.svgapps/storybook/src/stories/assets/youtube.svgis excluded by!**/*.svgbun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (29)
.gitignore(1 hunks).hintrc(1 hunks)apps/storybook/.gitignore(1 hunks)apps/storybook/.storybook/main.ts(1 hunks)apps/storybook/.storybook/preview.ts(1 hunks)apps/storybook/README.md(1 hunks)apps/storybook/eslint.config.js(1 hunks)apps/storybook/index.html(1 hunks)apps/storybook/package.json(1 hunks)apps/storybook/src/App.css(1 hunks)apps/storybook/src/App.tsx(1 hunks)apps/storybook/src/index.css(1 hunks)apps/storybook/src/main.tsx(1 hunks)apps/storybook/src/stories/Button.stories.ts(1 hunks)apps/storybook/src/stories/Button.tsx(1 hunks)apps/storybook/src/stories/Configure.mdx(1 hunks)apps/storybook/src/stories/Header.stories.ts(1 hunks)apps/storybook/src/stories/Header.tsx(1 hunks)apps/storybook/src/stories/Page.stories.ts(1 hunks)apps/storybook/src/stories/Page.tsx(1 hunks)apps/storybook/src/stories/button.css(1 hunks)apps/storybook/src/stories/header.css(1 hunks)apps/storybook/src/stories/page.css(1 hunks)apps/storybook/src/vite-env.d.ts(1 hunks)apps/storybook/tsconfig.app.json(1 hunks)apps/storybook/tsconfig.json(1 hunks)apps/storybook/tsconfig.node.json(1 hunks)apps/storybook/vite.config.ts(1 hunks)turbo.json(1 hunks)
✅ Files skipped from review due to trivial changes (10)
- .hintrc
- apps/storybook/src/vite-env.d.ts
- apps/storybook/tsconfig.json
- apps/storybook/index.html
- .gitignore
- apps/storybook/.gitignore
- apps/storybook/src/stories/button.css
- apps/storybook/src/stories/header.css
- apps/storybook/src/App.css
- apps/storybook/src/stories/page.css
🧰 Additional context used
🪛 Biome (1.9.4)
apps/storybook/tsconfig.app.json
[error] 10-10: JSON standard does not allow comments.
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: JSON standard does not allow comments.
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
apps/storybook/tsconfig.node.json
[error] 9-9: JSON standard does not allow comments.
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: JSON standard does not allow comments.
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (8)
apps/storybook/tsconfig.app.json (1)
10-16: Well-configured bundler settings!
The bundler mode configuration is well optimized for modern development workflows:
- Appropriate module resolution strategy
- Performance optimizations with isolated modules
- Correct React JSX configuration
🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: JSON standard does not allow comments.
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
turbo.json (1)
8-8: LGTM! Verify Storybook configuration.
The addition of storybook-static/** to the build outputs is correct for Storybook integration with Turborepo.
Let's verify the Storybook setup is properly configured:
✅ Verification successful
✅ Storybook setup is properly configured
The addition of storybook-static/** to Turborepo's outputs is valid and matches the existing Storybook setup:
- Found complete Storybook configuration in
apps/storybook/ - Proper dependencies and scripts in package.json
- Configuration files present at
apps/storybook/.storybook/ storybook-staticis correctly listed in.gitignore
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for Storybook configuration files and .gitignore entries
echo "Checking Storybook configuration..."
fd -t f "\.story(ies|y)\.(tsx|jsx|ts|js)$" || echo "No story files found"
fd -t f "\.storybook" || echo "No .storybook configuration directory found"
echo "Checking .gitignore..."
rg "storybook-static" .gitignore || echo "Warning: storybook-static not found in .gitignore"
Length of output: 254
Script:
#!/bin/bash
# Let's verify if there's a Storybook configuration in package.json or any build scripts
echo "Checking package.json for Storybook dependencies and scripts..."
fd package.json --exec rg -l "storybook|@storybook"
# Check for any Storybook configuration files with different extensions
echo -e "\nChecking for Storybook config files..."
fd "main\.(js|ts|cjs|mjs)" -p ".storybook/"
fd "preview\.(js|ts|cjs|mjs)" -p ".storybook/"
Length of output: 602
Script:
#!/bin/bash
# Let's check the Storybook app configuration since we found it in apps/storybook/
echo "Checking Storybook app configuration..."
cat apps/storybook/package.json
# Check for Storybook config in the app directory
echo -e "\nChecking for Storybook config files in apps/storybook/.storybook/..."
fd "main\.(js|ts|cjs|mjs)" -p "apps/storybook/.storybook/"
fd "preview\.(js|ts|cjs|mjs)" -p "apps/storybook/.storybook/"
Length of output: 1702
apps/storybook/README.md (1)
1-50: Add documentation for component stories
The README lacks information about how to create and maintain stories for the new sidebar components introduced in this PR.
Let me verify if any story files exist for the new components:
Would you like me to help create story templates for the new sidebar components?
apps/storybook/tsconfig.node.json (3)
9-15: LGTM: Bundler mode configuration is well-structured
The bundler mode settings are appropriately configured for a Vite-based Storybook setup, with proper module resolution and TypeScript features enabled.
🧰 Tools
🪛 Biome (1.9.4)
[error] 9-9: JSON standard does not allow comments.
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
23-23: LGTM: Appropriate file inclusion
The include pattern correctly targets only the Vite configuration file, which is appropriate for a node-specific TypeScript configuration.
🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
1-24: Verify Storybook configuration integration
Let's ensure this configuration properly integrates with the main Storybook setup.
✅ Verification successful
Let me check the app-specific TypeScript configuration to ensure complete verification.
TypeScript configuration is properly integrated with Storybook setup
The configuration in tsconfig.node.json is correctly set up and aligns well with the Storybook environment:
- It's properly referenced in the root
tsconfig.json - The settings complement the app-specific configuration in
tsconfig.app.json - The configuration supports the Vite-based Storybook setup as seen in
.storybook/main.ts - Source files show correct TypeScript/React imports and Storybook usage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for main tsconfig references and Storybook configuration
echo "Checking for main tsconfig.json..."
cat apps/storybook/tsconfig.json 2>/dev/null || echo "Main tsconfig.json not found"
echo -e "\nChecking for Storybook configuration..."
cat apps/storybook/.storybook/main.ts 2>/dev/null || echo "Storybook main.ts not found"
Length of output: 794
Script:
#!/bin/bash
# Check the app-specific TypeScript configuration
echo "Checking tsconfig.app.json..."
cat apps/storybook/tsconfig.app.json 2>/dev/null || echo "tsconfig.app.json not found"
# Check if there are any Storybook-related build or type issues
echo -e "\nChecking for any Storybook component files..."
fd -e tsx -e ts -e jsx -e js --full-path apps/storybook/src/ --exec head -n 1 {} 2>/dev/null
Length of output: 1319
🧰 Tools
🪛 Biome (1.9.4)
[error] 9-9: JSON standard does not allow comments.
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: JSON standard does not allow comments.
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
apps/storybook/eslint.config.js (1)
1-5: LGTM! Imports are well-structured
The imports include all necessary ESLint packages for TypeScript and React development.
apps/storybook/src/stories/Configure.mdx (1)
1-1: Verify PR scope and title alignment
The PR is titled "Feat/#123 new sidebar" but these files appear to be Storybook documentation setup. Consider:
- Verifying if these changes should be in a separate PR
- Updating the PR title to reflect the Storybook setup changes
- Including the sidebar-related changes if they're missing
✅ Verification successful
PR scope aligns with title - no issues found
The PR changes show active development of the sidebar feature with multiple related files:
- New sidebar components added:
app-sidebar.tsxandsidebar.tsx - Supporting components:
logo-icon-switcher.tsx,nav-main.tsx - Dashboard layout changes and sidebar assets
The Storybook setup appears to be an additional enhancement rather than a scope conflict, as it provides a development environment that will help document and test the new sidebar components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for sidebar-related changes that should be part of this PR
echo "Searching for sidebar-related files..."
fd -e tsx -e ts -e jsx -e js --type f | rg -i "sidebar"
echo "Searching for sidebar-related components..."
ast-grep --pattern 'class $_ extends $_ {
$$$
Sidebar
$$$
}'
Length of output: 401
Script:
#!/bin/bash
# Check if these sidebar files are newly added or modified in this PR
git diff --name-status origin/main -- apps/core/app/dashboard/_compnents/app-sidebar.tsx packages/ui/src/components/ui/sidebar.tsx
# Check the content of the PR title and description
gh pr view --json title,body
# List all modified files in this PR
git diff --name-status origin/main
Length of output: 6231
apps/storybook/README.md
Outdated
| # React + TypeScript + Vite | ||
|
|
||
| This template provides a minimal setup to get React working in Vite with HMR and some ESLint rules. | ||
|
|
||
| Currently, two official plugins are available: | ||
|
|
||
| - [@vitejs/plugin-react](https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react/README.md) uses [Babel](https://babeljs.io/) for Fast Refresh | ||
| - [@vitejs/plugin-react-swc](https://github.com/vitejs/vite-plugin-react-swc) uses [SWC](https://swc.rs/) for Fast Refresh | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add context specific to the Storybook implementation
While the basic React + Vite setup information is useful, this README should focus more on how Storybook is specifically implemented in this project, especially given that this PR introduces new sidebar components that should be documented in Storybook.
Consider adding:
# React + TypeScript + Vite
This template provides a minimal setup to get React working in Vite with HMR and some ESLint rules.
+## Storybook Setup
+
+This Storybook instance is configured to showcase our UI components, including:
+- AppSidebar
+- LogoIconSwitcher
+- NavMain
+- NavbarDashboard
+
+### Running Storybook
+
+```bash
+npm run storybook
+```
+
Currently, two official plugins are available:
apps/storybook/README.md
Outdated
| ```js | ||
| export default tseslint.config({ | ||
| languageOptions: { | ||
| // other options... | ||
| parserOptions: { | ||
| project: ['./tsconfig.node.json', './tsconfig.app.json'], | ||
| tsconfigRootDir: import.meta.dirname, | ||
| }, | ||
| }, | ||
| }) | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update tsconfig paths to match the project structure
The example configuration uses generic tsconfig paths that don't match the monorepo structure.
export default tseslint.config({
languageOptions: {
// other options...
parserOptions: {
- project: ['./tsconfig.node.json', './tsconfig.app.json'],
+ project: ['./apps/storybook/tsconfig.json'],
tsconfigRootDir: import.meta.dirname,
},
},
})
apps/storybook/eslint.config.js
Outdated
| files: ['**/*.{ts,tsx}'], | ||
| languageOptions: { | ||
| ecmaVersion: 2020, | ||
| globals: globals.browser, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider updating language options for modern development
The current ECMAVersion (2020) might be limiting. Consider updating to the latest ECMAScript version and explicitly setting the sourceType.
languageOptions: {
- ecmaVersion: 2020,
+ ecmaVersion: 2024,
globals: globals.browser,
+ sourceType: 'module',
},📝 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.
| files: ['**/*.{ts,tsx}'], | |
| languageOptions: { | |
| ecmaVersion: 2020, | |
| globals: globals.browser, | |
| }, | |
| files: ['**/*.{ts,tsx}'], | |
| languageOptions: { | |
| ecmaVersion: 2024, | |
| globals: globals.browser, | |
| sourceType: 'module', | |
| }, |
apps/storybook/eslint.config.js
Outdated
| plugins: { | ||
| 'react-hooks': reactHooks, | ||
| 'react-refresh': reactRefresh, | ||
| }, | ||
| rules: { | ||
| ...reactHooks.configs.recommended.rules, | ||
| 'react-refresh/only-export-components': [ | ||
| 'warn', | ||
| { allowConstantExport: true }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance React and Storybook-specific configuration
Consider adding React-specific settings and Storybook-related rules for a more comprehensive configuration.
plugins: {
'react-hooks': reactHooks,
'react-refresh': reactRefresh,
+ 'storybook': require('@storybook/eslint-plugin'),
},
rules: {
...reactHooks.configs.recommended.rules,
'react-refresh/only-export-components': [
'warn',
{ allowConstantExport: true },
],
+ 'storybook/prefer-pascal-case': 'error',
+ 'storybook/hierarchy-separator': 'error',
},
+ settings: {
+ react: {
+ version: 'detect'
+ }
+ },Committable suggestion skipped: line range outside the PR's diff.
apps/storybook/eslint.config.js
Outdated
| import js from '@eslint/js' | ||
| import globals from 'globals' | ||
| import reactHooks from 'eslint-plugin-react-hooks' | ||
| import reactRefresh from 'eslint-plugin-react-refresh' | ||
| import tseslint from 'typescript-eslint' | ||
|
|
||
| export default tseslint.config( | ||
| { ignores: ['dist'] }, | ||
| { | ||
| extends: [js.configs.recommended, ...tseslint.configs.recommended], | ||
| files: ['**/*.{ts,tsx}'], | ||
| languageOptions: { | ||
| ecmaVersion: 2020, | ||
| globals: globals.browser, | ||
| }, | ||
| plugins: { | ||
| 'react-hooks': reactHooks, | ||
| 'react-refresh': reactRefresh, | ||
| }, | ||
| rules: { | ||
| ...reactHooks.configs.recommended.rules, | ||
| 'react-refresh/only-export-components': [ | ||
| 'warn', | ||
| { allowConstantExport: true }, | ||
| ], | ||
| }, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
💡 Codebase verification
ESLint configuration in Storybook app needs alignment with shared configs
The Storybook ESLint configuration is using a different approach (flat config) compared to the shared ESLint configurations in packages/eslint-config/. While this isn't necessarily wrong, there are some inconsistencies to note:
- The shared configs use the traditional ESLint format and extend from
eslint:recommendedandturbo - The Storybook config doesn't inherit from the shared configs despite being in the same monorepo
- The shared configs include important settings like
import/resolverthat are missing in the Storybook config - TypeScript handling differs: Storybook uses
typescript-eslintflat config while shared configs use@typescript-eslintplugins
Recommendations:
- Either migrate the Storybook config to use the shared configs from
@repo/eslint-config - Or update the shared configs to use the new flat config format for consistency
🔗 Analysis chain
Verify ESLint configuration consistency across the project
Let's ensure this configuration aligns with other ESLint configurations in the project and doesn't create conflicts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all ESLint configuration files in the project
echo "Searching for other ESLint configuration files..."
fd -e js -e json -e yaml -e yml -e cjs --full-path "eslint.config|.eslintrc"
# Check for potential conflicting rules in package.json files
echo "Checking for ESLint configurations in package.json files..."
fd "package.json" -x jq -r 'select(.eslintConfig != null) | "File: {}, ESLint config: \(.eslintConfig)"'
Length of output: 595
Script:
#!/bin/bash
# Let's examine the content of these ESLint configurations to check for consistency
echo "=== Content of apps/storybook/eslint.config.js ==="
cat apps/storybook/eslint.config.js
echo -e "\n=== Content of packages/eslint-config/library.js ==="
cat packages/eslint-config/library.js
echo -e "\n=== Content of packages/eslint-config/next.js ==="
cat packages/eslint-config/next.js
echo -e "\n=== Content of packages/eslint-config/react-internal.js ==="
cat packages/eslint-config/react-internal.js
echo -e "\n=== Content of packages/eslint-config/package.json ==="
cat packages/eslint-config/package.json
Length of output: 3932
| // More on how to set up stories at: https://storybook.js.org/docs/writing-stories#default-export | ||
| const meta = { | ||
| title: 'Example/Button', | ||
| component: Button, | ||
| parameters: { | ||
| // Optional parameter to center the component in the Canvas. More info: https://storybook.js.org/docs/configure/story-layout | ||
| layout: 'centered', | ||
| }, | ||
| // This component will have an automatically generated Autodocs entry: https://storybook.js.org/docs/writing-docs/autodocs | ||
| tags: ['autodocs'], | ||
| // More on argTypes: https://storybook.js.org/docs/api/argtypes | ||
| argTypes: { | ||
| backgroundColor: { control: 'color' }, | ||
| }, | ||
| // Use `fn` to spy on the onClick arg, which will appear in the actions panel once invoked: https://storybook.js.org/docs/essentials/actions#action-args | ||
| args: { onClick: fn() }, | ||
| } satisfies Meta<typeof Button>; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility testing configurations
Consider adding accessibility testing parameters to ensure component meets WCAG standards.
const meta = {
title: 'Example/Button',
component: Button,
parameters: {
layout: 'centered',
+ a11y: {
+ config: {
+ rules: [
+ {
+ id: 'button-name',
+ enabled: true,
+ },
+ {
+ id: 'color-contrast',
+ enabled: true,
+ },
+ ],
+ },
+ },
},📝 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.
| // More on how to set up stories at: https://storybook.js.org/docs/writing-stories#default-export | |
| const meta = { | |
| title: 'Example/Button', | |
| component: Button, | |
| parameters: { | |
| // Optional parameter to center the component in the Canvas. More info: https://storybook.js.org/docs/configure/story-layout | |
| layout: 'centered', | |
| }, | |
| // This component will have an automatically generated Autodocs entry: https://storybook.js.org/docs/writing-docs/autodocs | |
| tags: ['autodocs'], | |
| // More on argTypes: https://storybook.js.org/docs/api/argtypes | |
| argTypes: { | |
| backgroundColor: { control: 'color' }, | |
| }, | |
| // Use `fn` to spy on the onClick arg, which will appear in the actions panel once invoked: https://storybook.js.org/docs/essentials/actions#action-args | |
| args: { onClick: fn() }, | |
| } satisfies Meta<typeof Button>; | |
| // More on how to set up stories at: https://storybook.js.org/docs/writing-stories#default-export | |
| const meta = { | |
| title: 'Example/Button', | |
| component: Button, | |
| parameters: { | |
| // Optional parameter to center the component in the Canvas. More info: https://storybook.js.org/docs/configure/story-layout | |
| layout: 'centered', | |
| a11y: { | |
| config: { | |
| rules: [ | |
| { | |
| id: 'button-name', | |
| enabled: true, | |
| }, | |
| { | |
| id: 'color-contrast', | |
| enabled: true, | |
| }, | |
| ], | |
| }, | |
| }, | |
| }, | |
| // This component will have an automatically generated Autodocs entry: https://storybook.js.org/docs/writing-docs/autodocs | |
| tags: ['autodocs'], | |
| // More on argTypes: https://storybook.js.org/docs/api/argtypes | |
| argTypes: { | |
| backgroundColor: { control: 'color' }, | |
| }, | |
| // Use `fn` to spy on the onClick arg, which will appear in the actions panel once invoked: https://storybook.js.org/docs/essentials/actions#action-args | |
| args: { onClick: fn() }, | |
| } satisfies Meta<typeof Button>; |
| export interface HeaderProps { | ||
| user?: User; | ||
| onLogin?: () => void; | ||
| onLogout?: () => void; | ||
| onCreateAccount?: () => void; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety for callback functions
Consider adding error handling types for callback functions and making them required if they're essential for component functionality.
export interface HeaderProps {
user?: User;
- onLogin?: () => void;
- onLogout?: () => void;
- onCreateAccount?: () => void;
+ onLogin: () => Promise<void>;
+ onLogout: () => Promise<void>;
+ onCreateAccount: () => Promise<void>;
}📝 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.
| export interface HeaderProps { | |
| user?: User; | |
| onLogin?: () => void; | |
| onLogout?: () => void; | |
| onCreateAccount?: () => void; | |
| } | |
| export interface HeaderProps { | |
| user?: User; | |
| onLogin: () => Promise<void>; | |
| onLogout: () => Promise<void>; | |
| onCreateAccount: () => Promise<void>; | |
| } |
| <svg width="32" height="32" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg"> | ||
| <g fill="none" fillRule="evenodd"> | ||
| <path | ||
| d="M10 0h12a10 10 0 0110 10v12a10 10 0 01-10 10H10A10 10 0 010 22V10A10 10 0 0110 0z" | ||
| fill="#FFF" | ||
| /> | ||
| <path | ||
| d="M5.3 10.6l10.4 6v11.1l-10.4-6v-11zm11.4-6.2l9.7 5.5-9.7 5.6V4.4z" | ||
| fill="#555AB9" | ||
| /> | ||
| <path | ||
| d="M27.2 10.6v11.2l-10.5 6V16.5l10.5-6zM15.7 4.4v11L6 10l9.7-5.5z" | ||
| fill="#91BAF8" | ||
| /> | ||
| </g> | ||
| </svg> |
There was a problem hiding this comment.
Add accessibility attributes to SVG logo
The SVG logo is missing accessibility attributes which are essential for screen readers.
- <svg width="32" height="32" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg">
+ <svg
+ width="32"
+ height="32"
+ viewBox="0 0 32 32"
+ xmlns="http://www.w3.org/2000/svg"
+ role="img"
+ aria-label="Acme logo"
+ >📝 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.
| <svg width="32" height="32" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg"> | |
| <g fill="none" fillRule="evenodd"> | |
| <path | |
| d="M10 0h12a10 10 0 0110 10v12a10 10 0 01-10 10H10A10 10 0 010 22V10A10 10 0 0110 0z" | |
| fill="#FFF" | |
| /> | |
| <path | |
| d="M5.3 10.6l10.4 6v11.1l-10.4-6v-11zm11.4-6.2l9.7 5.5-9.7 5.6V4.4z" | |
| fill="#555AB9" | |
| /> | |
| <path | |
| d="M27.2 10.6v11.2l-10.5 6V16.5l10.5-6zM15.7 4.4v11L6 10l9.7-5.5z" | |
| fill="#91BAF8" | |
| /> | |
| </g> | |
| </svg> | |
| <svg | |
| width="32" | |
| height="32" | |
| viewBox="0 0 32 32" | |
| xmlns="http://www.w3.org/2000/svg" | |
| role="img" | |
| aria-label="Acme logo" | |
| > | |
| <g fill="none" fillRule="evenodd"> | |
| <path | |
| d="M10 0h12a10 10 0 0110 10v12a10 10 0 01-10 10H10A10 10 0 010 22V10A10 10 0 0110 0z" | |
| fill="#FFF" | |
| /> | |
| <path | |
| d="M5.3 10.6l10.4 6v11.1l-10.4-6v-11zm11.4-6.2l9.7 5.5-9.7 5.6V4.4z" | |
| fill="#555AB9" | |
| /> | |
| <path | |
| d="M27.2 10.6v11.2l-10.5 6V16.5l10.5-6zM15.7 4.4v11L6 10l9.7-5.5z" | |
| fill="#91BAF8" | |
| /> | |
| </g> | |
| </svg> |
apps/storybook/src/stories/Page.tsx
Outdated
| <Header | ||
| user={user} | ||
| onLogin={() => setUser({ name: 'Jane Doe' })} | ||
| onLogout={() => setUser(undefined)} | ||
| onCreateAccount={() => setUser({ name: 'Jane Doe' })} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve callback naming and implementation
The onLogin and onCreateAccount callbacks have identical implementations. Consider consolidating them or differentiating their behavior.
- onLogin={() => setUser({ name: 'Jane Doe' })}
- onLogout={() => setUser(undefined)}
- onCreateAccount={() => setUser({ name: 'Jane Doe' })}
+ onLogin={() => setUser({ name: 'Jane Doe', id: 'existing-user' })}
+ onLogout={() => setUser(undefined)}
+ onCreateAccount={() => setUser({ name: 'New User', id: 'new-user' })}📝 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.
| <Header | |
| user={user} | |
| onLogin={() => setUser({ name: 'Jane Doe' })} | |
| onLogout={() => setUser(undefined)} | |
| onCreateAccount={() => setUser({ name: 'Jane Doe' })} | |
| /> | |
| <Header | |
| user={user} | |
| onLogin={() => setUser({ name: 'Jane Doe', id: 'existing-user' })} | |
| onLogout={() => setUser(undefined)} | |
| onCreateAccount={() => setUser({ name: 'New User', id: 'new-user' })} | |
| /> |
apps/storybook/src/stories/Page.tsx
Outdated
| <a href="https://componentdriven.org" target="_blank" rel="noopener noreferrer"> | ||
| <strong>component-driven</strong> | ||
| </a>{' '} | ||
| process starting with atomic components and ending with pages. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add security attributes to external link
The external link is missing recommended security attributes.
- <a href="https://componentdriven.org" target="_blank" rel="noopener noreferrer">
+ <a
+ href="https://componentdriven.org"
+ target="_blank"
+ rel="noopener noreferrer"
+ aria-label="Learn about component-driven development"
+ >📝 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.
| <a href="https://componentdriven.org" target="_blank" rel="noopener noreferrer"> | |
| <strong>component-driven</strong> | |
| </a>{' '} | |
| process starting with atomic components and ending with pages. | |
| <a | |
| href="https://componentdriven.org" | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| aria-label="Learn about component-driven development" | |
| > | |
| <strong>component-driven</strong> | |
| </a>{' '} | |
| process starting with atomic components and ending with pages. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/ui/src/components/molecules/sidebar.tsx (3)
22-27: Consider externalizing constants for better maintainabilityConstants like
SIDEBAR_COOKIE_NAME,SIDEBAR_COOKIE_MAX_AGE,SIDEBAR_WIDTH, etc., are defined within the component file. Moving them to a separate constants or configuration file can enhance reusability and maintain consistency across the codebase.
223-266: Split complex JSX into smaller components for better readabilityThe
Sidebarcomponent contains intricate JSX with nested ternary operations and conditional classes, which can be hard to read and maintain. Consider refactoring parts of the JSX into smaller, reusable components to enhance readability and reduce complexity.
551-599: Optimize tooltip rendering inSidebarMenuButtonThe
tooltipprop handling creates a new object on every render whentooltipis a string, potentially causing unnecessary re-renders of theTooltipContent. Consider memoizing the tooltip content or structuring the code to prevent redundant object creation.Apply this diff to optimize tooltip handling:
if (typeof tooltip === "string") { - tooltip = { + tooltipProps = { children: tooltip, }; } return ( <Tooltip> <TooltipTrigger asChild>{button}</TooltipTrigger> <TooltipContent side="right" align="center" hidden={state !== "collapsed" || isMobile} - {...tooltip} + {...tooltipProps} /> </Tooltip> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/storybook/src/stories/Button.tsx(2 hunks)apps/storybook/src/stories/Header.tsx(3 hunks)packages/ui/src/components/molecules/sidebar.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/storybook/src/stories/Header.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/storybook/src/stories/Button.tsx
🔇 Additional comments (2)
packages/ui/src/components/molecules/sidebar.tsx (2)
668-670:⚠️ Potential issueAvoid using random values during rendering to prevent hydration mismatches
Using
Math.random()inuseMemocan cause inconsistencies between server and client rendering in server-side rendered applications, leading to hydration errors. Consider using a fixed width or deriving the width from a prop or state that is consistent between server and client.Apply this diff to fix the issue:
- // Random width between 50 to 90%. - const width = React.useMemo(() => { - return `${Math.floor(Math.random() * 40) + 50}%`; - }, []); + // Use a fixed width or derive from props/state + const width = '70%'; // Example fixed widthAlternatively, pass the width as a prop to ensure consistency.
Likely invalid or redundant comment.
103-105: Ensure keyboard shortcut works consistently across platformsWhen handling keyboard events, relying on
event.metaKeyandevent.ctrlKeymight not cover all user scenarios, especially on different operating systems or browsers. Consider using a library likehotkeys-jsor standardizing the shortcut handling to ensure cross-platform compatibility.Run the following script to check the keyboard shortcut functionality across different browsers:
Ensure the output shows comprehensive handling of keyboard events.
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; | ||
| }, |
There was a problem hiding this comment.
Encode cookie values to prevent security vulnerabilities
Directly setting document.cookie with unencoded values can lead to security issues like XSS attacks. It's recommended to encode the cookie value using encodeURIComponent to ensure special characters are properly handled.
Apply this diff to fix the issue:
- document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`;
+ document.cookie = `${SIDEBAR_COOKIE_NAME}=${encodeURIComponent(openState)}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`;📝 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.
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; | |
| }, | |
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${encodeURIComponent(openState)}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; | |
| }, |
There was a problem hiding this comment.
Actionable comments posted: 47
🔭 Outside diff range comments (8)
packages/apis/package.json (1)
17-17: Duplicate Dependency for @types/js-cookie Detected.
The dependency"@types/js-cookie": "^3.0.6"appears both in the devDependencies (line 17) and in the dependencies (line 33). Since the intention is to shift it to a runtime dependency, please remove the duplicate entry from devDependencies to maintain clarity and avoid potential conflicts.Proposed removal diff from the devDependencies section:
- "@types/js-cookie": "^3.0.6",Also applies to: 33-33
apps/core/app/auth/set-password/page.tsx (2)
25-31: Address TODO comment and remove hardcoded values.The code contains:
- A TODO comment about fixing username retrieval
- Commented out code for getting username from search params
- Hardcoded username "[email protected]"
This could lead to security issues if not properly implemented.
- // TODO: Fix this - // const params = useSearchParams(); - // const username = params.get("username"); - - const username = "[email protected]"; + const params = useSearchParams(); + const username = params.get("username");
33-35:Details
🧩 Analysis chain
Verify password validation requirements.
The form uses Zod validation with
postForgetPasswordSchema, but the validation requirements aren't visible in the code. Please verify that the schema enforces:
- Minimum password length
- Password complexity (uppercase, lowercase, numbers, special characters)
- Password and confirm password match
Also applies to: 81-84
🏁 Script executed:
#!/bin/bash # Description: Search for password validation schema # Test: Find the schema definition rg -A 10 "postForgetPasswordSchema"Length of output: 7419
I'll dive deeper into the schema definition to confirm that the password requirements are indeed enforced. Let's inspect the file "packages/apis/src/services/core/forgot-password/post/post-forget-password.schema.ts" to look for validation rules related to password length, complexity, and match of confirm password fields.
🏁 Script executed:
#!/bin/bash # Display the content of the post-forget-password.schema.ts file rg -N "^" packages/apis/src/services/core/forgot-password/post/post-forget-password.schema.tsLength of output: 1161
Password Schema Validation Incomplete
The schema enforces a minimum password length (8 characters) for both new and confirm password fields and verifies that the two match via the.refinemethod. However, it does not enforce any complexity requirements (such as ensuring the presence of uppercase letters, lowercase letters, numbers, and special characters).
- The minimum length is set through
z.string().min(8, ...)in both thenewPasswordandconfirmPasswordfields.- Password confirmation is ensured using
.refine((data) => data.newPassword === data.confirmPassword, ...).- There is no regex or similar check to ensure password complexity.
apps/core/app/(landing)/product/[id]/page.tsx (2)
70-76: Add ARIA labels to interactive icons.The Heart and Message icons are clickable but lack proper accessibility attributes.
Add ARIA labels to make these interactive elements accessible:
- <Heart1icon size={28} className="cursor-pointer" /> + <Heart1icon + size={28} + className="cursor-pointer" + role="button" + aria-label="Like product" + /> - <Messagecircleicon size={28} className="cursor-pointer" /> + <Messagecircleicon + size={28} + className="cursor-pointer" + role="button" + aria-label="View comments" + />
96-101:Details
🧩 Analysis chain
Optimize parallax scroll performance.
The sticky positioning with a large scroll height (300vh) might affect scroll performance on lower-end devices.
Consider:
- Using CSS
will-changeproperty- Adding intersection observer to optimize off-screen content
- Testing performance on mobile devices
- <div className="relative w-full h-[300vh]" ref={targetRef}> + <div className="relative w-full h-[300vh] will-change-transform" ref={targetRef}>
🏁 Script executed:
#!/bin/bash # Verify whether the "will-change-transform" property has been applied in the target file rg "will-change-transform" "apps/core/app/(landing)/product/[id]/page.tsx"Length of output: 74
🏁 Script executed:
#!/bin/bash # Re-run search with escaped brackets for the file path to ensure proper matching rg "will-change-transform" apps/core/app/\(landing\)/product/\[id\]/page.tsxLength of output: 74
🏁 Script executed:
#!/bin/bash # Print context of lines 90 to 110 from the target file to manually verify applied changes sed -n '90,110p' apps/core/app/\(landing\)/product/\[id\]/page.tsxLength of output: 799
Optimize parallax scroll performance.
The code snippet still lacks the performance-related modifications—specifically, the addition of the "will-change-transform" property—which could help mitigate potential scroll performance issues on lower-end devices.
- The div with class "relative w-full h-[300vh]" does not include the suggested "will-change-transform" class.
- Applying the "will-change" property or leveraging an intersection observer for off-screen content could improve rendering performance.
apps/core/app/auth/login/page.tsx (1)
21-44: Remove commented-out code.If the social login functionality is no longer needed, it's better to remove the commented-out code rather than leaving it in the codebase. This improves code maintainability and reduces confusion. If this code might be needed in the future, it can be retrieved from version control history.
packages/ui/src/components/molecules/attachment/attachmentAdmin/attachmentAdmin.tsx (1)
61-73: Add missing key prop to the mapped elements.The outer motion.div is missing a key prop, which could cause issues with React's reconciliation process.
- <motion.div + <motion.div + key={file.name + file.size} exit={{ opacity: 0, scale: 1 }} initial={{ opacity: 0, scale: 0.7 }} animate={{ opacity: 1, scale: 1 }} > <AttachmentItem - key={file.name + file.size} file={file} handleRemove={handleRemove} /> </motion.div>🧰 Tools
🪛 Biome (1.9.4)
[error] 62-66: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
apps/core/app/(landing)/_components/desktop-navbar/feature-navbar-authenticated.tsx (1)
84-84: Fix typo in sign out text.There's a typo in the sign out text: "Sing Out" should be "Sign Out".
- Sing Out + Sign Out
♻️ Duplicate comments (1)
packages/ui/src/components/molecules/sidebar.tsx (1)
87-87:⚠️ Potential issueEncode cookie values to prevent security vulnerabilities.
Apply this diff to fix the issue:
- document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; + document.cookie = `${SIDEBAR_COOKIE_NAME}=${encodeURIComponent(openState)}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`;
🧹 Nitpick comments (70)
packages/design-system/src/tokens/token_spacing_Desktop.json (1)
1-114: Optional: Consider Sorting Token Keys
While JSON property order isn’t guaranteed, arranging these keys in ascending numerical order (e.g., spacing-1, spacing-2, spacing-3, …, spacing-96) could improve readability and maintainability for future updates.apps/core/app/auth/signup/otp/_components/signup-otp-form.tsx (1)
98-99: Consider removing redundant styling.The button has both a
variant="primary"prop and explicit primary-specific styling inclassName. This creates maintenance overhead and potential inconsistencies.- className="w-full text-lg font-bold bg-primary-600 hover:bg-primary-500" - variant="primary" + className="w-full text-lg font-bold" + variant="primary".github/workflows/build-design-system.yml (1)
12-19: Checkout Repository Step – Update Action Version
The checkout step uses "actions/checkout@v3". However, static analysis suggests this version may be outdated. Consider upgrading to a more recent version (for example, "actions/[email protected]" or the latest stable release) to benefit from updated runners and fixes.Suggested change:
- uses: actions/checkout@v3 + uses: actions/[email protected]🧰 Tools
🪛 actionlint (1.7.4)
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
apps/core/next.config.mjs (1)
3-5: Remove unnecessary image URL comments.These comments containing URLs don't provide meaningful documentation and could become stale. If these URLs are important, consider documenting them in a more appropriate location like a design document or configuration file.
-// https://s3-alpha-sig.figma.com/img/ce79/f737/ea54458a8146c4935 -//https://images.unsplash.com/photo-1730292422804-5bbb2bd2d3f0?q=80&w=2070&auto=format&fit=crop&ixlib=rb-4.0.3&ixid=M3wxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8fA%3D%3D -// config src imageapps/core/app/(landing)/test/page.tsx (2)
8-8: Improve spacing implementation for better responsiveness.The current spacing approach has some issues:
- Large hardcoded top margin (
mt-[10rem]) might cause layout issues- Fixed-height divs for spacing (
style={{ height: "50px" }}) aren't responsiveConsider using Tailwind's spacing utilities for a more responsive approach:
-<div className="mt-[10rem]"> +<div className="mt-16 md:mt-24 lg:mt-32"> -<div style={{ height: "50px" }}></div> +<div className="my-8 md:my-12"></div>Also applies to: 19-19, 30-30
14-16: Define proper TypeScript types for onChange handlers.Consider creating a type for the onChange handler to improve type safety and reusability:
type FileUploadHandler = (filesId: number[]) => Promise<void>; // Then use it in your components: onChange={async (filesId: number[]) => { try { // Handle the uploaded files } catch (error) { console.error('Error handling uploaded files:', error); } }} satisfies FileUploadHandlerAlso applies to: 26-28, 40-42
packages/ui/src/components/atoms/sidebar.tsx (5)
25-31: Consider adding secure cookie attributes when storing sidebar state.Currently, the sidebar state is stored in a plain cookie without attributes such as "Secure" or "SameSite". This might be acceptable for non-sensitive state, but adding these attributes would enhance security.
103-116: Guard against shortcut conflicts.CTRL/CMD + “b” could clash with common browser or OS shortcuts (e.g. bold in text editors). Consider making the keyboard shortcut configurable or offering an alternative.
304-322: Improve accessibility for the SidebarRail toggle region.Using a button with a cursor-resize style may confuse screen readers or keyboard-only users. Consider adding an ARIA label or an accessible name to clarify the toggle action.
656-666: Use a stable random value for skeleton width.While “useMemo” ensures the calculation only runs once per mount, the skeleton width will still change on each new mount. If you prefer a stable appearance across re-mounts, store the generated width in a React ref instead.
747-772: Consider modularizing or adding tests for small reusable components.This file defines many small components in one place. Splitting them into separate files and adding unit or integration tests would improve maintainability, discoverability, and test coverage.
.husky/pre-rebase (1)
10-11: Add a check for changes before stashing.While the stash operation works, it would be more robust to check if there are actually changes to stash first.
echo "🔄 Stashing changes before rebase..." -git stash push -m "Pre-rebase stash" +if ! git diff --quiet HEAD; then + git stash push -m "Pre-rebase stash" +else + echo "ℹ️ No changes to stash" +fipackages/icons/src/components/remove.tsx (1)
2-4: Consider using kebab-case for prop spreading.For better HTML attribute compatibility, consider using kebab-case when spreading props to the SVG element.
Apply this diff:
-const Removeicon = (props: IconProps) => { - const { size = 24, color = "currentColor", ...resProps } = props; +const RemoveIcon = (props: IconProps) => { + const { size = 24, color = "currentColor", ...restProps } = props; + const kebabProps = Object.fromEntries( + Object.entries(restProps).map(([key, value]) => [ + key.replace(/[A-Z]/g, (letter) => `-${letter.toLowerCase()}`), + value, + ]) + );Then update the SVG element:
<svg xmlns="http://www.w3.org/2000/svg" width={size} height={size} viewBox="0 0 24 24" fill="none" - {...resProps} + {...kebabProps} >packages/icons/src/components/security.tsx (1)
16-16: Make stroke color configurable.The stroke color is hardcoded to "#374151". Consider using the
colorprop to make the icon more reusable across different themes and color schemes.- stroke="#374151" + stroke={color}packages/icons/src/components/tonconnect.tsx (2)
6-12: Add missing resProps spread.The resProps are not spread to the SVG element, which limits the component's extensibility.
<svg xmlns="http://www.w3.org/2000/svg" width={size} height={size} viewBox="0 0 34 33" fill="none" + {...resProps} >
15-17: Make fill colors configurable.The fill colors are hardcoded which limits theme compatibility. Consider using the color prop.
<path d="M17 33C26.1127 33 33.5 25.6127 33.5 16.5C33.5 7.38729 26.1127 0 17 0C7.88729 0 0.5 7.38729 0.5 16.5C0.5 25.6127 7.88729 33 17 33Z" - fill="#0098EA" + fill={color} /> <path d="M22.6335 9.20923H11.3654C9.29359 9.20923 7.98043 11.4441 9.02275 13.2508L15.977 25.3045C16.4308 26.0916 17.5681 26.0916 18.0219 25.3045L24.9776 13.2508C26.0185 11.447 24.7054 9.20923 22.635 9.20923H22.6335ZM15.9714 21.6898L14.4569 18.7586L10.8025 12.2227C10.5614 11.8043 10.8591 11.2683 11.364 11.2683H15.97V21.6912L15.9714 21.6898ZM23.1937 12.2212L19.5407 18.76L18.0262 21.6898V11.2668H22.6321C23.137 11.2668 23.4347 11.8029 23.1937 12.2212Z" - fill="white" + fill="currentColor" />Also applies to: 19-21
packages/icons/src/components/icon-sets.tsx (1)
16-18: Make fill color configurable.The fill color is hardcoded to "white". Consider using the color prop for better theme compatibility.
<path d="M22.4876 4.6127C28.9421 -1.25355 38.7961 -1.25355 45.2474 4.6127L46.3979 5.7372L46.5084 5.84445L47.6329 6.99495C53.4991 13.4494 53.4991 23.3034 47.6329 29.7547C47.2104 30.2194 46.6026 30.4599 45.9754 30.4599H24.2004C22.8646 30.4599 21.7824 31.5422 21.7824 32.8779C21.7824 34.2137 22.8646 35.2959 24.2004 35.2959H37.4539C38.9196 35.2959 39.6509 35.2959 39.9889 35.5852C40.2814 35.8354 40.4406 36.2124 40.4081 36.5992C40.3724 37.0444 39.8556 37.5612 38.8189 38.5979L34.2916 43.1252L34.2039 43.2129C33.8372 43.6073 33.4248 43.9564 32.9754 44.2529C32.5431 44.5194 32.0719 44.7144 31.5779 44.8314C31.0091 44.9679 30.4306 44.9679 29.9724 44.9647H29.8489H13.4884C12.8774 44.9647 12.2859 44.9647 11.7821 44.9224C11.5026 44.8997 11.1971 44.8607 10.8786 44.7859L4.15761 51.5037C3.21511 52.4462 1.68436 52.4462 0.738612 51.5037C-0.207138 50.5612 -0.203888 49.0304 0.738612 48.0847L7.45961 41.3637C7.38486 41.0452 7.34586 40.7397 7.32311 40.4602C7.28086 39.9564 7.28086 39.3617 7.28086 38.7539V22.3934V22.2699C7.28086 21.8117 7.27761 21.2332 7.41411 20.6644C7.53111 20.1705 7.72936 19.6992 7.99261 19.2669C8.29811 18.7697 8.70761 18.3602 9.03261 18.0385L9.12036 17.9507L21.3339 5.7372L22.4876 4.6127Z" - fill="white" + fill={color} />packages/icons/src/components/mockups.tsx (1)
3-4: Consider standardizing the default size.The default size of 53px is inconsistent with other icons in the library (e.g., AfterEffectIcon uses 16px as default).
Consider standardizing the default size across all icons for consistency:
- const { size = 53, color = "currentColor", ...resProps } = props; + const { size = 16, color = "currentColor", ...resProps } = props;packages/icons/src/components/3dAsset.tsx (1)
4-4: Resolve size inconsistency.There's a mismatch between:
- Default size: 24px
- ViewBox size: 53x53
- Other icons' default size: 16px
Consider standardizing the size:
- const { size = 24, color = "currentColor", ...resProps } = props; + const { size = 16, color = "currentColor", ...resProps } = props;Also applies to: 11-11
apps/core/app/(landing)/_components/addtoCard.tsx (1)
81-81: Implement label container and sign out functionality.There are TODO comments for label container and sign out functionality that need to be addressed.
Would you like me to help implement these features? I can provide the implementation for both the label container and sign out functionality.
Also applies to: 90-91
packages/icons/src/components/cinema4D.tsx (1)
27-30: Consider optimizing the base64 encoded image.The large base64 encoded image string might impact the bundle size. Consider:
- Using an external image file
- Optimizing the PNG image before encoding
- Using a CDN for the image
packages/design-system/src/scripts/utills.ts (2)
25-25: Improve return format and add documentation.The current implementation returns a space-separated string, which might require additional parsing by consumers. Consider providing more flexible return options and adding documentation.
Consider these improvements:
+/** + * Converts a hex color string to HSL format. + * @param hex - The hex color string (e.g., "#ff0000", "ff0000", "#f00") + * @returns HSL values as a space-separated string (e.g., "0 100% 50%") + * @throws {Error} If the input is invalid + */ export const hexToHsl = (hex: string): string => { // ... existing code ... - return `${Math.round(h)} ${Math.round(s * 100)}% ${Math.round(l * 100)}%`; + // Option 1: Keep the current string format + return `${Math.round(h)} ${Math.round(s * 100)}% ${Math.round(l * 100)}%`; + + // Option 2: Return an object for more flexibility + // return { + // h: Math.round(h), + // s: Math.round(s * 100), + // l: Math.round(l * 100) + // }; };
1-26: Fix typo in filename.The filename "utills.ts" appears to be misspelled. The conventional spelling is "utils.ts".
Consider renaming the file:
- packages/design-system/src/scripts/utills.ts + packages/design-system/src/scripts/utils.tsapps/core/app/auth/otp/page.tsx (1)
58-58: Fix typos in text.There are typos in the text: "didnt recieved" should be "didn't receive".
- didnt recieved code yet? <Countdown date={Date.now() + 120000} /> + didn't receive code yet? <Countdown date={Date.now() + 120000} />apps/core/app/auth/set-password/page.tsx (1)
59-61: Enhance form submission security.Consider adding these security measures:
- Rate limiting for password reset attempts
- CSRF protection
- Clear form data after submission
apps/core/app/(landing)/product/[id]/page.tsx (3)
3-12: Consider consolidating icon imports.The large number of individual icon imports could be consolidated into a single import for better maintainability.
Consider using a barrel file for icons:
-import Adobexdicon from "@repo/icons/adobexd"; -import Chevronrighticon from "@repo/icons/chevron-right"; -import Circlecheckicon from "@repo/icons/Circle-check"; -import Clouddownloadicon from "@repo/icons/cloud-download"; -import Figmaicon from "@repo/icons/figma"; -import Heart1icon from "@repo/icons/heart1"; -import Instagram from "@repo/icons/instagram"; -import Messagecircleicon from "@repo/icons/message-circle"; -import Shopingcartplusicon from "@repo/icons/shopping-cart-plus"; -import Sketchicon from "@repo/icons/sketch"; +import { + Adobexdicon, + Chevronrighticon, + Circlecheckicon, + Clouddownloadicon, + Figmaicon, + Heart1icon, + Instagram, + Messagecircleicon, + Shopingcartplusicon, + Sketchicon +} from "@repo/icons";
22-81: Move OverLay component to a separate file.The OverLay component is a substantial piece of UI that would be better maintained in its own file.
Consider moving it to
_components/overlay.tsxfor better code organization and reusability.
39-51: Extract repeated icon group into a reusable component.The icon group with Figma, Adobe XD, and Sketch icons is duplicated. Consider extracting it into a reusable component.
Create a new component like
DesignToolIconsto avoid duplication:// _components/design-tool-icons.tsx export const DesignToolIcons = () => ( <div className="flex"> <div className="flex justify-center items-center bg-background rounded-full p-1.5"> <Figmaicon size={16} /> </div> <div className="flex -ml-2 justify-center items-center bg-background rounded-full p-1.5"> <Adobexdicon size={16} /> </div> <div className="flex -ml-3 justify-center items-center bg-background rounded-full p-1.5"> <Sketchicon size={16} /> </div> </div> );Also applies to: 226-234
packages/icons/src/components/wire-frame.tsx (1)
1-4: Add JSDoc documentation for the component and props.The component structure and prop handling look good. Consider adding JSDoc documentation to improve maintainability.
Add documentation above the component:
+/** + * WireFrameIcon component + * @param {IconProps} props - The component props + * @param {number} [props.size=53] - The size of the icon + * @param {string} [props.color="currentColor"] - The color of the icon + * @returns {JSX.Element} The WireFrameIcon component + */ const WireFrameIcon = (props: IconProps) => {packages/design-system/src/tailwind/config.ts (1)
4-10: Add type information to enhance type safety.Consider adding type information to the configuration object to ensure type safety and better IDE support.
-export const tailwindConfig = { +import type { Config } from 'tailwindcss' + +export const tailwindConfig: Config = {packages/design-system/src/scripts/build.ts (2)
12-18: Optimize build performance through parallelization.Consider running independent build tasks in parallel using Promise.all() to improve build performance.
- // Styles - console.log("Building SCSS styles..."); - buildPaletteScss(); - buildColorsScss("light"); - buildColorsScss("dark"); - buildMainScss(); + // Styles + console.log("Building SCSS styles..."); + await Promise.all([ + buildPaletteScss(), + buildColorsScss("light"), + buildColorsScss("dark"), + ]); + // Main SCSS must be built after other styles + await buildMainScss();
8-30: Add build status tracking.Consider adding build status tracking to provide more detailed feedback about which steps succeeded or failed.
+interface BuildStep { + name: string; + status: 'pending' | 'success' | 'failed'; + error?: Error; +} + +const buildSteps: BuildStep[] = [ + { name: 'Palette SCSS', status: 'pending' }, + { name: 'Colors SCSS (light)', status: 'pending' }, + { name: 'Colors SCSS (dark)', status: 'pending' }, + { name: 'Main SCSS', status: 'pending' }, + { name: 'Colors Tailwind', status: 'pending' }, + { name: 'Palette Tailwind', status: 'pending' }, + { name: 'Tailwind Config', status: 'pending' }, +]; + const main = () => { try { console.log("🎨 Building design system..."); + + const updateStepStatus = (stepName: string, status: 'success' | 'failed', error?: Error) => { + const step = buildSteps.find(s => s.name === stepName); + if (step) { + step.status = status; + if (error) step.error = error; + } + }; // ... rest of the build process with status updates ... console.log("✅ Design system built successfully!"); + console.log("\nBuild Summary:"); + buildSteps.forEach(step => { + const icon = step.status === 'success' ? '✅' : '❌'; + console.log(`${icon} ${step.name}: ${step.status}`); + if (step.error) console.log(` Error: ${step.error.message}`); + }); } catch (error) { console.error("❌ Build failed:", error); process.exit(1); } };packages/design-system/src/scripts/build-config-tailwind.ts (1)
39-41: Consider removing the immediate script execution.The immediate execution at the end of the file makes it harder to test and control the build process. Consider removing it and letting the main build script control the execution.
-// Run the script -buildTailwindConfig();packages/design-system/src/tailwind/colors.ts (1)
1-37: LGTM! Well-organized color system.The color system is well-structured with semantic naming and uses HSL notation with CSS custom properties, which is great for theming and color manipulation.
Consider adding JSDoc comments to document the expected HSL format of the CSS variables, e.g.:
+/** + * Color system using HSL notation. + * CSS variables should be in format: <hue> <saturation>% <lightness>% + * Example: --accent: 220 13% 91% + */ export const colors = {packages/design-system/src/scripts/build-palette-style.ts (2)
5-7: Consider making file paths configurable.Hardcoded paths make the script less flexible. Consider accepting paths as parameters or environment variables.
-const inputFilePath = path.join(__dirname, '../tokens/token_palette_primitive.json'); -const outputFilePath = path.join(__dirname, '../styles/base/_palette.scss'); +const inputFilePath = process.env.INPUT_PATH || path.join(__dirname, '../tokens/token_palette_primitive.json'); +const outputFilePath = process.env.OUTPUT_PATH || path.join(__dirname, '../styles/base/_palette.scss');
10-31: Improve type safety in generateScssContent function.The function uses
Record<string, any>which loses type safety. Consider defining proper interfaces for the token structure.+interface ColorToken { + $type: 'color'; + $value: string; +} + +interface TokenGroup { + [key: string]: ColorToken; +} + -const generateScssContent = (tokens: Record<string, any>): string => { +const generateScssContent = (tokens: Record<string, TokenGroup>): string => {packages/design-system/src/scripts/build-colors-style.ts (1)
7-9: Consider enhancing token reference resolution.The
resolveTokenReferencefunction could be more robust by:
- Validating input to ensure it's a string
- Handling nested references
- Adding error handling for malformed references
const resolveTokenReference = (value: string): string => { + if (typeof value !== 'string') { + throw new Error('Token reference must be a string'); + } return value.replace(/{(.+?)}/g, (_, match) => `var(--${match.replace(".", "-")})`); };packages/design-system/src/styles/base/_colors-light.scss (1)
35-35: Use CSS color function for transparency.Instead of using rgba directly, consider using the modern CSS color-mix function for better maintainability.
- --table-header: rgba(244, 244, 245, 0.5000); + --table-header: color-mix(in srgb, var(--zinc-100) 50%, transparent);packages/design-system/src/styles/base/_colors-dark.scss (1)
35-35: Use CSS color function for transparency.For consistency with the light theme suggestion, use color-mix here as well.
- --table-header: rgba(39, 39, 42, 0.5000); + --table-header: color-mix(in srgb, var(--zinc-900) 50%, transparent);packages/design-system/src/styles/base/_palette.scss (3)
3-13: Amber Color Palette Definition
The custom properties for the amber shades (50 through 950) are clearly defined using HSL values. The gradation appears consistent and should allow for versatile use in theming. Consider adding inline comments if there’s any design rationale for specific values to assist future maintainers.
114-124: Neutral Color Palette Definition
The neutral palette spans a complete range from almost white to nearly black. Such a spectrum is useful for backgrounds and text elements. Consider noting any usage guidance if these colors are intended to support accessibility standards.
138-149: Pink Color Palette Definition
The pink palette provides a smooth gradation and appears consistent with expectations for accent colors. If there are common UI elements that use these pink shades, consider documenting those associations in comments.packages/ui/src/components/molecules/attachment/useAttachment.ts (1)
87-139: Remove leftover console.log
Consider removing the console.log() call on line 122 to avoid clutter in production builds.- console.log();apps/core/app/(landing)/_components/card-cart-skeleton.tsx (2)
1-13: Consider using the Skeleton component from UI library.The component implements a custom skeleton loading state. However, since this is part of a larger application that likely has a UI library (based on the imports in other files), consider using a standardized Skeleton component from the UI library for consistency.
3-11: Consider making dimensions configurable via props.The skeleton uses fixed dimensions (
w-[112px],w-[166px], etc.) which might not be flexible enough for different use cases.Consider making the dimensions configurable:
-const CardSkeleton = () => { +interface CardSkeletonProps { + imageWidth?: string; + imageHeight?: string; + titleWidth?: string; + subtitleWidth?: string; +} + +const CardSkeleton = ({ + imageWidth = "112px", + imageHeight = "80px", + titleWidth = "166px", + subtitleWidth = "47px" +}: CardSkeletonProps) => { return ( <div className="bg-background p-4 flex gap-2 flex-1 rounded-lg"> <div> - <div className="rounded-lg w-[112px] h-[80px] bg-card"></div> + <div className={`rounded-lg w-[${imageWidth}] h-[${imageHeight}] bg-card`}></div> </div> <div className="flex flex-col gap-2"> - <div className="rounded-lg w-[166px] h-[18px] bg-card"></div> - <div className="rounded-lg w-[47px] h-[18px] bg-card"></div> + <div className={`rounded-lg w-[${titleWidth}] h-[18px] bg-card`}></div> + <div className={`rounded-lg w-[${subtitleWidth}] h-[18px] bg-card`}></div> </div> </div> ); };packages/apis/src/services/core/base/upload/post/post-uploadfile.types.ts (1)
4-5: Fix swapped comments for Request/Response.The comments are incorrectly placed:
- "Response" comment is above request type
- "Request" comment is above response type
Apply this diff to fix the comments:
-// Response +// Request export type PostUploadfileRequest = z.input<typeof postUploadfileSchema.request>; -// Request +// Response export type PostUploadfileResponse = z.input<typeof postUploadfileSchema.response>;Also applies to: 11-12
apps/core/app/(landing)/_components/browseMegaMenu/browse-mega-menu.tsx (2)
7-7: Clean up extra whitespace in className.There are multiple consecutive spaces in the className string.
Apply this diff to clean up the whitespace:
- <div className="flex flex-col relative items-center justify-center w-full bg-background px-5 pb-6 rounded-b-xl"> + <div className="flex flex-col relative items-center justify-center w-full bg-background px-5 pb-6 rounded-b-xl">
8-8: Consider making RetroGrid properties configurable via props.The RetroGrid properties (opacity, cellSize, angle) are hardcoded, which might limit reusability.
Consider making these properties configurable through props or theme configuration.
apps/core/app/(landing)/_components/desktop-navbar/features-navbar.tsx (1)
5-7: Use more idiomatic prop name.The prop name
isloginshould follow common React boolean prop naming conventions.Apply this diff to use a more idiomatic prop name:
export interface AuthStatusBarProps { - islogin: boolean; + isLoggedIn: boolean; }And update the usage:
const FeaturesNavbar = (props: AuthStatusBarProps) => { - const { islogin } = props; + const { isLoggedIn } = props; return ( <div className="flex items-center gap-3"> - {islogin && <FeatureNavbarAuthenticated />} - {!islogin && <FeatureNavbarGuest />} + {isLoggedIn && <FeatureNavbarAuthenticated />} + {!isLoggedIn && <FeatureNavbarGuest />} </div> ); };packages/ui/src/components/molecules/input.tsx (1)
5-9: Follow TypeScript naming conventions for interfaces.The interface name
textFieldPropsshould follow PascalCase convention as it's standard for TypeScript interfaces.-export interface textFieldProps extends Omit<BaseInputProps, "error"> { +export interface TextFieldProps extends Omit<BaseInputProps, "error"> { label?: React.ReactNode; helperText?: React.ReactNode; error?: string; }packages/icons/src/components/polygon.tsx (1)
7-14: Add accessibility attributes to SVG.The SVG element should include accessibility attributes for better screen reader support.
<svg width={size} height={size} viewBox="0 0 140 154" fill="none" xmlns="http://www.w3.org/2000/svg" + role="img" + aria-hidden="true" {...resProps} >packages/apis/src/services/core/base/upload/post/post-uploadfile.schema.ts (2)
4-5: Fix swapped Request/Response comments and typo in schema name.There are two issues:
- The "Response" and "Request" comments are swapped
- There's a typo in "Transofrmed" in the schema name
-// Response +// Request export const postUploadfileRequestSchemaTransformed = z -// Request +// Response -export const postUploadfileResponseSchemaTransofrmed = apiResponseSchema +export const postUploadfileResponseSchemaTransformed = apiResponseSchemaAlso applies to: 17-18
32-35: Update schema object to use corrected schema name.Update the schema object to use the corrected schema name.
export const postUploadfileSchema = { - response: postUploadfileResponseSchemaTransofrmed, + response: postUploadfileResponseSchemaTransformed, request: postUploadfileRequestSchemaTransformed, };packages/apis/src/services/core/base/upload/post/use-post-uploadfile.ts (1)
8-8: Remove empty import statement.The empty import statement is unnecessary and should be removed.
-import {} from "./post-uploadfile.schema";packages/ui/src/components/atoms/base-input.tsx (1)
33-44: Enhance input accessibility with ARIA attributes.While the implementation is solid, consider adding ARIA attributes to improve accessibility:
<input type={type} className={cn( "flex h-10 w-full rounded-md border border-border ring-offset-background file:border-0 file:bg-transparent transition-shadow duration-500 focus-visible:outline-none disabled:cursor-not-allowed disabled:opacity-50", "bg-card text-foreground", "text-sm file:text-sm placeholder:text-muted-foreground font-normal file:font-medium", "focus-visible:ring-2 focus-visible:ring-primary focus-visible:ring-offset-2", baseInputVariants({ size,error, className }), )} ref={ref} + aria-invalid={error ? "true" : "false"} {...resProps} />apps/core/app/foundation/page.tsx (1)
11-46: Enhance semantic structure with appropriate HTML elements.Consider using semantic HTML elements to improve accessibility and document structure:
- <div> + <section> - <Typography variant="heading/xs">Foundation Input</Typography> + <Typography variant="heading/xs" as="h2">Foundation Input</Typography> - <div className="bg-card p-4 px-8 rounded flex gap-10"> + <form className="bg-card p-4 px-8 rounded flex gap-10"> {/* ... input groups ... */} - </div> + </form> - </div> + </section>packages/ui/src/components/molecules/attachment/attachmentLanding/attachmentItem/attachmentItem.tsx (1)
42-48: Consider extracting the complex condition into a variable.The condition for showing the delete button is complex and could be made more readable by extracting it into a descriptive variable.
+ const showDeleteButton = !file.loading && (canDeleteFile || !file?.previousUploaded); + {showDeleteButton ? ( <Button onClick={handleClick} className="p-0 flex justify-center items-center absolute top-0 bg-error right-0 w-[20px] h-[20px] rounded-sm hover:bg-error" > <Trash2 color="#fff" size={14} /> </Button> ) : null} - {file.loading || (!canDeleteFile && file?.previousUploaded) ? null : ( - <Button - onClick={handleClick} - className="p-0 flex justify-center items-center absolute top-0 bg-error right-0 w-[20px] h-[20px] rounded-sm hover:bg-error" - > - <Trash2 color="#fff" size={14} /> - </Button> - )}apps/core/app/(landing)/_components/browseMegaMenu/topPart/top-part-menu.tsx (1)
11-15: Consider enhancing CategoryItemProps interface.The interface could be extended to support optional props for better reusability:
- className for custom styling
- iconSize for flexible icon dimensions
interface CategoryItemProps { title: string; icon: React.ReactNode; link: string; + className?: string; + iconSize?: number; }packages/ui/src/components/atoms/retroGrid.tsx (2)
63-63: Consider performance optimization for grid animation.The animate-grid class might cause performance issues on lower-end devices. Consider:
- Using CSS
will-changeproperty- Adding a prop to disable animation
- <div className="animate-grid [background-image:linear-gradient(to_right,var(--light-line)_1px,transparent_0),linear-gradient(to_bottom,var(--light-line)_1px,transparent_0)] [background-repeat:repeat] [background-size:var(--cell-size)_var(--cell-size)] [height:300vh] [inset:0%_0px] [margin-left:-200%] [transform-origin:100%_0_0] [width:600vw] dark:[background-image:linear-gradient(to_right,var(--dark-line)_1px,transparent_0),linear-gradient(to_bottom,var(--dark-line)_1px,transparent_0)]" /> + <div className="animate-grid will-change-transform [background-image:linear-gradient(to_right,var(--light-line)_1px,transparent_0),linear-gradient(to_bottom,var(--light-line)_1px,transparent_0)] [background-repeat:repeat] [background-size:var(--cell-size)_var(--cell-size)] [height:300vh] [inset:0%_0px] [margin-left:-200%] [transform-origin:100%_0_0] [width:600vw] dark:[background-image:linear-gradient(to_right,var(--dark-line)_1px,transparent_0),linear-gradient(to_bottom,var(--dark-line)_1px,transparent_0)]" />
3-33: Enhance type safety for color props.Consider using a union type for color props to restrict them to valid CSS color values.
+type CSSColorValue = `#${string}` | `rgb(${number},${number},${number})` | `rgba(${number},${number},${number},${number})` | string; interface RetroGridProps extends React.HTMLAttributes<HTMLDivElement> { className?: string; angle?: number; cellSize?: number; opacity?: number; - lightLineColor?: string; - darkLineColor?: string; + lightLineColor?: CSSColorValue; + darkLineColor?: CSSColorValue; }packages/ui/src/components/molecules/attachment/attachmentAdmin/attachmentItem/attachmentItem.tsx (2)
65-67: Improve file size formatting.The current file size formatting could be more user-friendly by:
- Using appropriate units (KB for small files)
- Rounding to reasonable precision
+const formatFileSize = (bytes: number) => { + if (bytes < 1024 * 1024) { + return `${(bytes / 1024).toFixed(1)}KB`; + } + return `${(bytes / 1024 / 1024).toFixed(1)}MB`; +}; - <p className="w-full text-center text-xs text-gray-500"> - {(file.size / 1024 / 1024).toFixed(2)}mb - </p> + <p className="w-full text-center text-xs text-gray-500"> + {formatFileSize(file.size)} + </p>
58-63: Extract file name handling logic.Consider extracting the file name manipulation logic into a utility function for better maintainability.
+const getFileNameParts = (fileName: string) => { + const lastDotIndex = fileName.lastIndexOf('.'); + return { + name: fileName.slice(0, lastDotIndex), + extension: fileName.slice(lastDotIndex) + }; +}; - <span className="inline-block truncate w-[80%]"> - {file.name.slice(0, file.name.lastIndexOf(".") - 1)} - </span> - <span className="inline-block w-[20%]"> - {file.name.slice(file.name.lastIndexOf("."), file.name.length)} - </span> + {(() => { + const { name, extension } = getFileNameParts(file.name); + return ( + <> + <span className="inline-block truncate w-[80%]">{name}</span> + <span className="inline-block w-[20%]">{extension}</span> + </> + ); + })()}apps/core/app/(landing)/_components/browseMegaMenu/bottomPart/bottom-part-menu.tsx (1)
2-10: Consider consolidating icon imports.Multiple individual icon imports could be consolidated using a barrel file.
-import IllustratorIcon from "@repo/icons/Illustrator"; -import Adobexdicon from "@repo/icons/adobexd"; -import Belendericon from "@repo/icons/belender"; -// ... other icon imports +import { + IllustratorIcon, + Adobexdicon, + Belendericon, + // ... other icons +} from "@repo/icons";apps/core/app/(landing)/_components/desktop-navbar/navbar.tsx (2)
35-36: Improve semantic HTML structure.Using a
<div>for navigation content instead of<nav>reduces semantic meaning.- <div className="bg-background shadow-box rounded-xl fixed top-12 -left-1/2 -right-1/2 z-40 w-[95%] sm:w-[90%] lg:w-[962px] max-w-none mx-auto"> - <div className="flex py-3 px-5 items-center h-full w-full justify-between "> + <nav className="bg-background shadow-box rounded-xl fixed top-12 -left-1/2 -right-1/2 z-40 w-[95%] sm:w-[90%] lg:w-[962px] max-w-none mx-auto"> + <div className="flex py-3 px-5 items-center h-full w-full justify-between">
87-89: Remove commented code.The commented mobile menu code should be removed if it's no longer needed.
- {/* mobile menu */} - {isMobile && <>{/* <Menu2icon size={25} color="white" /> */}</>}apps/core/app/auth/signup/page.tsx (1)
112-116: Consider removing commented code.Large blocks of commented code should be removed and tracked in version control instead.
- {/* <div className="flex items-center w-full gap-3 my-4"> - <div className="w-full h-[1px] bg-gray-700 rounded-full"></div> - <p className="text-base font-medium">OR</p> - <div className="w-full h-[1px] bg-gray-700 rounded-full"></div> - </div> */} - {/* Social Login Buttons */} - {/* <div className="flex w-full flex-col items-center gap-3"> - <Button - size="lg" - className="w-full text-lg bg-[#181818]" - variant="secondary" - > - <GoogleIcon size={24} className="mr-2" /> - Log in with Google - </Button> - <Button - size="lg" - className="w-full text-lg bg-[#181818]" - variant="secondary" - > - <LinkedinIcon size={24} className="mr-2" /> - Log in with LinkedIn - </Button> - </div> */}Also applies to: 119-136
packages/ui/src/components/molecules/attachment/attachmentThumbnail/attachmentThumbnail.tsx (1)
26-29: Extract magic values to constants.Hardcoded values should be extracted to named constants or configuration.
+const DEFAULT_ALLOWED_TYPES = ["jpg", "jpeg"] as const; +const DEFAULT_PRICE_UNIT = "$"; +const DEFAULT_PRICE = 0; const { avatar, - allowedTypes = ["jpg", "jpeg"], - price = 0, - priceUnit = "$", + allowedTypes = DEFAULT_ALLOWED_TYPES, + price = DEFAULT_PRICE, + priceUnit = DEFAULT_PRICE_UNIT, } = props;apps/core/app/(landing)/layout.tsx (1)
40-92: Consider refactoring repeated link patterns to reduce duplication.The footer contains three identical sets of links that could be refactored into a reusable component or mapped from a configuration array.
Example refactor:
+ const BROWSE_LINKS = [ + { text: "UI Kits", href: "/landing/product/id/page" }, + { text: "Featured products", href: "/landing/product/id/page" }, + { text: "Coded Templates", href: "/landing/product/id/page" }, + { text: "Wireframe kits", href: "/landing/product/id/page" }, + { text: "Illustrations", href: "/landing/product/id/page" }, + ]; + + const FooterLinkList = ({ links }: { links: Array<{ text: string, href: string }> }) => ( + <ul className="flex flex-col gap-4 font-normal"> + {links.map(({ text, href }) => ( + <Link key={text} href={href} prefetch={false}> + <li className="text-xs sm:text-sm">{text}</li> + </Link> + ))} + </ul> + ); <div className="flex gap-5"> - <div> - <ul className="flex flex-col gap-4 font-normal"> - <Link href="/landing/product/id/page" prefetch={false}> - <li className="text-xs sm:text-sm">UI Kits</li> - </Link> - ... - </ul> - </div> + {[...Array(3)].map((_, i) => ( + <div key={i}> + <FooterLinkList links={BROWSE_LINKS} /> + </div> + ))} </div>apps/core/app/(landing)/product/test/page.tsx (1)
153-196: Consider refactoring repeated highlight blocks to reduce duplication.The highlights section contains four identical blocks that could be refactored into a reusable component or mapped from a configuration array.
Example refactor:
+ const HIGHLIGHTS = [ + "Fully Oraganised Layers", + "Fully Oraganised Layers", + "Fully Oraganised Layers", + "Fully Oraganised Layers" + ]; + + const HighlightBlock = ({ text }: { text: string }) => ( + <div className="bg-gray-800 rounded-2xl p-4"> + <div className="flex gap-2 p-2 items-center"> + <Circlecheckicon + size={24} + className="opacity-15 text-success-400 bg-white rounded-full" + /> + <p className="text-sm font-semibold">{text}</p> + </div> + </div> + ); <div className="flex flex-col gap-2"> - <div className="bg-gray-800 rounded-2xl p-4"> - <div className="flex gap-2 p-2 items-center"> - <Circlecheckicon size={24} className="opacity-15 text-success-400 bg-white rounded-full" /> - <p className="text-sm font-semibold">Fully Oraganised Layers</p> - </div> - </div> - ...repeated blocks... + {HIGHLIGHTS.map((text, index) => ( + <HighlightBlock key={index} text={text} /> + ))} </div>packages/design-system/src/tokens/token_typography_Mode 1.json (1)
1-200: Typography Tokens Structure & Naming ConsistencyThe overall JSON structure is well-organized and clearly defines the typography tokens for colors, font families, line heights, sizes, and weights. Each token is properly annotated using "$type" and "$value".
However, there is a minor naming inconsistency in the "weight" section (lines 182-198):
• The token key "light" (line 194) is in lowercase, while the other weight tokens ("Bold", "Medium", "Regular") are capitalized.For consistency and to avoid potential lookup issues in consuming code, consider renaming "light" to "Light" (e.g., use diff below):
- "light": { - "$type": "string", - "$value": "Light" - } + "Light": { + "$type": "string", + "$value": "Light" + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (107)
.github/workflows/build-design-system.yml(1 hunks).hintrc(1 hunks).husky/pre-rebase(1 hunks)apps/core/app/(landing)/_components/addtoCard.tsx(1 hunks)apps/core/app/(landing)/_components/browseMegaMenu/bottomPart/bottom-part-menu.tsx(1 hunks)apps/core/app/(landing)/_components/browseMegaMenu/browse-mega-menu.tsx(1 hunks)apps/core/app/(landing)/_components/browseMegaMenu/topPart/top-part-menu.tsx(1 hunks)apps/core/app/(landing)/_components/card-cart-skeleton.tsx(1 hunks)apps/core/app/(landing)/_components/card.tsx(1 hunks)apps/core/app/(landing)/_components/desktop-navbar/animated-navbar.tsx(0 hunks)apps/core/app/(landing)/_components/desktop-navbar/feather-navbar.tsx(0 hunks)apps/core/app/(landing)/_components/desktop-navbar/feature-navbar-authenticated.tsx(2 hunks)apps/core/app/(landing)/_components/desktop-navbar/feature-navbar-guest.tsx(2 hunks)apps/core/app/(landing)/_components/desktop-navbar/features-navbar.tsx(1 hunks)apps/core/app/(landing)/_components/desktop-navbar/navbar-links.tsx(1 hunks)apps/core/app/(landing)/_components/desktop-navbar/navbar.tsx(4 hunks)apps/core/app/(landing)/_components/desktop-navbar/search-bar.tsx(1 hunks)apps/core/app/(landing)/become-auther/_components/auther-form.tsx(1 hunks)apps/core/app/(landing)/layout.tsx(1 hunks)apps/core/app/(landing)/page.tsx(1 hunks)apps/core/app/(landing)/product/[id]/_components/comment-section.tsx(1 hunks)apps/core/app/(landing)/product/[id]/_components/product-footer.tsx(1 hunks)apps/core/app/(landing)/product/[id]/page.tsx(1 hunks)apps/core/app/(landing)/product/test/page.tsx(1 hunks)apps/core/app/(landing)/store/cart-store.ts(1 hunks)apps/core/app/(landing)/test/megaMenuContent/page.tsx(1 hunks)apps/core/app/(landing)/test/page.tsx(1 hunks)apps/core/app/auth/forget-password/_components/form/forgetPasswordForm.tsx(1 hunks)apps/core/app/auth/layout.tsx(1 hunks)apps/core/app/auth/login/_components/form/loginForm.tsx(1 hunks)apps/core/app/auth/login/page.tsx(2 hunks)apps/core/app/auth/otp/page.tsx(2 hunks)apps/core/app/auth/set-password/page.tsx(1 hunks)apps/core/app/auth/signup/otp/_components/signup-otp-form.tsx(2 hunks)apps/core/app/auth/signup/page.tsx(5 hunks)apps/core/app/dashboard/page.tsx(3 hunks)apps/core/app/foundation/page.tsx(1 hunks)apps/core/app/layout.tsx(1 hunks)apps/core/middleware.ts(1 hunks)apps/core/next.config.mjs(1 hunks)apps/core/package.json(1 hunks)package.json(1 hunks)packages/apis/package.json(1 hunks)packages/apis/src/services/core/base/upload/post/post-uploadfile.schema.ts(1 hunks)packages/apis/src/services/core/base/upload/post/post-uploadfile.ts(1 hunks)packages/apis/src/services/core/base/upload/post/post-uploadfile.types.ts(1 hunks)packages/apis/src/services/core/base/upload/post/use-post-uploadfile.ts(1 hunks)packages/design-system/package.json(1 hunks)packages/design-system/src/scripts/build-colors-style.ts(1 hunks)packages/design-system/src/scripts/build-colors-tailwind.ts(1 hunks)packages/design-system/src/scripts/build-config-tailwind.ts(1 hunks)packages/design-system/src/scripts/build-main-style.ts(1 hunks)packages/design-system/src/scripts/build-palette-style.ts(1 hunks)packages/design-system/src/scripts/build-palette-tailwind.ts(1 hunks)packages/design-system/src/scripts/build.ts(1 hunks)packages/design-system/src/scripts/utills.ts(1 hunks)packages/design-system/src/styles/base/_colors-dark.scss(1 hunks)packages/design-system/src/styles/base/_colors-light.scss(1 hunks)packages/design-system/src/styles/base/_palette.scss(1 hunks)packages/design-system/src/styles/main.scss(1 hunks)packages/design-system/src/tailwind/colors.ts(1 hunks)packages/design-system/src/tailwind/config.ts(1 hunks)packages/design-system/src/tailwind/palette.ts(1 hunks)packages/design-system/src/tokens/token_Icon_size_Mode 1.json(1 hunks)packages/design-system/src/tokens/token_color_default.json(1 hunks)packages/design-system/src/tokens/token_color_light.json(1 hunks)packages/design-system/src/tokens/token_palette_primitive.json(1 hunks)packages/design-system/src/tokens/token_radius_size.json(1 hunks)packages/design-system/src/tokens/token_spacing_Desktop.json(1 hunks)packages/design-system/src/tokens/token_typography_Mode 1.json(1 hunks)packages/design-system/tsconfig.json(1 hunks)packages/design-system/tsconfig.lint.json(1 hunks)packages/icons/package.json(2 hunks)packages/icons/src/components/3dAsset.tsx(1 hunks)packages/icons/src/components/after-Effect.tsx(1 hunks)packages/icons/src/components/cinema4D.tsx(1 hunks)packages/icons/src/components/icon-sets.tsx(1 hunks)packages/icons/src/components/illustration.tsx(1 hunks)packages/icons/src/components/metamask.tsx(1 hunks)packages/icons/src/components/mockups.tsx(1 hunks)packages/icons/src/components/paypal.tsx(1 hunks)packages/icons/src/components/polygon.tsx(1 hunks)packages/icons/src/components/refresh.tsx(1 hunks)packages/icons/src/components/remove.tsx(1 hunks)packages/icons/src/components/security.tsx(1 hunks)packages/icons/src/components/tonconnect.tsx(1 hunks)packages/icons/src/components/ui-kit.tsx(1 hunks)packages/icons/src/components/wire-frame.tsx(1 hunks)packages/ui/package.json(5 hunks)packages/ui/postcss.config.mjs(1 hunks)packages/ui/src/components/atoms/base-input.tsx(1 hunks)packages/ui/src/components/atoms/button.tsx(2 hunks)packages/ui/src/components/atoms/carousel.tsx(1 hunks)packages/ui/src/components/atoms/input.tsx(0 hunks)packages/ui/src/components/atoms/navigation-menu.tsx(8 hunks)packages/ui/src/components/atoms/retroGrid.tsx(1 hunks)packages/ui/src/components/atoms/sidebar.tsx(1 hunks)packages/ui/src/components/atoms/tooltip.tsx(1 hunks)packages/ui/src/components/molecules/attachment/attachmentAdmin/attachmentAdmin.tsx(2 hunks)packages/ui/src/components/molecules/attachment/attachmentAdmin/attachmentItem/attachmentItem.tsx(3 hunks)packages/ui/src/components/molecules/attachment/attachmentLanding/attachmentItem/attachmentItem.tsx(2 hunks)packages/ui/src/components/molecules/attachment/attachmentLanding/attachmentLanding.tsx(4 hunks)packages/ui/src/components/molecules/attachment/attachmentThumbnail/attachmentThumbnail.tsx(1 hunks)packages/ui/src/components/molecules/attachment/useAttachment.ts(2 hunks)packages/ui/src/components/molecules/input.tsx(1 hunks)packages/ui/src/components/molecules/label-wrapper.tsx(1 hunks)packages/ui/src/components/molecules/sidebar.tsx(1 hunks)
⛔ Files not processed due to max files limit (9)
- packages/ui/src/components/organisms/foundation-color.tsx
- packages/ui/src/components/ui/attachment/attachmentAdmin/attachmentAdmin.tsx
- packages/ui/src/components/ui/attachment/attachmentAdmin/attachmentItem/attachmentItem.tsx
- packages/ui/src/globals.css
- packages/ui/src/styles/_tailinwd.scss
- packages/ui/src/styles/globals.scss
- packages/ui/src/template/ui-component.tsx
- packages/ui/tailwind.config.ts
- packages/ui/tsconfig.json
💤 Files with no reviewable changes (3)
- packages/ui/src/components/atoms/input.tsx
- apps/core/app/(landing)/_components/desktop-navbar/animated-navbar.tsx
- apps/core/app/(landing)/_components/desktop-navbar/feather-navbar.tsx
✅ Files skipped from review due to trivial changes (16)
- apps/core/app/(landing)/_components/desktop-navbar/search-bar.tsx
- packages/ui/src/components/atoms/carousel.tsx
- packages/design-system/tsconfig.lint.json
- apps/core/app/(landing)/product/[id]/_components/comment-section.tsx
- packages/design-system/src/tokens/token_Icon_size_Mode 1.json
- packages/design-system/src/styles/main.scss
- apps/core/app/(landing)/product/[id]/_components/product-footer.tsx
- apps/core/app/layout.tsx
- packages/design-system/tsconfig.json
- packages/ui/src/components/atoms/navigation-menu.tsx
- apps/core/app/auth/layout.tsx
- apps/core/app/(landing)/_components/desktop-navbar/feature-navbar-guest.tsx
- packages/design-system/package.json
- apps/core/app/(landing)/page.tsx
- apps/core/app/(landing)/become-auther/_components/auther-form.tsx
- packages/design-system/src/tokens/token_palette_primitive.json
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/core/app/dashboard/page.tsx
- packages/ui/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ui/src/components/atoms/button.tsx
[error] 53-53: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
apps/core/app/(landing)/_components/addtoCard.tsx
[error] 58-58: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
packages/ui/src/components/molecules/attachment/useAttachment.ts
[error] 54-54: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🪛 actionlint (1.7.4)
.github/workflows/build-design-system.yml
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (96)
packages/design-system/src/tokens/token_spacing_Desktop.json (2)
1-114: Overall JSON Structure and Consistency
This new JSON file is well-structured. Each spacing token is defined as an object with a "$type" set to "number" and a "$value" that follows the expected multiple-of-4 pattern, ensuring a standardized approach across the design system.
1-114: Verify Integration with UI Components
Ensure that these spacing tokens are properly consumed by the intended UI components (such as AppSidebar and LogoIconSwitcher) to maintain a consistent design language across desktop views..github/workflows/build-design-system.yml (8)
1-2: Descriptive Workflow Name
The workflow is named “Build Design System” which clearly indicates its purpose.
3-7: Precise Trigger Configuration
The trigger is scoped to pull requests that modify files under "packages/design-system/src/tokens/**", ensuring the workflow runs only when design tokens are updated.
8-11: Job Declaration and Runner Setup
The job "build-design-system" is properly declared and scheduled to run on "ubuntu-latest", which is suitable for the tasks that follow.
20-29: Caching Bun Dependencies
The cache step is well configured: it caches Bun's installation cache using a key based on the OS and the hash of "bun.lockb". This setup should help speed up subsequent builds by reusing cached dependencies.
30-35: Bun Setup Step
The action "oven-sh/setup-bun@v1" is used to set up Bun with the latest version. This is an appropriate choice to ensure your build environment is up-to-date.
36-41: Conditional Dependency Installation
The step "Install dependencies" is executed only if the cache was not restored. Using the condition "if: steps.cache-deps.outputs.cache-hit != 'true'" is a smart way to avoid redundant installs.
42-46: Executing the Build Script
The "Run build script" step correctly switches to the design system's working directory and executes "bun run build", ensuring that the build process is properly initiated.
47-55: Automated Commit and Push
The final step commits and pushes any changes in the design system directory using "stefanzweifel/git-auto-commit-action@v5". The configuration (commit message, file pattern, and branch reference) is well defined.apps/core/next.config.mjs (1)
8-10: Verify Unsplash domain usage in the codebase.The configuration is correctly structured. Let's verify that this domain is actually used in the new sidebar components.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for Unsplash image URLs in the codebase rg -i "images\.unsplash\.com" --type tsx --type jsxLength of output: 81
Let's re-run the search command across all files (without file type restrictions) to verify that the Unsplash domain is referenced somewhere in the codebase.
🏁 Script executed:
#!/bin/bash # Search for Unsplash image URLs in the entire codebase to confirm usage in the new sidebar components rg -i "images\.unsplash\.com"Length of output: 520
Unsplash Domain Confirmed in the Codebase
The Unsplash domain configuration in next.config.mjs is correctly set up and is actively utilized in the codebase. A reference to an Unsplash URL was found in the landing page card component (apps/core/app/(landing)/_components/card.tsx), confirming that the domain is indeed used.
packages/icons/src/components/refresh.tsx (1)
25-25: LGTM!The export statement follows common practices.
apps/core/app/(landing)/test/page.tsx (2)
1-4: LGTM! Proper setup of client-side component.The imports are well-organized and the client directive is correctly placed.
20-29: Apply the same improvements as AttachmentLanding.The same issues with console logging and error handling are present here.
packages/ui/src/components/atoms/sidebar.tsx (2)
73-75: Verify SSR compatibility for mobile detection logic.Since you’re using the custom “useIsMobile” hook, ensure that it safely avoids referencing window properties during server-side rendering if the component is ever rendered SSR.
89-91: Check the logic that writes to document.cookie.Writing to document.cookie is done in setOpen. If you rely on SSR at any point, be advised that document is not available on the server. You may want to confirm that this code always executes client-side, or add a safeguard.
packages/icons/src/components/ui-kit.tsx (3)
1-1: LGTM! Clean import statement.Good practice to keep types in a separate file.
3-5: LGTM! Well-structured component definition.Good practices observed:
- Clean props destructuring with sensible defaults
- Rest props pattern for flexibility
25-25: LGTM! Clean export statement..husky/pre-rebase (2)
1-2: LGTM! Proper script setup.The script correctly sets up the shell interpreter and sources the husky.sh helper.
4-8: LGTM! Good safety check.The script properly prevents nested rebases by checking for existing rebase directories and provides a clear error message.
packages/ui/postcss.config.mjs (1)
3-3: LGTM!The addition of SCSS syntax support aligns with the transition from CSS to SCSS in the project.
.hintrc (1)
5-13: Verify the impact of disabling accessibility and TypeScript rules.The changes disable the following rules:
typescript-config/consistent-casing: This might lead to inconsistent naming conventions.axe/formslabel: This could impact form accessibility.Please ensure these changes don't negatively impact code quality and accessibility standards.
Could you provide the rationale for disabling these rules? If they're causing false positives, we might want to configure them instead of disabling them completely.
apps/core/app/(landing)/store/cart-store.ts (1)
3-8: LGTM!The
CartItemtype definition is well-structured and includes essential properties.packages/icons/src/components/illustration.tsx (1)
3-4: Similar issues as in MockUpsIcon component.This component has the same issues as identified in the MockUpsIcon component:
- Hardcoded fill color instead of using the color prop
- Inconsistent default size (53px)
Please apply the same fixes as suggested for the MockUpsIcon component.
Also applies to: 17-17
packages/icons/src/components/after-Effect.tsx (1)
1-38: Well-structured implementation!The component demonstrates good practices:
- Appropriate default size (16px)
- Proper use of clip-path for complex shapes
- Brand-specific colors for the Adobe After Effects logo
packages/icons/src/components/3dAsset.tsx (1)
4-4: Use the color prop instead of hardcoding fill color.Similar to other icon components, the fill color is hardcoded to "white" instead of using the color prop.
Please apply the same fix as suggested for the MockUpsIcon component.
Also applies to: 17-17
packages/icons/src/components/paypal.tsx (1)
1-48: Well-structured icon component implementation!The component follows best practices with proper props handling and SVG implementation.
packages/icons/src/components/metamask.tsx (1)
1-262: Clean and well-structured MetaMask icon implementation!The component follows best practices with proper props handling and detailed SVG path definitions.
package.json (2)
10-10: New "dev:core" Script Added for Filtered Core Development.
The addition of"dev:core": "turbo dev --filter=core"(line 10) enables developers to target the core module specifically during development. Ensure that this new command is documented (e.g., in the project README) so that the team is aware of its purpose and usage.
15-17: Refined Script Configuration for "prepare", "deploy", and "rebase:main".
The updates on lines 15–17 remove extraneous spaces and introduce the"rebase:main"script, contributing to a cleaner and more consistent command setup. Please verify that these scripts work as intended in your CI/CD workflows.apps/core/package.json (1)
25-25: New "zustand" Dependency Added.
The introduction of"zustand": "^5.0.2"(line 25) provides state management support, which is especially useful for the new sidebar functionality. Confirm that this version aligns with your project’s compatibility requirements and update any related documentation if needed.packages/icons/package.json (2)
31-31: Updated Export Path for After-Effect Icon.
The export for the After-Effect icon has been renamed to"./After-Effect": "./src/components/after-effect.tsx"(line 31), which improves the consistency of naming conventions. Please verify that the referenced file name adheres to this naming style.
45-49: Addition of New Icon Exports.
New exports such as"./remove","./metamask","./tonconnect","./paypal", and"./security"(lines 45–49) expand the icon library. Confirm that these exports correctly point to their respective component files and follow the project’s naming guidelines.packages/design-system/src/scripts/utills.ts (2)
7-20: LGTM! Hue calculation is mathematically correct.The implementation properly handles all cases in the hue calculation, including edge cases when colors are equal (delta = 0).
22-23: LGTM! Saturation and lightness calculations are accurate.The implementation correctly follows the standard HSL color space formulas.
apps/core/app/(landing)/product/[id]/page.tsx (1)
55-60: Ensure consistent button styling after removing variants.The removal of
variant="default"from both buttons might affect styling consistency. While the first button has explicit styling, the second button lacks proper styling definitions.Consider one of these approaches:
- Keep the variants for consistent theming
- Define a new standardized button styling system
- Ensure both buttons have proper className definitions
Run this script to check for similar button variant removals across the codebase:
Also applies to: 62-66
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for commented out button variants and buttons without variants rg -U 'variant=|// variant=' --type tsxLength of output: 69
I'm going to re-run the check using a revised search pattern that targets TSX files by using a glob instead of a file type. Please run the following script:
This should provide us with better insight into the state of button variant usage across the codebase.
🏁 Script executed:
#!/bin/bash # Check for occurrences of "variant=" (whether active or commented out) in the specific file: rg 'variant=' "apps/core/app/(landing)/product/[id]/page.tsx" # Additionally, perform a global search in TSX files to see if similar patterns exist elsewhere: rg 'variant=' --glob '*.tsx'Length of output: 7341
Ensure consistent button styling across Button components
- Confirmed that in file apps/core/app/(landing)/product/[id]/page.tsx the variant prop has been commented out (i.e. “// variant="default"”), matching the snippet under review.
- A global search shows similar patterns elsewhere in the codebase, indicating this change is intentional in these components.
- Given that the first Button relies solely on explicit className values while others might not, consider verifying that styling remains consistent across all Button instances. You might want to either reinstate variant usage for consistency or ensure that all Buttons receive proper className styling.
packages/icons/src/components/wire-frame.tsx (1)
21-22: LGTM!The export statement follows React component conventions.
packages/design-system/src/scripts/build-main-style.ts (1)
4-6: Validate file paths.Consider validating the file paths to ensure they point to the correct locations.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify the existence of color files referenced in main.scss # Check if the color files exist fd -t f "_colors-(dark|light).scss$" "$(dirname "$0")/../styles/base"Length of output: 232
I'm generating a revised shell script to verify the existence of the directories and files constructed in build‐main‐style.ts relative to the repository root. This will help confirm whether the file paths (i.e. the output directory and output file) are valid. Please run the script and share the output.
🏁 Script executed:
#!/bin/bash # This script validates the file paths as computed in # packages/design-system/src/scripts/build-main-style.ts # The file under review constructs the output directory as: # path.join(__dirname, "../styles") # Given __dirname is "packages/design-system/src/scripts", the expected # directory should be "packages/design-system/src/styles". # # We verify the following: # 1. Directory "packages/design-system/src/styles" exists. # 2. File "packages/design-system/src/styles/main.scss" exists. # 3. (Optional) If there's a "base" subdirectory under "styles", list any _colors-(dark|light).scss files. OUTPUT_DIR="packages/design-system/src/styles" OUTPUT_FILE="${OUTPUT_DIR}/main.scss" BASE_DIR="${OUTPUT_DIR}/base" echo "Checking if '${OUTPUT_DIR}' exists:" if [ -d "$OUTPUT_DIR" ]; then echo "Directory exists: ${OUTPUT_DIR}" else echo "Directory does NOT exist: ${OUTPUT_DIR}" fi echo "" echo "Checking if '${OUTPUT_FILE}' exists:" if [ -f "$OUTPUT_FILE" ]; then echo "File exists: ${OUTPUT_FILE}" else echo "File does NOT exist: ${OUTPUT_FILE}" fi echo "" echo "Checking for '_colors-(dark|light).scss' files in '${BASE_DIR}':" if [ -d "$BASE_DIR" ]; then COLORS_FILES=$(find "$BASE_DIR" -maxdepth 1 -type f -regex ".*_colors-\(dark\|light\)\.scss") if [ -n "$COLORS_FILES" ]; then echo "Found the following color files:" echo "$COLORS_FILES" else echo "No _colors-(dark|light).scss files found in ${BASE_DIR}" fi else echo "Directory does NOT exist: ${BASE_DIR}" fiLength of output: 1731
File paths in build-main-style.ts are validated and correct.
- The output directory ("packages/design-system/src/styles") exists.
- The output file ("packages/design-system/src/styles/main.scss") exists.
- The expected color files ("_colors-light.scss" and "_colors-dark.scss") are present in the "base" subdirectory.
packages/design-system/src/scripts/build-palette-tailwind.ts (1)
34-51: Add error handling and input validation.Similar to build-palette-style.ts, this function needs proper error handling.
Please apply the same error handling improvements suggested for build-palette-style.ts.
packages/design-system/src/styles/base/_colors-dark.scss (1)
12-14: Review destructive color contrast in dark theme.The destructive colors might have insufficient contrast:
- Using rose-500 with black foreground might not meet WCAG contrast requirements
✅ Verification successful
🌐 Web query:
What is the contrast ratio between Tailwind's rose-500 and black colors?💡 Result:
<think> Okay, let's tackle this question about the contrast ratio between Tailwind's rose-500 and black colors. First, I need to understand what the user is asking. They want to know how much contrast there is between these two specific colors according to accessibility standards, probably for web design purposes. Looking at the search results provided, I'll start by checking which sources mention Tailwind's color palette and contrast ratios. Result [1] is the Tailwind CSS documentation, which details their color system. It lists all the color names and their variants, including rose-500. However, it doesn't directly provide contrast ratios. Next, results [2], [3], [4], [6], and [9] discuss contrast ratio checkers and WCAG guidelines. Among these, result [4] from colour-a11y.vercel.app specifically mentions testing Tailwind CSS colors for accessibility. This seems promising. According to result [4], when testing Tailwind's rose-500 against a white background, it scores "AA Large," but against a black background isn't explicitly mentioned. Wait, the user is asking about rose-500 against black, so maybe I need to check if there's data on that. Wait, in result [4], the site allows switching between white and black backgrounds. The example shows that rose-500 against white has certain scores, but when switching to a black background (dark mode), the scores might change. Let me check the details again. Looking at result [4], under the "rose" section, each color variant (50 to 900) is tested against a background (which can be toggled between white and black). For rose-500, when the background is white, it's "AA Large." But when the background is black (dark mode), what's the score? The example given in result [4] for rose-500 on a white background is AA Large. However, when switching to dark mode (black background), the same rose-500 would be text color on a black background. The site shows that for rose-500 against black (assuming dark mode is black), perhaps the contrast ratio would be different. Wait, result [4] says: "You can switch between a white and black background using the toggle below and the scores will change for that background." So if we toggle to black background (dark mode), then the text color would be rose-500 on black. Let me check if any of the entries in result [4] show this. Looking at the example in result [4], under "rose" it lists scores for each shade against the current background. For example, rose-50 on white fails, but when you switch to dark mode (black background), those same classes would be tested against black. However, in the provided snippet, only some scores are visible. For example, rose-800 and 900 on white get AAA, but when switching to black, maybe their scores change. But wait, how does rose-500 fare against black? The problem is that in the provided search results for [4], only some parts are visible. The snippet shows rose-500 against white as AA Large. To find out against black, perhaps we need to infer based on other colors or look for another source. Result [3] has a table with colors against white and black. For example, #000 (black) against white has a ratio of 21:1. But that's not directly helpful here since we need rose-500's hex value. Result [7] mentions that Tailwind's colors are manually adjusted for perceived brightness and contrast. But it doesn't give specific contrast ratios. Result [8] is a Tailwind color generator but doesn't provide contrast data. Result [10] explains how to use Tailwind's background colors but doesn't discuss contrast ratios. So going back to result [4], since it's specifically testing Tailwind colors with WCAG ratings and allows switching backgrounds, it's likely the best source. However, in the provided snippet for result [4], when you switch to dark mode (black background), how does rose-500 perform? The snippet shows that for rose-600 and above on white background get AA or AAA ratings. But when using dark mode (black background), perhaps lighter colors like rose-500 would have lower contrast against black. Wait, in result [4], when you switch to dark mode (black background), the text color would be lighter on dark. So if we're testing rose-500 (which is a mid-tone pink) on a black background, what's the ratio? Alternatively, perhaps we can calculate it using another tool mentioned in the search results like result [2] or [6]. But since those are external tools not directly linked here except through search results snippets. Alternatively, maybe Tailwind's documentation in result [1] provides hex values for rose-500 so we can compute it manually using the formula from result [3]. Result [3] mentions that contrast ratio ranges from 1:1 to 21:1 and provides a formula. From result [1], looking at the code snippet under "Colors" for Tailwind CSS: In result [1], under "--color-emerald-..." etc., there's a section where they define colors using oklch values. For example: --color-emerald-500: oklch(0.696 0.17 162.48); But looking for rose-500 in result [1]: Searching through result [1], there's a part where they list all color definitions. Let me find where they define rose colors. In result [1], towards the end of the code block: --color-slate-950: oklch(0.129 0.042 264.695); --color-gray-50: oklch(0.985 0.002 247.839); --color-gray-100: oklch(0.967 0.003 264.542); ... Then later: --color-stone-950: oklch(0.147 0.004 49.25); --color-black: #000; --color-white: #fff; Wait, maybe I missed where they define rose colors? Let me check again. In result [1], under "@theme { ... }", there are various color definitions like: --color-green-500: oklch(0.723 0.219 149.579); ... --color-emerald-50: oklch(0.979 0.021 166.113); ... --color-teal-50: oklch(0.984 0.014 180.72); ... But I don't see --color-rose entries here. Wait earlier in result [1], there's a list of color names including "rose". The default palette includes "rose" as one of the colors with steps from 50 to 950. But in the code snippets provided in search result [1], perhaps they don't list all color definitions explicitly except as examples. Alternatively, maybe I need to find another source within search results that gives hex codes for Tailwind's rose-500. Result [8] is a Tailwind CSS color generator which might show hex values for rose-500. Looking at result [8], which says "Create and customize beautiful Tailwind CSS color palettes." If I could access this tool, I could find rose-500's hex value. But within the provided search results snippet for result [8], there’s no specific data about rose’s hex codes. Alternatively, since I can't access external links beyond what’s provided in search results snippets, perhaps there’s another way. Alternatively, perhaps from result [7], which discusses tweaking Tailwind colors: In result [7], an example mentions `pink-500` having hue 330 and `rose-500` hue 350. But without knowing saturation and lightness values or hex codes for rose-500 from these snippets, calculating manually isn't feasible here. Alternatively, maybe using WCAG formula from search results: Result[2]: Polypane uses WCAG or APCA algorithms. Result[3]: Contrast ratio formula gives ratio from 1:1 to 21:1. Result[6]: Siege Media tool based on WCAG guidelines. But without knowing exact hex codes of Tailwind’s rose-500 and black (#000), I can't compute it manually here based solely on search results provided. Wait wait—search result[1] does mention that --color-black is #000 and --color-white is #fff towards the end: --color-black: #000; --color-white: #fff; So assuming that Tailwind’s default 'black' is #000000. Now need to find out what hex code corresponds to Tailwind’s 'rose-500'. Looking through search result[1]: In one of the code blocks under @theme definitions: There are lines like: --color-green-950: oklch(0.266 0.065 152.934); --color-emerald-50: oklch(0.979 0.021 166.113); ... But no explicit mention of 'rose' in these variables except possibly further down? Continuing through code blocks: Further down: --color-slate-950: oklch(0...); Then after slate comes gray definitions: --color-gray-50: ...; ... Then stone definitions: --color-stone Citations: - 1: https://tailwindcss.com/docs/colors - 2: https://polypane.app/color-contrast/ - 3: https://www.msfw.com/services/contrastratiocalculator - 4: https://colour-a11y.vercel.app - 5: https://tailwindcss.com/docs/filter-contrast - 6: https://www.siegemedia.com/contrast-ratio - 7: https://www.stackbit.com/blog/tweaking-tailwind-colors - 8: https://uicolors.app/create - 9: https://getbootstrap.com/docs/5.0/customize/color/ - 10: https://windframe.dev/tailwind/classes/tailwind-background-color
Contrast Assessment: Rose-500 vs Black
Based on our research and calculations, Tailwind’s rose-500 (commonly #f43f5e) on a black (#000000) background yields an estimated contrast ratio of roughly 5.7:1. This satisfies the WCAG AA requirement for normal text (minimum 4.5:1), though it falls short of the AAA guideline of 7:1.
• If your design targets typical AA compliance, this pairing is acceptable.
• Consider adjusting the shade or foreground if AAA-level contrast is needed or if the text is very small.packages/design-system/src/styles/base/_palette.scss (22)
1-2: CSS Layer and Root Setup
The declaration of the CSS layer using “@layer base” and the use of “:root” for defining custom properties are properly implemented. This organization helps in scoping the color variables.
15-25: Blue Color Palette Definition
The blue color variables are well organized and follow a clear numeric progression. Ensuring that these values match your design tokens will give a unified look across the UI.
27-37: Cyan Color Palette Definition
The cyan color palette is defined over an adequate range of shades. The HSL values smoothly transition between light and dark variants, which is beneficial for various UI contexts.
39-49: Emerald Color Palette Definition
The emerald palette is comprehensively defined with a good range of HSL values. The fast progression in saturation and lightness is excellent for emphasizing elements in the UI.
51-61: Fuchsia Color Palette Definition
The fuchsia color variables show a consistent scale from very light to very dark, which should serve well for accents and highlights.
63-64: General Black and White Variables
Defining “--general-black” and “--general-white” provides essential defaults for the design system. Their simple HSL definitions (0 0% 0% and 0 0% 100%) ensure standard colors are available at all times.
66-76: Gray Color Palette Definition
The gray palette, covering shades 50 to 950, is structured to offer subtle gradations, which can be vital for background elements and borders. Consider verifying that the contrast ratios meet accessibility guidelines where these are applied.
78-89: Green Color Palette Definition
The green palette values provide a gentle transition from light to dark, which should fit well in accents or status indicators. The HSL values appear consistent with typical design trends.
90-101: Indigo Color Palette Definition
The indigo color variables are well laid out with a smooth transition across the range. Double-check these values against the design specifications to ensure consistency across themes.
102-112: Lime Color Palette Definition
The lime color definitions are clear and provide the expected range of motion from very light to deep tones. Ensuring consistency with other accent colors might improve overall visual harmony.
126-137: Orange Color Palette Definition
The orange values are precisely defined for a versatile range of shades. Ensure that these hues align with the overall mood and energy intended in the design system.
150-160: Purple Color Palette Definition
The purple color set is systematically defined with a balanced progression. It’s ready for use in components that may require a royal or creative touch.
161-172: Red Color Palette Definition
The red palette is clearly defined, offering a range of intensities suitable for error indicators, alerts, or highlights. Verifying these shades against design standards is recommended.
173-184: Rose Color Palette Definition
The rose shades are well represented from very light to strikingly dark, which should help in creating visually appealing accents.
185-196: Sky Color Palette Definition
The sky palette is carefully specified to offer gentle, airy tones. Confirm that these values support the desired mood for background or highlight elements in the sidebar and dashboard environments.
197-208: Slate Color Palette Definition
The slate colors provide a robust and versatile set of shades suitable for text and interface elements. These values can be particularly beneficial in a dark theme context.
209-220: Stone Color Palette Definition
The stone color variables cover a natural and neutral spectrum, which can ground the design elements. Checking for sufficient color contrast when used in UI components is advisable.
221-232: Teal Color Palette Definition
The teal shades are well structured and provide a clear progression from light to dark. This range is ideal for accentuating call-to-action elements without overwhelming the design.
233-244: Violet Color Palette Definition
The violet color definitions offer a stylistic range that can complement both light and dark UI elements. Verify with design mockups to ensure these values meet the visual requirements.
245-256: Yellow Color Palette Definition
The yellow palette is defined with a consistent scale, providing bright and bold options that are suitable for attention-grabbing UI components. Double-check the usability, especially regarding contrast and readability when used as background or text colors.
257-268: Zinc Color Palette Definition
The zinc color variables are systematically declared, offering subtle transition shades that can be used for borders or muted backgrounds. This definition fits well with the overall design system approach.
269-270: Closure of the CSS Block
The closing braces properly terminate the “:root” and “@layer base” blocks. The structure is complete and adheres to standard SCSS practices.packages/ui/src/components/molecules/attachment/useAttachment.ts (9)
3-4: Updated import for 'UsePostUploadfile'
No issues identified.
15-18: New optional props
The addition of canDeleteFile, value, disabled, and error broadens the hook’s flexibility and adaptability.
21-21: New optional fields for FileState
Allowing optional id and previousUploaded provides a clear way to handle previously uploaded files.Also applies to: 27-28
29-33: New 'AttachmentItemProps' interface
Defining AttachmentItemProps clarifies responsibilities for file rendering and deletion.
39-39: Destructuring 'canDeleteFile'
Including canDeleteFile in the hook props helps manage file deletion behavior.
80-85: handleRemove method
Removing files by matching either the name (string) or id (number) is a clear and effective approach.
141-144: handleChange logic
File input handling, including multiple checks and calls to getUploadFiles, appears consistent.
147-158: Drag-and-drop handlers
The handleDrop and handleDragOver functions are implemented cleanly and should enhance usability.
165-166: Returning handleDragOver and handleDrop
Exposing both handlers from the hook is appropriate for external components.apps/core/app/(landing)/test/megaMenuContent/page.tsx (1)
1-9: New 'Page' Component
Straightforward implementation of the Page component with BrowseMegaMenu.apps/core/app/(landing)/_components/desktop-navbar/navbar-links.tsx (1)
1-16: Verify link destinations
Both anchor tags use "/browse" as their href. If this duplication is unintentional, please correct it.packages/ui/src/components/molecules/input.tsx (1)
11-27: LGTM! Well-structured component implementation.The component implementation is clean and follows best practices:
- Uses
forwardRefcorrectly for ref forwarding- Properly destructures and spreads props
- Correctly converts error string to boolean
packages/icons/src/components/polygon.tsx (1)
3-21: LGTM! Clean icon implementation.The component implementation follows best practices:
- Proper TypeScript typing with IconProps
- Good default values for size and color
- Clean SVG structure
packages/ui/src/components/atoms/tooltip.tsx (2)
1-7: LGTM! Good use of "use client" directive and imports.The component is correctly marked for client-side rendering, and imports are well-organized.
14-28: LGTM! Well-implemented TooltipContent component.The implementation follows React best practices:
- Proper use of forwardRef for ref forwarding
- Good type definitions with ElementRef and ComponentPropsWithoutRef
- Well-structured animations and transitions using data attributes
packages/ui/src/components/molecules/attachment/attachmentLanding/attachmentItem/attachmentItem.tsx (3)
2-6: LGTM!The imports and props interface changes look good. The addition of
canDeleteFileprop enhances the component's flexibility by allowing conditional rendering of the delete button.Also applies to: 8-12
14-14: LGTM!Good use of the fallback pattern to handle cases where
file.idmight not be available.
40-41: LGTM!Good improvement in using the
Typographycomponent instead of astrongtag, which better aligns with the design system.packages/ui/src/components/molecules/label-wrapper.tsx (2)
18-45: LGTM!The component implementation follows React best practices:
- Uses forwardRef for proper ref handling
- Correctly spreads remaining props
- Handles required fields with proper visual indication
51-60: LGTM!The helper text implementation is well-structured:
- Uses conditional classes for visibility
- Properly handles error states
- Maintains consistency with Typography component
apps/core/app/auth/forget-password/_components/form/forgetPasswordForm.tsx (1)
51-52: LGTM!The button styling changes align with the design system and correctly use semantic color classes. The variant change to "primary" is appropriate for this primary action.
apps/core/app/auth/login/_components/form/loginForm.tsx (1)
80-86: LGTM! Button styling aligns with design system.The changes to the Button component improve consistency by using design system tokens (
bg-primary,text-foreground) and standardizing the button size.packages/ui/src/components/molecules/attachment/attachmentAdmin/attachmentAdmin.tsx (1)
31-49: LGTM! Enhanced text semantics with Typography component.The introduction of the Typography component improves accessibility and maintains consistent text styling across the application.
apps/core/app/(landing)/_components/desktop-navbar/feature-navbar-authenticated.tsx (1)
43-46: Address TODO comment for user info.The TODO comment indicates missing user information implementation.
Could you provide more details about the user information that needs to be added here? This will help me generate the appropriate implementation.
packages/ui/src/components/molecules/attachment/attachmentLanding/attachmentLanding.tsx (1)
11-104: LGTM! Well-structured component with proper drag and drop implementation.The changes demonstrate good practices:
- Proper event handling for drag and drop
- Consistent use of Typography component
- Clear prop management with defaults
- Semantic HTML structure
apps/core/app/(landing)/_components/desktop-navbar/navbar.tsx (2)
4-11: LGTM! New imports enhance navigation functionality.The added imports support the new navigation components and align with the PR objectives.
32-32: Verify mobile responsiveness.Setting
isMobileto a hardcodedfalsemight affect mobile responsiveness. Consider using a dynamic check.apps/core/app/(landing)/layout.tsx (2)
12-12: LGTM!The change to
islogin={true}aligns with the PR objectives for sidebar redesign.
40-132: Verify placeholder URLs in footer links.All footer links currently point to "/landing/product/id/page". Please verify if these are intended to be placeholder URLs or if they should point to actual pages.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if the placeholder URLs exist in the routing system # Search for route definitions matching the placeholder pattern echo "Searching for route definitions..." fd -e tsx -e ts "page" apps/core/app/landing/productLength of output: 248
I'm going to further investigate the routing by searching for all occurrences of the URL "/landing/product/id/page" in the repository to verify whether that's intended as a placeholder or if an actual route definition exists.
🏁 Script executed:
#!/bin/bash echo "Searching for '/landing/product/id/page' references in the codebase..." rg "/landing/product/id/page" .Length of output: 2684
Footer links are still using placeholder URLs
Our search results confirm that the URL "/landing/product/id/page" only appears in the footer layout without any corresponding route definition elsewhere in the repository. This indicates that these links remain as placeholders rather than linking to actual pages. Please confirm whether this intentional placeholder setup should be updated to point to real product pages.
apps/core/app/(landing)/product/test/page.tsx (2)
56-57: LGTM!The commented out variant props align with the PR objectives for UI consistency.
Also applies to: 63-64
131-145: Replace Lorem ipsum placeholder text with actual product description.The product summary section contains Lorem ipsum placeholder text. Please replace it with actual product description content.
packages/ui/src/components/molecules/sidebar.tsx (1)
99-113: LGTM! Well-implemented keyboard shortcut handling.The keyboard shortcut implementation is clean and properly handles cleanup through useEffect.
packages/design-system/src/tokens/token_radius_size.json (1)
1-38: LGTM! Well-structured design tokens.The radius size tokens follow a logical scale progression and adhere to design token best practices.
| <div className="pb-7"> | ||
| <p> | ||
| didnt recieved code yet? <Countdown date={Date.now() + 120000} /> | ||
| didnt recieveddqqdqdqdqdqdqdqdqdq code yet? <Countdown date={Date.now() + 120000} /> |
There was a problem hiding this comment.
Fix typos in the message text.
The text contains both spelling errors and unintended character repetition.
- didnt recieveddqqdqdqdqdqdqdqdqdq code yet? <Countdown date={Date.now() + 120000} />
+ didn't receive code yet? <Countdown date={Date.now() + 120000} />📝 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.
| didnt recieveddqqdqdqdqdqdqdqdqdq code yet? <Countdown date={Date.now() + 120000} /> | |
| didn't receive code yet? <Countdown date={Date.now() + 120000} /> |
| // if (isDashboard) { | ||
| // if (!accessToken || !refreshToken) { | ||
| // return NextResponse.redirect(new URL("/auth/login", request.url)); | ||
| // } | ||
| // } |
There was a problem hiding this comment.
CRITICAL: Restore authentication checks for dashboard routes.
Commenting out the authentication middleware for dashboard routes creates a significant security vulnerability by allowing unauthorized access to potentially sensitive dashboard content. This change doesn't appear to be necessary for implementing the new sidebar feature.
Please restore the authentication checks by removing the comments:
- // if (isDashboard) {
- // if (!accessToken || !refreshToken) {
- // return NextResponse.redirect(new URL("/auth/login", request.url));
- // }
- // }
+ if (isDashboard) {
+ if (!accessToken || !refreshToken) {
+ return NextResponse.redirect(new URL("/auth/login", request.url));
+ }
+ }If there's a specific reason why authentication needs to be disabled for the dashboard, please:
- Explain the rationale in the PR description
- Consider adding a temporary development-only flag instead of removing the checks entirely
- Create a separate security-focused PR for this change instead of bundling it with UI updates
📝 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.
| // if (isDashboard) { | |
| // if (!accessToken || !refreshToken) { | |
| // return NextResponse.redirect(new URL("/auth/login", request.url)); | |
| // } | |
| // } | |
| if (isDashboard) { | |
| if (!accessToken || !refreshToken) { | |
| return NextResponse.redirect(new URL("/auth/login", request.url)); | |
| } | |
| } |
| <svg | ||
| width="46" | ||
| height="46" | ||
| viewBox="0 0 46 46" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <path | ||
| d="M43 20.4998C42.3886 16.1003 40.3477 12.0239 37.1916 8.89849C34.0355 5.77308 29.9394 3.77205 25.5342 3.20364C21.129 2.63522 16.6591 3.53096 12.813 5.75287C8.96695 7.97478 5.95812 11.3996 4.25 15.4998M3 5.49975V15.4998H13M3 25.4997C3.6114 29.8992 5.65233 33.9756 8.8084 37.101C11.9645 40.2264 16.0606 42.2274 20.4658 42.7958C24.871 43.3643 29.3409 42.4685 33.187 40.2466C37.033 38.0247 40.0419 34.5999 41.75 30.4997M43 40.4997V30.4997H33" | ||
| stroke="white" | ||
| strokeWidth="5" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| /> | ||
| </svg> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve SVG implementation for better reusability and accessibility.
The SVG implementation has several areas for improvement:
- Fixed dimensions prevent the
sizeprop from working - Hardcoded white stroke color prevents color customization
- Missing accessibility attributes
Apply these improvements:
<svg
- width="46"
- height="46"
+ width={size}
+ height={size}
viewBox="0 0 46 46"
fill="none"
xmlns="http://www.w3.org/2000/svg"
+ role="img"
+ aria-label="Refresh icon"
>
<path
d="M43 20.4998C42.3886 16.1003 40.3477 12.0239 37.1916 8.89849C34.0355 5.77308 29.9394 3.77205 25.5342 3.20364C21.129 2.63522 16.6591 3.53096 12.813 5.75287C8.96695 7.97478 5.95812 11.3996 4.25 15.4998M3 5.49975V15.4998H13M3 25.4997C3.6114 29.8992 5.65233 33.9756 8.8084 37.101C11.9645 40.2264 16.0606 42.2274 20.4658 42.7958C24.871 43.3643 29.3409 42.4685 33.187 40.2466C37.033 38.0247 40.0419 34.5999 41.75 30.4997M43 40.4997V30.4997H33"
- stroke="white"
+ stroke="currentColor"
strokeWidth="5"
strokeLinecap="round"
strokeLinejoin="round"
/>
</svg>📝 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.
| <svg | |
| width="46" | |
| height="46" | |
| viewBox="0 0 46 46" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| > | |
| <path | |
| d="M43 20.4998C42.3886 16.1003 40.3477 12.0239 37.1916 8.89849C34.0355 5.77308 29.9394 3.77205 25.5342 3.20364C21.129 2.63522 16.6591 3.53096 12.813 5.75287C8.96695 7.97478 5.95812 11.3996 4.25 15.4998M3 5.49975V15.4998H13M3 25.4997C3.6114 29.8992 5.65233 33.9756 8.8084 37.101C11.9645 40.2264 16.0606 42.2274 20.4658 42.7958C24.871 43.3643 29.3409 42.4685 33.187 40.2466C37.033 38.0247 40.0419 34.5999 41.75 30.4997M43 40.4997V30.4997H33" | |
| stroke="white" | |
| strokeWidth="5" | |
| strokeLinecap="round" | |
| strokeLinejoin="round" | |
| /> | |
| </svg> | |
| <svg | |
| width={size} | |
| height={size} | |
| viewBox="0 0 46 46" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| role="img" | |
| aria-label="Refresh icon" | |
| > | |
| <path | |
| d="M43 20.4998C42.3886 16.1003 40.3477 12.0239 37.1916 8.89849C34.0355 5.77308 29.9394 3.77205 25.5342 3.20364C21.129 2.63522 16.6591 3.53096 12.813 5.75287C8.96695 7.97478 5.95812 11.3996 4.25 15.4998M3 5.49975V15.4998H13M3 25.4997C3.6114 29.8992 5.65233 33.9756 8.8084 37.101C11.9645 40.2264 16.0606 42.2274 20.4658 42.7958C24.871 43.3643 29.3409 42.4685 33.187 40.2466C37.033 38.0247 40.0419 34.5999 41.75 30.4997M43 40.4997V30.4997H33" | |
| stroke="currentColor" | |
| strokeWidth="5" | |
| strokeLinecap="round" | |
| strokeLinejoin="round" | |
| /> | |
| </svg> |
| const Refresh = (props: IconProps) => { | ||
| const { size = 24, color = "currentColor", ...resProps } = props; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align size prop with SVG dimensions and remove unused props.
The component has several inconsistencies:
- The default size (24) doesn't match the SVG's dimensions (46)
- The
colorprop is destructured but never used - The
resPropsspread is unused
Consider this implementation:
-const Refresh = (props: IconProps) => {
- const { size = 24, color = "currentColor", ...resProps } = props;
+const Refresh = ({ size = 46 }: IconProps) => {📝 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.
| const Refresh = (props: IconProps) => { | |
| const { size = 24, color = "currentColor", ...resProps } = props; | |
| const Refresh = ({ size = 46 }: IconProps) => { |
| <AttachmentLanding | ||
| title="Upload Your CV and Portfolio" | ||
| allowedTypes={["pdf", "jpg", "jpeg", "png"]} | ||
| maxSize={10} | ||
| multiple | ||
| onChange={(filesId: number[]) => { | ||
| console.log(filesId); | ||
| }} | ||
| fileCategory="cv" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling and remove debug logging.
A few suggestions for improvement:
- Console logging should not be present in production code
- The onChange handler lacks error handling
- Consider using MIME types for more precise file type validation
Here's a suggested implementation:
<AttachmentLanding
title="Upload Your CV and Portfolio"
- allowedTypes={["pdf", "jpg", "jpeg", "png"]}
+ allowedTypes={[
+ "application/pdf",
+ "image/jpeg",
+ "image/png"
+ ]}
maxSize={10}
multiple
onChange={(filesId: number[]) => {
- console.log(filesId);
+ try {
+ // Handle the uploaded files
+ // e.g., update form state, trigger API call, etc.
+ } catch (error) {
+ // Handle error appropriately
+ console.error('Error handling uploaded files:', error);
+ }
}}
fileCategory="cv"
/>| export type ButtonStatusColor = "success" | "warning" | "error"; | ||
|
|
||
| const statusColorHandler = ({ | ||
| statusColor, | ||
| variant, | ||
| disabled, | ||
| }: { | ||
| variant?: "primary" | "secondary" | "tertiary" | null; | ||
| statusColor?: ButtonStatusColor; | ||
| disabled?: boolean; | ||
| }): string | undefined => { | ||
| if (disabled) return ""; | ||
|
|
||
| switch (variant) { | ||
| case "secondary": | ||
| case "tertiary": | ||
| if (statusColor === "success") return "text-success-500"; | ||
| if (statusColor === "warning") return "text-warning-500"; | ||
| if (statusColor === "error") return "text-error-500"; | ||
| break; | ||
| case "primary": | ||
| default: | ||
| if (statusColor === "success") return "bg-success-500"; | ||
| if (statusColor === "warning") return "bg-warning-500"; | ||
| if (statusColor === "error") return "bg-error-500"; | ||
| break; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Fix switch statement structure.
The switch statement has a useless case clause as flagged by static analysis.
switch (variant) {
- case "secondary":
- case "tertiary":
+ case "secondary": case "tertiary":
if (statusColor === "success") return "text-success-500";
if (statusColor === "warning") return "text-warning-500";
if (statusColor === "error") return "text-error-500";
break;📝 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.
| export type ButtonStatusColor = "success" | "warning" | "error"; | |
| const statusColorHandler = ({ | |
| statusColor, | |
| variant, | |
| disabled, | |
| }: { | |
| variant?: "primary" | "secondary" | "tertiary" | null; | |
| statusColor?: ButtonStatusColor; | |
| disabled?: boolean; | |
| }): string | undefined => { | |
| if (disabled) return ""; | |
| switch (variant) { | |
| case "secondary": | |
| case "tertiary": | |
| if (statusColor === "success") return "text-success-500"; | |
| if (statusColor === "warning") return "text-warning-500"; | |
| if (statusColor === "error") return "text-error-500"; | |
| break; | |
| case "primary": | |
| default: | |
| if (statusColor === "success") return "bg-success-500"; | |
| if (statusColor === "warning") return "bg-warning-500"; | |
| if (statusColor === "error") return "bg-error-500"; | |
| break; | |
| } | |
| }; | |
| export type ButtonStatusColor = "success" | "warning" | "error"; | |
| const statusColorHandler = ({ | |
| statusColor, | |
| variant, | |
| disabled, | |
| }: { | |
| variant?: "primary" | "secondary" | "tertiary" | null; | |
| statusColor?: ButtonStatusColor; | |
| disabled?: boolean; | |
| }): string | undefined => { | |
| if (disabled) return ""; | |
| switch (variant) { | |
| case "secondary": case "tertiary": | |
| if (statusColor === "success") return "text-success-500"; | |
| if (statusColor === "warning") return "text-warning-500"; | |
| if (statusColor === "error") return "text-error-500"; | |
| break; | |
| case "primary": | |
| default: | |
| if (statusColor === "success") return "bg-success-500"; | |
| if (statusColor === "warning") return "bg-warning-500"; | |
| if (statusColor === "error") return "bg-error-500"; | |
| break; | |
| } | |
| }; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
| </AnimatePresence> | ||
| </div> | ||
| <div className="flex flex-wrap justify-between text-base mb-2"> | ||
| <Typography className="text-base font-bold">News App UI KIT</Typography> |
There was a problem hiding this comment.
Remove hardcoded product name.
The product name "News App UI KIT" should come from props instead of being hardcoded.
- <Typography className="text-base font-bold">News App UI KIT</Typography>
+ <Typography className="text-base font-bold">{productName}</Typography>| ) : ( | ||
| <Avatar /> | ||
| )} | ||
| <Typography className="text-[10px] font-normal">Ali Ebrahimi Kashef</Typography> |
There was a problem hiding this comment.
Use provided username prop.
The username "Ali Ebrahimi Kashef" is hardcoded instead of using the provided username prop.
- <Typography className="text-[10px] font-normal">Ali Ebrahimi Kashef</Typography>
+ <Typography className="text-[10px] font-normal">{username}</Typography>📝 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.
| <Typography className="text-[10px] font-normal">Ali Ebrahimi Kashef</Typography> | |
| <Typography className="text-[10px] font-normal">{username}</Typography> |
| { | ||
| "color": { | ||
| "accent": { | ||
| "$type": "color", | ||
| "$value": "{zinc.100}", | ||
| "$description": "Used for accents such as hover effects on <DropdownMenuItem>, <SelectItem>...etc" | ||
| }, | ||
| "accent-foreground": { | ||
| "$type": "color", | ||
| "$value": "{zinc.900}", | ||
| "$description": "Used for accents such as hover effects on <DropdownMenuItem>, <SelectItem>...etc" | ||
| }, | ||
| "background": { | ||
| "$type": "color", | ||
| "$value": "{general.white}", | ||
| "$description": "Default background color of <body />...etc" | ||
| }, | ||
| "black": { | ||
| "$type": "color", | ||
| "$value": "{general.black}" | ||
| }, | ||
| "border": { | ||
| "$type": "color", | ||
| "$value": "{zinc.200}", | ||
| "$description": "Default border color" | ||
| }, | ||
| "card": { | ||
| "$type": "color", | ||
| "$value": "{zinc.100}", | ||
| "$description": "Background color for <Card />" | ||
| }, | ||
| "card-foreground": { | ||
| "$type": "color", | ||
| "$value": "{slate.900}", | ||
| "$description": "Foreground color for <Card />" | ||
| }, | ||
| "destructive": { | ||
| "$type": "color", | ||
| "$value": "{red.600}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "destructive-foreground": { | ||
| "$type": "color", | ||
| "$value": "{general.white}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "error": { | ||
| "$type": "color", | ||
| "$value": "{red.500}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "error-border": { | ||
| "$type": "color", | ||
| "$value": "{red.200}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "error-foreground": { | ||
| "$type": "color", | ||
| "$value": "{red.600}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "foreground": { | ||
| "$type": "color", | ||
| "$value": "{zinc.950}", | ||
| "$description": "Default foreground color of <body />...etc" | ||
| }, | ||
| "information": { | ||
| "$type": "color", | ||
| "$value": "{blue.100}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "information-border": { | ||
| "$type": "color", | ||
| "$value": "{blue.200}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "information-foreground": { | ||
| "$type": "color", | ||
| "$value": "{blue.700}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "input": { | ||
| "$type": "color", | ||
| "$value": "{zinc.200}", | ||
| "$description": "Border color for inputs such as <Input />, <Select />, <Textarea />" | ||
| }, | ||
| "muted": { | ||
| "$type": "color", | ||
| "$value": "{zinc.100}", | ||
| "$description": "Muted backgrounds such as <TabsList />, <Skeleton /> and <Switch />" | ||
| }, | ||
| "muted-foreground": { | ||
| "$type": "color", | ||
| "$value": "{zinc.500}", | ||
| "$description": "Muted Foreground such as <TabsList />, <Skeleton /> and <Switch />" | ||
| }, | ||
| "popover": { | ||
| "$type": "color", | ||
| "$value": "{general.white}", | ||
| "$description": "Background color for popovers such as <DropdownMenu />, <HoverCard />, <Popover />" | ||
| }, | ||
| "popover-foreground": { | ||
| "$type": "color", | ||
| "$value": "{slate.900}", | ||
| "$description": "Foreground color for popovers such as <DropdownMenu />, <HoverCard />, <Popover />" | ||
| }, | ||
| "primary": { | ||
| "$type": "color", | ||
| "$value": "{purple.500}", | ||
| "$description": "Primary colors for <Button />" | ||
| }, | ||
| "primary-foreground": { | ||
| "$type": "color", | ||
| "$value": "{general.white}", | ||
| "$description": "Secondary colors for <Button />" | ||
| }, | ||
| "primary-light": { | ||
| "$type": "color", | ||
| "$value": "{purple.100}" | ||
| }, | ||
| "ring": { | ||
| "$type": "color", | ||
| "$value": "{zinc.400}", | ||
| "$description": "Used for focus ring" | ||
| }, | ||
| "secondary": { | ||
| "$type": "color", | ||
| "$value": "{zinc.100}", | ||
| "$description": "Secondary colors for <Button />" | ||
| }, | ||
| "secondary-foreground": { | ||
| "$type": "color", | ||
| "$value": "{zinc.900}", | ||
| "$description": "Secondary colors for <Button />" | ||
| }, | ||
| "success": { | ||
| "$type": "color", | ||
| "$value": "{green.100}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "success-border": { | ||
| "$type": "color", | ||
| "$value": "{green.200}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "success-foreground": { | ||
| "$type": "color", | ||
| "$value": "{green.700}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "table-header": { | ||
| "$type": "color", | ||
| "$value": "rgba(244, 244, 245, 0.5000)", | ||
| "$description": "Muted backgrounds such as <TabsList />, <Skeleton /> and <Switch />" | ||
| }, | ||
| "warning": { | ||
| "$type": "color", | ||
| "$value": "{amber.500}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "warning-border": { | ||
| "$type": "color", | ||
| "$value": "{amber.200}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "warning-foreground": { | ||
| "$type": "color", | ||
| "$value": "{general.white}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "white": { | ||
| "$type": "color", | ||
| "$value": "{general.white}" | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Light Theme Color Tokens: Structure & Descriptions
The color tokens for the light theme are comprehensively defined with consistent use of "$type", "$value", and "$description". This format should promote easy usage in styling across the UI.
However, several "$description" fields appear to be copy-paste artifacts that may confuse downstream usage:
• The tokens under "information" (lines 67–81) have a description stating "Used for destructive actions such as <Button variant="destructive">", which is unexpected for informational tokens.
• Similarly, the "success" tokens (around line 137) and the "warning" tokens (around line 156) also carry descriptions referring to "destructive actions", which do not align with their intended semantic use.
Please review and update these descriptions to more accurately describe their usage (e.g., "information" tokens for informative messages, "success" tokens for confirmation states, and "warning" tokens for cautionary alerts).
| { | ||
| "color": { | ||
| "accent": { | ||
| "$type": "color", | ||
| "$value": "{zinc.800}", | ||
| "$description": "Used for accents such as hover effects on <DropdownMenuItem>, <SelectItem>...etc" | ||
| }, | ||
| "accent-foreground": { | ||
| "$type": "color", | ||
| "$value": "{general.white}", | ||
| "$description": "Used for accents such as hover effects on <DropdownMenuItem>, <SelectItem>...etc" | ||
| }, | ||
| "background": { | ||
| "$type": "color", | ||
| "$value": "{zinc.950}", | ||
| "$description": "Default background color of <body />...etc" | ||
| }, | ||
| "black": { | ||
| "$type": "color", | ||
| "$value": "{general.black}" | ||
| }, | ||
| "border": { | ||
| "$type": "color", | ||
| "$value": "{zinc.800}", | ||
| "$description": "Default border color" | ||
| }, | ||
| "card": { | ||
| "$type": "color", | ||
| "$value": "{zinc.900}", | ||
| "$description": "Background color for <Card />" | ||
| }, | ||
| "card-foreground": { | ||
| "$type": "color", | ||
| "$value": "{general.white}", | ||
| "$description": "Foreground color for <Card />" | ||
| }, | ||
| "destructive": { | ||
| "$type": "color", | ||
| "$value": "{rose.500}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "destructive-foreground": { | ||
| "$type": "color", | ||
| "$value": "{general.black}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "error": { | ||
| "$type": "color", | ||
| "$value": "{rose.500}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "error-border": { | ||
| "$type": "color", | ||
| "$value": "{red.900}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "error-foreground": { | ||
| "$type": "color", | ||
| "$value": "{red.100}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "foreground": { | ||
| "$type": "color", | ||
| "$value": "{zinc.50}", | ||
| "$description": "Default foreground color of <body />...etc" | ||
| }, | ||
| "information": { | ||
| "$type": "color", | ||
| "$value": "{sky.500}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "information-border": { | ||
| "$type": "color", | ||
| "$value": "{blue.900}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "information-foreground": { | ||
| "$type": "color", | ||
| "$value": "{blue.100}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "input": { | ||
| "$type": "color", | ||
| "$value": "{zinc.800}", | ||
| "$description": "Border color for inputs such as <Input />, <Select />, <Textarea />" | ||
| }, | ||
| "muted": { | ||
| "$type": "color", | ||
| "$value": "{zinc.800}", | ||
| "$description": "Muted backgrounds such as <TabsList />, <Skeleton /> and <Switch />" | ||
| }, | ||
| "muted-foreground": { | ||
| "$type": "color", | ||
| "$value": "{zinc.400}", | ||
| "$description": "Muted Foreground such as <TabsList />, <Skeleton /> and <Switch />" | ||
| }, | ||
| "popover": { | ||
| "$type": "color", | ||
| "$value": "{zinc.950}", | ||
| "$description": "Background color for popovers such as <DropdownMenu />, <HoverCard />, <Popover />" | ||
| }, | ||
| "popover-foreground": { | ||
| "$type": "color", | ||
| "$value": "{general.white}", | ||
| "$description": "Foreground color for popovers such as <DropdownMenu />, <HoverCard />, <Popover />" | ||
| }, | ||
| "primary": { | ||
| "$type": "color", | ||
| "$value": "{purple.500}", | ||
| "$description": "Primary colors for <Button />" | ||
| }, | ||
| "primary-foreground": { | ||
| "$type": "color", | ||
| "$value": "{general.white}", | ||
| "$description": "Secondary colors for <Button />" | ||
| }, | ||
| "primary-light": { | ||
| "$type": "color", | ||
| "$value": "{purple.100}" | ||
| }, | ||
| "ring": { | ||
| "$type": "color", | ||
| "$value": "{zinc.600}", | ||
| "$description": "Used for focus ring" | ||
| }, | ||
| "secondary": { | ||
| "$type": "color", | ||
| "$value": "{zinc.800}", | ||
| "$description": "Secondary colors for <Button />" | ||
| }, | ||
| "secondary-foreground": { | ||
| "$type": "color", | ||
| "$value": "{general.white}", | ||
| "$description": "Secondary colors for <Button />" | ||
| }, | ||
| "success": { | ||
| "$type": "color", | ||
| "$value": "{emerald.500}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "success-border": { | ||
| "$type": "color", | ||
| "$value": "{green.900}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "success-foreground": { | ||
| "$type": "color", | ||
| "$value": "{green.50}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "table-header": { | ||
| "$type": "color", | ||
| "$value": "rgba(39, 39, 42, 0.5000)", | ||
| "$description": "Muted backgrounds such as <TabsList />, <Skeleton /> and <Switch />" | ||
| }, | ||
| "warning": { | ||
| "$type": "color", | ||
| "$value": "{amber.500}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "warning-border": { | ||
| "$type": "color", | ||
| "$value": "{amber.900}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "warning-foreground": { | ||
| "$type": "color", | ||
| "$value": "{amber.100}", | ||
| "$description": "Used for destructive actions such as <Button variant=\"destructive\">" | ||
| }, | ||
| "white": { | ||
| "$type": "color", | ||
| "$value": "{general.white}" | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Default Theme Color Tokens: Review of Descriptive Fields
The default theme color tokens are defined clearly and follow a similar structure to the light theme tokens, ensuring consistency across themes. Each token includes a descriptive "$description" field to guide its use.
Similar to the light theme, there are notable issues with the descriptions:
• Tokens such as "error", "information", "success", and "warning" (e.g., lines 47–61, 67–81, 137–150, and 157–170 respectively) currently include descriptions that reference "destructive actions". This is likely a copy-paste error.
• Each token’s description should instead be revised to reflect its actual purpose, for example:
– "error": used for error states and messages.
– "information": used for informational messages.
– "success": used for confirming successful actions.
– "warning": used for cautionary alerts.
Updating these descriptions will improve clarity and prevent potential misuse of the tokens in the design system.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.husky/pre-rebase (2)
19-21: Make commit message configurable.The commit message is hardcoded and specific to this PR. Consider making it configurable or using the PR title.
# Create a new single commit -git commit -m "FEAT: Implement Sidebar with ShadCN UI" +COMMIT_MSG=${2:-"FEAT: Implement Sidebar with ShadCN UI"} +if ! git commit -m "$COMMIT_MSG"; then + echo "⚠️ Failed to create commit" + exit 1 +fi
22-24: Add stash pop verification.While the script handles stash pop failures with
||, it should verify if there are stashed changes before attempting to pop.-echo "✅ Rebase completed. Applying stash..." -git stash pop || echo "ℹ️ No stashed changes to apply." +echo "✅ Squash completed. Checking for stashed changes..." +if git stash list | grep -q "Pre-rebase stash"; then + echo "🔄 Applying stashed changes..." + if ! git stash pop; then + echo "⚠️ Failed to apply stashed changes. Please resolve conflicts manually." + exit 1 + fi +else + echo "ℹ️ No stashed changes to apply." +fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.husky/pre-rebase(1 hunks)
🔇 Additional comments (4)
.husky/pre-rebase (4)
1-3: LGTM!The script setup is correct with proper shebang and husky initialization.
4-9: LGTM!Good defensive programming by checking for an existing rebase operation.
16-18: Add error handling and quote variables.The reset operation needs proper error handling and variable quoting.
echo "📌 Squashing all commits..." -git reset --soft $(git merge-base $TARGET_BRANCH $(git branch --show-current)) +MERGE_BASE=$(git merge-base "$TARGET_BRANCH" "$(git branch --show-current)" 2>/dev/null) +if [ -z "$MERGE_BASE" ]; then + echo "⚠️ Failed to find merge base with $TARGET_BRANCH" + exit 1 +fi + +if ! git reset --soft "$MERGE_BASE"; then + echo "⚠️ Reset failed" + exit 1 +fi
25-25: LGTM!Clear success message indicating completion.
.husky/pre-rebase
Outdated
| echo "🔄 Stashing any uncommitted changes..." | ||
| git stash push -m "Pre-rebase stash" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for stash operation.
The stash operation should be verified for success.
echo "🔄 Stashing any uncommitted changes..."
-git stash push -m "Pre-rebase stash"
+if ! git stash push -m "Pre-rebase stash"; then
+ echo "⚠️ Failed to stash changes"
+ exit 1
+fi📝 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.
| echo "🔄 Stashing any uncommitted changes..." | |
| git stash push -m "Pre-rebase stash" | |
| echo "🔄 Stashing any uncommitted changes..." | |
| if ! git stash push -m "Pre-rebase stash"; then | |
| echo "⚠️ Failed to stash changes" | |
| exit 1 | |
| fi |
.husky/pre-rebase
Outdated
| # Find the target branch (e.g., main) | ||
| TARGET_BRANCH=${1:-main} | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for target branch existence.
The script should verify that the target branch exists before proceeding.
# Find the target branch (e.g., main)
TARGET_BRANCH=${1:-main}
+
+# Verify target branch exists
+if ! git rev-parse --verify "origin/$TARGET_BRANCH" >/dev/null 2>&1; then
+ echo "⚠️ Target branch 'origin/$TARGET_BRANCH' does not exist"
+ exit 1
+fi📝 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.
| # Find the target branch (e.g., main) | |
| TARGET_BRANCH=${1:-main} | |
| # Find the target branch (e.g., main) | |
| TARGET_BRANCH=${1:-main} | |
| # Verify target branch exists | |
| if ! git rev-parse --verify "origin/$TARGET_BRANCH" >/dev/null 2>&1; then | |
| echo "⚠️ Target branch 'origin/$TARGET_BRANCH' does not exist" | |
| exit 1 | |
| fi |
mrbadri
force-pushed
the
feat/#123-new-sidebar
branch
from
a0f4ec8 to
ee40226
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (12)
packages/ui/src/components/atoms/sidebar.tsx (4)
1-51: LGTM! Consider documenting the keyboard shortcut.The context implementation and state management are well-structured. The keyboard shortcut (Ctrl/Cmd + B) for toggling the sidebar is a nice touch.
Consider adding a comment or documentation to inform users about the keyboard shortcut functionality.
53-168: Consider adding security attributes to the cookie.The SidebarProvider implementation is robust, but the cookie could benefit from additional security attributes.
Consider adding security attributes to the cookie:
- document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; + document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}; SameSite=Strict; Secure`;
273-324: Add aria-expanded attribute for better accessibility.The interactive components are well-implemented, but the trigger button could benefit from additional accessibility attributes.
Add aria-expanded to improve accessibility:
<Button ref={ref} data-sidebar="trigger" + aria-expanded={open} className={cn("h-7 w-7", className)} onClick={(event) => { onClick?.(event); toggleSidebar(); }} {...props} >
326-496: Consider adding ARIA roles for semantic clarity.The layout components are well-implemented, but some could benefit from explicit ARIA roles for better accessibility.
Consider adding roles to clarify the semantic structure:
<div ref={ref} data-sidebar="header" + role="banner" className={cn("flex flex-col gap-2 p-2", className)} {...props} /> <div ref={ref} data-sidebar="footer" + role="contentinfo" className={cn("flex flex-col gap-2 p-2", className)} {...props} />apps/storybook/eslint.config.js (2)
1-5: Add Storybook ESLint plugin for enhanced linting.Consider adding the Storybook ESLint plugin to enforce Storybook-specific best practices.
import reactHooks from 'eslint-plugin-react-hooks' import reactRefresh from 'eslint-plugin-react-refresh' import tseslint from 'typescript-eslint' +import storybook from '@storybook/eslint-plugin'
7-8: Consider ignoring additional build directories.Add more common build and cache directories to the ignore list.
- { ignores: ['dist'] }, + { ignores: ['dist', '.storybook', 'storybook-static', 'node_modules'] },apps/storybook/.storybook/main.ts (1)
5-10: Add essential addons for sidebar development.Consider adding these addons to enhance development and testing capabilities:
@storybook/addon-a11yfor accessibility testing@storybook/addon-viewportfor responsive testing@storybook/addon-controlsfor dynamic props manipulationApply this diff to add the essential addons:
addons: [ "@storybook/addon-onboarding", "@storybook/addon-essentials", "@chromatic-com/storybook", "@storybook/addon-interactions", + "@storybook/addon-a11y", + "@storybook/addon-viewport", + "@storybook/addon-controls", ],apps/storybook/src/stories/Header.tsx (1)
48-66: Improve conditional rendering with early return pattern.The component uses a ternary operator for conditional rendering which can be harder to read. Consider using the early return pattern for better readability.
Apply this diff to improve the code:
- <div> - {user ? ( - <> - <span className="welcome"> - Welcome, <b>{user.name}</b>! - </span> - <Button size="small" onClick={onLogout} label="Log out" /> - </> - ) : ( - <> - <Button size="small" onClick={onLogin} label="Log in" /> - <Button - primary - size="small" - onClick={onCreateAccount} - label="Sign up" - /> - </> - )} - </div> + <div> + {user && ( + <> + <span className="welcome"> + Welcome, <b>{user.name}</b>! + </span> + <Button size="small" onClick={onLogout} label="Log out" /> + </> + )} + {!user && ( + <> + <Button size="small" onClick={onLogin} label="Log in" /> + <Button + primary + size="small" + onClick={onCreateAccount} + label="Sign up" + /> + </> + )} + </div>packages/ui/src/components/molecules/sidebar.tsx (2)
100-113: Improve keyboard event handling.The keyboard event handler should check for composing events to avoid conflicts with IME input methods.
Apply this diff to improve the code:
React.useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { + if (event.isComposing) return; if ( event.key === SIDEBAR_KEYBOARD_SHORTCUT && (event.metaKey || event.ctrlKey) ) { event.preventDefault(); toggleSidebar(); } }; window.addEventListener("keydown", handleKeyDown); return () => window.removeEventListener("keydown", handleKeyDown); }, [toggleSidebar]);
203-221: Add error boundary for mobile sheet component.The mobile sheet component should be wrapped in an error boundary to handle potential rendering errors gracefully.
Consider adding an error boundary component to handle potential errors in the mobile sheet component:
+ if (isMobile) { + return ( + <ErrorBoundary fallback={<div>Something went wrong</div>}> <Sheet open={openMobile} onOpenChange={setOpenMobile} {...props}> <SheetContent data-sidebar="sidebar" data-mobile="true" className="w-[--sidebar-width] bg-sidebar p-0 text-sidebar-foreground [&>button]:hidden" style={ { "--sidebar-width": SIDEBAR_WIDTH_MOBILE, } as React.CSSProperties } side={side} > <div className="flex h-full w-full flex-col">{children}</div> </SheetContent> </Sheet> + </ErrorBoundary> ); }apps/storybook/tsconfig.app.json (1)
2-24: Consider adding additional compiler options for better type safety.The configuration could benefit from additional compiler options to enhance type safety and catch potential issues earlier.
Consider adding these compiler options:
"compilerOptions": { "tsBuildInfoFile": "./node_modules/.tmp/tsconfig.app.tsbuildinfo", "target": "ES2020", "useDefineForClassFields": true, "lib": ["ES2020", "DOM", "DOM.Iterable"], "module": "ESNext", "skipLibCheck": true, /* Bundler mode */ "moduleResolution": "bundler", "allowImportingTsExtensions": true, "isolatedModules": true, "moduleDetection": "force", "noEmit": true, "jsx": "react-jsx", /* Linting */ "strict": true, "noUnusedLocals": true, "noUnusedParameters": true, "noFallthroughCasesInSwitch": true, - "noUncheckedSideEffectImports": true + "noUncheckedSideEffectImports": true, + "noImplicitReturns": true, + "noImplicitOverride": true, + "exactOptionalPropertyTypes": true },🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: JSON standard does not allow comments.
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 11-11: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 11-11: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 11-11: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 12-12: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 12-12: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 12-12: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 12-12: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 13-13: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 13-13: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 13-13: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 13-13: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 14-14: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 14-14: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 14-14: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 14-14: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 15-15: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 15-15: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 15-15: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 15-15: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 16-16: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 16-16: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 16-16: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 16-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 18-18: JSON standard does not allow comments.
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 22-22: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 22-22: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 22-22: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 22-22: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 24-24: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
apps/storybook/src/stories/Configure.mdx (1)
208-364: Inline CSS Styling for Storybook Documentation
The style block at the end is well organized to handle layout, grids, responsiveness, and addon presentation. The use of media queries ensures adaptability for smaller screen sizes. Overall, the custom styling is consistent and should support a good user experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (27)
apps/core/public/images/clip-path-group-sidebar.svgis excluded by!**/*.svgapps/storybook/%SystemDrive%/ProgramData/Microsoft/Windows/Caches/cversions.2.dbis excluded by!**/*.dbapps/storybook/%SystemDrive%/ProgramData/Microsoft/Windows/Caches/{6AF0698E-D558-4F6E-9B3C-3716689AF493}.2.ver0x0000000000000001.dbis excluded by!**/*.dbapps/storybook/%SystemDrive%/ProgramData/Microsoft/Windows/Caches/{7D350FE0-5F96-4752-A63A-05699029D904}.2.ver0x0000000000000001.dbis excluded by!**/*.dbapps/storybook/%SystemDrive%/ProgramData/Microsoft/Windows/Caches/{DDF571F2-BE98-426D-8288-1A9A39C3FDA2}.2.ver0x0000000000000001.dbis excluded by!**/*.dbapps/storybook/.cache/storybook/default/dev-server/storybook-0c9aba2732f8798ca2ae104c09855accis excluded by!**/.cache/**apps/storybook/.cache/storybook/default/dev-server/storybook-21d6f40cfb511982e4424e0e250a9557is excluded by!**/.cache/**apps/storybook/.cache/storybook/default/dev-server/storybook-e35710dcf9c6d4f837d5dae15e56122eis excluded by!**/.cache/**apps/storybook/package-lock.jsonis excluded by!**/package-lock.jsonapps/storybook/public/vite.svgis excluded by!**/*.svgapps/storybook/src/assets/react.svgis excluded by!**/*.svgapps/storybook/src/stories/assets/accessibility.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/accessibility.svgis excluded by!**/*.svgapps/storybook/src/stories/assets/addon-library.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/assets.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/context.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/discord.svgis excluded by!**/*.svgapps/storybook/src/stories/assets/docs.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/figma-plugin.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/github.svgis excluded by!**/*.svgapps/storybook/src/stories/assets/share.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/styling.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/testing.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/theming.pngis excluded by!**/*.pngapps/storybook/src/stories/assets/tutorials.svgis excluded by!**/*.svgapps/storybook/src/stories/assets/youtube.svgis excluded by!**/*.svgbun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (44)
.hintrc(1 hunks).husky/pre-rebase(1 hunks)apps/core/app/dashboard/_compnents/app-sidebar.tsx(1 hunks)apps/core/app/dashboard/_compnents/logo-icon-switcher.tsx(1 hunks)apps/core/app/dashboard/_compnents/nav-main.tsx(1 hunks)apps/core/app/dashboard/_compnents/navbar-dashboard.tsx(1 hunks)apps/core/app/dashboard/_compnents/sidebar.tsx(0 hunks)apps/core/app/dashboard/layout.tsx(1 hunks)apps/core/app/dashboard/page.tsx(1 hunks)apps/core/middleware.ts(1 hunks)apps/storybook/.gitignore(1 hunks)apps/storybook/.storybook/main.ts(1 hunks)apps/storybook/.storybook/preview.ts(1 hunks)apps/storybook/README.md(1 hunks)apps/storybook/eslint.config.js(1 hunks)apps/storybook/index.html(1 hunks)apps/storybook/package.json(1 hunks)apps/storybook/src/App.css(1 hunks)apps/storybook/src/App.tsx(1 hunks)apps/storybook/src/index.css(1 hunks)apps/storybook/src/main.tsx(1 hunks)apps/storybook/src/stories/Button.stories.ts(1 hunks)apps/storybook/src/stories/Button.tsx(1 hunks)apps/storybook/src/stories/Configure.mdx(1 hunks)apps/storybook/src/stories/Header.stories.ts(1 hunks)apps/storybook/src/stories/Header.tsx(1 hunks)apps/storybook/src/stories/Page.stories.ts(1 hunks)apps/storybook/src/stories/Page.tsx(1 hunks)apps/storybook/src/stories/button.css(1 hunks)apps/storybook/src/stories/header.css(1 hunks)apps/storybook/src/stories/page.css(1 hunks)apps/storybook/src/vite-env.d.ts(1 hunks)apps/storybook/tsconfig.app.json(1 hunks)apps/storybook/tsconfig.json(1 hunks)apps/storybook/tsconfig.node.json(1 hunks)apps/storybook/vite.config.ts(1 hunks)packages/icons/src/components/menu.tsx(1 hunks)packages/ui/package.json(3 hunks)packages/ui/src/components/atoms/sidebar.tsx(1 hunks)packages/ui/src/components/atoms/tooltip.tsx(1 hunks)packages/ui/src/components/molecules/sidebar.tsx(1 hunks)packages/ui/src/components/ui/tooltip.tsx(1 hunks)packages/ui/src/hooks/use-mobile.tsx(1 hunks)packages/ui/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/core/app/dashboard/_compnents/sidebar.tsx
✅ Files skipped from review due to trivial changes (9)
- apps/storybook/tsconfig.json
- apps/storybook/vite.config.ts
- apps/storybook/src/vite-env.d.ts
- apps/storybook/src/main.tsx
- apps/storybook/.gitignore
- apps/storybook/src/stories/page.css
- apps/storybook/index.html
- apps/storybook/src/stories/header.css
- apps/storybook/src/stories/button.css
🚧 Files skipped from review as they are similar to previous changes (14)
- apps/core/middleware.ts
- .hintrc
- apps/core/app/dashboard/_compnents/navbar-dashboard.tsx
- packages/ui/src/components/atoms/tooltip.tsx
- .husky/pre-rebase
- apps/core/app/dashboard/layout.tsx
- packages/ui/tsconfig.json
- packages/icons/src/components/menu.tsx
- apps/core/app/dashboard/_compnents/logo-icon-switcher.tsx
- apps/core/app/dashboard/page.tsx
- apps/core/app/dashboard/_compnents/app-sidebar.tsx
- packages/ui/src/hooks/use-mobile.tsx
- apps/core/app/dashboard/_compnents/nav-main.tsx
- packages/ui/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
apps/storybook/tsconfig.app.json
[error] 10-10: JSON standard does not allow comments.
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: JSON standard does not allow comments.
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
apps/storybook/tsconfig.node.json
[error] 9-9: JSON standard does not allow comments.
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: JSON standard does not allow comments.
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (48)
apps/storybook/tsconfig.node.json (7)
1-8: Configuration Options Approved.
The compiler options for target, lib, module, and skipLibCheck are clearly defined and appropriate for an ES2022/ES2023 environment.
9-9: Potential Issue: Non-standard JSON Comments.
Line 9 contains a C-style comment (/* Bundler mode */). Although TypeScript configuration files often use comments (i.e., JSONC), the JSON standard itself doesn’t support them. Verify that the tooling parsing this file accepts JSON with comments, or consider removing these if strict JSON compliance is required.🧰 Tools
🪛 Biome (1.9.4)
[error] 9-9: JSON standard does not allow comments.
(parse)
10-14: Bundler Mode Settings Validated.
The configuration options under bundler mode—including moduleResolution, allowImportingTsExtensions, isolatedModules, moduleDetection, and noEmit—are properly set to support the intended setup.🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 10-10: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 10-10: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 10-10: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 11-11: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 11-11: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 11-11: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 11-11: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 12-12: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 12-12: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 12-12: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 12-12: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 13-13: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 13-13: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 13-13: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 13-13: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 14-14: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 14-14: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 14-14: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
16-16: Potential Issue: Comment in Linting Section.
Similar to line 9, the comment at line 16 (/* Linting */) may trigger static analysis errors due to JSON’s lack of support for comments. Confirm that your environment supports JSON with comments (JSONC) or remove this comment to avoid potential parser issues.🧰 Tools
🪛 Biome (1.9.4)
[error] 14-16: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 16-16: JSON standard does not allow comments.
(parse)
17-21: Linting Rules Configured Correctly.
The linting settings (strict, noUnusedLocals, noUnusedParameters, noFallthroughCasesInSwitch, and noUncheckedSideEffectImports) are well defined to enforce strong type safety and clean code practices.🧰 Tools
🪛 Biome (1.9.4)
[error] 17-17: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 17-17: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 17-17: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 17-17: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 18-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 18-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 18-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 18-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 19-19: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 20-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
22-23: Include Directive Confirmation.
The "include" array at line 23 correctly specifies that "vite.config.ts" should be incorporated into the project’s compilation process. Ensure that including only this file is intentional for your setup.🧰 Tools
🪛 Biome (1.9.4)
[error] 22-22: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
24-25: File Termination Validity and Static Analysis Warnings.
The file ends as expected; however, several static analysis messages about “End of file expected” may be false positives triggered by the presence of comments. Verify that your build and analysis tools support JSON with comments (JSONC), or consider migrating to a different format if necessary.packages/ui/src/components/atoms/sidebar.tsx (3)
170-271: Excellent implementation with comprehensive responsive design!The Sidebar component is well-structured with proper handling of different variants, collapsible modes, and mobile responsiveness. The accessibility attributes are properly implemented.
497-745: Excellent menu system implementation!The menu components are well-designed with proper accessibility, loading states, and interactive features. The random width implementation for skeletons is a nice touch for visual variety.
747-772: LGTM! Well-organized exports.The exports are comprehensive and follow a consistent naming convention.
apps/storybook/eslint.config.js (3)
12-15: Update language options for modern development.The current ECMAVersion (2020) is outdated and sourceType is missing.
languageOptions: { - ecmaVersion: 2020, + ecmaVersion: 2024, globals: globals.browser, + sourceType: 'module', },
16-26: Enhance React and Storybook configuration.Add Storybook-specific rules and React settings.
plugins: { 'react-hooks': reactHooks, 'react-refresh': reactRefresh, + 'storybook': storybook, }, rules: { ...reactHooks.configs.recommended.rules, 'react-refresh/only-export-components': [ 'warn', { allowConstantExport: true }, ], + 'storybook/prefer-pascal-case': 'error', + 'storybook/hierarchy-separator': 'error', }, + settings: { + react: { + version: 'detect' + } + },
1-28: ESLint configuration needs alignment with shared configs.The Storybook ESLint configuration uses a different approach (flat config) compared to the shared ESLint configurations in
packages/eslint-config/.Consider either:
- Migrating to use the shared configs from
@repo/eslint-config- Or updating the shared configs to use the new flat config format for consistency
apps/storybook/.storybook/preview.ts (1)
3-12: Enhance Storybook configuration for sidebar development.Given this PR implements a new sidebar feature, consider adding essential Storybook configurations for viewport presets, theme configuration, and global decorators for sidebar context.
apps/storybook/src/stories/Header.stories.ts (1)
1-34: LGTM!The story configuration follows best practices with:
- Proper use of mock functions
- Coverage of different user states
- Automatic documentation setup
apps/storybook/src/stories/Button.tsx (2)
3-14: Enhance type safety and accessibility.The ButtonProps interface needs better type safety and accessibility support.
28-35:⚠️ Potential issueRestrict prop spreading for security.
Using prop spreading (
{...props}) can expose the button to injection of unauthorized props. Consider explicitly passing only the required props.Apply this diff to fix the security issue:
<button type="button" className={["storybook-button", `storybook-button--${size}`, mode].join( " ", )} style={{ backgroundColor }} - {...props} + onClick={props.onClick} >Likely invalid or redundant comment.
apps/storybook/src/stories/Page.stories.ts (1)
1-32: LGTM! Well-implemented interaction testingThe story implementation follows Storybook best practices with proper interaction testing for login/logout flows.
packages/ui/src/components/ui/tooltip.tsx (1)
14-27: Add accessibility enhancements to tooltipConsider adding default accessibility properties to improve user experience.
Apply these changes:
const TooltipContent = React.forwardRef< React.ElementRef<typeof TooltipPrimitive.Content>, React.ComponentPropsWithoutRef<typeof TooltipPrimitive.Content> ->(({ className, sideOffset = 4, ...props }, ref) => ( +>(({ + className, + sideOffset = 4, + delayDuration = 200, + skipDelayDuration = 300, + ...props + }, ref) => ( <TooltipPrimitive.Content ref={ref} sideOffset={sideOffset} + delayDuration={delayDuration} + skipDelayDuration={skipDelayDuration} className={cn( "z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", className )} + aria-live="polite" {...props} /> ))apps/storybook/src/stories/Button.stories.ts (1)
6-22: Add accessibility testing configurationsConsider adding accessibility testing parameters to ensure the component meets WCAG standards.
Apply these changes:
const meta = { title: 'Example/Button', component: Button, parameters: { layout: 'centered', + a11y: { + config: { + rules: [ + { + id: 'button-name', + enabled: true, + }, + { + id: 'color-contrast', + enabled: true, + }, + ], + }, + }, },apps/storybook/src/stories/Header.tsx (2)
8-13: Improve type safety for callback functions.The callback functions are marked as optional which could lead to runtime errors if they're essential for component functionality.
Apply this diff to improve type safety:
export interface HeaderProps { user?: User; - onLogin?: () => void; - onLogout?: () => void; - onCreateAccount?: () => void; + onLogin: () => Promise<void>; + onLogout: () => Promise<void>; + onCreateAccount: () => Promise<void>; }
24-44: Add accessibility attributes to SVG logo.The SVG logo is missing accessibility attributes which are essential for screen readers.
Apply this diff to improve accessibility:
- <svg - width="32" - height="32" - viewBox="0 0 32 32" - xmlns="http://www.w3.org/2000/svg" - > + <svg + width="32" + height="32" + viewBox="0 0 32 32" + xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Acme logo" + >apps/storybook/src/stories/Page.tsx (2)
15-20: Improve callback naming and implementation.The
onLoginandonCreateAccountcallbacks have identical implementations which could lead to confusion.Apply this diff to differentiate their behavior:
- onLogin={() => setUser({ name: 'Jane Doe' })} - onLogout={() => setUser(undefined)} - onCreateAccount={() => setUser({ name: 'Jane Doe' })} + onLogin={() => setUser({ name: 'Jane Doe', id: 'existing-user' })} + onLogout={() => setUser(undefined)} + onCreateAccount={() => setUser({ name: 'New User', id: 'new-user' })}
48-54: Add security attributes to external links.The external links are missing recommended security attributes.
Apply this diff to improve security:
- <a href="https://storybook.js.org/tutorials/" target="_blank" rel="noopener noreferrer"> + <a + href="https://storybook.js.org/tutorials/" + target="_blank" + rel="noopener noreferrer" + aria-label="Learn about Storybook tutorials" + >packages/ui/src/components/molecules/sidebar.tsx (1)
87-88: Encode cookie values to prevent security vulnerabilities.Directly setting
document.cookiewith unencoded values can lead to security issues like XSS attacks.Apply this diff to fix the issue:
- document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; + document.cookie = `${SIDEBAR_COOKIE_NAME}=${encodeURIComponent(openState)}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`;apps/storybook/src/App.css (8)
1-6: Strong Base Styles for Root Container
The #root selector sets a clear maximum width, centers content, and adds padding. This provides a solid foundation for your layout.
8-13: Effective Logo Styling with Transitions
The .logo class uses defined height, padding, and smooth transition effects with “will-change: filter.” This is good for performance and visual quality.
14-16: Clear Hover Feedback for Logo
Using a drop-shadow on hover for the logo gives immediate visual feedback. The implementation is straightforward and effective.
17-19: Differentiated Hover Behavior for React Logos
The .logo.react:hover rule correctly applies a variant drop-shadow for React-specific logos. This conditional styling is clear and purposeful.
21-27: Well-Defined Keyframe Animation
The keyframes for logo-spin are implemented correctly to rotate the element. This will add a dynamic element without being overly complex.
29-34: Media Query for Reduced Motion Consideration
The media query applies the logo-spin animation only when the user’s reduced-motion preference isn’t active. One note: using a:nth-of-type(2) for selecting the element could be brittle if the markup changes; consider using a more semantic selector if possible.
36-38: Simple and Effective Card Spacing
The .card class adds sufficient padding for content separation. This minimal approach keeps the design clean.
40-42: Muted Styling for Documentation Text
The .read-the-docs class applies a subdued color, which is appropriate for supplementary information or documentation links.apps/storybook/package.json (3)
1-13: Well-Organized Project Metadata and Scripts
The header section (name, private flag, version, type) along with the defined scripts are clear and align with expected workflows (development, build, lint, preview, and Storybook operations).
14-17: React Dependency Version Caution
The dependencies for "react" and "react-dom" are specified as "^18.3.1". Note that previous review feedback indicated that this version may be premature since React 18.3.1 is not officially released. Please verify if this is intentional or if reverting to a stable version (e.g., ^18.2.0) is more appropriate.
18-40: Review ESLint and TypeScript-ESLint Setup
Within devDependencies, packages like "@eslint/js", "eslint", and "typescript-eslint" are present. There is prior feedback suggesting alignment (and possibly using scoped packages for TypeScript-ESLint). Ensure these versions are deliberate and that they align with the rest of your codebase’s configuration.apps/storybook/src/index.css (7)
1-14: Solid Root Styles for Global Consistency
The :root selector defines key font, color, and rendering properties that will help ensure a consistent look across the application.
16-20: Clear Anchor Tag Styling
The styling for anchor tags (a) is straightforward and provides good visual cues with clear hover states.
21-23: Effective Hover State for Links
The a:hover rule appropriately changes the text color to enhance interactivity.
25-31: Layout Caution: Body Styling May Impact Dashboard
The body element is styled with display: flex and place-items: center along with min-width/min-height. While this often works well for centering content, note that using “place-items: center” could conflict with a dynamic sidebar layout in a dashboard context. It might be beneficial to reassess this if the sidebar appears misaligned.
33-36: Header Styling is Clear and Bold
The h1 element properties define a strong and readable header, setting an appropriate font size and line-height.
38-48: Button Styling Balances Function and Aesthetics
The button styles add good visual weight with proper padding, border, and transition effects. The hover and focus states further enhance usability.
57-68: Adaptive Light Theme Overrides
The media query for (prefers-color-scheme: light) correctly overrides root styles, link hovers, and button backgrounds for a lighter theme, ensuring a responsive design based on user preferences.apps/storybook/README.md (3)
1-9: Concise Introduction and Plugin Overview
The introduction clearly sets the stage for the React + TypeScript + Vite template and outlines the two Storybook plugins available. This is a good high-level overview for new users.
10-30: Detailed Guidance on Expanding ESLint Configuration
The section on ESLint configuration provides actionable steps and code examples. The instructions are clear; however, verify that the use of tseslint.config aligns with your project’s ESLint strategy. Some teams opt for the scoped @typescript-eslint packages instead.
32-50: Instructive ESLint Configuration Example
The code snippet for configuring ESLint (including setting the React version and extending recommended rules) is well presented. Ensure that the specified React version (set as "18.3") matches your dependency requirements and that all configuration paths are updated for your monorepo structure.apps/storybook/src/stories/Configure.mdx (2)
1-17: Well-Structured Asset Imports
The file begins by importing all necessary SVG and PNG assets. This organization makes it easy to update or replace assets in the future.
34-207: Comprehensive Documentation for Configuring Storybook
The remainder of the MDX document is detailed and guides users through various configuration aspects including styling, context, asset loading, autodocs, and addons. The clear sectioning, descriptive text, and use of the RightArrow component in links make for an engaging and instructive documentation page.
apps/storybook/eslint.config.js
Outdated
| extends: [js.configs.recommended, ...tseslint.configs.recommended], | ||
| files: ['**/*.{ts,tsx}'], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add React recommended configurations.
Include React's recommended configurations for comprehensive React-specific linting.
- extends: [js.configs.recommended, ...tseslint.configs.recommended],
+ extends: [
+ js.configs.recommended,
+ ...tseslint.configs.recommended,
+ 'plugin:react/recommended',
+ 'plugin:react/jsx-runtime'
+ ],📝 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.
| extends: [js.configs.recommended, ...tseslint.configs.recommended], | |
| files: ['**/*.{ts,tsx}'], | |
| extends: [ | |
| js.configs.recommended, | |
| ...tseslint.configs.recommended, | |
| 'plugin:react/recommended', | |
| 'plugin:react/jsx-runtime' | |
| ], | |
| files: ['**/*.{ts,tsx}'], |
| export const RightArrow = () => <svg | ||
| viewBox="0 0 14 14" | ||
| width="8px" | ||
| height="14px" | ||
| style={{ | ||
| marginLeft: '4px', | ||
| display: 'inline-block', | ||
| shapeRendering: 'inherit', | ||
| verticalAlign: 'middle', | ||
| fill: 'currentColor', | ||
| 'path fill': 'currentColor' | ||
| }} | ||
| > | ||
| <path d="m11.1 7.35-5.5 5.5a.5.5 0 0 1-.7-.7L10.04 7 4.9 1.85a.5.5 0 1 1 .7-.7l5.5 5.5c.2.2.2.5 0 .7Z" /> | ||
| </svg> |
There was a problem hiding this comment.
Critical Issue: Incorrect Inline Style Property in SVG Component
In the RightArrow component, the style object includes a property 'path fill' at line 28. This is not a valid CSS property for inline styles. It is likely intended to be just "fill".
Apply this diff:
- 'path fill': 'currentColor'
+ fill: 'currentColor'
This correction will ensure the SVG renders with the intended color.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
packages/icons/src/components/user.tsx (1)
13-15: 🛠️ Refactor suggestionConvert SVG attributes to camelCase for React.
React expects DOM properties to be in camelCase. Convert the following SVG attributes:
- stroke-width="2" - stroke-linecap="round" - stroke-linejoin="round" + strokeWidth="2" + strokeLinecap="round" + strokeLineJoin="round"packages/icons/package.json (1)
63-63:⚠️ Potential issueRemove trailing comma in Asset3dIcon export path.
The export path contains an unintended trailing comma that could cause module resolution issues.
- "./Asset3dIcon": "./src/components/3dAsset.tsx,", + "./Asset3dIcon": "./src/components/3dAsset.tsx"packages/ui/src/components/molecules/sidebar.tsx (1)
109-110:⚠️ Potential issueEncode cookie values to prevent security vulnerabilities.
The cookie value should be encoded to prevent XSS attacks.
Apply this diff to fix the issue:
- document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; + document.cookie = `${SIDEBAR_COOKIE_NAME}=${encodeURIComponent(openState)}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`;
🧹 Nitpick comments (9)
apps/core/next.config.mjs (1)
5-5: Consider using GitHub's CDN for avatar images.For GitHub avatar images, it's recommended to use the
avatars.githubusercontent.comCDN instead ofgithub.comfor better reliability and performance. The CDN URL format would be:https://avatars.githubusercontent.com/u/USERNAMEAlso applies to: 10-10
packages/icons/package.json (1)
51-64: Standardize export path naming convention.The export paths use inconsistent casing (e.g., "Usericon" vs "user-circle"). Consider standardizing to kebab-case for consistency with most other exports.
- "./Usercircleicon": "./src/components/user-circle.tsx", - "./Usericon": "./src/components/user.tsx", - "./Useractionpixelicon": "./src/components/user-action-pixel.tsx", - "./Settingsicon": "./src/components/settings.tsx", - "./Doorexiticon": "./src/components/door-exit.tsx", - "./Moonicon": "./src/components/moon.tsx", + "./user-circle-icon": "./src/components/user-circle.tsx", + "./user-icon": "./src/components/user.tsx", + "./user-action-pixel-icon": "./src/components/user-action-pixel.tsx", + "./settings-icon": "./src/components/settings.tsx", + "./door-exit-icon": "./src/components/door-exit.tsx", + "./moon-icon": "./src/components/moon.tsx",apps/core/app/dashboard/_compnents/app-sidebar.tsx (3)
14-23: Maintain consistent import grouping for icons.The
Settingsiconimport is separated from other icon imports. Move it to the icons group for better organization.//icons import BoxPackage from "@repo/icons/box-package"; import ChartBarPopular from "@repo/icons/chart-bar-popular"; import LayoutDashboard from "@repo/icons/layout-dashboard"; import ReportMoney from "@repo/icons/report-money"; import Usericon from "@repo/icons/user"; +import Settingsicon from "@repo/icons/settings"; //components sidebar -import Settingsicon from "@repo/icons/settings"; import { LogoIconSwitcher } from "./logo-icon-switcher"; import { NavMain } from "./nav-main";
26-27: Improve comment clarity.The comment "This is items of dashboard" could be more descriptive.
-// This is items of dashboard. +// Navigation items configuration for the dashboard sidebar.
27-60: Consider using URL constants for better maintainability.Hardcoded URLs can be difficult to maintain. Consider extracting them into constants or using a route configuration system.
// Example route constants const ROUTES = { DASHBOARD: '/dashboard', PRODUCTS: '/products', SALES: '/sales', PROFILE: '/profile', PAYOUTS: '/payouts', SETTINGS: '/settings', } as const; const data = { navMain: [ { title: "Dashboard", url: ROUTES.DASHBOARD, icon: LayoutDashboard, }, // ... other items using ROUTES constants ], };packages/ui/src/components/atoms/sidebar.tsx (4)
24-29: Consider making constants configurable via environment variables.The hardcoded constants for cookie name, max age, and dimensions could be made more flexible by allowing configuration through environment variables.
-const SIDEBAR_COOKIE_NAME = "sidebar:state"; -const SIDEBAR_COOKIE_MAX_AGE = 60 * 60 * 24 * 7; -const SIDEBAR_WIDTH = "16rem"; -const SIDEBAR_WIDTH_MOBILE = "18rem"; -const SIDEBAR_WIDTH_ICON = "3rem"; -const SIDEBAR_KEYBOARD_SHORTCUT = "b"; +const SIDEBAR_COOKIE_NAME = process.env.NEXT_PUBLIC_SIDEBAR_COOKIE_NAME ?? "sidebar:state"; +const SIDEBAR_COOKIE_MAX_AGE = parseInt(process.env.NEXT_PUBLIC_SIDEBAR_COOKIE_MAX_AGE ?? String(60 * 60 * 24 * 7)); +const SIDEBAR_WIDTH = process.env.NEXT_PUBLIC_SIDEBAR_WIDTH ?? "16rem"; +const SIDEBAR_WIDTH_MOBILE = process.env.NEXT_PUBLIC_SIDEBAR_WIDTH_MOBILE ?? "18rem"; +const SIDEBAR_WIDTH_ICON = process.env.NEXT_PUBLIC_SIDEBAR_WIDTH_ICON ?? "3rem"; +const SIDEBAR_KEYBOARD_SHORTCUT = process.env.NEXT_PUBLIC_SIDEBAR_KEYBOARD_SHORTCUT ?? "b";
102-115: Improve keyboard shortcut accessibility.The keyboard shortcut implementation could be enhanced with ARIA attributes and user notifications.
React.useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { if ( event.key === SIDEBAR_KEYBOARD_SHORTCUT && (event.metaKey || event.ctrlKey) ) { event.preventDefault(); + const message = `Sidebar ${open ? 'collapsed' : 'expanded'}`; + const ariaLive = document.createElement('div'); + ariaLive.setAttribute('role', 'status'); + ariaLive.setAttribute('aria-live', 'polite'); + ariaLive.className = 'sr-only'; + ariaLive.textContent = message; + document.body.appendChild(ariaLive); toggleSidebar(); + setTimeout(() => document.body.removeChild(ariaLive), 1000); } };
169-270: Consider memoizing complex calculations and callbacks.The Sidebar component could benefit from performance optimizations by memoizing complex calculations and callbacks.
+const useSidebarCallbacks = (isMobile: boolean, state: string, openMobile: boolean, setOpenMobile: (open: boolean) => void) => { + return React.useMemo( + () => ({ + handleMobileChange: React.useCallback((open: boolean) => { + setOpenMobile(open); + }, [setOpenMobile]), + }), + [isMobile, state, openMobile, setOpenMobile] + ); +}; const Sidebar = React.forwardRef< HTMLDivElement, React.ComponentProps<"div"> & { side?: "left" | "right"; variant?: "sidebar" | "floating" | "inset"; collapsible?: "offcanvas" | "icon" | "none"; } >( ({ side = "left", variant = "sidebar", collapsible = "offcanvas", className, children, ...props }, ref) => { const { isMobile, state, openMobile, setOpenMobile } = useSidebar(); + const { handleMobileChange } = useSidebarCallbacks(isMobile, state, openMobile, setOpenMobile);
272-745: Add JSDoc comments for better documentation.The UI components would benefit from comprehensive JSDoc comments describing their purpose, props, and usage examples.
Example for one component:
+/** + * A button component that triggers the sidebar's open/close state. + * @component + * @param {React.ComponentProps<typeof Button>} props - The component props + * @param {string} [props.className] - Additional CSS classes + * @param {function} [props.onClick] - Click handler + * @returns {JSX.Element} The rendered component + * @example + * ```tsx + * <SidebarTrigger className="my-class" onClick={() => console.log('clicked')} /> + * ``` + */ const SidebarTrigger = React.forwardRef< React.ElementRef<typeof Button>, React.ComponentProps<typeof Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (9)
.husky/pre-rebase(1 hunks)apps/core/app/dashboard/_compnents/app-sidebar.tsx(1 hunks)apps/core/app/dashboard/_compnents/navbar-dashboard.tsx(1 hunks)apps/core/next.config.mjs(1 hunks)packages/icons/package.json(2 hunks)packages/icons/src/components/moon.tsx(1 hunks)packages/icons/src/components/user.tsx(1 hunks)packages/ui/src/components/atoms/sidebar.tsx(1 hunks)packages/ui/src/components/molecules/sidebar.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .husky/pre-rebase
- apps/core/app/dashboard/_compnents/navbar-dashboard.tsx
🧰 Additional context used
🧠 Learnings (1)
apps/core/app/dashboard/_compnents/app-sidebar.tsx (1)
Learnt from: mrbadri
PR: pixel-genius/pixel-client#133
File: apps/core/app/dashboard/_compnents/app-sidebar.tsx:17-20
Timestamp: 2024-12-22T16:57:42.240Z
Learning: We always use '@repo/icons' path alias instead of relative paths for icons across the codebase for maintainability.
🔇 Additional comments (7)
packages/ui/src/components/atoms/sidebar.tsx (2)
43-50: LGTM! Well-implemented custom hook with proper error handling.The
useSidebarhook follows React best practices by checking for context availability and providing a clear error message.
746-771: LGTM! Well-organized exports.The exports are properly organized and follow a consistent alphabetical order.
packages/ui/src/components/molecules/sidebar.tsx (5)
1-70: Well-structured context and hook implementation!The context setup, constants definition, and hook implementation follow React best practices with proper error handling.
122-135: Well-implemented keyboard shortcut handling!The keyboard shortcut implementation properly handles cleanup and prevents default browser behavior.
189-290: Excellent responsive sidebar implementation!The component handles different view modes and collapsible states effectively, with clean conditional rendering for mobile and desktop views.
394-843: Excellent component architecture!The utility components are well-structured with:
- Proper use of forwardRef and displayNames
- Consistent styling patterns
- Clear prop types and variants
844-870: Well-organized exports!All components are properly exported and organized alphabetically.
| transpilePackages: ["@repo/ui"], | ||
| images: { | ||
| domains: ["images.unsplash.com"], | ||
| domains: ["images.unsplash.com", "github.com/shadcn.png"], |
There was a problem hiding this comment.
Fix the incorrect domain format for GitHub images.
The domain "github.com/shadcn.png" is incorrectly formatted as it includes the full path. Next.js domains should only include the hostname.
Apply this diff to fix the domain:
- domains: ["images.unsplash.com", "github.com/shadcn.png"],
+ domains: ["images.unsplash.com", "github.com", "avatars.githubusercontent.com"],📝 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.
| domains: ["images.unsplash.com", "github.com/shadcn.png"], | |
| domains: ["images.unsplash.com", "github.com", "avatars.githubusercontent.com"], |
| stroke-width="2" | ||
| stroke-linecap="round" | ||
| stroke-linejoin="round" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Convert SVG attributes to camelCase for React.
React expects DOM properties to be in camelCase. Convert the following SVG attributes:
- stroke-width="2"
- stroke-linecap="round"
- stroke-linejoin="round"
+ strokeWidth="2"
+ strokeLinecap="round"
+ strokeLineJoin="round"📝 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.
| stroke-width="2" | |
| stroke-linecap="round" | |
| stroke-linejoin="round" | |
| strokeWidth="2" | |
| strokeLinecap="round" | |
| strokeLineJoin="round" |
| export function AppSidebar({ ...props }: React.ComponentProps<typeof Sidebar>) { | ||
| const { open, isMobile } = useSidebar(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused destructured variables.
The open and isMobile variables are destructured from useSidebar but never used.
export function AppSidebar({ ...props }: React.ComponentProps<typeof Sidebar>) {
- const { open, isMobile } = useSidebar();
+ useSidebar(); // Remove if not needed, or keep only required variables📝 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.
| export function AppSidebar({ ...props }: React.ComponentProps<typeof Sidebar>) { | |
| const { open, isMobile } = useSidebar(); | |
| export function AppSidebar({ ...props }: React.ComponentProps<typeof Sidebar>) { | |
| useSidebar(); // Remove if not needed, or keep only required variables |
| } | ||
|
|
||
| // This sets the cookie to keep the sidebar state. | ||
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance cookie security attributes.
The cookie is set without secure attributes, which could pose security risks.
- document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`;
+ document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}; SameSite=Strict${location.protocol === 'https:' ? '; Secure' : ''}`;📝 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.
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; | |
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}; SameSite=Strict${location.protocol === 'https:' ? '; Secure' : ''}`; |
| // <Button | ||
| // ref={ref} | ||
| // data-sidebar="trigger" | ||
| // className={cn("h-7 w-7", className)} | ||
| // onClick={(event) => { | ||
| // onClick?.(event); | ||
| // toggleSidebar(); | ||
| // }} | ||
| // {...props} | ||
| // > | ||
| // <MenuIcon /> | ||
| // <span className="sr-only">Toggle Sidebar</span> | ||
| // </Button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented-out code.
The commented-out implementation should be removed to maintain code cleanliness.
| {/* TODO: Add User info */} | ||
| <AvatarImage | ||
| src="https://github.com/shadcn.png" | ||
| alt="@shadcn" | ||
| /> | ||
| <AvatarFallback>SH</AvatarFallback> | ||
| </Avatar> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent className="z-50"> | ||
| <DropdownMenuLabel className="text-white text-base font-medium"> | ||
| Ali Ebrahimi Kashef | ||
| <p className="text-xs font-light text-gray-600"> | ||
| [email protected] | ||
| </p> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Parameterize user information.
Replace hardcoded user information with props or context values.
Apply this diff to parameterize the user information:
- <AvatarImage src="https://github.com/shadcn.png" alt="@shadcn" />
- <AvatarFallback>SH</AvatarFallback>
+ <AvatarImage src={user.avatarUrl} alt={user.username} />
+ <AvatarFallback>{user.initials}</AvatarFallback>- <DropdownMenuLabel className="text-white text-base font-medium">
- Ali Ebrahimi Kashef
- <p className="text-xs font-light text-gray-600">
- [email protected]
- </p>
- </DropdownMenuLabel>
+ <DropdownMenuLabel className="text-white text-base font-medium">
+ {user.fullName}
+ <p className="text-xs font-light text-gray-600">
+ {user.email}
+ </p>
+ </DropdownMenuLabel>📝 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.
| {/* TODO: Add User info */} | |
| <AvatarImage | |
| src="https://github.com/shadcn.png" | |
| alt="@shadcn" | |
| /> | |
| <AvatarFallback>SH</AvatarFallback> | |
| </Avatar> | |
| </DropdownMenuTrigger> | |
| <DropdownMenuContent className="z-50"> | |
| <DropdownMenuLabel className="text-white text-base font-medium"> | |
| Ali Ebrahimi Kashef | |
| <p className="text-xs font-light text-gray-600"> | |
| alikashef27@gmail.com | |
| </p> | |
| {/* TODO: Add User info */} | |
| <AvatarImage | |
| src={user.avatarUrl} | |
| alt={user.username} | |
| /> | |
| <AvatarFallback>{user.initials}</AvatarFallback> | |
| </Avatar> | |
| </DropdownMenuTrigger> | |
| <DropdownMenuContent className="z-50"> | |
| <DropdownMenuLabel className="text-white text-base font-medium"> | |
| {user.fullName} | |
| <p className="text-xs font-light text-gray-600"> | |
| {user.email} | |
| </p> | |
| </DropdownMenuLabel> |
mrbadri
left a comment
mrbadri
left a comment
There was a problem hiding this comment.
please handle inner padding sam as figma
Code:
Figma:
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/ui/src/components/atoms/sidebar.tsx (1)
89-89:⚠️ Potential issueEnhance cookie security attributes.
The cookie is set without secure attributes, which could pose security risks.
Apply this diff to add secure attributes:
- document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; + document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}; SameSite=Strict${location.protocol === 'https:' ? '; Secure' : ''}`;
🧹 Nitpick comments (6)
packages/ui/src/components/atoms/sidebar.tsx (6)
102-115: Add ARIA attributes for keyboard shortcuts.Enhance accessibility by adding ARIA attributes to indicate the keyboard shortcut.
Add
aria-keyshortcutsattribute to the sidebar trigger:<Button ref={ref} data-sidebar="trigger" + aria-keyshortcuts="Control+B" className={cn("h-7 w-7", className)} onClick={(event) => { onClick?.(event); toggleSidebar(); }} {...props} >
72-73: Consider debouncing mobile detection.The
useIsMobilehook might trigger frequent re-renders on window resize. Consider debouncing the mobile detection to improve performance.Consider using a debounced version of the mobile detection:
const useDebounce = <T>(value: T, delay: number): T => { const [debouncedValue, setDebouncedValue] = React.useState(value); React.useEffect(() => { const handler = setTimeout(() => { setDebouncedValue(value); }, delay); return () => { clearTimeout(handler); }; }, [value, delay]); return debouncedValue; }; // Usage in component const isMobileRaw = useIsMobile(); const isMobile = useDebounce(isMobileRaw, 150);
207-221: Enhance mobile sheet accessibility.The mobile sheet component could benefit from additional ARIA attributes for better screen reader support.
Add ARIA attributes to improve accessibility:
<Sheet open={openMobile} onOpenChange={setOpenMobile} {...props}> <SheetContent data-sidebar="sidebar" data-mobile="true" + role="navigation" + aria-label="Mobile Navigation Menu" + aria-expanded={openMobile} className="w-[--sidebar-width] bg-sidebar p-0 text-sidebar-foreground [&>button]:hidden" style={ { "--sidebar-width": SIDEBAR_WIDTH_MOBILE, } as React.CSSProperties } side={side} > <div className="flex h-full w-full flex-col">{children}</div> </SheetContent> </Sheet>
496-507: Optimize performance with memoization.Consider memoizing the
SidebarMenucomponent to prevent unnecessary re-renders of the menu items.Wrap the component with
React.memo:-const SidebarMenu = React.forwardRef< +const SidebarMenu = React.memo(React.forwardRef< HTMLUListElement, React.ComponentProps<"ul"> >(({ className, ...props }, ref) => ( <ul ref={ref} data-sidebar="menu" className={cn("flex w-full min-w-0 flex-col gap-1", className)} {...props} /> -)); +)));
546-550: Strengthen type safety for tooltip props.The tooltip prop type could be more strictly defined to ensure type safety.
Consider using a more specific type for tooltip props:
- tooltip?: string | React.ComponentProps<typeof TooltipContent>; + tooltip?: string | (Omit<React.ComponentProps<typeof TooltipContent>, 'side' | 'align' | 'hidden'> & { + side?: 'right'; + align?: 'center'; + });
746-771: Add JSDoc documentation for exported components.Consider adding JSDoc documentation to improve developer experience and code maintainability.
Add documentation for the exported components:
/** Provider component that manages sidebar state and context */ export { SidebarProvider }; /** Hook to access sidebar context and state */ export { useSidebar }; /** Main sidebar component that renders the sidebar container */ export { Sidebar }; // Add similar documentation for other exports...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.husky/pre-rebase(1 hunks)packages/design-system/src/styles/base/_colors-dark.scss(1 hunks)packages/ui/src/components/atoms/sidebar.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .husky/pre-rebase
🔇 Additional comments (1)
packages/design-system/src/styles/base/_colors-dark.scss (1)
43-43: Table Header Opacity SimplificationThe change from
rgba(39, 39, 42, 0.5000)torgba(39, 39, 42, 0.5)at line 43 simplifies the opacity value without altering the appearance. This update improves readability while preserving styling.
| --sidebar: var(--zinc-900); | ||
| --sidebar-accent: var(--zinc-950); | ||
| --sidebar-accent-foreground: var(--general-white); | ||
| --sidebar-border: rgba(255, 255, 255, 0); | ||
| --sidebar-forground: var(--general-white); | ||
| --sidebar-primary: hsl(0, 100%, 50%); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Sidebar Color Properties Update with Potential Typo
The updated sidebar properties (lines 32–37) correctly adopt the new design palette values. However, note that on line 36 the custom property is named --sidebar-forground instead of the expected --sidebar-foreground. This typo may lead to styling issues if components reference the correctly spelled variable.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/ui/src/components/molecules/sidebar.tsx (2)
109-110:⚠️ Potential issueEncode cookie values to prevent security vulnerabilities.
Directly setting
document.cookiewith unencoded values can lead to security issues like XSS attacks. It's recommended to encode the cookie value usingencodeURIComponentto ensure special characters are properly handled.Apply this diff to fix the issue:
- document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; + document.cookie = `${SIDEBAR_COOKIE_NAME}=${encodeURIComponent(openState)}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`;
338-351: 🛠️ Refactor suggestionParameterize user information.
Replace hardcoded user information with props or context values.
Apply this diff to parameterize the user information:
- <AvatarImage src="https://github.com/shadcn.png" alt="@shadcn" /> - <AvatarFallback>SH</AvatarFallback> + <AvatarImage src={user.avatarUrl} alt={user.username} /> + <AvatarFallback>{user.initials}</AvatarFallback>- <DropdownMenuLabel className="text-white text-base font-medium"> - Ali Ebrahimi Kashef - <p className="text-xs font-light text-gray-600"> - [email protected] - </p> - </DropdownMenuLabel> + <DropdownMenuLabel className="text-white text-base font-medium"> + {user.fullName} + <p className="text-xs font-light text-gray-600"> + {user.email} + </p> + </DropdownMenuLabel>
🧹 Nitpick comments (5)
packages/ui/src/components/molecules/sidebar.tsx (5)
72-110: Add cookie reading on initial load.The SidebarProvider persists the state to cookies when changed, but it doesn't read from cookies on initial load. This means user preferences won't be maintained across page refreshes or sessions.
Add cookie reading logic in the component initialization:
const SidebarProvider = React.forwardRef< HTMLDivElement, React.ComponentProps<"div"> & { defaultOpen?: boolean; open?: boolean; onOpenChange?: (open: boolean) => void; } >( ( { defaultOpen = true, open: openProp, onOpenChange: setOpenProp, className, style, children, ...props }, ref, ) => { const isMobile = useIsMobile(); const [openMobile, setOpenMobile] = React.useState(false); + // Read the saved state from cookie on initial load + const getSavedState = React.useCallback(() => { + if (typeof document === "undefined") return defaultOpen; + const cookies = document.cookie.split(';'); + const cookie = cookies.find(c => c.trim().startsWith(`${SIDEBAR_COOKIE_NAME}=`)); + if (!cookie) return defaultOpen; + const value = cookie.split('=')[1]; + return value === "true"; + }, [defaultOpen]); // This is the internal state of the sidebar. // We use openProp and setOpenProp for control from outside the component. - const [_open, _setOpen] = React.useState(defaultOpen); + const [_open, _setOpen] = React.useState(getSavedState);
232-232: Remove commented code.The commented out
bg-sidebarappears to be leftover from development.className="w-[--sidebar-width] p-0 bg-card text-sidebar-foreground [&>button]:hidden" - // bg-sidebar style={
382-382: Fix typo in "Sign Out" text.There's a typo in the sign out text which should be corrected.
- <Link href="/" className="pl-1 text-error-500">Sing Out</Link> + <Link href="/" className="pl-1 text-error-500">Sign Out</Link>
682-685: Use a new variable for tooltip object conversion.Reassigning the
tooltipparameter can lead to type checking issues. Use a new variable instead.if (typeof tooltip === "string") { - tooltip = { - children: tooltip, - }; + const tooltipContent = { children: tooltip }; + return ( + <Tooltip> + <TooltipTrigger asChild>{button}</TooltipTrigger> + <TooltipContent + side="right" + align="center" + hidden={state !== "collapsed" || isMobile} + {...tooltipContent} + /> + </Tooltip> + ); }
761-763: Use useRef instead of useMemo for stable random width.The current implementation with
useMemocould still cause the width to change on re-renders that invalidate the dependency array. UsinguseRefwould ensure the width remains stable throughout the component's lifecycle.- const width = React.useMemo(() => { - return `${Math.floor(Math.random() * 40) + 50}%`; - }, []); + const width = React.useRef(`${Math.floor(Math.random() * 40) + 50}%`).current;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/core/app/dashboard/_compnents/app-sidebar.tsx(1 hunks)apps/core/app/dashboard/_compnents/logo-icon-switcher.tsx(1 hunks)apps/core/app/dashboard/_compnents/navbar-dashboard.tsx(1 hunks)apps/core/app/dashboard/layout.tsx(1 hunks)packages/ui/src/components/molecules/sidebar.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/core/app/dashboard/_compnents/navbar-dashboard.tsx
- apps/core/app/dashboard/_compnents/logo-icon-switcher.tsx
- apps/core/app/dashboard/layout.tsx
- apps/core/app/dashboard/_compnents/app-sidebar.tsx
🔇 Additional comments (1)
packages/ui/src/components/molecules/sidebar.tsx (1)
300-312: Remove commented-out code.The commented-out implementation should be removed to maintain code cleanliness.
mrbadri
force-pushed
the
feat/#123-new-sidebar
branch
from
ecea9f8 to
108f92a
Compare
Summary by CodeRabbit