-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/#171 products list #195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning Rate limit exceeded@PooriaKazemirad has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 15 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughThis pull request adds multiple new React components spanning the UI and API layers. It introduces the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant PL as ProductsListPage
participant PT as ProductTabs
participant Q as UseGetShopProductList
participant G as getShopProductList
participant API as API Server
U->>PL: Open Product List Page
PL->>PT: Render tabs & initialize state
U->>PT: Select a tab
PT->>PL: Update tab state
PL->>Q: Trigger query with parameters
Q->>G: Fetch product list data
G->>API: Make API request
API-->>G: Return product data
G-->>Q: Process & validate response
Q-->>PL: Return processed data
PL->>U: Render updated product table
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
🧹 Nitpick comments (11)
apps/core/app/dashboard/products/page.tsx (1)
9-12: Move tabs configuration to constants fileThe tabs array should be moved to a constants file for better maintainability and reuse.
+// constants/products.ts +export const PRODUCT_TABS = [ + { value: "all", label: "Products" }, + { value: "drafts", label: "Drafts" }, +] as const;apps/core/app/dashboard/products/_components/tabs/productTabs.tsx (2)
26-26: Remove empty line in JSXThere's an unnecessary empty line within the TabsTrigger component.
22-22: Improve key prop for tab itemsUsing array index in key prop along with label is redundant since label should be unique.
- key={tab.label + "-" + index} + key={tab.value}packages/icons/src/components/circle-plus.tsx (1)
16-16: Consider making the stroke color configurable.The stroke color is hardcoded to "white", which limits the icon's reusability across different themes or backgrounds. Consider using the
colorprop instead.- stroke="white" + stroke={color}apps/core/app/dashboard/products/_components/table/productTablePages/productTablePages.tsx (1)
11-16: Consider type consistency in onChange callback.The
onChangecallback uses string type whilepageprop is number. Consider maintaining consistent types throughout the component.interface ProductTablePageProps { page?: number; limit: number; total: number; - onChange?: (pageNum: string) => void; + onChange?: (pageNum: number) => void; }Then update the onValueChange handler:
- onChange?.(value); + onChange?.(Number(value));apps/core/app/dashboard/products/_components/table/productTableHead/productTableHead.tsx (1)
7-41: Consider extracting repeated className patterns.Multiple TableHead components share the same className patterns. Consider extracting these into a constant or utility class for better maintainability.
const tableHeadClass = "border-b-2 border-background"; const typographyClass = "font-bold text-foreground"; // Then use them in JSX: <TableHead className={tableHeadClass}> <Typography className={typographyClass} variant="label/sm">packages/icons/src/components/dotsVertical.tsx (2)
3-3: Consider using "currentColor" as default.Using "#fff" as the default color might not work well with dark themes. Consider using "currentColor" to inherit from parent element.
- const { size = 25, color = "#fff", ...resProps } = props; + const { size = 25, color = "currentColor", ...resProps } = props;
14-34: Consider simplifying repeated SVG paths.The three dot paths share many properties. Consider using a loop or transform attribute to reduce code duplication.
Example approach using transform:
const dots = [5.5, 12.5, 19.5].map((cx) => ( <path key={cx} d="M4.5 12.3333C4.5 12.5985 4.60536 12.8528 4.79289 13.0404C4.98043 13.2279 5.23478 13.3333 5.5 13.3333C5.76522 13.3333 6.01957 13.2279 6.20711 13.0404C6.39464 12.8528 6.5 12.5985 6.5 12.3333C6.5 12.068 6.39464 11.8137 6.20711 11.6261C6.01957 11.4386 5.76522 11.3333 5.5 11.3333C5.23478 11.3333 4.98043 11.4386 4.79289 11.6261C4.60536 11.8137 4.5 12.068 4.5 12.3333Z" stroke={color} strokeWidth="2" strokeLinecap="round" strokeLinejoin="round" transform={`translate(${cx - 5.5})`} /> ));packages/ui/src/components/atoms/table.tsx (1)
54-67: Consider adding aria- attributes for better accessibility.*The TableRow, TableHead, and TableCell components would benefit from additional accessibility attributes:
- Add
role="row"to TableRow- Add
scope="col"to TableHead- Add
role="cell"to TableCellconst TableRow = React.forwardRef< HTMLTableRowElement, React.HTMLAttributes<HTMLTableRowElement> >(({ className, ...props }, ref) => ( <tr ref={ref} + role="row" className={cn( "border-b transition-colors hover:bg-muted/50 data-[state=selected]:bg-muted", className, )} {...props} /> )); const TableHead = React.forwardRef< HTMLTableCellElement, React.ThHTMLAttributes<HTMLTableCellElement> >(({ className, ...props }, ref) => ( <th ref={ref} + scope="col" className={cn( "h-12 px-4 text-left align-middle font-medium text-muted-foreground [&:has([role=checkbox])]:pr-0", className, )} {...props} /> )); const TableCell = React.forwardRef< HTMLTableCellElement, React.TdHTMLAttributes<HTMLTableCellElement> >(({ className, ...props }, ref) => ( <td ref={ref} + role="cell" className={cn("p-4 align-middle [&:has([role=checkbox])]:pr-0", className)} {...props} /> ));Also applies to: 69-94
apps/core/app/dashboard/products/_components/table/productTableRow/productTableRow.tsx (2)
41-41: Remove commented out code.The commented line appears to be unrelated to the implementation and should be removed.
57-58: Fix typo in className and improve truncation logic.There's a typo in the className (-86) and the truncation might need improvement for better responsiveness.
- <div className="flex flex-wrap -86"> - <Typography variant="label/sm" className="max-w-[200px] truncate">{productName}</Typography> + <div className="flex flex-wrap gap-2"> + <Typography + variant="label/sm" + className="max-w-[200px] truncate" + title={productName} + > + {productName} + </Typography>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/core/app/dashboard/products/_components/table/productTable.tsx(1 hunks)apps/core/app/dashboard/products/_components/table/productTableHead/productTableHead.tsx(1 hunks)apps/core/app/dashboard/products/_components/table/productTablePages/productTablePages.tsx(1 hunks)apps/core/app/dashboard/products/_components/table/productTableRow/productTableRow.tsx(1 hunks)apps/core/app/dashboard/products/_components/tabs/productTabs.tsx(1 hunks)apps/core/app/dashboard/products/page.tsx(1 hunks)packages/icons/src/components/circle-plus.tsx(1 hunks)packages/icons/src/components/dotsVertical.tsx(1 hunks)packages/icons/src/components/message.tsx(1 hunks)packages/ui/src/components/atoms/table.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/core/app/dashboard/products/_components/table/productTablePages/productTablePages.tsx
[error] 45-45: 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)
🔇 Additional comments (3)
apps/core/app/dashboard/products/page.tsx (1)
15-15: Add error handling for undefined tab valueThe default tab value could be undefined if the tabs array is empty.
- const [productTab, setProductTab] = useState<string>(tabs[0]?.value || ""); + const [productTab, setProductTab] = useState<string>(tabs[0]?.value || "all");✅ Verification successful
Review suggestion is correct and should be implemented
The proposed change aligns with the codebase where "all" is the default tab value in the ProductTabs component and the first tab in the tabs array. Using "all" instead of an empty string as fallback ensures consistency and proper error handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for tabs array definition and usage in product-related files rg -A 5 "const tabs\s*=\s*\[" apps/core/app/dashboard/products/ # Search for "all" string usage in tab-related contexts rg -B 2 -A 2 '"all"' apps/core/app/dashboard/products/ # Find any references to productTab state rg "productTab" apps/core/app/dashboard/products/Length of output: 2113
apps/core/app/dashboard/products/_components/tabs/productTabs.tsx (1)
11-15: Add validation for empty tabs arrayThe component should handle cases where the tabs array is empty.
const ProductTabs = (props: ProductTabsProps) => { const { tabs, setProductTab } = props; + + if (!tabs.length) { + return null; + } + const handleValueChange = (value: string) => {✅ Verification successful
Empty tabs validation is a good practice
The suggested change to return
nullfor empty tabs array is valid and beneficial. It prevents rendering unnecessary DOM elements and maintains a clean UI by not showing an empty tab container.🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Get the full component implementation cat apps/core/app/dashboard/products/_components/tabs/productTabs.tsx # Search for similar tab components fd -e tsx -e ts -H tabsLength of output: 1235
Script:
#!/bin/bash # Check the base Tabs component implementation cat packages/ui/src/components/atoms/tabs.tsx # Look for similar validation patterns in UI components rg "if.*!.*length.*return null" packages/ui/src/components/Length of output: 2013
packages/ui/src/components/atoms/table.tsx (1)
5-17: LGTM! Well-structured table component with proper overflow handling.The Table component is well-implemented with proper TypeScript types, ref forwarding, and overflow handling.
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width={size} | ||
| height={size} | ||
| viewBox="0 0 20 20" | ||
| fill="none" | ||
| {...resProps} | ||
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add accessibility attributes to SVG
The SVG element is missing important accessibility attributes.
<svg
xmlns="http://www.w3.org/2000/svg"
width={size}
height={size}
viewBox="0 0 20 20"
fill="none"
+ role="img"
+ aria-label="message"
{...resProps}
>📝 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 | |
| xmlns="http://www.w3.org/2000/svg" | |
| width={size} | |
| height={size} | |
| viewBox="0 0 20 20" | |
| fill="none" | |
| {...resProps} | |
| > | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| width={size} | |
| height={size} | |
| viewBox="0 0 20 20" | |
| fill="none" | |
| role="img" | |
| aria-label="message" | |
| {...resProps} | |
| > |
| @@ -0,0 +1,25 @@ | |||
| import { IconProps } from "../types/types"; | |||
|
|
|||
| const MessegeIcon = (props: IconProps) => { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in component name: "MessegeIcon" → "MessageIcon"
The component name contains a spelling error. This should be corrected to maintain code quality and prevent confusion.
-const MessegeIcon = (props: IconProps) => {
+const MessageIcon = (props: 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 MessegeIcon = (props: IconProps) => { | |
| const MessageIcon = (props: IconProps) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| d="M6.66667 8.16667H13.3333M6.66667 11.5H11.6667M7.5 15.6667H5C4.33696 15.6667 3.70107 15.4033 3.23223 14.9344C2.76339 14.4656 2.5 13.8297 2.5 13.1667V6.5C2.5 5.83696 2.76339 5.20107 3.23223 4.73223C3.70107 4.26339 4.33696 4 5 4H15C15.663 4 16.2989 4.26339 16.7678 4.73223C17.2366 5.20107 17.5 5.83696 17.5 6.5V13.1667C17.5 13.8297 17.2366 14.4656 16.7678 14.9344C16.2989 15.4033 15.663 15.6667 15 15.6667H12.5L10 18.1667L7.5 15.6667Z" | ||
| stroke="#A1A1AA" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the color prop instead of hardcoding stroke color
The component accepts a color prop but uses a hardcoded stroke color "#A1A1AA". This makes the color prop ineffective.
- stroke="#A1A1AA"
+ stroke={color}Committable suggestion skipped: line range outside the PR's diff.
| @@ -0,0 +1,31 @@ | |||
| import { Table, TableBody, TableHeader } from "@repo/ui/components/table"; | |||
| import { ProdcutTableHead } from "./productTableHead/productTableHead"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in import: "ProdcutTableHead" → "ProductTableHead"
The imported component name contains a spelling error.
-import { ProdcutTableHead } from "./productTableHead/productTableHead";
+import { ProductTableHead } from "./productTableHead/productTableHead";📝 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 { ProdcutTableHead } from "./productTableHead/productTableHead"; | |
| import { ProductTableHead } from "./productTableHead/productTableHead"; |
| const ProductTable = () => { | ||
| return ( | ||
| <> | ||
| <Table className="mb-4"> | ||
| <TableHeader className="mb-1"> | ||
| <ProdcutTableHead /> | ||
| </TableHeader> | ||
| <TableBody> | ||
| <ProductTableRow | ||
| productName="Crypite - Saas Crypto Currency website design" | ||
| comments={20} | ||
| likes={40} | ||
| earning="0.00$" | ||
| sales={0} | ||
| status="Published" | ||
| version="1.5.4" | ||
| totalPrice="0.00$" | ||
| rowIndex={1} | ||
| /> | ||
| </TableBody> | ||
| </Table> | ||
| <ProductTablePages page={1} limit={10} total={20}/> | ||
| </> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Refactor hardcoded data into props or mock data file
The component contains hardcoded product data which reduces reusability and makes testing difficult. Consider:
- Accepting data as props
- Moving mock data to a separate file for development/testing
Example refactor:
-const ProductTable = () => {
+interface Product {
+ productName: string;
+ comments: number;
+ likes: number;
+ earning: string;
+ sales: number;
+ status: string;
+ version: string;
+ totalPrice: string;
+}
+
+interface ProductTableProps {
+ products: Product[];
+ pagination: {
+ page: number;
+ limit: number;
+ total: number;
+ };
+}
+
+const ProductTable = ({ products, pagination }: ProductTableProps) => {Committable suggestion skipped: line range outside the PR's diff.
| import { TableHead, TableRow } from "@repo/ui/components/table"; | ||
| import Typography from "@repo/ui/components/typography"; | ||
|
|
||
| const ProdcutTableHead = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in component name.
The component name has a typo: "Prodcut" should be "Product".
-const ProdcutTableHead = () => {
+const ProductTableHead = () => {
-export { ProdcutTableHead };
+export { ProductTableHead };Also applies to: 46-46
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| </TableHead> | ||
| <TableHead className="border-b-2 border-background"> | ||
| <Typography className="font-bold text-foreground" variant="label/sm"> | ||
| Earining |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in column header.
The "Earining" column header has a typo.
- Earining
+ Earning📝 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.
| Earining | |
| Earning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| <Image | ||
| src={imgSrc} | ||
| alt="Product Placeholder" | ||
| width={91} | ||
| height={61} | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for image loading.
Consider adding error handling for image load failures and implementing a fallback UI:
<Image
src={imgSrc}
alt="Product Placeholder"
width={91}
height={61}
+ onError={(e) => {
+ e.currentTarget.src = "/images/product-placeholder.jpg";
+ }}
/>📝 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.
| <Image | |
| src={imgSrc} | |
| alt="Product Placeholder" | |
| width={91} | |
| height={61} | |
| /> | |
| <Image | |
| src={imgSrc} | |
| alt="Product Placeholder" | |
| width={91} | |
| height={61} | |
| onError={(e) => { | |
| e.currentTarget.src = "/images/product-placeholder.jpg"; | |
| }} | |
| /> |
| <DropdownMenu> | ||
| <DropdownMenuTrigger> | ||
| <span className="cursor-pointer"> | ||
| <DotsVerticalIcon /> | ||
| </span> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent> | ||
| <DropdownMenuItem onClick={() => {}} className="cursor-pointer"> | ||
| <Typography variant="label/xs">Edit</Typography> | ||
| </DropdownMenuItem> | ||
| <DropdownMenuItem onClick={() => {}} className="cursor-pointer"> | ||
| <Typography variant="label/xs">View product</Typography> | ||
| </DropdownMenuItem> | ||
| <DropdownMenuItem onClick={() => {}} className="cursor-pointer"> | ||
| <Typography variant="label/xs" className="text-error"> | ||
| Delete | ||
| </Typography> | ||
| </DropdownMenuItem> | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implement proper action handlers and add confirmation for delete action.
The dropdown menu actions have empty onClick handlers. Additionally, the delete action should have a confirmation dialog to prevent accidental deletions.
<DropdownMenu>
<DropdownMenuTrigger>
- <span className="cursor-pointer">
+ <span className="cursor-pointer" role="button" aria-label="More options">
<DotsVerticalIcon />
</span>
</DropdownMenuTrigger>
<DropdownMenuContent>
- <DropdownMenuItem onClick={() => {}} className="cursor-pointer">
+ <DropdownMenuItem
+ onClick={() => handleEdit()}
+ className="cursor-pointer"
+ >
<Typography variant="label/xs">Edit</Typography>
</DropdownMenuItem>
- <DropdownMenuItem onClick={() => {}} className="cursor-pointer">
+ <DropdownMenuItem
+ onClick={() => handleView()}
+ className="cursor-pointer"
+ >
<Typography variant="label/xs">View product</Typography>
</DropdownMenuItem>
- <DropdownMenuItem onClick={() => {}} className="cursor-pointer">
+ <DropdownMenuItem
+ onClick={() => handleDeleteWithConfirmation()}
+ className="cursor-pointer"
+ >
<Typography variant="label/xs" className="text-error">
Delete
</Typography>
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>Committable suggestion skipped: line range outside the PR's diff.
| <TableRow className="!bg-card"> | ||
| <TableCell className="border-b-2 border-background"> | ||
| <Typography variant="label/sm">{rowIndex}</Typography> | ||
| </TableCell> | ||
| <TableCell className="border-b-2 border-background max-w-[400px]"> | ||
| <div className="flex gap-[10px]"> | ||
| <div> | ||
| <Image | ||
| src={imgSrc} | ||
| alt="Product Placeholder" | ||
| width={91} | ||
| height={61} | ||
| /> | ||
| </div> | ||
| <div className="flex flex-wrap -86"> | ||
| <Typography variant="label/sm" className="max-w-[200px] truncate">{productName}</Typography> | ||
| <div className="flex gap-2 w-full"> | ||
| <span className="inline-flex items-center gap-1"> | ||
| <IconHeart size={20} /> | ||
| <Typography variant="label/sm">{likes || 0}</Typography> | ||
| </span> | ||
| <span className="inline-flex items-center gap-1"> | ||
| <MessegeIcon size={20} /> | ||
| <Typography variant="label/sm">{comments || 0}</Typography> | ||
| </span> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </TableCell> | ||
| <TableCell className="border-b-2 border-background"> | ||
| <Badge | ||
| className={`${status==='Published' ? 'bg-success-400' : 'bg-secondary'} p-[8px] rounded-lg`} | ||
| > | ||
| <Typography variant="label/sm">{status}</Typography> | ||
| </Badge> | ||
| </TableCell> | ||
| <TableCell className="border-b-2 border-background"> | ||
| <Typography variant="label/sm"> | ||
| <span>{sales}</span> | ||
| <span className="text-muted-foreground">({totalPrice})</span> | ||
| </Typography> | ||
| </TableCell> | ||
| <TableCell className="border-b-2 border-background"> | ||
| <Typography variant="label/sm">{version}</Typography> | ||
| </TableCell> | ||
| <TableCell className="border-b-2 border-background">{earning}</TableCell> | ||
| <TableCell className="border-b-2 border-background"> | ||
| <DropdownMenu> | ||
| <DropdownMenuTrigger> | ||
| <span className="cursor-pointer"> | ||
| <DotsVerticalIcon /> | ||
| </span> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent> | ||
| <DropdownMenuItem onClick={() => {}} className="cursor-pointer"> | ||
| <Typography variant="label/xs">Edit</Typography> | ||
| </DropdownMenuItem> | ||
| <DropdownMenuItem onClick={() => {}} className="cursor-pointer"> | ||
| <Typography variant="label/xs">View product</Typography> | ||
| </DropdownMenuItem> | ||
| <DropdownMenuItem onClick={() => {}} className="cursor-pointer"> | ||
| <Typography variant="label/xs" className="text-error"> | ||
| Delete | ||
| </Typography> | ||
| </DropdownMenuItem> | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> | ||
| </TableCell> | ||
| </TableRow> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance keyboard navigation and accessibility.
The table row lacks proper keyboard navigation and accessibility attributes:
- Add keyboard navigation for the dropdown menu
- Add aria labels for interactive elements
- Add role attributes for better screen reader support
- <TableRow className="!bg-card">
+ <TableRow
+ className="!bg-card"
+ role="row"
+ aria-label={`Product: ${productName}`}
+ >Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/apis/src/services/core/shop/products/get/get-shop-product-list.schema.ts (1)
13-13: Fix typo in schema name.The schema name contains a typo: "Transofrmed" should be "Transformed".
-export const getShopProductListResponseSchemaTransofrmed = apiResponseSchema +export const getShopProductListResponseSchemaTransformed = apiResponseSchemapackages/apis/src/services/core/shop/products/get/get-shop-product-list.types.ts (1)
7-9: Fix typo in type name.The type name contains a typo: "Transofrmed" should be "Transformed".
-export type GetShopProductListRequestTransofrmed = z.infer< +export type GetShopProductListRequestTransformed = z.infer<packages/apis/src/services/core/shop/products/get/get-shop-product-list.ts (2)
12-12: Enhance URL path construction.Using
path.join()for URL paths can be problematic as it uses OS-specific path separators. Consider using URL constructor or string template literals for URL paths.-const getShopProductListURL = () => path.join("/api/shop-product-list"); +const getShopProductListURL = () => "/api/shop-product-list";
28-30: Avoid double parsing of response data.The response is already parsed by the request handler using
schema.response._def.schema. The additional parsing on line 28 is redundant.- const dataParsed = schema.response.parse(response.data); - return { ...response, data: dataParsed }; + return response;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/apis/src/services/core/shop/products/get/get-shop-product-list.schema.ts(1 hunks)packages/apis/src/services/core/shop/products/get/get-shop-product-list.ts(1 hunks)packages/apis/src/services/core/shop/products/get/get-shop-product-list.types.ts(1 hunks)packages/apis/src/services/core/shop/products/get/use-get-shop-product-list.ts(1 hunks)
🔇 Additional comments (1)
packages/apis/src/services/core/shop/products/get/get-shop-product-list.schema.ts (1)
39-39: Define structure for highlights object.An empty object schema (
z.object({})) will reject any properties. Consider defining the expected structure or usez.record()if the properties are dynamic.✅ Verification successful
Empty highlights object schema needs structure definition
The codebase shows no existing implementation or usage patterns for the
highlightsfield. Sincez.object({})rejects all properties, you should either:
- Define the expected properties in the schema if the structure is known
- Use
z.record()if properties are dynamic- Add a TODO comment if the structure is pending definition
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of highlights field to understand its structure rg -A 5 'highlights.*:' --type typescriptLength of output: 78
Script:
#!/bin/bash # Search for highlights field usage with correct file types rg -t ts -t js -A 5 'highlights[\s]*[:=]' # Search for type definitions or interfaces containing highlights ast-grep --pattern 'interface $_ { $$$ highlights $$$ }' # Search for specific Zod schema definitions related to highlights rg -t ts -t js 'highlights.*z\.' -A 5Length of output: 1538
Script:
#!/bin/bash # Search for test files related to shop products fd -e test.ts -e spec.ts -e test.js -e spec.js shop.*product --exec rg -l 'highlights' # Search for API documentation fd -e md -e yaml -e yml api.*doc --exec rg -l -i 'highlights' # Search for example responses rg -t json -t ts -t js '"highlights"' -B 5 -A 5Length of output: 202
| z.object({ | ||
| id: z.number(), | ||
| is_published: z.boolean(), | ||
| created_at: z.date(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Standardize date handling across schemas.
Inconsistent date handling detected:
created_atin the root object usesz.date()created_atin nested objects usesz.string()
This could lead to parsing issues. Consider standardizing to either all z.date() or all z.string().
Also applies to: 60-60, 69-69, 77-77, 84-84
| WithParams<GetShopProductListRequest> | ||
| >; | ||
|
|
||
| export const getShopProductListQueryKey = () => ["getShopProductList"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Include parameters in query key for proper caching.
The query key should include the parameters to ensure proper caching and refetching when parameters change.
-export const getShopProductListQueryKey = () => ["getShopProductList"];
+export const getShopProductListQueryKey = (params: GetShopProductListRequest) =>
+ ["getShopProductList", params];
// In the hook:
- queryKey: getShopProductListQueryKey(),
+ queryKey: getShopProductListQueryKey(params),Also applies to: 24-26
|
|
||
| export const getShopProductListQueryKey = () => ["getShopProductList"]; | ||
|
|
||
| export const UseGetShopProductList = (props: UseGetShopProductListProps) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Follow React hook naming convention.
React hooks should start with a lowercase 'use'. Rename UseGetShopProductList to useGetShopProductList.
-export const UseGetShopProductList = (props: UseGetShopProductListProps) => {
+export const useGetShopProductList = (props: UseGetShopProductListProps) => {📝 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 const UseGetShopProductList = (props: UseGetShopProductListProps) => { | |
| export const useGetShopProductList = (props: UseGetShopProductListProps) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/core/app/dashboard/products/_components/table/ProductTableHead.tsx (1)
1-31: Consider implementing data fetching and state management.Since this is a mock UI for a product list, consider the following architectural improvements:
- Implement a data fetching layer using React Query or SWR
- Add state management for filters and sorting
- Consider implementing virtual scrolling for large lists
- Add error boundaries for better error handling
Would you like me to provide examples of these implementations?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/core/app/dashboard/products/_components/table/ProductTableHead.tsx(1 hunks)apps/core/app/dashboard/products/page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/core/app/dashboard/products/page.tsx
| const ProductTable = () => { | ||
| return ( | ||
| <> | ||
| <Table className="mb-4"> | ||
| <TableHeader> | ||
| <ProdcutTableHead /> | ||
| </TableHeader> | ||
| <TableBody> | ||
| <ProductTableRow | ||
| productName="Crypite - Saas Crypto Currency website design" | ||
| comments={20} | ||
| likes={40} | ||
| earning="0.00$" | ||
| sales={0} | ||
| status="Published" | ||
| version="1.5.4" | ||
| totalPrice="0.00$" | ||
| rowIndex={1} | ||
| /> | ||
| </TableBody> | ||
| </Table> | ||
| <ProductTablePages page={1} limit={10} total={20} /> | ||
| </> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Convert hardcoded values to dynamic data and add TypeScript types.
The component currently uses hardcoded values and lacks TypeScript types. Consider the following improvements:
- Accept product data as props
- Support multiple rows
- Add proper TypeScript interfaces
- Handle loading and error states
Here's a suggested implementation:
interface Product {
productName: string;
comments: number;
likes: number;
earning: string;
sales: number;
status: string;
version: string;
totalPrice: string;
}
interface ProductTableProps {
products: Product[];
isLoading?: boolean;
error?: Error;
pagination: {
page: number;
limit: number;
total: number;
};
onPageChange: (page: number) => void;
}
const ProductTable = ({
products,
isLoading,
error,
pagination,
onPageChange
}: ProductTableProps) => {
if (isLoading) return <div>Loading...</div>;
if (error) return <div>Error: {error.message}</div>;
return (
<>
<Table className="mb-4">
<TableHeader>
<ProductTableHead />
</TableHeader>
<TableBody>
{products.map((product, index) => (
<ProductTableRow
key={index}
{...product}
rowIndex={index + 1}
/>
))}
</TableBody>
</Table>
<ProductTablePages
page={pagination.page}
limit={pagination.limit}
total={pagination.total}
onPageChange={onPageChange}
/>
</>
);
};| @@ -0,0 +1,31 @@ | |||
| import { Table, TableBody, TableHeader } from "@repo/ui/components/table"; | |||
| import { ProdcutTableHead } from "./productTableHead/productTableHead"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in component name "ProdcutTableHead".
There's a typo in the imported component name. It should be "ProductTableHead" instead of "ProdcutTableHead".
-import { ProdcutTableHead } from "./productTableHead/productTableHead";
+import { ProductTableHead } from "./productTableHead/productTableHead";📝 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 { ProdcutTableHead } from "./productTableHead/productTableHead"; | |
| import { ProductTableHead } from "./productTableHead/productTableHead"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| @@ -0,0 +1,31 @@ | |||
| import { Table, TableBody, TableHeader } from "@repo/ui/components/table"; | |||
| import { ProdcutTableHead } from "./productTableHead/productTableHead"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @@ -0,0 +1,31 @@ | |||
| import { Table, TableBody, TableHeader } from "@repo/ui/components/table"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please Convert all File name to kebab case
| import { TableHead, TableRow } from "@repo/ui/components/table"; | ||
| import Typography from "@repo/ui/components/typography"; | ||
|
|
||
| const ProdcutTableHead = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| </TableHead> | ||
| <TableHead className="border-b-2 border-background"> | ||
| <Typography className="font-bold text-foreground" variant="label/sm"> | ||
| Earining |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| {getPages().map((number) => ( | ||
| <SelectItem value={String(number)}>{number}</SelectItem> | ||
| ))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| > | ||
| <path | ||
| d="M9 12H15M12 9V15M3 12C3 13.1819 3.23279 14.3522 3.68508 15.4442C4.13738 16.5361 4.80031 17.5282 5.63604 18.364C6.47177 19.1997 7.46392 19.8626 8.55585 20.3149C9.64778 20.7672 10.8181 21 12 21C13.1819 21 14.3522 20.7672 15.4442 20.3149C16.5361 19.8626 17.5282 19.1997 18.364 18.364C19.1997 17.5282 19.8626 16.5361 20.3149 15.4442C20.7672 14.3522 21 13.1819 21 12C21 10.8181 20.7672 9.64778 20.3149 8.55585C19.8626 7.46392 19.1997 6.47177 18.364 5.63604C17.5282 4.80031 16.5361 4.13738 15.4442 3.68508C14.3522 3.23279 13.1819 3 12 3C10.8181 3 9.64778 3.23279 8.55585 3.68508C7.46392 4.13738 6.47177 4.80031 5.63604 5.63604C4.80031 6.47177 4.13738 7.46392 3.68508 8.55585C3.23279 9.64778 3 10.8181 3 12Z" | ||
| stroke="white" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PooriaKazemirad
please use Current color
| @@ -0,0 +1,39 @@ | |||
| import { IconProps } from "../types/types"; | |||
| const DotsVerticalIcon = (props: IconProps) => { | |||
| const { size = 25, color = "#fff", ...resProps } = props; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PooriaKazemirad
please use Current color
| @@ -0,0 +1,25 @@ | |||
| import { IconProps } from "../types/types"; | |||
|
|
|||
| const MessegeIcon = (props: IconProps) => { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| > | ||
| <path | ||
| d="M6.66667 8.16667H13.3333M6.66667 11.5H11.6667M7.5 15.6667H5C4.33696 15.6667 3.70107 15.4033 3.23223 14.9344C2.76339 14.4656 2.5 13.8297 2.5 13.1667V6.5C2.5 5.83696 2.76339 5.20107 3.23223 4.73223C3.70107 4.26339 4.33696 4 5 4H15C15.663 4 16.2989 4.26339 16.7678 4.73223C17.2366 5.20107 17.5 5.83696 17.5 6.5V13.1667C17.5 13.8297 17.2366 14.4656 16.7678 14.9344C16.2989 15.4033 15.663 15.6667 15 15.6667H12.5L10 18.1667L7.5 15.6667Z" | ||
| stroke="#A1A1AA" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PooriaKazemirad
please use Current color
8e3e1fd to
556a2a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
🧹 Nitpick comments (12)
apps/core/app/dashboard/products/_components/tabs/product-tabs.tsx (4)
18-18: Consider making the default tab value dynamic.The
defaultValueis hardcoded as "all", but there's no guarantee that this value exists in the providedtabsarray. To make the component more flexible, consider using the first tab's value as the default or accepting a prop for the default tab.- <Tabs defaultValue="all" onValueChange={handleValueChange}> + <Tabs defaultValue={tabs[0]?.value || "all"} onValueChange={handleValueChange}>
22-22: Improve key generation in the map function.While using index in keys works, it's generally better to use a unique identifier from your data when possible. If
tab.valueis guaranteed to be unique, consider using just that instead of combining it with the label and index.- key={tab.label + "-" + index} + key={tab.value}
26-26: Remove unnecessary empty line.There's an empty line that doesn't serve any purpose and should be removed for cleaner code.
-
23-23: Consider extracting common class names to a variable.The class names for TabsTrigger might be reused across similar components. Consider extracting them to a constant or a utility function for consistency and maintainability.
packages/apis/src/services/core/shop/products/get/get-shop-product-list.schema.ts (2)
93-93: Remove unnecessary transform function.The transform function doesn't modify the data, making it redundant.
- .transform((data) => data);
7-8: Add validation for page and pageSize parameters.Consider adding validation to ensure the page number is positive and the page size is within reasonable limits.
- page: z.number(), - pageSize: z.number(), + page: z.number().int().positive(), + pageSize: z.number().int().min(1).max(100),apps/core/app/dashboard/products/_components/table/product-table-head.tsx (1)
6-30: Consider making ProductTable more dynamic to handle real dataThe current implementation uses hardcoded values and a single row. Consider modifying the component to accept an array of products and render multiple rows dynamically.
-const ProductTable = () => { +interface ProductTableProps { + products: Array<{ + productName: string; + comments: number; + likes: number; + earning: string; + sales: number; + status: string; + version: string; + totalPrice: string; + }>; + page: number; + limit: number; + total: number; + onPageChange?: (page: string) => void; +} + +const ProductTable = ({ products = [], page = 1, limit = 10, total = 0, onPageChange }: ProductTableProps) => { return ( <> <Table className="mb-4"> <TableHeader> - <ProdcutTableHead /> + <ProductTableHead /> </TableHeader> <TableBody> - <ProductTableRow - productName="Crypite - Saas Crypto Currency website design" - comments={20} - likes={40} - earning="0.00$" - sales={0} - status="Published" - version="1.5.4" - totalPrice="0.00$" - rowIndex={1} - /> + {products.map((product, index) => ( + <ProductTableRow + key={index} + productName={product.productName} + comments={product.comments} + likes={product.likes} + earning={product.earning} + sales={product.sales} + status={product.status} + version={product.version} + totalPrice={product.totalPrice} + rowIndex={index + 1} + /> + ))} </TableBody> </Table> - <ProductTablePages page={1} limit={10} total={20} /> + <ProductTablePages page={page} limit={limit} total={total} onChange={onPageChange} /> </> ); };apps/core/app/dashboard/products/_components/table/product-table-pages/product-table-pages.tsx (1)
15-15: Consider changing onChange to accept a number instead of stringSince page numbers are numeric, it would be more type-appropriate to have the onChange handler work with numbers rather than strings.
- onChange?: (pageNum: string) => void; + onChange?: (pageNum: number) => void;Then update the onValueChange handler:
onValueChange={(value) => { setSelectPage(value); - onChange?.(value); + onChange?.(parseInt(value, 10)); }}apps/core/app/dashboard/products/_components/table/product-table-row/product-table-row.tsx (3)
41-41: Remove commented-out codeThere appears to be a commented-out product name that should be removed.
- // Crypite - Saas Crypto Currency website design
74-74: Improve conditional styling using Tailwind's conditional classesThe current conditional styling could be improved for better readability using template literals.
- <Badge - className={`${status==='Published' ? 'bg-success-400' : 'bg-secondary'} p-[8px] rounded-lg`} - > + <Badge + className={`p-[8px] rounded-lg ${ + status === 'Published' ? 'bg-success-400' : 'bg-secondary' + }`} + >
50-55: Improve image accessibility with more descriptive alt textThe alt text "Product Placeholder" doesn't provide context about which product is being displayed. Consider using the product name for better accessibility.
<Image src={imgSrc} - alt="Product Placeholder" + alt={`Product image: ${productName}`} width={91} height={61} />packages/ui/src/components/atoms/table.tsx (1)
1-117: Consider enhancing accessibility attributes.While the component structure is solid, consider adding accessibility enhancements such as:
aria-sortfor sortable columns in TableHeadaria-labeloraria-labelledbyfor the Table component- Keyboard navigation support
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/core/app/dashboard/products/_components/table/product-table-head.tsx(1 hunks)apps/core/app/dashboard/products/_components/table/product-table-head/product-table-head.tsx(1 hunks)apps/core/app/dashboard/products/_components/table/product-table-pages/product-table-pages.tsx(1 hunks)apps/core/app/dashboard/products/_components/table/product-table-row/product-table-row.tsx(1 hunks)apps/core/app/dashboard/products/_components/tabs/product-tabs.tsx(1 hunks)apps/core/app/dashboard/products/page.tsx(1 hunks)packages/apis/src/services/core/shop/products/get/get-shop-product-list.schema.ts(1 hunks)packages/apis/src/services/core/shop/products/get/get-shop-product-list.ts(1 hunks)packages/apis/src/services/core/shop/products/get/get-shop-product-list.types.ts(1 hunks)packages/apis/src/services/core/shop/products/get/use-get-shop-product-list.ts(1 hunks)packages/icons/src/components/circle-plus.tsx(1 hunks)packages/icons/src/components/dotsVertical.tsx(1 hunks)packages/icons/src/components/message.tsx(1 hunks)packages/ui/src/components/atoms/table.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/icons/src/components/circle-plus.tsx
- packages/icons/src/components/dotsVertical.tsx
- packages/apis/src/services/core/shop/products/get/get-shop-product-list.ts
- apps/core/app/dashboard/products/page.tsx
- packages/apis/src/services/core/shop/products/get/get-shop-product-list.types.ts
- packages/icons/src/components/message.tsx
- packages/apis/src/services/core/shop/products/get/use-get-shop-product-list.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/core/app/dashboard/products/_components/table/product-table-pages/product-table-pages.tsx
[error] 45-45: 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)
🔇 Additional comments (15)
apps/core/app/dashboard/products/_components/tabs/product-tabs.tsx (3)
19-19: Verify accessibility for custom-styled tabs.The custom styling with
className="bg-transparent gap-[8px]"should maintain proper accessibility. Ensure that the tabs have sufficient color contrast and that focus states are visible for keyboard navigation.
6-9: Props interface looks good.The interface for the component props is well-defined with appropriate types for the tabs array and state setter function.
11-16: Callback function implementation is clean and effective.The component properly destructures props and implements the value change handler efficiently.
packages/apis/src/services/core/shop/products/get/get-shop-product-list.schema.ts (2)
23-23: Standardize date handling across schema.Inconsistent date handling detected:
created_atin the root object usesz.date()created_atin nested objects usesz.string()This could lead to parsing issues. Consider standardizing to either all
z.date()or allz.string().Also applies to: 60-60, 69-69, 77-77, 84-84
39-39: Review the empty highlights schema definition.The
highlightsfield is defined as an empty object schema. If this is intentional (for future extension), consider adding a comment explaining this. Otherwise, define the expected structure.- highlights: z.object({}), + highlights: z.record(z.string()).optional(), // Map of highlight key-value pairsapps/core/app/dashboard/products/_components/table/product-table-row/product-table-row.tsx (1)
97-107: Implement actual functionality for menu itemsThe dropdown menu items have empty onClick handlers. Consider implementing actual functionality or using placeholders that indicate future implementation.
- <DropdownMenuItem onClick={() => {}} className="cursor-pointer"> + <DropdownMenuItem + onClick={() => { + console.log('Edit product:', productName); + // TODO: Implement edit functionality + }} + className="cursor-pointer" + > <Typography variant="label/xs">Edit</Typography> </DropdownMenuItem>Implement similar changes for the "View product" and "Delete" menu items. For the "Delete" action, consider adding a confirmation dialog since it's a destructive action.
packages/ui/src/components/atoms/table.tsx (9)
1-4: Clean import structure with good separation of concerns.The imports are well-organized, with React imported separately from utility functions. The use of the
cnutility from a shared library demonstrates good code reusability.
5-17: Well-implemented Table component with responsive considerations.The Table component correctly uses
React.forwardRefand wraps the table in a div with overflow handling, which is good practice for responsive tables. Setting the displayName is helpful for debugging in React DevTools.
19-25: TableHeader component looks good!Proper type definitions and consistent styling approach with the other table components.
27-37: TableBody implementation is clean and follows the component pattern.Good use of Tailwind's nesting syntax to remove borders from the last child row.
54-67: TableRow has good interactive state handling.The component includes appropriate hover effects and selected states using data attributes, with a clean transition effect.
84-94: TableCell implementation is consistent with TableHead.The component follows the same pattern as TableHead with appropriate styling differences for regular cells.
96-106: TableCaption styling is appropriate.Good use of margin and text styling to differentiate the caption from the table content.
108-117: Clean named exports.Named exports for all components follow best practices for tree-shaking and selective importing.
69-82:Details
✅ Verification successful
Consider browser compatibility for :has() selector.
The
:has()selector in the TableHead component is a relatively modern CSS feature and might not be supported in all browsers.
🌐 Web query:
What is the current browser support for CSS :has() selector?💡 Result:
The CSS
:has()pseudo-class, which allows selecting parent elements based on their children, now has full support in all major modern browsers as of late 2023/early 2024:Browser Support Overview
Browser Supported Versions Release Date Window Chrome 105+ August 2022 Edge 105+ August 2022 Safari 15.4+ (macOS/iOS) March 2022 Firefox 121+ December 2023 Opera 91+ Based on Chromium Key Details
- Global adoption: ~92% of users worldwide have browsers supporting
:has()as of September 2024[3][4]- Mobile support: Available in Chrome/Android 105+, Safari/iOS 15.4+, and Firefox/Android[4][6]
- Firefox timeline: Full unflagged support arrived in Firefox 121 (Dec 2023)[2][3][8]
Compatibility Strategies
For projects requiring backward compatibility:
@supports selector(:has(*)) { /* Modern :has() styles */ } @supports not selector(:has(*)) { /* Fallback styles */ }Performance Note: While powerful,
:has()can impact rendering performance when used with complex selectors or in dynamic DOM environments[1]. Use judiciously for critical rendering paths.Browser support is now robust enough for production use in most projects, particularly when paired with progressive enhancement strategies[7][9].
Citations:
- 1: https://blog.logrocket.com/advanced-guide-css-has-selector/
- 2: https://connect.mozilla.org/t5/ideas/when-is-has-css-selector-going-to-be-fully-implemented-in/idi-p/23794
- 3: https://www.joshwcomeau.com/css/has/
- 4: https://caniuse.com/css-has
- 5: https://css-tricks.com/the-css-has-selector/
- 6: https://www.lambdatest.com/web-technologies/css-has
- 7: https://css-irl.info/detecting-css-selector-support/
- 8: https://dev.to/yuridevat/css-has-pseudo-class-p6g
- 9: https://www.sitepoint.com/css-is-where-has-pseudo-class-selectors/
- 10: https://developer.mozilla.org/en-US/docs/Web/CSS/:has
CSS :has() Compatibility Confirmed
The web query confirms that the CSS
:has()pseudo-class is now fully supported in major browsers (Chrome 105+, Edge 105+, Safari 15.4+, Firefox 121+, Opera 91+). As long as our audience uses these modern browsers, no changes are required. If legacy browser support is a concern, consider adding appropriate fallbacks.
| .transform((data) => ({ ...data, page_size: data.pageSize })); | ||
|
|
||
| // Response | ||
| export const getShopProductListResponseSchemaTransofrmed = apiResponseSchema |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix the typo in schema name.
The schema name has a typo: "TransOfrmed" should be "Transformed".
-export const getShopProductListResponseSchemaTransofrmed = apiResponseSchema
+export const getShopProductListResponseSchemaTransformed = apiResponseSchema📝 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 const getShopProductListResponseSchemaTransofrmed = apiResponseSchema | |
| export const getShopProductListResponseSchemaTransformed = apiResponseSchema |
| next: z.string(), | ||
| previous: z.string(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider making pagination links optional.
The next and previous pagination links should be optional or nullable since they might not always be present (e.g., for the first or last page).
- next: z.string(),
- previous: z.string(),
+ next: z.string().nullable(),
+ previous: z.string().nullable(),📝 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.
| next: z.string(), | |
| previous: z.string(), | |
| next: z.string().nullable(), | |
| previous: z.string().nullable(), |
| .transform((data) => data); | ||
|
|
||
| export const getShopProductListSchema = { | ||
| response: getShopProductListResponseSchemaTransofrmed, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix schema reference in the exported object.
The exported schema references the misspelled response schema name.
- response: getShopProductListResponseSchemaTransofrmed,
+ response: getShopProductListResponseSchemaTransformed,This needs to be fixed after correcting the typo in the schema name (line 13).
📝 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.
| response: getShopProductListResponseSchemaTransofrmed, | |
| response: getShopProductListResponseSchemaTransformed, |
| </TableHead> | ||
| <TableHead className="border-b-2 border-background"> | ||
| <Typography className="font-bold text-foreground" variant="label/sm"> | ||
| Earining |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix typo in table header: "Earining" should be "Earning"
There's a typo in the table header text.
- Earining
+ Earning📝 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.
| Earining | |
| Earning |
| const ProdcutTableHead = () => { | ||
| return ( | ||
| <TableRow className="!bg-card"> | ||
| <TableHead className="border-b-2 border-background"> | ||
| <Typography className="font-bold text-foreground" variant="label/sm"> | ||
| # | ||
| </Typography> | ||
| </TableHead> | ||
| <TableHead className="border-b-2 border-background"> | ||
| <Typography className="font-bold text-foreground" variant="label/sm"> | ||
| Product Name | ||
| </Typography> | ||
| </TableHead> | ||
| <TableHead className="border-b-2 border-background"> | ||
| <Typography className="font-bold text-foreground" variant="label/sm"> | ||
| Status | ||
| </Typography> | ||
| </TableHead> | ||
| <TableHead className="border-b-2 border-background"> | ||
| <Typography className="font-bold text-foreground" variant="label/sm"> | ||
| Sales | ||
| </Typography> | ||
| </TableHead> | ||
| <TableHead className="border-b-2 border-background"> | ||
| <Typography className="font-bold text-foreground" variant="label/sm"> | ||
| Version | ||
| </Typography> | ||
| </TableHead> | ||
| <TableHead className="border-b-2 border-background"> | ||
| <Typography className="font-bold text-foreground" variant="label/sm"> | ||
| Earining | ||
| </Typography> | ||
| </TableHead> | ||
| <TableHead className="border-b-2 border-background"> | ||
| <Typography className="font-bold text-foreground" variant="label/sm"> | ||
| Actions | ||
| </Typography> | ||
| </TableHead> | ||
| </TableRow> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix typo in component name: "Prodcut" should be "Product"
There's a typo in the component name ProdcutTableHead. This should be renamed to ProductTableHead for consistency and to avoid confusion.
-const ProdcutTableHead = () => {
+const ProductTableHead = () => {Don't forget to update the export statement as well:
-export { ProdcutTableHead };
+export { ProductTableHead };This will require updating imports in other files that use this component.
📝 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 ProdcutTableHead = () => { | |
| return ( | |
| <TableRow className="!bg-card"> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| # | |
| </Typography> | |
| </TableHead> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| Product Name | |
| </Typography> | |
| </TableHead> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| Status | |
| </Typography> | |
| </TableHead> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| Sales | |
| </Typography> | |
| </TableHead> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| Version | |
| </Typography> | |
| </TableHead> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| Earining | |
| </Typography> | |
| </TableHead> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| Actions | |
| </Typography> | |
| </TableHead> | |
| </TableRow> | |
| ); | |
| }; | |
| const ProductTableHead = () => { | |
| return ( | |
| <TableRow className="!bg-card"> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| # | |
| </Typography> | |
| </TableHead> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| Product Name | |
| </Typography> | |
| </TableHead> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| Status | |
| </Typography> | |
| </TableHead> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| Sales | |
| </Typography> | |
| </TableHead> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| Version | |
| </Typography> | |
| </TableHead> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| Earining | |
| </Typography> | |
| </TableHead> | |
| <TableHead className="border-b-2 border-background"> | |
| <Typography className="font-bold text-foreground" variant="label/sm"> | |
| Actions | |
| </Typography> | |
| </TableHead> | |
| </TableRow> | |
| ); | |
| }; | |
| export { ProductTableHead }; |
| @@ -0,0 +1,31 @@ | |||
| import { Table, TableBody, TableHeader } from "@repo/ui/components/table"; | |||
| import { ProdcutTableHead } from "./product-table-head/product-table-head"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Update import to use corrected component name
Once you've fixed the typo in the ProductTableHead component name, you'll need to update this import.
-import { ProdcutTableHead } from "./product-table-head/product-table-head";
+import { ProductTableHead } from "./product-table-head/product-table-head";📝 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 { ProdcutTableHead } from "./product-table-head/product-table-head"; | |
| import { ProductTableHead } from "./product-table-head/product-table-head"; |
| </SelectTrigger> | ||
| <SelectContent> | ||
| {getPages().map((number) => ( | ||
| <SelectItem value={String(number)}>{number}</SelectItem> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add missing key prop to mapped elements
React requires a unique key prop for elements in an iterable to help with efficient rendering and updates.
{getPages().map((number) => (
- <SelectItem value={String(number)}>{number}</SelectItem>
+ <SelectItem key={number} value={String(number)}>{number}</SelectItem>
))}📝 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.
| <SelectItem value={String(number)}>{number}</SelectItem> | |
| {getPages().map((number) => ( | |
| <SelectItem key={number} value={String(number)}>{number}</SelectItem> | |
| ))} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 45-45: 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)
| const getPages = () => { | ||
| const pages: number[] = []; | ||
| for (let i = 1; i <= Math.floor(total / limit); i++) { | ||
| pages.push(i); | ||
| } | ||
| return pages; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix pagination calculation to handle remainders
The current pagination calculation doesn't account for remainders, which means if you have a total that's not divisible by the limit, you'll be missing a page.
const getPages = () => {
const pages: number[] = [];
- for (let i = 1; i <= Math.floor(total / limit); i++) {
+ const totalPages = Math.ceil(total / limit);
+ for (let i = 1; i <= totalPages; i++) {
pages.push(i);
}
return pages;
};📝 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 getPages = () => { | |
| const pages: number[] = []; | |
| for (let i = 1; i <= Math.floor(total / limit); i++) { | |
| pages.push(i); | |
| } | |
| return pages; | |
| }; | |
| const getPages = () => { | |
| const pages: number[] = []; | |
| const totalPages = Math.ceil(total / limit); | |
| for (let i = 1; i <= totalPages; i++) { | |
| pages.push(i); | |
| } | |
| return pages; | |
| }; |
| <div className="flex flex-wrap -86"> | ||
| <Typography variant="label/sm" className="max-w-[200px] truncate">{productName}</Typography> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect CSS class and improve component structure
There appears to be an incorrect CSS class -86 and the structure could be improved.
- <div className="flex flex-wrap -86">
- <Typography variant="label/sm" className="max-w-[200px] truncate">{productName}</Typography>
+ <div className="flex flex-wrap">
+ <Typography variant="label/sm" className="max-w-[200px] truncate w-full">{productName}</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.
| <div className="flex flex-wrap -86"> | |
| <Typography variant="label/sm" className="max-w-[200px] truncate">{productName}</Typography> | |
| <div className="flex flex-wrap"> | |
| <Typography variant="label/sm" className="max-w-[200px] truncate w-full">{productName}</Typography> |
| const TableFooter = React.forwardRef< | ||
| HTMLTableSectionElement, | ||
| React.HTMLAttributes<HTMLTableSectionElement> | ||
| >(({ className, ...props }, ref) => ( | ||
| <tfoot | ||
| ref={ref} | ||
| className={cn( | ||
| "border-t bg-muted/50 font-medium [&>tr]:last:border-b-0", | ||
| className, | ||
| )} | ||
| {...props} | ||
| /> | ||
| )); | ||
| TableFooter.displayName = "TableFooter"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix selector syntax in TableFooter className.
There appears to be a syntax issue in the Tailwind class.
<tfoot
ref={ref}
className={cn(
- "border-t bg-muted/50 font-medium [&>tr]:last:border-b-0",
+ "border-t bg-muted/50 font-medium [&>tr:last-child]:border-b-0",
className,
)}
{...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 TableFooter = React.forwardRef< | |
| HTMLTableSectionElement, | |
| React.HTMLAttributes<HTMLTableSectionElement> | |
| >(({ className, ...props }, ref) => ( | |
| <tfoot | |
| ref={ref} | |
| className={cn( | |
| "border-t bg-muted/50 font-medium [&>tr]:last:border-b-0", | |
| className, | |
| )} | |
| {...props} | |
| /> | |
| )); | |
| TableFooter.displayName = "TableFooter"; | |
| const TableFooter = React.forwardRef< | |
| HTMLTableSectionElement, | |
| React.HTMLAttributes<HTMLTableSectionElement> | |
| >(({ className, ...props }, ref) => ( | |
| <tfoot | |
| ref={ref} | |
| className={cn( | |
| "border-t bg-muted/50 font-medium [&>tr:last-child]:border-b-0", | |
| className, | |
| )} | |
| {...props} | |
| /> | |
| )); | |
| TableFooter.displayName = "TableFooter"; |
product list mock ui
Summary by CodeRabbit