docs(ui): add ButtonGroup stories to storybook#1964
docs(ui): add ButtonGroup stories to storybook#1964cylewaitforit wants to merge 6 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThis pull request refactors the Storybook configuration and Button component stories. It adds internationalization support via the storybook-i18n addon, creates a dedicated theme module for the npmx dark theme, and updates preview and manager configurations to use this theme. The Button component's story structure is reorganised, moving from a single Base.stories.ts file to separate ButtonBase.stories.ts and ButtonGroup.stories.ts files with enhanced documentation transforms. Both Button components receive explicit component name declarations via defineOptions. Dependencies are updated in package.json and pnpm-workspace.yaml, and unused dependency ignores are removed from knip.ts. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.storybook/preview.ts (1)
85-85: Avoid usinganytype for i18n instance.The
i18nInstanceis typed asany, which bypasses TypeScript's type checking. Consider using the appropriate type fromvue-i18nor at minimum a more descriptive interface.🛠️ Suggested improvement
- let i18nInstance: any = null + let i18nInstance: { setLocale: (locale: string) => void } | null = null
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b33c7bfb-2b5b-4119-9765-c992b32e7ac6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.storybook/main.ts.storybook/manager.ts.storybook/preview-head.html.storybook/preview.ts.storybook/theme.tsapp/components/Button/Base.stories.tsapp/components/Button/Base.vueapp/components/Button/Button.stories.tsapp/components/Button/ButtonGroup.stories.tsapp/components/Button/Group.vuepackage.jsonpnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- app/components/Button/Base.stories.ts
| <style> | ||
| /* Override docs story canvas background to match npmx theme */ | ||
| .docs-story { | ||
| background-color: var(--bg, oklch(0.171 0 0)) !important; | ||
| } | ||
| </style> |
There was a problem hiding this comment.
The docs canvas background never follows the selected theme.
Line 4 depends on --bg, but Storybook never sets that variable in the provided setup: .storybook/preview.ts:62-82 only updates --accent-color, and .storybook/theme.ts:1-12 sets appContentBg for the manager UI rather than a CSS custom property. As a result, this always falls back to the dark value, so Light theme docs still render on a dark canvas. Please either initialise --bg in Storybook or switch this rule to a selector that keys off data-theme directly.
There was a problem hiding this comment.
The backgrounds in the autodocs pages look correct with this when viewing the light versions of components and removing this makes them not display correctly. Would love to find a way to fix this in the storybook theme overrides instead of needing this preview-head file but at this time have been unsuccessful.
There was a problem hiding this comment.
@cylewaitforit, thanks for the context! This is actually a well-known Storybook limitation. The dark theme is applied correctly in the canvas view but not in the Docs tab — the DOM inside Docs has extra wrappers and inline styles that override the background and theme styles. This is a tracked open bug (#30928) in Storybook, so your preview-head.html workaround is reasonable and pragmatic.
For a cleaner alternative (when you want to revisit), the closest officially-documented approach is:
-
Use
parameters.docs.themewith anappContentBgvalue. Storybook Docs uses the same theme system as Storybook's UI but is themed independently from the main UI. The default theme for Docs is always the "light" theme, regardless of the main UI theme. You can pass a custom theme specifically to docs in.storybook/preview.ts:import { npmxDark } from './theme'; // in preview parameters: docs: { theme: npmxDark, // applies appContentBg to the docs page wrapper }
However, this styles the docs page background, not the individual
.docs-storycanvas iframes — which is whypreview-head.htmlis still needed for the story canvas itself. -
Acknowledge the
!importantrisk: Sincepreview-head.htmluses!importantto override.docs-story, it will prevent any future per-story background customisation. If you later need story-specific backgrounds, you'd need to revisit this rule.
Given the upstream bug, the current preview-head.html approach is a valid and widely-used workaround. I'm withdrawing my original concern — the fix is intentional and appropriate for the current state of Storybook.
(ﾐ^ ◡ ^ﾐ) ✦ noted for future reviews!
✏️ Learnings added
Learnt from: cylewaitforit
Repo: npmx-dev/npmx.dev PR: 1964
File: .storybook/preview-head.html:1-6
Timestamp: 2026-03-06T17:44:59.085Z
Learning: In the npmx.dev project, the `.storybook/preview-head.html` file sets `background-color` on `.docs-story` via the `--bg` CSS custom property to fix the Autodocs canvas background. This is an intentional workaround for a known Storybook bug (storybookjs/storybook#30928) where `addon-themes` and `parameters.docs.theme` do not apply to the individual story canvas iframes in the Autodocs tab. Removing this file causes the autodocs pages to not display correctly.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: alexdln
Repo: npmx-dev/npmx.dev PR: 1042
File: app/assets/main.css:120-129
Timestamp: 2026-02-06T11:09:31.529Z
Learning: In the npmx.dev project's app/assets/main.css, the `:root[data-theme='light'][data-bg-theme='black']` variant intentionally only overrides `--bg-subtle` and reuses the default light theme values for `--bg`, `--bg-muted`, and `--bg-elevated`. This is by design because the default light theme is mostly neutral, so selective override is sufficient for the black theme variant.
| '@storybook/[email protected]': | ||
| resolution: {integrity: sha512-FcOqPAXACP0I3oJ/ws6/rrPT9WGhu915Cg8D02a9YxLo0DE9zI+a9A5gRGvmQ09fiWPukqI8ZAEoQEdWUKMQdQ==} | ||
|
|
||
| '@storybook/[email protected]': |
There was a problem hiding this comment.
storybook-i18n introduces an older version of the @storybook/icons library. Opened stevensacks/storybook-i18n#29 so this can be addressed this in a future version of storybook-i18n.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c3e058ab-44b3-47cc-ab0d-2d13215a73a5
📒 Files selected for processing (1)
app/components/Button/ButtonBase.stories.ts
| export const Default: Story = { | ||
| args: { | ||
| default: 'Button Text', | ||
| }, | ||
| } |
There was a problem hiding this comment.
Default story may not render slot content without a render function.
All other stories use explicit render functions with templates to handle slot content, but Default relies on args.default without a render function. In Vue Storybook, this pattern typically requires either a render function or specific argTypes configuration to pass slot content properly.
Consider adding a render function for consistency and to ensure the button text displays:
Proposed fix
export const Default: Story = {
- args: {
- default: 'Button Text',
- },
+ render: args => ({
+ components: { ButtonBase },
+ setup() {
+ return { args }
+ },
+ template: `<ButtonBase v-bind="args">Button Text</ButtonBase>`,
+ }),
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const Default: Story = { | |
| args: { | |
| default: 'Button Text', | |
| }, | |
| } | |
| export const Default: Story = { | |
| render: args => ({ | |
| components: { ButtonBase }, | |
| setup() { | |
| return { args } | |
| }, | |
| template: `<ButtonBase v-bind="args">Button Text</ButtonBase>`, | |
| }), | |
| } |
There was a problem hiding this comment.
Keeping the default story with the default render allow to someone viewing the storybook to change the story to alternative text in place of Button Text. The other stories use the template render in order for the internationalization to work.
There was a problem hiding this comment.
@cylewaitforit, that makes perfect sense! Using args.default without a render function is the correct approach here — it exposes the slot content as a Storybook control, allowing users to interactively edit the button text directly from the controls panel. The render function approach in the other stories is needed for i18n to work, but would prevent that interactive editing for the Default story. Thanks for the clarification!
ʕ •`ᴥ´• ʔ
✏️ Learnings added
Learnt from: cylewaitforit
Repo: npmx-dev/npmx.dev PR: 1964
File: app/components/Button/ButtonBase.stories.ts:21-25
Timestamp: 2026-03-06T18:16:31.538Z
Learning: In the npmx.dev project's ButtonBase.stories.ts (app/components/Button/ButtonBase.stories.ts), the `Default` story intentionally uses `args.default` without a render function so that Storybook users can interactively edit the slot content (button text) via the controls panel. Other stories (e.g. Primary, Secondary, etc.) use explicit render functions with templates specifically to support storybook-i18n internationalization. Do not flag the `Default` story's lack of a render function as an issue.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: userquin
Repo: npmx-dev/npmx.dev PR: 1335
File: app/components/Compare/FacetSelector.vue:72-78
Timestamp: 2026-02-10T15:47:40.710Z
Learning: In the npmx.dev project, the `ButtonBase` component (app/components/ButtonBase.vue or similar) provides default `border border-border` styles. When styling ButtonBase instances, avoid adding duplicate `border` classes as this triggers the HTML validator error `no-dup-class` and fails CI.
Learnt from: cylewaitforit
Repo: npmx-dev/npmx.dev PR: 1964
File: .storybook/preview-head.html:1-6
Timestamp: 2026-03-06T17:44:59.085Z
Learning: In the npmx.dev project, the `.storybook/preview-head.html` file sets `background-color` on `.docs-story` via the `--bg` CSS custom property to fix the Autodocs canvas background. This is an intentional workaround for a known Storybook bug (storybookjs/storybook#30928) where `addon-themes` and `parameters.docs.theme` do not apply to the individual story canvas iframes in the Autodocs tab. Removing this file causes the autodocs pages to not display correctly.
1 participant
🔗 Linked issue
Part of #1841
🧭 Context
Adds stories for ButtonGroup
Preview npmx Storybook on Chromatic
📚 Description