migration b2 #841
migration b2 #841
Conversation
WalkthroughThe pull request involves updates to several Storybook story files across different UI components. The changes primarily focus on improving type annotations, adding React imports, and simplifying render function signatures. Modifications are consistent across Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
src/components/ui/Code/stories/Code.stories.tsx (2)
Line range hint
11-24: Consider improving component composition.The render function has duplicate Code components with similar props. Consider extracting the common structure to reduce duplication.
render: (args: React.JSX.IntrinsicAttributes & CodeProps) => <SandboxEditor> <div> <div className='flex space-x-2'> - <Code {...args}> - requestAnimationFrame() - </Code> - <Code {...args}> - {Code_TEXT} - </Code> + {[ + 'requestAnimationFrame()', + Code_TEXT + ].map((text, index) => ( + <Code key={index} {...args}>{text}</Code> + ))} </div> </div> </SandboxEditor>
31-31: Standardize className formatting.The className prop has inconsistent spacing in different stories.
- className :'space-x-1' + className: 'space-x-1'- className:'space-x-1', + className: 'space-x-1',Also applies to: 37-37
src/components/ui/Card/stories/Card.stories.tsx (2)
12-12: Remove commented code about Avatar size prop.Instead of keeping commented code, document the Avatar component's limitations in the component's documentation or remove the comment if it's no longer relevant.
- // size='lg' avatar doesn't have size prop
Line range hint
34-39: Consider using Array.from for better readability.The Template function uses Array(10).fill(0) which is less idiomatic than Array.from.
const Template = () => <div> <SandboxEditor className="gap-3 grid grid-cols-4"> - {Array(10).fill(0).map((_, i) => <CardStory key={i}/>)} + {Array.from({ length: 10 }, (_, i) => <CardStory key={i}/>)} </SandboxEditor> </div>;src/components/ui/Disclosure/stories/Disclosure.stories.tsx (2)
1-4: Consolidate React imports.The React and JSX imports can be combined into a single import statement.
-import { JSX } from 'react'; -import Disclosure, { DisclosureProps } from '../Disclosure' +import React, { JSX } from 'react'; +import Disclosure, { DisclosureProps } from '../Disclosure'; import SandboxEditor from '~/components/tools/SandboxEditor/SandboxEditor'; -import React from 'react';
Line range hint
36-44: Add type definition for disclosureItems.Consider adding a type definition for the disclosureItems array to improve type safety.
+interface DisclosureItem { + title: string; + content: string; +} + -const disclosureItems = [ +const disclosureItems: DisclosureItem[] = [ { title: 'Section 1', content: 'Content for section 1' }, { title: 'Section 2', content: 'Content for section 2' }, ]src/components/ui/Dropdown/stories/Dropdown.stories.tsx (2)
11-12: Replace inline styles with CSS classes.Consider moving inline styles to CSS classes for better maintainability.
- <div className=' bg-gray-400 p-4 overflow-auto' style={{ height: '200px' }}> - <div className='block' style={{ height: '400px' }}> + <div className='bg-gray-400 p-4 overflow-auto h-[200px]'> + <div className='block h-[400px]'>
Line range hint
18-35: Extract long text content to constants.The dropdown content contains repeated text that should be extracted to a constant for better maintainability.
+const LONG_TEXT = 'Hello this is a dropdown. '.repeat(5); + <Dropdown onSelect={() => {}} label={'Bello'} list={[ { - value: `hello Hello this is a dropdown. Hello this is a dropdown. - Hello this is a dropdown. Hello`, + value: LONG_TEXT, label: 'hello' } ]}> <div> - Hello this is a dropdown. Hello this is a dropdown. Hello this is a dropdown. Hello this is a - dropdown. Hello this is a dropdown. Hello this is a dropdown. Hello this is a dropdown. Hello - this is a dropdown. Hello this is a dropdown. Hello this is a dropdown. Hello this is a - dropdown. Hello this is a dropdown. Hello this is a dropdown. Hello this is a dropdown. Hello + {LONG_TEXT} </div> </Dropdown>src/components/ui/Callout/stories/Callout.stories.tsx (2)
Line range hint
14-27: Consider adding proper type annotations to the render function.While removing unused args parameter simplifies the code, we should maintain type safety by properly typing the render function.
- render: () => <> + render: React.FC = () => <>
Line range hint
1-54: Consider a more thorough TypeScript migration approach.While the file has been successfully converted to TSX, the migration could be more thorough:
- Story metadata could be typed using
MetaandStoryObjfrom@storybook/react- Template functions could use proper component prop types
- Story arguments could be properly typed
This would provide better type safety and improved developer experience.
Example structure:
import type { Meta, StoryObj } from '@storybook/react'; const meta: Meta<typeof Callout> = { title: 'Components/Callout', component: Callout, // ... other metadata }; export default meta; type Story = StoryObj<typeof Callout>; export const All: Story = { args: { className: '' } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/ui/Callout/stories/Callout.stories.tsx(4 hunks)src/components/ui/Card/stories/Card.stories.tsx(2 hunks)src/components/ui/Code/stories/Code.stories.tsx(3 hunks)src/components/ui/Disclosure/stories/Disclosure.stories.tsx(1 hunks)src/components/ui/Dropdown/stories/Dropdown.stories.tsx(1 hunks)
🔇 Additional comments (3)
src/components/ui/Code/stories/Code.stories.tsx (1)
1-2: LGTM! Good TypeScript migration.The addition of React import and CodeProps type improves type safety.
src/components/ui/Callout/stories/Callout.stories.tsx (2)
4-4: LGTM! Required React import added.The React import is necessary for TSX files and is correctly placed.
Line range hint
38-44: Replaceanytype with proper Callout component props type.Using
anytype defeats the purpose of TypeScript migration. We should use the proper props type from the Callout component.Let's verify the available types:
Apply this improvement:
-const DefaultTextTemplate = (args: any) => { +const DefaultTextTemplate = (args: React.ComponentProps<typeof Callout>) => { -const RedTextTemplate = (args: any) => { +const RedTextTemplate = (args: React.ComponentProps<typeof Callout>) => {Also applies to: 48-54
✅ Verification successful
The review comment's suggestion is correct and provides a better solution
Using
React.ComponentProps<typeof Callout>is indeed better thananyas it:
- Provides proper type safety
- Automatically stays in sync with component changes
- Eliminates the need for explicit type imports
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if Callout component exports its props type ast-grep --pattern 'export (type|interface) CalloutProps' # Alternative pattern to find props definition ast-grep --pattern 'interface CalloutProps'Length of output: 105
Script:
#!/bin/bash # Find and show Callout component implementation ast-grep --pattern 'const Callout = $_' -A 10 # Look for any type definitions in files containing Callout rg "type\s+\w+Props|interface\s+\w+Props" --type ts --type tsx -C 2Length of output: 976
Script:
#!/bin/bash # Check Callout.tsx file for type definitions cat src/components/ui/Callout/Callout.tsx # Fixed ripgrep command for TypeScript/TSX files rg "type\s+\w+Props|interface\s+\w+Props" -t typescript -t tsx -C 2Length of output: 847
kotAPI
added
the
automerge
2 participants
js to tsx stories
Summary by CodeRabbit
Chores
Documentation
Note: These changes are primarily internal improvements to the development process and do not directly impact end-user functionality.