fix(ErrorPage): Fix Storybook controls and button theming#58
Conversation
Stories fixes: - Convert render function stories to args-based stories for controls - Add Default story at top with full control support - Add argTypes with descriptions for type, size, code, title, description, showHomeButton, showBackButton, homeHref - Add default args for consistent baseline state - Keep render stories for special components (ServerErrorWithDetails, OfflineWithRetry, MaintenanceWithStatus) - Add new stories: AccessDenied, Unauthorized, LargeSize Component fixes: - Replace custom ActionButton with design system Button component - Import Button from '../Button/Button' - Use Button's built-in variant props (primary/secondary) - Fixes dark mode text contrast issue on buttons - Button now uses proper theming tokens for both light and dark mode
There was a problem hiding this comment.
Pull request overview
This PR improves Storybook story controls for the ErrorPage component and attempts to replace custom ActionButton styling with the design system Button component to fix dark mode text contrast issues.
Changes:
- Convert ErrorPage stories from render functions to args-based stories with comprehensive controls
- Add new stories: Default, AccessDenied, Unauthorized, and LargeSize
- Replace custom ActionButton implementation with design system Button component
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/components/ErrorPage/ErrorPage.tsx | Refactored ActionButton to use design system Button component with variant and size props |
| src/components/ErrorPage/ErrorPage.stories.tsx | Converted stories to args-based format, added argTypes with descriptions, added new story variants, removed unused imports |
Comments suppressed due to low confidence (1)
src/components/ErrorPage/ErrorPage.stories.tsx:7
- The React import was removed but the file still uses JSX in render functions (ServerErrorWithDetails, OfflineWithRetry, MaintenanceWithStatus stories). While modern React with automatic JSX runtime may not strictly require this import, the codebase convention appears to be importing React explicitly in story files that use JSX. For example, src/components/AI/AI.stories.tsx imports React at line 2. This should be retained for consistency with the codebase conventions.
import type { Meta, StoryObj } from '@storybook/react';
import {
ErrorPage,
ServerErrorPage,
OfflinePage,
MaintenancePage,
} from './ErrorPage';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <Button | ||
| as="a" | ||
| href={href} | ||
| className={cn(baseClasses, variantClasses, 'inline-block text-center')} | ||
| variant={variant} | ||
| size="md" | ||
| className="min-w-[140px]" | ||
| > | ||
| {label} | ||
| </a> | ||
| </Button> |
There was a problem hiding this comment.
The Button component does not support the as prop or href attribute. The Button component (imported from '../Button/Button') only renders as a button element and does not have polymorphic rendering capabilities. This will cause a TypeScript error and runtime issues.
To fix this, you have a few options:
- Keep the existing implementation with a native anchor tag styled to match the Button component
- Extend the Button component to support polymorphic rendering with an
asprop - Create a wrapper component that conditionally renders either a Button or a styled anchor tag
The original implementation with the anchor tag and manual styling was functionally correct for this use case.
- Button component doesn't support 'as' prop for polymorphism - Use buttonVariants directly on anchor tag for link buttons - Import buttonVariants alongside Button from Button component - Maintains consistent styling between button and link variants
Deploying ui with
|
| Latest commit: |
dee0303
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1883744a.ui-6d0.pages.dev |
| Branch Preview URL: | https://feature-error-page-update.ui-6d0.pages.dev |
|
Fixed the TypeScript error in the follow-up commit. The Button component doesn't support an ✅ |
Stories fixes:
Component fixes:
error-page.mov