Susmoyproducterp #49
Susmoyproducterp #49
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This pull request adds a products showcase section to the homepage, featuring two products (StormCom and StormERP) with detailed information displayed in modal dialogs.
Changes:
- New Dialog component using Radix UI primitives for accessible modals
- ProductsSection component with product cards and detailed modal views
- Integration of the products section into the homepage
- Dashboard images for the StormERP product
Reviewed changes
Copilot reviewed 3 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/components/ui/dialog.tsx |
New Dialog component wrapping Radix UI primitives with Tailwind styling |
src/components/home/products-section.tsx |
Products showcase with cards and modal details for StormCom and StormERP |
src/app/page.tsx |
Added ProductsSection to homepage between projects and team sections |
public/products/stormerp-dashboard.svg |
SVG dashboard mockup for StormERP product |
public/products/stormerp-dashboard.png |
PNG dashboard image for StormERP product |
| } | ||
|
|
||
| export default function ProductsSection({ className }: ProductsSectionProps) { | ||
| const [selectedProduct, setSelectedProduct] = useState<Product | null>(null) |
There was a problem hiding this comment.
The selectedProduct state is declared but never used in the component. This state is set when clicking "View Details" but serves no functional purpose since the Dialog component doesn't rely on it. Consider removing this unused state to keep the code clean.
| "24/7 Performance Monitoring" | ||
| ], | ||
| productUrl: "https://codestormhub.live", | ||
| dashboardImage: "/products/stormcom-dashboard.png", |
There was a problem hiding this comment.
The image path references /products/stormerp-dashboard.png but there's no corresponding image for the first product (StormCom) at /products/stormcom-dashboard.png. This will cause a 404 error and broken image for the StormCom product. Add the missing image file or update the path to use a placeholder.
| dashboardImage: "/products/stormcom-dashboard.png", | |
| dashboardImage: "/products/stormerp-dashboard.png", |
| export { | ||
| Dialog, | ||
| DialogPortal, | ||
| DialogOverlay, | ||
| DialogClose, | ||
| DialogTrigger, | ||
| DialogContent, | ||
| DialogHeader, | ||
| DialogFooter, | ||
| DialogTitle, | ||
| DialogDescription, | ||
| } |
There was a problem hiding this comment.
Component prop types should be exported alongside the component according to the coding guidelines. The Dialog component and its sub-components don't export their prop types, which reduces reusability and makes it harder for consumers to understand the API. Consider exporting the prop types or use type inference helpers.
| <DialogContent className="max-w-5xl max-h-[90vh] overflow-y-auto"> | ||
| <DialogHeader> | ||
| <div className="flex items-center gap-3"> | ||
| <div className="w-12 h-12 rounded-xl bg-primary/20 flex items-center justify-center"> | ||
| <RocketIcon className="h-6 w-6 text-primary" /> | ||
| </div> | ||
| <div> | ||
| <DialogTitle className="text-2xl">{product.name}</DialogTitle> | ||
| <DialogDescription className="text-primary font-medium"> | ||
| {product.tagline} | ||
| </DialogDescription> | ||
| </div> | ||
| </div> | ||
| </DialogHeader> |
There was a problem hiding this comment.
The Dialog is missing a proper accessible description. While DialogDescription is provided as a component, the modal content should include more context about what the dialog contains for screen reader users. Consider adding an accessible description that explains the purpose of the dialog.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
… types, improve accessibility Co-authored-by: rafiqul4 <124497017+rafiqul4@users.noreply.github.com>
No description provided.