add variant and size for callout#924
Conversation
WalkthroughThis pull request introduces new documentation components for the Callout feature. Three new code usage files asynchronously retrieve source code from respective example files using a shared utility function and export a constant object. Additionally, three new React components—demonstrating callout variants, sizes, and colors—have been added. The documentation page is updated to import these examples and usage constants, adding dedicated sections to display the variations. Changes
Sequence Diagram(s)sequenceDiagram
participant D as Documentation Page
participant CU as Code Usage File
participant E as Example Component
D->>+CU: Import usage file (color/size/variant)
CU->>+E: Call getSourceCodeFromPath (asynchronously fetch source)
E-->>-CU: Return source code
CU-->>-D: Export code constant
D->>D: Render Documentation.ComponentHero with example & code
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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 (8)
docs/app/docs/components/callout/examples/CalloutSizes.tsx (2)
9-21: Consider using better keys for list renderingThe component currently uses array indices as React keys, which could potentially cause issues with component rerendering if the lists are modified. Consider using more stable and unique keys.
-{variants.map((variant, index) => { - return <div key={index} className='flex justify-center flex-wrap gap-4'> +{variants.map((variant) => { + return <div key={variant} className='flex justify-center flex-wrap gap-4'>And similarly for the nested map:
-{sizes.map((size, index) => { - return ( - <Callout key={index} size={size} variant={variant}> +{sizes.map((size) => { + return ( + <Callout key={`${variant}-${size}`} size={size} variant={variant}>
5-22: TypeScript type annotations would improve maintainabilityConsider adding TypeScript type annotations for the component's props, variables, and return type to improve code maintainability and catch potential type errors early.
-const CalloutSizes = () => { +const CalloutSizes: React.FC = () => { const sizes = ['small', 'medium', 'large', 'x-large']; const variants = ['soft', 'outline'];Additionally, you could create type definitions for sizes and variants:
+type CalloutSize = 'small' | 'medium' | 'large' | 'x-large'; +type CalloutVariant = 'soft' | 'outline'; const CalloutSizes: React.FC = () => { - const sizes = ['small', 'medium', 'large', 'x-large']; - const variants = ['soft', 'outline']; + const sizes: CalloutSize[] = ['small', 'medium', 'large', 'x-large']; + const variants: CalloutVariant[] = ['soft', 'outline'];docs/app/docs/components/callout/examples/CalloutColor.tsx (2)
6-7: Consider extending color demonstration beyond 'pink'The component name
CalloutColorsuggests it demonstrates multiple colors, but it only shows 'pink' in the implementation. Consider showing multiple color options to better align with the component name and purpose.
11-18: Avoid using array index as React key when possibleUsing array indices as keys (key={index}) can lead to unexpected behavior if the array items get reordered. Consider using a more stable identifier or a composite key.
- {sizes.map((size, index) => { - return ( - <Callout key={index} size={size} variant={variant} color='pink'> + {sizes.map((size) => { + return ( + <Callout key={`${variant}-${size}`} size={size} variant={variant} color='pink'>docs/app/docs/components/callout/examples/CalloutVariants.tsx (2)
9-12: Improve variant descriptions for clarity and grammarThe descriptions in
calloutStyleDescriptionhave grammatical errors and don't clearly differentiate between variants.const calloutStyleDescription = { - soft: 'Soft callout have a soft background color and a border.', - outline: 'Outline callout have a border and a background color.', + soft: 'Soft callouts have a subtle background color with a matching border.', + outline: 'Outline callouts have a prominent border with a minimal background color.' };
24-24: Use className instead of inline styles for consistencyThe component uses className for styling elsewhere but switches to inline styles for the Separator. For consistency, consider using className throughout.
- <Separator orientation="horizontal" style={{ marginTop: 20 }} /> + <Separator orientation="horizontal" className="mt-5" />docs/app/docs/components/callout/page.mdx (2)
28-39: LGTM! Good organization of component variationsThe documentation is well structured with separate sections for variants, sizes, and colors. The implementation effectively showcases the different features of the Callout component.
However, consider adding brief introductory text for each section to explain the purpose and use cases for different variants, sizes, and colors.
31-32: Remove unnecessary blank lineThere's an extra blank line between sections that disrupts the consistent spacing throughout the document.
</Documentation.ComponentHero> - - <Documentation.ComponentHero title='Sizes' codeUsage={sizeCodeUsage}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/app/docs/components/callout/docs/colorCodeUsage.js(1 hunks)docs/app/docs/components/callout/docs/sizeCodeUsage.js(1 hunks)docs/app/docs/components/callout/docs/variantCodeUsage.js(1 hunks)docs/app/docs/components/callout/examples/CalloutColor.tsx(1 hunks)docs/app/docs/components/callout/examples/CalloutSizes.tsx(1 hunks)docs/app/docs/components/callout/examples/CalloutVariants.tsx(1 hunks)docs/app/docs/components/callout/page.mdx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
docs/app/docs/components/callout/docs/variantCodeUsage.js (1)
1-9: Code structure looks good!This file correctly imports the utility function and asynchronously fetches the source code for the Callout variants example. The export structure follows a consistent pattern that appears to be used throughout the documentation system.
docs/app/docs/components/callout/docs/sizeCodeUsage.js (1)
1-9: Code structure looks good!This file correctly imports the utility function and asynchronously fetches the source code for the Callout sizes example. The export structure follows a consistent pattern that appears to be used throughout the documentation system.
docs/app/docs/components/callout/docs/colorCodeUsage.js (1)
1-9: Code structure looks good!This file correctly imports the utility function and asynchronously fetches the source code for the Callout color example. The export structure follows a consistent pattern that appears to be used throughout the documentation system.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
styles/themes/components/callout.scss (2)
20-25: Simplify border declarations for better readabilityThe border declarations for the "soft" variant can be simplified using CSS shorthand notation instead of defining each side separately.
&[data-callout-variant="soft"]{ - border-top: 1px solid var(--rad-ui-color-accent-600); - border-left: 1px solid var(--rad-ui-color-accent-600); - border-right: 1px solid var(--rad-ui-color-accent-600); - border-bottom: 1px solid var(--rad-ui-color-accent-600); + border: 1px solid var(--rad-ui-color-accent-600); color: var(--rad-ui-color-accent-950); background-color: var(--rad-ui-color-accent-400);
33-37: Consider using explicit font size values for better consistencyCurrently, the size variants use relative keywords (small, medium, large, x-large) for font-size which can vary across browsers. Consider using explicit values (px, rem) for better consistency.
You could replace the relative keywords with explicit values, for example:
&[data-callout-size="small"]{ - font-size: small; + font-size: 12px; padding: 4px 8px; height: 24px; } &[data-callout-size="medium"]{ - font-size: medium; + font-size: 14px; padding: 4px 12px; height: 32px; } &[data-callout-size="large"]{ - font-size: large; + font-size: 16px; padding: 4px 16px; height: 40px; } &[data-callout-size="x-large"]{ - font-size: x-large; + font-size: 18px; padding: 4px 24px; height: 48px; }Also applies to: 39-43, 45-49, 51-55
src/components/ui/Callout/fragments/CalloutRoot.tsx (1)
28-32: Remove redundant data-accent-color attribute assignmentThe
data-accent-colorattribute is being set twice: once in the data_attributes object and once directly in the JSX.if (color) { data_attributes['data-accent-color'] = color; } - return <div className={clsx(rootClass, className)} data-accent-color={color ?? undefined} {...data_attributes} {...props}> + return <div className={clsx(rootClass, className)} {...data_attributes} {...props}> {children} </div>;src/components/ui/Callout/stories/Callout.stories.tsx (3)
69-73: Improve rendering of Callout components with size propsThe code is functional but could be more readable. Consider adding spacing between the Callout components to improve visual clarity in the storybook.
<span key={index} className="inline-flex items-start space-x-2"> {Sizes.map((size, index) => { - return <Callout key={index} size={size} variant={variant} > + return <Callout key={index} size={size} variant={variant} className="mr-4 mb-2"> <InfoIcon/> <span>This is a Callout</span> </Callout>; })}
89-93: Fix excessive indentation and whitespaceThe code has excessive indentation which makes it inconsistent with the surrounding code structure.
<div className='flex space-x-2'> - - {Variants.map((variant, index) => { - return <Callout key={index} variant={variant} > - <InfoIcon/> <span>This is a Callout</span> - </Callout>; - })} + {Variants.map((variant, index) => { + return <Callout key={index} variant={variant}> + <InfoIcon/> <span>This is a Callout</span> + </Callout>; + })}
90-90: Remove extra whitespace in variant propThere's an extra space between
key={index}andvariant={variant}.- return <Callout key={index} variant={variant} > + return <Callout key={index} variant={variant}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/ui/Callout/Callout.tsx(1 hunks)src/components/ui/Callout/fragments/CalloutRoot.tsx(1 hunks)src/components/ui/Callout/stories/Callout.stories.tsx(2 hunks)styles/themes/components/callout.scss(1 hunks)
🧰 Additional context used
🪛 ESLint
src/components/ui/Callout/Callout.tsx
[error] 19-19: Operator '=' must be spaced.
(space-infix-ops)
[error] 19-19: Operator '=' must be spaced.
(space-infix-ops)
src/components/ui/Callout/fragments/CalloutRoot.tsx
[error] 16-16: Operator '=' must be spaced.
(space-infix-ops)
[error] 16-16: Operator '=' must be spaced.
(space-infix-ops)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
| } | ||
|
|
||
| const CalloutRoot = ({ children, className, color, customRootClass, ...props }: CalloutRootProps) => { | ||
| const CalloutRoot = ({ children, className, color, variant='', size='', customRootClass, ...props }: CalloutRootProps) => { |
There was a problem hiding this comment.
Fix spacing around default value assignments
The ESLint hint indicates missing spaces around the equals signs for default value assignments.
-const CalloutRoot = ({ children, className, color, variant='', size='', customRootClass, ...props }: CalloutRootProps) => {
+const CalloutRoot = ({ children, className, color, variant = '', size = '', customRootClass, ...props }: CalloutRootProps) => {📝 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.
| const CalloutRoot = ({ children, className, color, variant='', size='', customRootClass, ...props }: CalloutRootProps) => { | |
| const CalloutRoot = ({ children, className, color, variant = '', size = '', customRootClass, ...props }: CalloutRootProps) => { |
🧰 Tools
🪛 ESLint
[error] 16-16: Operator '=' must be spaced.
(space-infix-ops)
[error] 16-16: Operator '=' must be spaced.
(space-infix-ops)
| const COMPONENT_NAME = 'Callout'; | ||
| const Callout = ({ children, className = '', color, customRootClass, ...props }: CalloutProps) => { | ||
| return (<CalloutRoot customRootClass={customRootClass} className={clsx(className)} color={color ?? undefined} {...props}> | ||
| const Callout = ({ children, className = '', color, variant='', size='', customRootClass, ...props }: CalloutProps) => { |
There was a problem hiding this comment.
Fix spacing around default value assignments
The ESLint hint indicates missing spaces around the equals signs for default value assignments.
-const Callout = ({ children, className = '', color, variant='', size='', customRootClass, ...props }: CalloutProps) => {
+const Callout = ({ children, className = '', color, variant = '', size = '', customRootClass, ...props }: CalloutProps) => {📝 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.
| const Callout = ({ children, className = '', color, variant='', size='', customRootClass, ...props }: CalloutProps) => { | |
| const Callout = ({ children, className = '', color, variant = '', size = '', customRootClass, ...props }: CalloutProps) => { |
🧰 Tools
🪛 ESLint
[error] 19-19: Operator '=' must be spaced.
(space-infix-ops)
[error] 19-19: Operator '=' must be spaced.
(space-infix-ops)
2 participants
Callout component
Docs Page
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Documentation