-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat(storybook): add fumadocs ui components #211
feat(storybook): add fumadocs ui components #211
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces enhanced Storybook configurations for multiple components from the Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (5)
libs/docs/components/ui/callout.stories.tsx (3)
11-17
: Consider expanding argTypes configurationThe current configuration only includes the 'type' control. Consider adding other important props like 'title' and children to provide a more complete Storybook documentation.
argTypes: { type: { options: - ['info', 'warn', 'error'], + ['info', 'warn', 'error'], control: { type: 'select' }, }, + title: { + control: 'text', + description: 'The title of the callout', + }, + children: { + control: 'text', + description: 'The content of the callout', + }, },
24-32
: Add story description and comprehensive argsThe Default story could benefit from additional documentation and props demonstration.
export const Default: Story = { + parameters: { + docs: { + description: { + story: 'Basic usage of the Callout component with a title only.', + }, + }, + }, args: { - title: 'Title' + title: 'Information', + type: 'info', }, render: args => (
34-45
: Enhance story with realistic contentThe WithDescription story uses placeholder text. Consider using more realistic content to better demonstrate the component's usage.
export const WithDescription: Story = { + parameters: { + docs: { + description: { + story: 'Callout component with both title and descriptive content.', + }, + }, + }, args: { - title: 'Title' + title: 'Important Note', + type: 'warn', }, render: args => ( <div> <Callout {...args}> - children + This is an important message that provides additional context or warnings + to the user. It can contain multiple lines of text and even markdown content. </Callout> </div> ),libs/docs/components/ui/card.stories.tsx (2)
21-29
: Document story and avoid hardcoded URLsThe story uses a hardcoded documentation URL and lacks description. Consider making the URL configurable and adding proper documentation.
export const Default: Story = { + parameters: { + docs: { + description: { + story: 'Basic card component with title, description, and link.', + }, + }, + }, args: { title: 'Title', description: 'Description', - href: 'https://fumadocs.vercel.app/docs/ui/mdx/card#card' + href: '/docs/example' },
31-42
: Enhance icon demonstrationConsider adding more icon variations and documenting icon usage guidelines.
export const WithIcon: Story = { + parameters: { + docs: { + description: { + story: 'Card component with an icon. Icons should be 24x24px and preferably from the lucide-react library.', + }, + }, + }, args: { title: 'Icon', - description: 'You can specify an icon to cards', + description: 'Cards can include icons from lucide-react or other icon libraries', - href: 'https://fumadocs.vercel.app/docs/ui/mdx/card#card' + href: '/docs/example' },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/docs/components/ui/callout.stories.tsx
(1 hunks)libs/docs/components/ui/card.stories.tsx
(1 hunks)
argTypes: { | ||
|
||
}, |
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.
🛠️ Refactor suggestion
Add comprehensive argTypes configuration
The argTypes configuration is empty. Consider documenting all available props for better component documentation.
argTypes: {
-
+ title: {
+ control: 'text',
+ description: 'The title of the card',
+ },
+ description: {
+ control: 'text',
+ description: 'The description text for the card',
+ },
+ href: {
+ control: 'text',
+ description: 'The destination URL when the card is clicked',
+ },
+ icon: {
+ control: 'object',
+ description: 'Optional icon component to display',
+ },
},
📝 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.
argTypes: { | |
}, | |
argTypes: { | |
title: { | |
control: 'text', | |
description: 'The title of the card', | |
}, | |
description: { | |
control: 'text', | |
description: 'The description text for the card', | |
}, | |
href: { | |
control: 'text', | |
description: 'The destination URL when the card is clicked', | |
}, | |
icon: { | |
control: 'object', | |
description: 'Optional icon component to display', | |
}, | |
}, |
2596933
to
0a328cc
Compare
0a328cc
to
82e0605
Compare
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.
Actionable comments posted: 2
🧹 Nitpick comments (5)
libs/docs/components/ui/card.stories.tsx (3)
5-29
: Consider enhancing the component metadataThe metadata configuration is good, but could be improved:
- Add a
description
parameter to provide more context about the Card component- Consider adding
defaultValue
for optional props in argTypesconst meta: Meta<typeof Card> = { title: '📚 Docs Site/Card', parameters: { layout: 'centered', + description: 'A clickable card component that displays a title, description, and optional icon', }, component: Card, tags: ['autodocs'], argTypes: { icon: { description: 'Optional icon component to display', + defaultValue: undefined, }, // ... other props }, }
41-45
: Consider removing unnecessary div wrapperThe
div
wrapper in the render function doesn't seem to serve any purpose. Unless specific styling or layout is needed, it can be removed for cleaner code.render: args => ( - <div> <Card {...args} /> - </div> ),
54-61
: Duplicate code in render functionsThe render functions in
Icon
andWithoutHref
stories contain identical wrapper code. Consider extracting a shared render function.+const renderCard = (args: typeof Card) => <Card {...args} /> + export const Icon: Story = { args: { // ... }, - render: args => ( - <div> - <Card - {...args} - icon={<HomeIcon />} - /> - </div> - ), + render: args => renderCard({ ...args, icon: <HomeIcon /> }), }Also applies to: 69-76
libs/docs/components/ui/callout.stories.tsx (2)
11-25
: Enhance argTypes configurationThe argTypes configuration could be improved by:
- Adding default values
- Making the type property required
- Adding examples for better documentation
argTypes: { type: { options: ['info', 'warn', 'error'], control: { type: 'select' }, description: 'The type of callout (info, warn, error)', + required: true, + defaultValue: 'info', }, title: { control: 'text', description: 'The title of the callout', + defaultValue: '', + table: { + type: { summary: 'string' }, + defaultValue: { summary: '' }, + }, }, children: { control: 'text', description: 'The content inside the callout', + required: true, }, },
32-43
: Reduce code duplication in story render functionsThe render function is duplicated across all stories. Consider extracting it to a shared function.
+const renderCallout = (args: typeof Callout) => ( + <div> + <Callout {...args}>{args.children}</Callout> + </div> +) + export const Default: Story = { args: { title: 'Default Callout', type: 'info', children: 'This is a default callout. You can edit this text.', }, - render: args => ( - <div> - <Callout {...args}>{args.children}</Callout> - </div> - ), + render: renderCallout, }Also applies to: 45-56, 58-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/docs/tailwind.config.js
(1 hunks)libs/docs/components/ui/callout.stories.tsx
(1 hunks)libs/docs/components/ui/card.stories.tsx
(1 hunks)tailwind.config.ts
(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
libs/docs/components/ui/callout.stories.tsx
[error] 71-71: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (2)
tailwind.config.ts (2)
82-86
: LGTM! Improved readability of transform properties
The formatting changes make the keyframe transform properties more readable while maintaining the same functionality.
6-6
: Consider optimizing the content scanning pattern
The current pattern scans all JS files in the Fumadocs UI dist folder, which might impact build performance. Consider using a more specific pattern targeting only the component files you need.
✅ Verification successful
The Tailwind configuration is correct for Fumadocs UI integration
The configuration is appropriate as:
- Fumadocs UI is correctly listed as a dependency (v14.4.2)
- The path
./node_modules/fumadocs-ui/dist/**/*.js
is the recommended pattern for Tailwind to scan Fumadocs UI components - The package not being present in node_modules is expected in this verification environment, as build artifacts are not available in the sandbox
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the actual files being included by the pattern
echo "Files matched by the current pattern:"
find ./node_modules/fumadocs-ui/dist -name "*.js" -type f
# Check if source files are available as an alternative
echo -e "\nChecking for source files:"
find ./node_modules/fumadocs-ui/src -name "*.tsx" -type f 2>/dev/null
Length of output: 340
Script:
#!/bin/bash
# Let's verify if the package is actually installed and check its structure
echo "Checking if fumadocs-ui is installed:"
ls -la ./node_modules/fumadocs-ui 2>/dev/null || echo "Package not found in node_modules"
echo -e "\nChecking package.json for fumadocs-ui dependency:"
cat package.json | jq '.dependencies["fumadocs-ui"] // .devDependencies["fumadocs-ui"] // "Not found"'
echo -e "\nChecking for any fumadocs-related files in the project:"
fd fumadocs
Length of output: 568
export const Error: Story = { | ||
args: { | ||
title: 'Error Callout', | ||
type: 'error', | ||
children: 'This is an error callout. Action may be required.', | ||
}, | ||
render: args => ( | ||
<div> | ||
<Callout {...args}>{args.children}</Callout> | ||
</div> | ||
), | ||
} |
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.
Rename 'Error' story to avoid shadowing built-in Error object
The static analysis tool correctly flags that naming a story 'Error' shadows the global Error object. Consider a more descriptive name.
-export const Error: Story = {
+export const ErrorCallout: Story = {
args: {
title: 'Error Callout',
type: 'error',
children: 'This is an error callout. Action may be required.',
},
render: args => (
<div>
<Callout {...args}>{args.children}</Callout>
</div>
),
}
📝 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 Error: Story = { | |
args: { | |
title: 'Error Callout', | |
type: 'error', | |
children: 'This is an error callout. Action may be required.', | |
}, | |
render: args => ( | |
<div> | |
<Callout {...args}>{args.children}</Callout> | |
</div> | |
), | |
} | |
export const ErrorCallout: Story = { | |
args: { | |
title: 'Error Callout', | |
type: 'error', | |
children: 'This is an error callout. Action may be required.', | |
}, | |
render: args => ( | |
<div> | |
<Callout {...args}>{args.children}</Callout> | |
</div> | |
), | |
} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 71-71: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
@@ -7,6 +7,7 @@ import TailwindAnimate from 'tailwindcss-animate' | |||
export default { | |||
content: [ | |||
'../../libs/docs/components/**/*.{ts,tsx}', | |||
'../../libs/docs/components/**/*.stories.{ts,tsx}', |
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.
💡 Codebase verification
The path pattern in tailwind.config.js needs to be corrected
The current path '../../libs/docs/components/**/*.stories.{ts,tsx}'
is correct in its glob pattern but incorrect in its base path. The verification shows that the story files exist at libs/docs/components/ui/*.stories.tsx
, so the path should be relative to the apps/docs
directory without going up two levels.
The path should be changed to: '../libs/docs/components/**/*.stories.{ts,tsx}'
🔗 Analysis chain
Verify the stories path pattern
The added path pattern looks correct but verify that it captures all story files correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the path pattern captures all story files
# and doesn't miss any stories in subdirectories
# List all story files in the components directory
fd -e ts -e tsx . "../../libs/docs/components" | grep "stories\."
# Verify the pattern matches all found files
for file in $(fd -e ts -e tsx . "../../libs/docs/components" | grep "stories\."); do
echo "Checking $file..."
if [[ ! $file =~ \.stories\.(ts|tsx)$ ]]; then
echo "Warning: File $file might not be captured by the pattern"
fi
done
Length of output: 361
Script:
#!/bin/bash
# Let's first check if the libs/docs/components directory exists and list all story files
fd -e ts -e tsx . libs/docs/components | grep "stories\."
# Also check the apps/docs directory structure to understand the relative path
ls -la apps/docs/
# Let's also check if there are any story files in the entire repository
fd -e ts -e tsx . | grep "stories\."
Length of output: 3188
82e0605
to
b387e6b
Compare
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.
Actionable comments posted: 4
🧹 Nitpick comments (8)
libs/docs/components/ui/type-table.stories.tsx (2)
11-24
: Consider extracting shared type definition to avoid duplicationThe type definition is duplicated between
argTypes
and the story'sargs
. Consider extracting it to a constant to maintain DRY principles.+const TYPE_DEFINITION = { + percentage: { + description: 'The percentage of scroll position to display the roll button', + type: 'number', + default: 0.2, + }, +} argTypes: { type: { control: 'object', description: 'The object describing the types and their properties', - defaultValue: { - percentage: { - description: 'The percentage of scroll position to display the roll button', - type: 'number', - default: 0.2, - }, - }, + defaultValue: TYPE_DEFINITION, }, },
1-41
: Clarify component purpose in documentationThe component is named
TypeTable
but the only property described relates to a roll button's scroll position. Consider:
- Adding a clear description in the component's metadata about its primary purpose
- Including examples of how it's used to display types
libs/docs/components/ui/callout.stories.tsx (1)
38-42
: Consider removing unnecessary wrapper divEach story's render function wraps the Callout in a div without apparent styling or layout purpose. If the wrapper isn't needed for styling or layout, consider removing it for cleaner code.
render: args => ( - <div> <Callout {...args}>{args.children}</Callout> - </div> ),Also applies to: 51-55, 64-68
libs/docs/components/ui/accordion.stories.tsx (2)
41-43
: Review props spreading order to prevent overridesThe props are spread before specific props, which could lead to unintended overrides. Consider spreading props last to ensure specific props take precedence.
-<Accordion {...args} title="What do we love?"> +<Accordion title="What do we love?" {...args}>
16-19
: Enhance asChild prop documentationThe description for asChild prop could be more detailed to explain its purpose and usage patterns.
Consider expanding the description to explain:
- When to use this prop
- What child elements are supported
- Common use cases
libs/docs/components/ui/tabs.stories.tsx (2)
4-32
: LGTM! Consider enhancing prop documentation.The metadata configuration is well-structured with appropriate controls and descriptions. However, consider adding
@example
usage snippets in the descriptions to make it even more helpful for developers.Example enhancement for the
groupId
description:groupId: { control: 'text', - description: 'The group id for persistent tabs (used as localStorage key)', + description: 'The group id for persistent tabs (used as localStorage key). @example "language-tabs"', },
38-50
: Consider enhancing demo content for better documentation.While the implementation is correct, the tab content could be more meaningful. Consider adding code snippets or realistic examples in each tab to better demonstrate the component's intended use case.
Example enhancement:
<Tabs items={args.items} defaultIndex={args.defaultIndex}> - <Tab value="Javascript">Hello World in Javascript</Tab> + <Tab value="Javascript"> + {`console.log("Hello World!");`} + </Tab> - <Tab value="Rust">Rust is fast</Tab> + <Tab value="Rust"> + {`fn main() { + println!("Hello World!"); +}`} + </Tab>libs/docs/components/ui/folder.stories.tsx (1)
68-82
: Replace emoji icons with proper icon components.While emojis work for demonstration, they're not ideal for production use. Consider using proper SVG icons or an icon library to demonstrate real-world usage.
Example using a hypothetical icon library:
- <Folder name="app" defaultOpen icon={<span>📁</span>}> + <Folder name="app" defaultOpen icon={<FolderIcon className="w-4 h-4" />}> - <File name="layout.tsx" icon={<span>📄</span>} /> + <File name="layout.tsx" icon={<FileIcon className="w-4 h-4" />} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/docs/tailwind.config.js
(1 hunks)libs/docs/components/ui/accordion.stories.tsx
(1 hunks)libs/docs/components/ui/callout.stories.tsx
(1 hunks)libs/docs/components/ui/card.stories.tsx
(1 hunks)libs/docs/components/ui/folder.stories.tsx
(1 hunks)libs/docs/components/ui/tabs.stories.tsx
(1 hunks)libs/docs/components/ui/type-table.stories.tsx
(1 hunks)tailwind.config.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tailwind.config.ts
- apps/docs/tailwind.config.js
- libs/docs/components/ui/card.stories.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
libs/docs/components/ui/callout.stories.tsx
[error] 58-58: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (3)
libs/docs/components/ui/callout.stories.tsx (2)
58-69
: Rename 'Error' story to avoid shadowing built-in Error object
The story name 'Error' shadows the global Error object. Consider a more descriptive name.
🧰 Tools
🪛 Biome (1.9.4)
[error] 58-58: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
12-25
: LGTM! Comprehensive type coverage
The argTypes effectively document all available types (info, warn, error) and control types are appropriately chosen for each prop.
libs/docs/components/ui/accordion.stories.tsx (1)
32-75
: LGTM! Comprehensive story coverage
The stories effectively demonstrate key features:
- Default behavior with multiple items
- Disabled state
- Multiple selection mode
export const Default: Story = { | ||
args: { | ||
type: { | ||
percentage: { | ||
description: 'The percentage of scroll position to display the roll button', | ||
type: 'number', | ||
default: 0.2, | ||
}, | ||
}, | ||
}, | ||
render: args => <TypeTable {...args} />, | ||
} |
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.
🛠️ Refactor suggestion
Add more story variations to showcase component features
The component currently only has a Default story. Consider adding more stories to demonstrate different use cases and edge cases.
Examples of additional stories to consider:
- Empty type table
- Table with multiple type definitions
- Table with different scroll positions
export const Persistent: Story = { | ||
args: { | ||
items: ['Javascript', 'Rust'], | ||
persist: true, | ||
groupId: 'language', | ||
}, | ||
render: args => ( | ||
<Tabs items={args.items} persist groupId={args.groupId}> | ||
<Tab value="Javascript">Value is shared! Try refreshing to see persistence.</Tab> | ||
<Tab value="Rust">Rust is fast and persistent!</Tab> | ||
</Tabs> | ||
), | ||
} |
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.
Add error handling for localStorage availability.
The persistent tabs feature relies on localStorage, but there's no handling for cases where it might be unavailable (e.g., in private browsing mode or when storage quota is exceeded).
Consider wrapping the persistence logic in a try-catch:
export const Persistent: Story = {
args: {
items: ['Javascript', 'Rust'],
persist: true,
groupId: 'language',
},
render: args => (
<Tabs
items={args.items}
persist
groupId={args.groupId}
+ onError={(error) => console.warn('Storage error:', error)}
>
<Tab value="Javascript">Value is shared! Try refreshing to see persistence.</Tab>
<Tab value="Rust">Rust is fast and persistent!</Tab>
</Tabs>
),
}
Committable suggestion skipped: line range outside the PR's diff.
import type { Meta, StoryObj } from '@storybook/react' | ||
import { File, Files, Folder } from 'fumadocs-ui/components/files' | ||
|
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.
Address missing tests and documentation.
According to the PR objectives, tests and documentation updates are pending. Please ensure:
- Add unit tests for the Storybook stories (e.g., using @storybook/testing-library)
- Update component documentation with usage examples and prop descriptions
Would you like help setting up the test infrastructure or documentation templates?
const meta: Meta<typeof Files> = { | ||
title: '📚 Docs Site/Files', | ||
component: Files, | ||
tags: ['autodocs'], | ||
parameters: { | ||
layout: 'centered', | ||
}, | ||
argTypes: { | ||
name: { | ||
control: 'text', | ||
description: 'The name of the file or folder', | ||
}, | ||
disabled: { | ||
control: 'boolean', | ||
description: 'Disables the folder interaction', | ||
defaultValue: false, | ||
}, | ||
icon: { | ||
control: 'object', | ||
description: 'Optional icon component to display', | ||
}, | ||
}, | ||
} |
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.
🛠️ Refactor suggestion
Document accessibility features in component metadata.
The metadata should include information about keyboard navigation and ARIA attributes supported by the Files component.
Add accessibility documentation to the metadata:
parameters: {
layout: 'centered',
+ a11y: {
+ config: {
+ rules: [
+ {
+ id: 'button-name',
+ enabled: true
+ }
+ ]
+ }
+ }
},
Committable suggestion skipped: line range outside the PR's diff.
🎉 This PR is included in version 1.15.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Pull Request Template
PR Requirements Checklist
Summary by CodeRabbit
Summary by CodeRabbit
Callout
,Card
,Accordion
,Files
,Tabs
, andTypeTable
components.Default
,Warn
,Error
,Icon
,WithoutHref
,Disabled
,Multiple
,FolderDisabled
,CustomIcons
,Persistent
, andDefaultValue
.