feat(CheckrIntegration): redesign with design system components and theming#49
Conversation
There was a problem hiding this comment.
Pull request overview
This PR redesigns the CheckrIntegration component to use design system components and theming for dark mode support. The changes replace hardcoded colors with design system tokens, swap native HTML elements for design system components (Button, Input, Select, Modal), and add selection functionality with a status summary bar and footer actions.
Changes:
- Replace hardcoded colors with design system tokens (e.g.,
bg-emerald-600→bg-success,text-red-600→text-destructive) - Replace native HTML buttons with
Buttoncomponent and inputs withInput/Selectcomponents - Add report selection functionality with checkbox UI, status summary bar, and footer with Export/View Details buttons
- Replace FontAwesome icons with inline SVG icons
- Add comprehensive Storybook argTypes and remove deprecated CSVColumnMapper label properties
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/components/CheckrIntegration/CheckrIntegration.tsx | Redesigned component with design system tokens, replaced HTML elements with design system components, added selection functionality and status summary |
| src/components/CheckrIntegration/CheckrIntegration.stories.tsx | Added comprehensive argTypes for all props and callbacks, moved meta definition after sample data, updated event handlers |
| src/components/CSVColumnMapper/CSVColumnMapper.stories.tsx | Removed deprecated label properties (ignore, include, incomingSample, fieldType) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ntrols - Add Playground story that uses args directly so all controls work - Rename Default to InteractiveDemo (uses custom render with state) - Controls now properly work for: connected, loading, error, account, reports, packages - InteractiveDemo retains full interactive functionality for connect/disconnect flow
Deploying ui with
|
| Latest commit: |
a253a7c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c78c2481.ui-6d0.pages.dev |
| Branch Preview URL: | https://feature-check-integration-up.ui-6d0.pages.dev |
check-integration-updstes.mov |
- Fix Tailwind class order: bg-success/10 positioned after layout classes - Fix Tailwind class order: text-success positioned after sizing classes - Add aria-checked and role=checkbox to selection button for accessibility - Add required prop to Select component for form validation consistency
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Change selection button from circular to square for checkbox role consistency - Reorder handleViewSelected logic to prioritize onViewSelected over onViewReport fallback
- Select component does not support required prop in its type definition - Form validation is still handled by the submit handler logic
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…fix comment - Add useEffect to reset selectedReports when reports or connected changes - Fix comment from 'Selection Radio' to 'Selection Checkbox' to match ARIA role - Note: Select required prop suggestion ignored - component type doesn't support it
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div className="flex items-center gap-3"> | ||
| <div className="flex h-12 w-12 items-center justify-center rounded-lg bg-emerald-100"> | ||
| <i className="fas fa-user-shield text-xl text-emerald-600" /> | ||
| <div className="bg-success/10 flex h-12 w-12 items-center justify-center rounded-lg"> |
There was a problem hiding this comment.
The Tailwind class order is incorrect. The bg-success/10 class should appear after position/display classes. Reorder to flex h-12 w-12 items-center justify-center rounded-lg bg-success/10 for consistency with the project's class ordering convention.
| <div className="bg-success/10 flex h-12 w-12 items-center justify-center rounded-lg"> | |
| <div className="flex h-12 w-12 items-center justify-center rounded-lg bg-success/10"> |
| label={packageLabel} | ||
| options={packageOptions} | ||
| value={selectedPackage} | ||
| onValueChange={setSelectedPackage} |
There was a problem hiding this comment.
The Select component is missing a required attribute while the original native select had required. This removes form validation that was previously enforced, which could allow form submission without a package selection.
| onValueChange={setSelectedPackage} | |
| onValueChange={setSelectedPackage} | |
| required |
| </div> | ||
|
|
||
| {/* Footer */} | ||
| <div className="border-border bg-muted/30 flex items-center justify-between border-t px-4 py-3"> |
There was a problem hiding this comment.
The Tailwind class order is incorrect. The bg-muted/30 class should appear after position/display classes. Reorder to flex items-center justify-between border-t border-border bg-muted/30 px-4 py-3 for consistency with the project's class ordering convention.
| <div className="border-border bg-muted/30 flex items-center justify-between border-t px-4 py-3"> | |
| <div className="flex items-center justify-between border-t border-border bg-muted/30 px-4 py-3"> |
feat(CheckrIntegration): redesign with design system components and theming
check-integration-updstes.mov