Clean up products section code review feedback#50
Clean up products section code review feedback#50rezwana-karim merged 4 commits intosusmoyproducterpfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
… types, improve accessibility Co-authored-by: rafiqul4 <124497017+rafiqul4@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses code review feedback from PR #49 by cleaning up unused state, exporting component prop types, and enhancing accessibility for the Dialog component in the products section.
Changes:
- Removed unused
selectedProductstate and its setter from products-section.tsx - Added comprehensive type exports for all Dialog component props (DialogOverlayProps, DialogContentProps, etc.)
- Enhanced Dialog accessibility by linking aria-describedby to a description section
- Updated StormERP product URL to point to the dashboard route
- Package-lock.json peer dependency flags automatically removed by npm
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/ui/dialog.tsx | Adds type exports for all Dialog component props to follow strict TypeScript guidelines |
| src/components/home/products-section.tsx | Removes unused state, updates product URL, and adds aria-describedby for accessibility |
| package-lock.json | Automatic npm dependency resolution updates (peer flags removed) |
| <DialogContent className="max-w-5xl max-h-[90vh] overflow-y-auto"> | ||
| <DialogContent | ||
| className="max-w-5xl max-h-[90vh] overflow-y-auto" | ||
| aria-describedby="product-description" |
There was a problem hiding this comment.
The aria-describedby attribute references a hardcoded ID "product-description", but this ID is duplicated for each product in the map loop (line 224). HTML IDs must be unique within the document. Consider using a dynamic ID such as product-description-${product.id} for both the aria-describedby attribute and the corresponding id on line 224.
|
|
||
| {/* Description */} | ||
| <div className="space-y-3"> | ||
| <div className="space-y-3" id="product-description"> |
There was a problem hiding this comment.
This ID "product-description" is duplicated for each product in the map loop. It needs to be unique by incorporating the product identifier, such as product-description-${product.id}, to match the aria-describedby reference on line 194.
| <div className="space-y-3" id="product-description"> | |
| <div className="space-y-3" id={`product-description-${product.id}`}> |
Addresses code review comments from PR #49 regarding unused state, missing type exports, and accessibility improvements.
Changes
selectedProductstate - State was declared but never consumed by Dialog componentDialogOverlayProps,DialogContentProps, etc.) per coding guidelinesaria-describedbyto description section ID for screen reader context💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.