feat(ProviderSelector): add Storybook controls and theme variable support#71
Conversation
…port - Add args and argTypes to meta config for interactive Docs controls - Configure controls for label, placeholder, searchPlaceholder, disabled, searchable, isLoading, size, and className props - Disable controls for complex props (selectedProvider, providers, onSelect) - Replace all 22 hardcoded blue-* colors with theme CSS variables - Remove all dark: variant prefixes (theme variables handle dark mode) - Refine typography in trigger: add leading-tight, use text-sm and text-[11px] Tested in both light and dark modes.
There was a problem hiding this comment.
Pull request overview
This PR enhances the ProviderSelector component with Storybook controls and migrates it to use theme CSS variables for better dark mode support. It also adds similar Storybook controls to the PermissionsEditor component.
Changes:
- Added interactive Storybook args and argTypes to ProviderSelector and PermissionsEditor for better documentation and testing
- Replaced hardcoded color classes with semantic theme CSS variables (text-foreground, text-muted-foreground, bg-background, bg-muted, border-input, etc.)
- Refined typography in the trigger button with leading-tight, text-sm, and text-[11px] for improved visual hierarchy
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/ProviderSelector/ProviderSelector.tsx | Migrated from hardcoded color classes to theme CSS variables; refined typography with tighter leading and smaller text sizes |
| src/components/ProviderSelector/ProviderSelector.stories.tsx | Added args/argTypes for interactive controls in Storybook; moved Default story to use args-driven approach; updated demo styling to use theme variables |
| src/components/PermissionsEditor/PermissionsEditor.stories.tsx | Added argTypes configuration and updated Default story to use args-driven approach for better Storybook controls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /> | ||
| ) : ( | ||
| <div className="flex h-6 w-6 items-center justify-center rounded bg-blue-100 text-xs font-medium text-blue-600 dark:bg-blue-900 dark:text-blue-300"> | ||
| <div className="bg-primary-100 text-primary-700 dark:bg-primary-900 dark:text-primary-300 flex h-6 w-6 items-center justify-center rounded text-xs font-medium"> |
There was a problem hiding this comment.
The PR description states "Remove all dark: variant prefixes (theme variables handle dark mode)", but this line still uses dark: prefixes for primary color classes. This creates a discrepancy between the stated goal and the implementation.
The pattern bg-primary-100 text-primary-700 dark:bg-primary-900 dark:text-primary-300 is actually consistent with the existing codebase (e.g., ProviderCard.tsx line 100-101), suggesting the dark: prefixes are still necessary for primary color variants. Either the PR description should be updated to clarify that dark: prefixes were only removed for semantic colors (background, foreground, muted, etc.), or the dark: prefixes should indeed be removed here if the theme system fully handles dark mode for primary colors.
| 'hover:bg-muted', | ||
| selectedProvider?.id === provider.id && | ||
| 'bg-blue-50 dark:bg-blue-900/30' | ||
| 'bg-primary-50 dark:bg-primary-900/30' |
There was a problem hiding this comment.
Similar to line 189, the PR description states "Remove all dark: variant prefixes (theme variables handle dark mode)", but this line still uses a dark: prefix for the primary color. The pattern bg-primary-50 dark:bg-primary-900/30 is consistent with the existing codebase patterns. Either the PR description should be clarified to indicate that dark: prefixes were only removed for semantic colors, or these dark: prefixes should be removed if the theme system is intended to handle dark mode automatically for primary colors.
…view Address PR feedback from Copilot: - Replace bg-primary-100/700 dark:bg-primary-900/300 with bg-primary/10 text-primary - Replace bg-primary-50 dark:bg-primary-900/30 with bg-primary/10 These theme-aware opacity classes work in both light and dark mode.
Deploying ui with
|
| Latest commit: |
cade60e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c7834094.ui-6d0.pages.dev |
| Branch Preview URL: | https://feature-provider-selector-up.ui-6d0.pages.dev |
Tested in both light and dark modes.
provider-selector.mov