-
-
Notifications
You must be signed in to change notification settings - Fork 53
BlockQuote color prop with test, docs and storybook #797
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
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
docs/app/docs/components/blockquote/page.jsOops! Something went wrong! :( ESLint: 8.56.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/docs/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThis pull request introduces a color customization feature for the Changes
Sequence DiagramsequenceDiagram
participant Component as BlockQuote
participant Props as Props
participant Render as Rendered HTML
Props->>Component: Pass color prop
Component->>Component: Create data-attributes
Component->>Render: Render with data-accent-color
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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 (7)
src/components/ui/BlockQuote/BlockQuote.tsx (3)
12-12: Add JSDoc documentation for the color prop.Add documentation to describe the purpose and expected values of the color prop.
+/** Accent color for the blockquote. */ color?: string;
18-22: Follow TypeScript naming conventions.Use camelCase for variable names instead of snake_case.
- const data_attributes: Record<string, string> = {}; + const dataAttributes: Record<string, string> = {}; if (color) { - data_attributes['data-accent-color'] = color; + dataAttributes['data-accent-color'] = color; }
24-24: Consider validating color values.The color prop accepts any string value without validation. Consider adding validation or using a union type of allowed colors.
Example implementation:
type AllowedColors = 'blue' | 'green' | 'red' | string; export type BlockQuoteProps = { // ...other props /** Accent color for the blockquote. */ color?: AllowedColors; }src/components/ui/BlockQuote/tests/BlockQuote.test.tsx (1)
30-33: Enhance test coverage for the color prop.Consider adding more test cases to cover:
- Empty color value
- Multiple color values
- Color prop updates
Example additional tests:
test('renders BlockQuote without color when color prop is empty', () => { render(<BlockQuote color="">BlockQuote</BlockQuote>); expect(screen.getByText('BlockQuote')).not.toHaveAttribute('data-accent-color'); }); test('updates BlockQuote color when prop changes', () => { const { rerender } = render(<BlockQuote color="blue">BlockQuote</BlockQuote>); expect(screen.getByText('BlockQuote')).toHaveAttribute('data-accent-color', 'blue'); rerender(<BlockQuote color="green">BlockQuote</BlockQuote>); expect(screen.getByText('BlockQuote')).toHaveAttribute('data-accent-color', 'green'); });src/components/ui/BlockQuote/stories/BlockQuote.stories.js (1)
36-40: Enhance color story documentation and examples.Consider adding:
- ArgTypes documentation for the color prop
- Multiple stories showcasing different colors
- Description of available color options
Example implementation:
export default { // ... other config argTypes: { color: { description: 'Accent color for the blockquote', control: 'select', options: ['blue', 'green', 'red'], } } }; export const Colors = { render: () => ( <div className="space-y-4"> <BlockQuote color="blue">Blue accent</BlockQuote> <BlockQuote color="green">Green accent</BlockQuote> <BlockQuote color="red">Red accent</BlockQuote> </div> ) };docs/app/docs/components/blockquote/page.js (2)
21-23: Enhance color prop documentation.The current documentation lacks details about:
- Available color options
- Usage examples with different colors
- Best practices for color selection
Example improved documentation:
const data = [ { prop: 'color', type: 'string', default: 'null', description: 'Accent color of the quote. Supports standard color values (e.g., "blue", "green", "red") or custom color values.', id: 'color' } ];
48-50: Consider adding usage examples section.Add a section demonstrating different color usage examples to help users understand the available options.
Example implementation:
<Documentation.Section title="Color Examples"> <div className="space-y-4"> <BlockQuote color="blue">Blue accent example</BlockQuote> <BlockQuote color="green">Green accent example</BlockQuote> <BlockQuote color="red">Red accent example</BlockQuote> </div> </Documentation.Section>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/app/docs/components/blockquote/page.js(2 hunks)src/components/ui/BlockQuote/BlockQuote.tsx(1 hunks)src/components/ui/BlockQuote/stories/BlockQuote.stories.js(2 hunks)src/components/ui/BlockQuote/tests/BlockQuote.test.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
| customRootClass?: string; | ||
| className?: string; | ||
| color?:string; | ||
| props?: Record<string, any>[] |
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.
Fix props type definition.
The props type should be a generic object, not an array.
- props?: Record<string, any>[]
+ props?: Record<string, any>📝 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.
| props?: Record<string, any>[] | |
| props?: Record<string, any> |
BlockQuote color prop with test, docs and storybook
Summary by CodeRabbit
New Features
colorprop toBlockQuotecomponent for customizing accent colorBlockQuoteDocumentation
BlockQuotecomponent propertiesTests
BlockQuote