-
-
Notifications
You must be signed in to change notification settings - Fork 53
code color prop with test, docs and sb #809
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
Conversation
WalkthroughThis pull request introduces a new structure for displaying documentation related to code properties within the Changes
Sequence DiagramsequenceDiagram
participant Component as Code Component
participant Props as Props Input
participant Render as Rendered Element
Props->>Component: color prop (optional)
alt Color prop provided
Component->>Component: Create data-accent-color attribute
end
Component->>Render: Render code element with optional color attribute
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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)
src/components/ui/Code/Code.tsx (1)
14-14: Consider using undefined instead of empty string as default.The empty string default for the color prop will still create a
data-accent-colorattribute with an empty value. Usingundefinedwould be more semantically correct and would prevent the attribute from being added when no color is specified.-const Code = ({ children, customRootClass = '', color='' }: CodeProps) => { +const Code = ({ children, customRootClass = '', color }: CodeProps) => {Also applies to: 17-21
src/components/ui/Code/stories/Code.stories.js (2)
13-13: Fix inconsistent indentation in JSX.The indentation of the Code components and their children is inconsistent.
- <Code className='space-x-1' {...args}> - requestAnimationFrame() - </Code> + <Code className='space-x-1' {...args}> + requestAnimationFrame() + </Code>Also applies to: 17-17
34-38: Enhance color story examples.Consider adding more color variations and documenting the available color options in the story.
export const Color = { + parameters: { + docs: { + description: { + story: 'The Code component supports various accent colors through the `color` prop.', + }, + }, + }, args: { color: 'green' } -} +}; + +export const ColorVariants = { + parameters: { + docs: { + description: { + story: 'Examples of different color variants.', + }, + }, + }, + render: () => ( + <SandboxEditor> + <div className='flex space-x-2'> + <Code color='red'>Error</Code> + <Code color='yellow'>Warning</Code> + <Code color='green'>Success</Code> + <Code color='blue'>Info</Code> + </div> + </SandboxEditor> + ), +};docs/app/docs/components/code/page.js (2)
17-19: Enhance color prop documentation.The description of the color prop could be more detailed:
- List available/recommended colors
- Add validation information
- Provide usage examples
const data = [ - {prop: 'color', type: 'string', default: 'null', description: 'Accent Color of the code', id: 'color'}, + { + prop: 'color', + type: 'string', + default: 'undefined', + description: 'Sets the accent color of the code block. Recommended values: "red" (errors), "yellow" (warnings), "green" (success), "blue" (info).', + id: 'color' + }, ];
34-36: Consider table positioning.The table div could benefit from proper spacing and semantic HTML structure.
- <div className="max-w-screen-md"> - <Documentation.Table columns={columns} data={data} /> - </div> + <section className="max-w-screen-md mt-8"> + <h2 className="text-xl font-semibold mb-4">Props</h2> + <Documentation.Table columns={columns} data={data} /> + </section>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/app/docs/components/code/page.js(2 hunks)src/components/ui/Code/Code.tsx(1 hunks)src/components/ui/Code/stories/Code.stories.js(2 hunks)src/components/ui/Code/tests/Code.test.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
src/components/ui/Code/Code.tsx (1)
11-11: LGTM! Well-typed optional prop.The
colorprop is correctly typed as an optional string.
| it('renders without crashing', () => { | ||
| const { container } = render(<Code color='blue'>console.log('Hello world!');</Code>); | ||
|
|
||
| const codeElement = container.querySelector('code'); | ||
| expect(codeElement).toBeInTheDocument(); | ||
| expect(codeElement).toHaveAttribute('data-accent-color', 'blue'); | ||
| }); |
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
Test suite needs improvement.
- The test has the same name as the previous test ("renders without crashing"). Please use a more descriptive name.
- Additional test cases are needed:
- Test with undefined/empty color
- Test the combination of color with customRootClass
- it('renders without crashing', () => {
+ it('applies color as data-accent-color attribute', () => {
const { container } = render(<Code color='blue'>console.log('Hello world!');</Code>);
const codeElement = container.querySelector('code');
expect(codeElement).toBeInTheDocument();
expect(codeElement).toHaveAttribute('data-accent-color', 'blue');
});
+
+ it('does not add data-accent-color when color is undefined', () => {
+ const { container } = render(<Code>console.log('Hello world!');</Code>);
+ const codeElement = container.querySelector('code');
+ expect(codeElement).not.toHaveAttribute('data-accent-color');
+ });
+
+ it('combines color with customRootClass correctly', () => {
+ const { container } = render(
+ <Code color='blue' customRootClass='test-class'>
+ console.log('Hello world!');
+ </Code>
+ );
+ const codeElement = container.querySelector('code');
+ expect(codeElement).toHaveAttribute('data-accent-color', 'blue');
+ expect(codeElement).toHaveClass('test-class');
+ });Committable suggestion skipped: line range outside the PR's diff.
| expect(codeElement).toHaveTextContent("console.log('Hello world!');"); | ||
| }); | ||
|
|
||
| it('renders without crashing', () => { |
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.
duplicate test
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.
will update
Summary by CodeRabbit
New Features
Codecomponent for custom color styling.CodeDocscomponent.Documentation
Codecomponent with a green color variant.Tests
Codecomponent.