-
-
Notifications
You must be signed in to change notification settings - Fork 328
docs(ui): add ButtonGroup stories to storybook #1964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3e4ec12
476316b
20dd233
ae7c4be
d4c4404
4c8e9d5
a923dd3
51ec2db
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,6 @@ | ||
| import { addons } from 'storybook/manager-api' | ||
| import { create } from 'storybook/theming' | ||
|
|
||
| const npmxTheme = create({ | ||
| brandTitle: 'npmx Storybook', | ||
| brandImage: '/npmx-storybook.svg', | ||
| }) | ||
| import npmxDark from './theme' | ||
|
|
||
| addons.setConfig({ | ||
| theme: npmxTheme, | ||
| theme: npmxDark, | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| <style> | ||
| /* Override docs story canvas background to match npmx theme */ | ||
| .docs-story { | ||
| background-color: var(--bg, oklch(0.171 0 0)) !important; | ||
| } | ||
| </style> | ||
|
Comment on lines
+1
to
+6
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The docs canvas background never follows the selected theme. Line 4 depends on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For a cleaner alternative (when you want to revisit), the closest officially-documented approach is:
Given the upstream bug, the current (ミ^ ◡ ^ミ) ✦ noted for future reviews! ✏️ Learnings added
🧠 Learnings used |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import { create } from 'storybook/theming/create' | ||
|
|
||
| const npmxDark = create({ | ||
| base: 'dark', | ||
|
|
||
| brandTitle: 'npmx Storybook', | ||
| brandImage: '/npmx-storybook.svg', | ||
|
|
||
| // UI | ||
| appContentBg: '#101010', // oklch(0.171 0 0) | ||
| }) | ||
|
|
||
| export default npmxDark |
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,116 @@ | ||||||||||||||||||||||||||||||
| import type { Meta, StoryObj } from '@storybook-vue/nuxt' | ||||||||||||||||||||||||||||||
| import ButtonBase from './Base.vue' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const meta = { | ||||||||||||||||||||||||||||||
| component: ButtonBase, | ||||||||||||||||||||||||||||||
| parameters: { | ||||||||||||||||||||||||||||||
| docs: { | ||||||||||||||||||||||||||||||
| source: { | ||||||||||||||||||||||||||||||
| type: 'dynamic', | ||||||||||||||||||||||||||||||
| transform: (code: string) => | ||||||||||||||||||||||||||||||
| code.replace(/<Base\b/g, '<ButtonBase').replace(/<\/Base>/g, '</ButtonBase>'), | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| tags: ['autodocs'], | ||||||||||||||||||||||||||||||
| } satisfies Meta<typeof ButtonBase> | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export default meta | ||||||||||||||||||||||||||||||
| type Story = StoryObj<typeof meta> | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export const Default: Story = { | ||||||||||||||||||||||||||||||
| args: { | ||||||||||||||||||||||||||||||
| default: 'Button Text', | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+25
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 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
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keeping the default story with the default render allow to someone viewing the storybook to change the story to alternative text in place of
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
ʕ •`ᴥ´• ʔ ✏️ Learnings added
🧠 Learnings used |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export const Primary: Story = { | ||||||||||||||||||||||||||||||
| args: { | ||||||||||||||||||||||||||||||
| variant: 'primary', | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| render: args => ({ | ||||||||||||||||||||||||||||||
| components: { ButtonBase }, | ||||||||||||||||||||||||||||||
| setup() { | ||||||||||||||||||||||||||||||
| return { args } | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| template: `<ButtonBase v-bind="args">{{ $t("nav.settings") }}</ButtonBase>`, | ||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export const Secondary: Story = { | ||||||||||||||||||||||||||||||
| args: { | ||||||||||||||||||||||||||||||
| variant: 'secondary', | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| render: args => ({ | ||||||||||||||||||||||||||||||
| components: { ButtonBase }, | ||||||||||||||||||||||||||||||
| setup() { | ||||||||||||||||||||||||||||||
| return { args } | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| template: `<ButtonBase v-bind="args">{{ $t("nav.settings") }}</ButtonBase>`, | ||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export const Small: Story = { | ||||||||||||||||||||||||||||||
| args: { | ||||||||||||||||||||||||||||||
| size: 'small', | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| render: args => ({ | ||||||||||||||||||||||||||||||
| components: { ButtonBase }, | ||||||||||||||||||||||||||||||
| setup() { | ||||||||||||||||||||||||||||||
| return { args } | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| template: `<ButtonBase v-bind="args">{{ $t("nav.settings") }}</ButtonBase>`, | ||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export const Disabled: Story = { | ||||||||||||||||||||||||||||||
| args: { | ||||||||||||||||||||||||||||||
| disabled: true, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| render: args => ({ | ||||||||||||||||||||||||||||||
| components: { ButtonBase }, | ||||||||||||||||||||||||||||||
| setup() { | ||||||||||||||||||||||||||||||
| return { args } | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| template: `<ButtonBase v-bind="args">{{ $t("nav.settings") }}</ButtonBase>`, | ||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export const WithIcon: Story = { | ||||||||||||||||||||||||||||||
| args: { | ||||||||||||||||||||||||||||||
| classicon: 'i-lucide:search', | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| render: args => ({ | ||||||||||||||||||||||||||||||
| components: { ButtonBase }, | ||||||||||||||||||||||||||||||
| setup() { | ||||||||||||||||||||||||||||||
| return { args } | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| template: `<ButtonBase v-bind="args">{{ $t("search.button") }}</ButtonBase>`, | ||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export const WithKeyboardShortcut: Story = { | ||||||||||||||||||||||||||||||
| args: { | ||||||||||||||||||||||||||||||
| ariaKeyshortcuts: '/', | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| render: args => ({ | ||||||||||||||||||||||||||||||
| components: { ButtonBase }, | ||||||||||||||||||||||||||||||
| setup() { | ||||||||||||||||||||||||||||||
| return { args } | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| template: `<ButtonBase v-bind="args">{{ $t("search.button") }}</ButtonBase>`, | ||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export const Block: Story = { | ||||||||||||||||||||||||||||||
| args: { | ||||||||||||||||||||||||||||||
| block: true, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| render: args => ({ | ||||||||||||||||||||||||||||||
| components: { ButtonBase }, | ||||||||||||||||||||||||||||||
| setup() { | ||||||||||||||||||||||||||||||
| return { args } | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| template: `<ButtonBase v-bind="args">{{ $t("nav.settings") }}</ButtonBase>`, | ||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm very curious to hear why this is no longer necessary? 🤔 @cylewaitforit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why it's not needed I am not sure, while troubleshooting getting the autodocs to generate from the jsdoc comments for ButtonBase and ButtonGroup I removed it. Builds still worked.
Since it said it was needed to get builds to not fail, I didn't feel like it needed to be added it back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it more, it's probably because these two components don't import their types from other .vue files.
Maybe that is okay, since I believe @danielroe added that as a known limitation.
npmx.dev/CONTRIBUTING.md
Line 997 in 3de681d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I am also not married to keeping this removal in this PR. I'm all for deleting code but if it feels out of scope here happy to discuss in a smaller scoped PR.)