textArea updated #1663
textArea updated #1663
Conversation
🦋 Changeset detectedLatest commit: c80a283 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR enhances the TextArea component with color and radius API support. It extends TextAreaProps with new properties (placeholder, disabled, readonly), integrates TextAreaRootProps and TextAreaInputProps types, adds data-attribute-driven styling modifiers for resize/radius/size/variant options, and includes new story variants demonstrating the updated functionality. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ui/TextArea/TextArea.tsx (1)
20-25: Remove duplicate children rendering.Children are rendered twice: once inside
TextAreaInput(lines 22-23) and again after it (line 24). SinceTextAreaInputwraps an HTML<textarea>element, which doesn't support children (textareas use thevalueprop), this causes incorrect rendering behavior.🔎 Proposed fix
const TextArea = React.forwardRef<React.ElementRef<'div'>, TextAreaProps>(({ customRootClass = '', placeholder = '', className = '', disabled = false, readonly = false, children, ...props }, ref) => { return ( <TextAreaRoot ref={ref} customRootClass={customRootClass} className={clsx(className)} {...props}> - <TextAreaInput placeholder={placeholder} disabled={disabled} readOnly={readonly}> - {children} - </TextAreaInput> - {children} + <TextAreaInput placeholder={placeholder} disabled={disabled} readOnly={readonly} /> </TextAreaRoot> ); }) as TextAreaComponent;Additionally, consider removing
childrenfrom the component signature entirely, as textarea components shouldn't accept children:-const TextArea = React.forwardRef<React.ElementRef<'div'>, TextAreaProps>(({ customRootClass = '', placeholder = '', className = '', disabled = false, readonly = false, children, ...props }, ref) => { +const TextArea = React.forwardRef<React.ElementRef<'div'>, TextAreaProps>(({ customRootClass = '', placeholder = '', className = '', disabled = false, readonly = false, ...props }, ref) => { return ( <TextAreaRoot ref={ref} customRootClass={customRootClass} className={clsx(className)} {...props}> <TextAreaInput placeholder={placeholder} disabled={disabled} readOnly={readonly} /> </TextAreaRoot> ); }) as TextAreaComponent;
🧹 Nitpick comments (1)
styles/themes/components/textarea.scss (1)
7-7: Consider aligning the default resize behavior with the component prop.The CSS default is
resize: none, butTextAreaRoot.tsx(line 18) defaults toresize='both'. While the component prop will override the CSS, aligning these defaults would improve clarity and consistency.Either:
- Change the CSS default to match the component:
- resize: none; + resize: both;Or:
2. Change the component default to match the CSS inTextAreaRoot.tsx:-({ children, customRootClass = '', className = '', variant = '', size = '', resize = 'both', color = '', radius = '', ...props }, ref) => { +({ children, customRootClass = '', className = '', variant = '', size = '', resize = 'none', color = '', radius = '', ...props }, ref) => {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/rude-forks-admire.md(1 hunks)src/components/ui/TextArea/TextArea.tsx(1 hunks)src/components/ui/TextArea/fragments/TextAreaRoot.tsx(2 hunks)src/components/ui/TextArea/stories/TextArea.stories.tsx(2 hunks)styles/themes/components/textarea.scss(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T08:34:33.079Z
Learnt from: decipher-cs
Repo: rad-ui/ui PR: 417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Applied to files:
src/components/ui/TextArea/stories/TextArea.stories.tsx
🧬 Code graph analysis (3)
src/components/ui/TextArea/stories/TextArea.stories.tsx (1)
src/components/ui/Radio/stories/Radio.stories.tsx (2)
Variants(44-65)Sizes(67-92)
src/components/ui/TextArea/fragments/TextAreaRoot.tsx (1)
src/core/hooks/createDataAttribute/index.ts (3)
useCreateDataAttribute(10-25)useCreateDataAccentColorAttribute(48-55)useComposeAttributes(33-40)
src/components/ui/TextArea/TextArea.tsx (2)
src/components/ui/TextArea/fragments/TextAreaRoot.tsx (1)
TextAreaRootProps(8-15)src/components/ui/TextArea/fragments/TextAreaInput.tsx (1)
TextAreaInputProps(3-3)
🪛 GitHub Actions: Lint
styles/themes/components/textarea.scss
[error] 133-133: no-descending-specificity: Expected selector ".rad-ui-text-area textarea:disabled" to come before selector ".rad-ui-text-area[data-textarea-variant="ghost"] textarea:focus".
[error] 134-134: no-descending-specificity: Expected selector ".rad-ui-text-area textarea:read-only" to come before selector ".rad-ui-text-area[data-textarea-variant="ghost"] textarea:focus".
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: coverage
🔇 Additional comments (5)
.changeset/rude-forks-admire.md (1)
1-5: LGTM!The changeset correctly documents the addition of color and radius API support in TextArea as a minor release.
src/components/ui/TextArea/stories/TextArea.stories.tsx (1)
36-66: LGTM!The new story variants (Size, Variant, Resize, TextAreaRadius) follow a consistent pattern and effectively demonstrate the new TextArea props.
src/components/ui/TextArea/fragments/TextAreaRoot.tsx (1)
4-25: LGTM!The integration of
colorandradiusprops is well-implemented. The use ofuseComposeAttributesto merge base data attributes with accent color attributes follows the established pattern and correctly generates the necessary data attributes for styling.styles/themes/components/textarea.scss (1)
1-122: LGTM!The data-attribute-driven styling approach is well-structured and provides comprehensive coverage for all TextArea variants, sizes, radii, and resize options. The base styles, focus states, and scrollbar customization enhance the user experience.
src/components/ui/TextArea/TextArea.tsx (1)
4-7: LGTM!The prop composition pattern correctly extends
TextAreaPropswithTextAreaRootPropsandTextAreaInputProps, allowing the TextArea component to accept all props from its fragments.
| const Variants = ['solid', 'soft', 'outline', 'ghost']; | ||
| const Sizes = ['small', 'medium', 'large']; | ||
| const Resizes: Array<'none' | 'vertical' | 'horizontal' | 'both' | undefined> = ['none', 'vertical', 'horizontal', 'both', undefined ]; | ||
| const Radius = ['none', 'small', 'medium', 'large']; |
There was a problem hiding this comment.
Remove undefined from the Resizes array or handle it separately.
Including undefined in the Resizes array will cause a React key warning when the array is mapped in the Resize story (line 55), since undefined cannot be used as a valid key.
🔎 Proposed fix
-const Resizes: Array<'none' | 'vertical' | 'horizontal' | 'both' | undefined> = ['none', 'vertical', 'horizontal', 'both', undefined ];
+const Resizes: Array<'none' | 'vertical' | 'horizontal' | 'both'> = ['none', 'vertical', 'horizontal', 'both'];If you need to test the default/undefined behavior, create a separate story instance rather than including it in the mapped array.
📝 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 Variants = ['solid', 'soft', 'outline', 'ghost']; | |
| const Sizes = ['small', 'medium', 'large']; | |
| const Resizes: Array<'none' | 'vertical' | 'horizontal' | 'both' | undefined> = ['none', 'vertical', 'horizontal', 'both', undefined ]; | |
| const Radius = ['none', 'small', 'medium', 'large']; | |
| const Variants = ['solid', 'soft', 'outline', 'ghost']; | |
| const Sizes = ['small', 'medium', 'large']; | |
| const Resizes: Array<'none' | 'vertical' | 'horizontal' | 'both'> = ['none', 'vertical', 'horizontal', 'both']; | |
| const Radius = ['none', 'small', 'medium', 'large']; |
🤖 Prompt for AI Agents
In src/components/ui/TextArea/stories/TextArea.stories.tsx around lines 5 to 8,
the Resizes array includes undefined which will produce React key warnings when
mapped; remove undefined from the Resizes array and, if you need to demonstrate
the default/undefined behavior, create a separate story instance (or add a
specific case) that renders the TextArea without a resize prop instead of
including undefined in the mapped list.
| <TextArea disabled placeholder='this textarea is disabled'/> | ||
|
|
||
|
|
||
| <TextArea readonly className='w-10' placeholder='this textarea is readonly'/> |
There was a problem hiding this comment.
Verify the className 'w-10' for the readonly textarea.
The className='w-10' applies a width of 2.5rem (40px), which is extremely narrow for a textarea. This appears to be a typo.
Consider removing the className or using w-full for full width:
🔎 Proposed fix
- <TextArea readonly className='w-10' placeholder='this textarea is readonly'/>
+ <TextArea readonly placeholder='this textarea is readonly'/>📝 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.
| <TextArea readonly className='w-10' placeholder='this textarea is readonly'/> | |
| <TextArea readonly placeholder='this textarea is readonly'/> |
🤖 Prompt for AI Agents
In src/components/ui/TextArea/stories/TextArea.stories.tsx around line 18, the
readonly TextArea is given className='w-10' which sets a very narrow width
(2.5rem) likely by typo; remove the className or replace it with a more
appropriate utility such as 'w-full' (or no className) so the readonly textarea
renders at a usable width.
| readonly ?: boolean; | ||
| disabled ?: boolean; |
There was a problem hiding this comment.
Remove extra spaces before the optional type indicators.
The readonly and disabled props have unnecessary spaces before the ? operator.
🔎 Proposed fix
- readonly ?: boolean;
- disabled ?: boolean;
+ readonly?: boolean;
+ disabled?: boolean;📝 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.
| readonly ?: boolean; | |
| disabled ?: boolean; | |
| readonly?: boolean; | |
| disabled?: boolean; |
🤖 Prompt for AI Agents
In src/components/ui/TextArea/TextArea.tsx around lines 9 to 10, the prop
declarations use an extra space before the optional operator (`readonly ?:
boolean;` and `disabled ?: boolean;`); remove the space so the declarations read
`readonly?: boolean;` and `disabled?: boolean;` to match TypeScript syntax and
project linting.
| .rad-ui-text-area textarea:disabled, | ||
| .rad-ui-text-area textarea:read-only { | ||
| background-color: var(--rad-ui-color-accent-200); | ||
| color: var(--rad-ui-color-accent-600); | ||
| cursor: not-allowed; | ||
| } | ||
|
|
||
| .rad-ui-text-area textarea:disabled::placeholder, | ||
| .rad-ui-text-area textarea:read-only::placeholder { | ||
| color: var(--rad-ui-color-accent-500); | ||
| } |
There was a problem hiding this comment.
Fix CSS specificity ordering to resolve pipeline failures.
The linter reports that .rad-ui-text-area textarea:disabled and .rad-ui-text-area textarea:read-only selectors should come before .rad-ui-text-area[data-textarea-variant="ghost"] textarea:focus to maintain proper specificity ordering.
🔎 Proposed fix
Move the disabled and read-only styles before the ghost variant styles:
.rad-ui-text-area[data-textarea-variant="solid"] textarea {
background-color: var(--rad-ui-color-accent-100);
border: 2px solid var(--rad-ui-color-accent-400);
}
+.rad-ui-text-area textarea:disabled,
+.rad-ui-text-area textarea:read-only {
+ background-color: var(--rad-ui-color-accent-200);
+ color: var(--rad-ui-color-accent-600);
+ cursor: not-allowed;
+}
+
+.rad-ui-text-area textarea:disabled::placeholder,
+.rad-ui-text-area textarea:read-only::placeholder {
+ color: var(--rad-ui-color-accent-500);
+}
.rad-ui-text-area[data-textarea-variant="ghost"] textarea {
background-color: transparent;
border: 2px solid transparent;
}
.rad-ui-text-area[data-textarea-variant="ghost"] textarea:focus {
border-color: var(--rad-ui-color-accent-500);
}
-.rad-ui-text-area textarea:disabled,
-.rad-ui-text-area textarea:read-only {
- background-color: var(--rad-ui-color-accent-200);
- color: var(--rad-ui-color-accent-600);
- cursor: not-allowed;
-}
-
-.rad-ui-text-area textarea:disabled::placeholder,
-.rad-ui-text-area textarea:read-only::placeholder {
- color: var(--rad-ui-color-accent-500);
-}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Lint
[error] 133-133: no-descending-specificity: Expected selector ".rad-ui-text-area textarea:disabled" to come before selector ".rad-ui-text-area[data-textarea-variant="ghost"] textarea:focus".
[error] 134-134: no-descending-specificity: Expected selector ".rad-ui-text-area textarea:read-only" to come before selector ".rad-ui-text-area[data-textarea-variant="ghost"] textarea:focus".
🤖 Prompt for AI Agents
In styles/themes/components/textarea.scss around lines 133 to 143, the
disabled/read-only selector block is placed after higher-specificity
ghost-variant focus rules which breaks the linter's specificity ordering; move
the entire block for `.rad-ui-text-area textarea:disabled, .rad-ui-text-area
textarea:read-only { ... }` and its placeholder rules immediately before the
`.rad-ui-text-area[data-textarea-variant="ghost"] textarea:focus` rules
(preserving all property names and values) so disabled/read-only rules appear
earlier than the ghost-variant focus selectors to satisfy specificity ordering.
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.