Add Storybook baseline for UI primitives#807
Conversation
Install storybook and @storybook/vue3-vite (10.3.x for Vite 8 compatibility). Add storybook and storybook:build npm scripts.
Configure .storybook/main.ts with vue3-vite framework, vue-component-meta docgen, and a viteFinal hook that strips the PWA plugin to prevent workbox precache errors during Storybook builds. Configure .storybook/preview.ts to import design tokens and set dark obsidian background as default to match the app theme.
Cover all variants (primary, secondary, ghost, danger), all sizes (sm, md, lg), disabled, loading states, and combined galleries.
…ldWrapper) Cover default, value, error, disabled, readonly states and a composed form example using TdFieldWrapper with TdInput.
…TdTooltip) Cover interactive open/close, positional variants, alignment options, and backdrop behavior.
…r, TdSkeleton) Cover all semantic variants (info, success, warning, error), sizes, dismissible state, and a composed skeleton card layout.
Cover all badge variants and sizes, tag colors and removable state, and empty state with icon/action slots.
The previous approach stripped all array-type Vite plugins. Now it inspects array members for PWA-related names before deciding to drop.
Adversarial Self-ReviewCRITICAL findingsNone. HIGH findings
MEDIUM findings
LOW findings
Verification results
|
Self-Review Fix SummaryHIGH #1 fixed in commit 7d8b397: All checks re-verified after the fix:
|
There was a problem hiding this comment.
Pull request overview
Adds a Storybook baseline to document and visually validate Taskdeck’s Vue 3 UI primitives, including configuration, scripts, and initial stories for each primitive.
Changes:
- Added Storybook (Vue 3 + Vite) configuration under
.storybook/and wired design tokens into the preview. - Added 17 CSF stories under
src/stories/covering state/variant examples for eachTd*primitive. - Updated frontend workspace scripts/deps to run and build Storybook; ignored
storybook-staticoutput.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/taskdeck-web/.storybook/main.ts | Adds Storybook config and custom Vite plugin stripping logic. |
| frontend/taskdeck-web/.storybook/preview.ts | Sets global preview parameters + decorator and imports design tokens. |
| frontend/taskdeck-web/src/stories/TdBadge.stories.ts | Badge variant/size stories. |
| frontend/taskdeck-web/src/stories/TdButton.stories.ts | Button variant/size/disabled/loading stories. |
| frontend/taskdeck-web/src/stories/TdDialog.stories.ts | Dialog open/variant examples. |
| frontend/taskdeck-web/src/stories/TdDropdown.stories.ts | Dropdown open/alignment examples. |
| frontend/taskdeck-web/src/stories/TdEmptyState.stories.ts | Empty state slot + description/action examples. |
| frontend/taskdeck-web/src/stories/TdFieldWrapper.stories.ts | Field wrapper composed examples with input. |
| frontend/taskdeck-web/src/stories/TdIconButton.stories.ts | Icon button variant/size/loading/disabled examples. |
| frontend/taskdeck-web/src/stories/TdInlineAlert.stories.ts | Inline alert variants + dismissible examples. |
| frontend/taskdeck-web/src/stories/TdInput.stories.ts | Input types + error/disabled/readonly examples. |
| frontend/taskdeck-web/src/stories/TdPopover.stories.ts | Popover open/alignment/position examples. |
| frontend/taskdeck-web/src/stories/TdSelect.stories.ts | Select placeholder/disabled/error/selection examples. |
| frontend/taskdeck-web/src/stories/TdSkeleton.stories.ts | Skeleton shapes + composed layout example. |
| frontend/taskdeck-web/src/stories/TdSpinner.stories.ts | Spinner sizes + label examples. |
| frontend/taskdeck-web/src/stories/TdTag.stories.ts | Tag custom color + removable examples. |
| frontend/taskdeck-web/src/stories/TdTextarea.stories.ts | Textarea rows + error/disabled/readonly examples. |
| frontend/taskdeck-web/src/stories/TdToast.stories.ts | Toast variants + dismissibility examples. |
| frontend/taskdeck-web/src/stories/TdTooltip.stories.ts | Tooltip positional + delay examples. |
| frontend/taskdeck-web/package.json | Adds Storybook scripts and dev dependencies. |
| frontend/taskdeck-web/package-lock.json | Locks Storybook dependency graph. |
| frontend/taskdeck-web/.gitignore | Ignores built Storybook output directory. |
Files not reviewed (1)
- frontend/taskdeck-web/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function isPwaPlugin(plugin: unknown): boolean { | ||
| if (plugin && typeof plugin === 'object' && 'name' in plugin) { | ||
| const name = (plugin as { name: string }).name | ||
| return name.includes('pwa') || name.includes('workbox') || name.includes('PWA') | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| function stripPwaPlugins(plugins: unknown[]): unknown[] { | ||
| return plugins.filter((plugin) => { | ||
| // Some Vite plugins (including VitePWA) return arrays of sub-plugins. | ||
| // Check each element to decide whether the whole array should be dropped. | ||
| if (Array.isArray(plugin)) { | ||
| return !plugin.some(isPwaPlugin) |
There was a problem hiding this comment.
stripPwaPlugins currently drops any plugin entries that are arrays (Array.isArray(plugin) => false). Vite plugins other than the PWA plugin can also be returned as arrays, so this risks unintentionally removing non-PWA functionality from the Storybook Vite config. Consider recursively filtering nested plugin arrays (or flattening config.plugins) and only removing plugins whose name matches the PWA/workbox patterns, while preserving other array-contained plugins.
| const meta = { | ||
| title: 'UI Primitives/TdPopover', | ||
| component: TdPopover, | ||
| tags: ['autodocs'], | ||
| argTypes: { | ||
| open: { control: 'boolean' }, | ||
| align: { | ||
| control: 'select', | ||
| options: ['left', 'right', 'center'], | ||
| }, | ||
| position: { | ||
| control: 'select', | ||
| options: ['top', 'bottom'], | ||
| }, | ||
| }, | ||
| args: { | ||
| open: false, | ||
| align: 'left', | ||
| position: 'bottom', | ||
| }, |
There was a problem hiding this comment.
This story sets argTypes/args (including open, align, position), but the story render implementations don't consume args (they use an internal isOpen ref and hard-coded props). As a result, Storybook Controls for these props won't work and the docs panel will be misleading. Either wire args into the render (e.g., bind open/align/position from args and update args.open on @close), or remove the unused controls from argTypes/args.
| const meta = { | ||
| title: 'UI Primitives/TdDropdown', | ||
| component: TdDropdown, | ||
| tags: ['autodocs'], | ||
| argTypes: { | ||
| open: { control: 'boolean' }, | ||
| align: { | ||
| control: 'select', | ||
| options: ['left', 'right'], | ||
| }, | ||
| }, | ||
| args: { | ||
| open: false, | ||
| align: 'left', | ||
| }, |
There was a problem hiding this comment.
This story defines argTypes/args for open and align, but the render blocks don't use args (they manage isOpen locally and hard-code align). This makes the Controls panel ineffective for these props. Suggest either binding args into the template (and syncing args.open on @close) or removing the unused argTypes entries.
| const meta = { | ||
| title: 'UI Primitives/TdDialog', | ||
| component: TdDialog, | ||
| tags: ['autodocs'], | ||
| argTypes: { | ||
| open: { control: 'boolean' }, | ||
| title: { control: 'text' }, | ||
| description: { control: 'text' }, | ||
| closeOnBackdrop: { control: 'boolean' }, | ||
| }, | ||
| args: { | ||
| open: false, | ||
| title: 'Confirm Action', | ||
| description: 'Are you sure you want to proceed?', | ||
| closeOnBackdrop: true, | ||
| }, |
There was a problem hiding this comment.
argTypes/args are defined for open, title, description, and closeOnBackdrop, but the stories render hard-coded values and a local isOpen ref instead of consuming args. This means Storybook Controls won't actually affect the rendered dialog and autodocs will be out of sync. Consider binding args into the template and updating args.open on @close, or remove the unused controls.
|
|
||
| export const ErrorBadge: Story = { | ||
| args: { variant: 'error' }, | ||
| render: (args) => ({ | ||
| components: { TdBadge }, | ||
| setup() { return { args } }, | ||
| template: '<TdBadge v-bind="args">Failed</TdBadge>', | ||
| }), |
There was a problem hiding this comment.
Story export name ErrorBadge is inconsistent with the other variant stories (Default, Primary, Success, Warning, Info). Renaming it to Error would keep the story list consistent and easier to scan.
frontend/taskdeck-web/package.json
Outdated
| "mutation:test": "npx stryker run", | ||
| "storybook": "storybook dev -p 6006", | ||
| "storybook:build": "storybook build -o storybook-static" |
There was a problem hiding this comment.
PR description/issue acceptance criteria mention documenting Storybook ownership/maintenance expectations in active docs, but this PR only adds configuration/scripts/stories and doesn't appear to update any documentation (no Storybook references found in workspace markdown). If docs are still required for #251, please add/update the relevant doc(s) and link from the README or existing quality gate docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7130d4403a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (Array.isArray(plugin)) { | ||
| return false // PWA returns an array; but be safe, check names |
There was a problem hiding this comment.
stripPwaPlugins currently drops every array entry in config.plugins, not just PWA-related plugins. Vite plugins commonly return Plugin[], so any future non-PWA array plugin (or an existing one from Storybook internals) would be silently removed and its transforms/hooks would stop running in Storybook builds. This makes the Storybook config brittle and can cause hard-to-diagnose build/runtime failures as soon as another array-based plugin is introduced.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates Storybook into the frontend project by adding the necessary configuration files, updating dependencies in package.json, and creating story files for various UI components. My review identified two improvements: defining a specific interface for Vite plugins to enhance type safety and updating the viteFinal configuration to treat the configuration object as immutable, preventing potential side effects.
| // Strip PWA plugin — it is app-specific and breaks the Storybook build | ||
| // because the Storybook output includes large JS bundles that exceed | ||
| // the workbox precache size limit. |
There was a problem hiding this comment.
The viteFinal configuration directly mutates the config.plugins array. It is safer to treat the configuration object as immutable and return a new object to avoid potential side effects in the Vite build pipeline.
| // Strip PWA plugin — it is app-specific and breaks the Storybook build | |
| // because the Storybook output includes large JS bundles that exceed | |
| // the workbox precache size limit. | |
| return { | |
| ...config, | |
| plugins: config.plugins ? stripPwaPlugins(config.plugins as unknown[]) : config.plugins, | |
| } |
Independent Adversarial Review (Round 2)Reviewed all 22 changed files (config, package.json, 17 stories) against their corresponding components and the broader codebase. MEDIUM FindingsM1. Missing ADR for Storybook version selection ( M2. Storybook preview imports only Importing M3. Variable shadowing in M4. Story files included in app's TypeScript compilation ( LOW FindingsL1. No CI guard for L2. Docker build will install Storybook devDependencies ( L3. Stories not co-located with components ( L4. Potential Storybook backgrounds API deprecation ( INFO FindingsI1. CI failure on I2. All 17 component stories have correct prop types I3. Summary
The PR is clean from a build-safety and correctness standpoint. None of the findings block merge. The most actionable items are M2 (style fidelity gap) and M4 (tsconfig isolation), which represent real maintenance risks that could bite later. Consider addressing them in follow-up work. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23a545ba67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export const NoBackdropClose: Story = { | ||
| render: (args) => ({ |
There was a problem hiding this comment.
NoBackdropClose inherits the meta default closeOnBackdrop: true, so backdrop clicks still dismiss the dialog and the story never demonstrates the non-dismissible behavior it is named for. This makes Storybook documentation misleading and weakens regression coverage for dialogs that must require explicit user acknowledgement.
Useful? React with 👍 / 👎.
| export const AlignRight: Story = { | ||
| render: (args) => ({ |
There was a problem hiding this comment.
AlignRight does not override align, so it keeps the meta default 'left' and renders the same alignment as the default story. This removes intended coverage for right-edge positioning and can hide regressions where right-aligned dropdown panels clip or anchor incorrectly.
Useful? React with 👍 / 👎.
| export const TopPosition: Story = { | ||
| render: (args) => ({ |
There was a problem hiding this comment.
TopPosition does not provide args.position = 'top', so it inherits the meta default 'bottom' and never exercises the top-placement path. That leaves the top-position variant effectively untested in Storybook and can mask placement/animation regressions for that mode.
Useful? React with 👍 / 👎.
| export const CenterAligned: Story = { | ||
| render: (args) => ({ |
There was a problem hiding this comment.
CenterAligned also omits an align override, so it inherits 'left' from meta and does not render centered positioning. This makes the named variant inaccurate and reduces regression detection for center alignment behavior near constrained container widths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab4bee6c3c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
frontend/taskdeck-web/package.json
Outdated
| "storybook:build": "storybook build -o storybook-static" | ||
| "test:visual": "playwright test --config playwright.visual.config.ts", |
There was a problem hiding this comment.
The scripts object is invalid JSON because "storybook:build" is not followed by a comma before "test:visual". With this syntax error, Node/npm cannot parse package.json, so any frontend npm command (npm install, npm run build, npm run storybook, etc.) fails immediately instead of running.
Useful? React with 👍 / 👎.
- package.json: Add missing comma after storybook:build and add storybook:docs script - main.ts: Fix stripPwaPlugins to preserve nested array structure instead of flattening - Stories: Add story-specific args to wire controls to render functions - TdPopover: Add position/align args for variant stories - TdDropdown: Add align arg for AlignRight story - TdDialog: Add title/description/closeOnBackdrop args for variant stories
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
Closes #251
npm run storybook(dev server on :6006) andnpm run storybook:buildscriptsComponents covered (17 total)
Storybook version note
The issue requested Storybook 8.x, but Storybook 8 only supports Vite up to v6. Since this project uses Vite 8, Storybook 10.3.x is required (first version with Vite 8 peer dependency support). The API surface and story format (CSF3 with
satisfies) are the same.Maintenance expectations
src/stories/npm run storybook:buildcan be added to CI to catch rendering regressionsTest plan
npm run typecheckpassesnpm run build(production build) passes -- no regressionnpm run lintpassesnpm run storybook:buildcompletes successfullynpm run storybooklaunches dev server and all stories render