add variant and size for blockquote#922
Conversation
WalkthroughThis pull request introduces new documentation and example files for the BlockQuote component. Three new usage files asynchronously load example source code and export a structured code object. Corresponding React examples—demonstrating color, size, and variant implementations—are added. The main BlockQuote component has been updated to accept new optional properties ( Changes
Sequence Diagram(s)sequenceDiagram
participant DocUsage as Documentation Usage File
participant Util as getSourceCodeFromPath
participant Example as Example Component Source
participant Export as Code Object Export
DocUsage->>Util: Call getSourceCodeFromPath(example path)
Util-->>DocUsage: Return source code asynchronously
DocUsage->>Export: Structure and export code object with retrieved source
sequenceDiagram
participant Parent as Parent Component/Story
participant BlockQuote as BlockQuote Component
participant DOM as Rendered Element
Parent->>BlockQuote: Render with props (variant, size, etc.)
BlockQuote->>BlockQuote: Check for new props and conditionally add data attributes
BlockQuote->>DOM: Render element with additional data attributes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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/docs/variantCodeUsage.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. docs/app/docs/components/blockquote/docs/sizeCodeUsage.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. docs/app/docs/components/blockquote/docs/colorCodeUsage.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.
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 2
🧹 Nitpick comments (13)
styles/themes/components/blockquote.scss (3)
6-6: Remove or update the TODO comment since variants are being implementedThe TODO comment about BlockQuote variants should be removed or updated since this PR is implementing variants for the component.
- // BlockQuote Variants - TODO + // BlockQuote Variants
9-31: Consider varying padding proportionally with different sizesCurrently, all size variants use the same padding (4px 10px) regardless of font size. It would be more consistent with design patterns to have proportionally larger padding for larger font sizes.
Also, there are several unnecessary blank lines that could be removed for better code consistency.
/** BlockQuote Sizes */ &[data-block-quote-size="small"]{ font-size: small; padding: 4px 10px; - } &[data-block-quote-size="medium"]{ font-size: medium; padding: 4px 10px; - } &[data-block-quote-size="large"]{ font-size: large; - padding: 4px 10px; - + padding: 6px 12px; } &[data-block-quote-size="x-large"]{ font-size: x-large; - padding: 4px 10px; - + padding: 8px 14px; }
33-34: Remove unnecessary blank linesThere are extra blank lines at the end of the file that should be removed.
docs/app/docs/components/blockquote/examples/BlockQuoteColor.tsx (1)
7-11: Consider using size as key instead of indexSince the sizes array contains unique string values, it would be more appropriate to use the size value as the key rather than the array index.
- {sizes.map((size, index) => { - return <BlockQuote key={index} size={size} color='pink'> + {sizes.map((size) => { + return <BlockQuote key={size} size={size} color='pink'>docs/app/docs/components/blockquote/examples/BlockQuoteSizes.tsx (4)
8-12: Consider using size as key instead of indexSince the sizes array contains unique string values, it would be more appropriate to use the size value as the key rather than the array index.
- {sizes.map((size, index) => { - return <BlockQuote key={index} size={size} > + {sizes.map((size) => { + return <BlockQuote key={size} size={size} >
6-13: Fix indentation and whitespaceThere are some inconsistent indentation and extra whitespace in the JSX. Consider cleaning up the formatting:
- return <div className='flex flex-col gap-4'> - - {sizes.map((size, index) => { - return <BlockQuote key={index} size={size} > - <span>"Behind every great man is a woman rolling her eyes." — Jim Carrey</span> - </BlockQuote> - })} - </div> + return <div className='flex flex-col gap-4'> + {sizes.map((size, index) => { + return <BlockQuote key={index} size={size}> + <span>"Behind every great man is a woman rolling her eyes." — Jim Carrey</span> + </BlockQuote> + })} + </div>
10-11: Ensure consistency with other examplesIn
BlockQuoteColor.tsx, the quote text is directly inside the BlockQuote, but here it's wrapped in a span. Consider being consistent across examples:- <span>"Behind every great man is a woman rolling her eyes." — Jim Carrey</span> + "Behind every great man is a woman rolling her eyes." — Jim CarreyAlternatively, ensure that all examples consistently use a span or other wrapper if there's a specific reason for it.
14-15: Remove extra whitespaceThere's unnecessary whitespace at line 14 that should be removed for cleaner code.
- +src/components/ui/BlockQuote/stories/BlockQuote.stories.tsx (2)
14-15: Consider removing unused array or implement VariantsThe
Variantsarray is defined but not used in the code. Either:
- Remove it until it's needed
- Implement it with the intended variant values
- Add a more descriptive TODO comment explaining what variants will be used
-const Variants = []; // TODO +// TODO: Define variant options once finalized
67-79: Enhance the TODO comment for Variant implementationThe Variant story has a TODO comment but doesn't provide much context. Consider adding a more descriptive comment about what variants will be implemented:
<div className='flex flex-col gap-3'> - {/* TODO */} + {/* TODO: Implement different variant examples once variants are finalized */} <BlockQuote> <div>{BLOCKQUOTE_TEXT}</div> </BlockQuote> </div>Also, add a newline at the end of the file to fix the ESLint error:
}; +🧰 Tools
🪛 ESLint
[error] 79-79: Newline required at end of file but not found.
(eol-last)
src/components/ui/BlockQuote/BlockQuote.tsx (3)
12-14: Clear addition of new props to enhance BlockQuote customization.The addition of
variantandsizeprops expands the component's flexibility while maintaining the existing API. Consider improving type safety by using union types (e.g.,variant?: 'bordered' | 'filled') if there are specific predefined variants and sizes.
30-33: Remove extra empty line.There's an unnecessary blank line before the color check conditional.
- if (color) {🧰 Tools
🪛 ESLint
[error] 30-31: More than 1 blank line not allowed.
(no-multiple-empty-lines)
34-34: Remove trailing whitespace.There's trailing whitespace at the end of line 34.
- +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/app/docs/components/blockquote/docs/colorCodeUsage.js(1 hunks)docs/app/docs/components/blockquote/docs/sizeCodeUsage.js(1 hunks)docs/app/docs/components/blockquote/docs/variantCodeUsage.js(1 hunks)docs/app/docs/components/blockquote/examples/BlockQuoteColor.tsx(1 hunks)docs/app/docs/components/blockquote/examples/BlockQuoteSizes.tsx(1 hunks)docs/app/docs/components/blockquote/examples/BlockQuoteVariants.tsx(1 hunks)docs/app/docs/components/blockquote/page.mdx(2 hunks)src/components/ui/BlockQuote/BlockQuote.tsx(1 hunks)src/components/ui/BlockQuote/stories/BlockQuote.stories.tsx(2 hunks)styles/themes/components/blockquote.scss(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/app/docs/components/blockquote/examples/BlockQuoteVariants.tsx
🧰 Additional context used
🪛 ESLint
src/components/ui/BlockQuote/BlockQuote.tsx
[error] 17-17: Operator '=' must be spaced.
(space-infix-ops)
[error] 17-17: Operator '=' must be spaced.
(space-infix-ops)
[error] 30-31: More than 1 blank line not allowed.
(no-multiple-empty-lines)
src/components/ui/BlockQuote/stories/BlockQuote.stories.tsx
[error] 79-79: Newline required at end of file but not found.
(eol-last)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (9)
docs/app/docs/components/blockquote/docs/sizeCodeUsage.js (1)
1-9: LGTM! Documentation file well structured.The file correctly imports the source code parsing utility and exports the code in the expected format for documentation purposes.
docs/app/docs/components/blockquote/docs/variantCodeUsage.js (1)
1-9: LGTM! Documentation file well structured.The file correctly imports the source code parsing utility and exports the code in the expected format for documentation purposes.
docs/app/docs/components/blockquote/docs/colorCodeUsage.js (1)
1-9: LGTM! Documentation file well structured.The file correctly imports the source code parsing utility and exports the code in the expected format for documentation purposes.
docs/app/docs/components/blockquote/examples/BlockQuoteColor.tsx (1)
1-15: Clean and well-structured example componentThe component is well-organized and clearly demonstrates the different size options for the BlockQuote component with color.
docs/app/docs/components/blockquote/page.mdx (2)
4-9: Clean organization of importsThe imports are well-organized and follow a logical structure, with code usages and their corresponding component examples grouped together.
24-35: Well-structured documentation sectionsThe new ComponentHero sections for Variants, Sizes, and Color are clearly organized and consistent with the existing documentation pattern. Each section appropriately combines the code examples with the visual representation.
src/components/ui/BlockQuote/stories/BlockQuote.stories.tsx (1)
46-65: Well-implemented Size storyThe Size story is well-structured and effectively demonstrates the different size options for the BlockQuote component.
src/components/ui/BlockQuote/BlockQuote.tsx (2)
22-24: Well-implemented variant attribute handling.The implementation correctly sets the data attribute only when a variant is provided, following the established pattern used for the color attribute.
26-28: Well-implemented size attribute handling.The implementation correctly sets the data attribute only when a size is provided, following the established pattern used for the color attribute.
| </BlockQuote> | ||
| </div> | ||
| </SandboxEditor>; | ||
| }; No newline at end of file |
There was a problem hiding this comment.
Add newline at end of file
Add a newline at the end of the file to fix the ESLint error "Newline required at end of file but not found."
};
+📝 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.
| }; | |
| }; |
🧰 Tools
🪛 ESLint
[error] 79-79: Newline required at end of file but not found.
(eol-last)
| props?: Record<string, any>[] | ||
| } | ||
| const BlockQuote = ({ children, customRootClass = '', className = '', color = '', ...props }: BlockQuoteProps) => { | ||
| const BlockQuote = ({ children, customRootClass = '', className = '', color = '', variant= '', size= '', ...props }: BlockQuoteProps) => { |
There was a problem hiding this comment.
Fix spacing around assignment operators.
There are spacing issues with the default value assignments for variant and size.
-const BlockQuote = ({ children, customRootClass = '', className = '', color = '', variant= '', size= '', ...props }: BlockQuoteProps) => {
+const BlockQuote = ({ children, customRootClass = '', className = '', color = '', variant = '', size = '', ...props }: BlockQuoteProps) => {📝 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 BlockQuote = ({ children, customRootClass = '', className = '', color = '', variant= '', size= '', ...props }: BlockQuoteProps) => { | |
| const BlockQuote = ({ children, customRootClass = '', className = '', color = '', variant = '', size = '', ...props }: BlockQuoteProps) => { |
🧰 Tools
🪛 ESLint
[error] 17-17: Operator '=' must be spaced.
(space-infix-ops)
[error] 17-17: Operator '=' must be spaced.
(space-infix-ops)
kotAPI
added
the
automerge
2 participants
Summary by CodeRabbit
New Features
Documentation
Style