fixed textarea issues and lint #1670
Conversation
|
📝 WalkthroughWalkthroughTest updated to pass an explicit placeholder to TextArea and assert it; minor whitespace/formatting and punctuation edits across TextArea fragments, stories, and Combobox files; textarea SCSS selectors were nested and disabled/read-only rules consolidated without property changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (5 passed)
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: 0
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/Combobox/Combobox.tsx (1)
27-27: Add missing semicolon for consistency.Line 27 is missing a semicolon, which is inconsistent with the other property declarations in the interface (lines 20-26 all have semicolons). Since this PR addresses linting issues, this should be fixed.
🔧 Proposed fix
- Search: typeof ComboboxSearch + Search: typeof ComboboxSearch;
🧹 Nitpick comments (1)
styles/themes/components/textarea.scss (1)
32-35: Consider using a more muted placeholder color for disabled/read-only states.The placeholder color for disabled and read-only states matches the normal state (both use
accent-500). Typically, disabled states benefit from more muted colors to provide clearer visual distinction.💡 Suggested color adjustment
.rad-ui-text-area textarea:disabled::placeholder, .rad-ui-text-area textarea:read-only::placeholder { - color: var(--rad-ui-color-accent-500); + color: var(--rad-ui-color-accent-400); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
src/components/ui/Combobox/Combobox.tsxsrc/components/ui/Combobox/stories/Combobox.stories.tsxstyles/themes/components/textarea.scss
💤 Files with no reviewable changes (1)
- src/components/ui/Combobox/stories/Combobox.stories.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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`.
⏰ 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). (3)
- GitHub Check: coverage
- GitHub Check: Jest (1/2)
- GitHub Check: Jest (2/2)
🔇 Additional comments (2)
styles/themes/components/textarea.scss (1)
25-30: LGTM! Disabled and read-only styles are well-defined.The explicit styling for disabled and read-only states provides clear visual feedback with appropriate color changes and cursor behavior.
src/components/ui/Combobox/Combobox.tsx (1)
39-39: Good addition of semicolon for consistency.This change aligns with the other property assignments (lines 32-38) and improves code style consistency.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
styles/themes/components/textarea.scss (2)
25-34: Consider different cursor for read-only state.The
cursor: not-allowedon line 29 applies to both:disabledand:read-onlystates. Read-only fields are typically still selectable and copyable, so atextordefaultcursor might better communicate "viewable but not editable" rather than "blocked."🔎 Optional refinement
- &:disabled, - &:read-only { + &:disabled { background-color: var(--rad-ui-color-accent-200); color: var(--rad-ui-color-accent-600); cursor: not-allowed; &::placeholder { color: var(--rad-ui-color-accent-500); } } + + &:read-only { + background-color: var(--rad-ui-color-accent-200); + color: var(--rad-ui-color-accent-600); + cursor: text; + + &::placeholder { + color: var(--rad-ui-color-accent-500); + } + }
42-59: Note: Scrollbar styling is webkit-only.The scrollbar customization uses webkit-specific pseudo-elements (
::-webkit-scrollbar-*), which won't apply in Firefox or other non-webkit browsers. This is likely intentional progressive enhancement, but if cross-browser scrollbar styling is needed, consider adding Firefox support usingscrollbar-colorandscrollbar-width.🔎 Optional cross-browser enhancement
Add Firefox scrollbar support within the
textareablock:/* Firefox scrollbar styling */ scrollbar-width: thin; scrollbar-color: var(--rad-ui-color-accent-400) transparent;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
styles/themes/components/textarea.scss
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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`.
⏰ 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). (2)
- GitHub Check: Build
- GitHub Check: coverage
🔇 Additional comments (1)
styles/themes/components/textarea.scss (1)
1-134: LGTM! Clean SCSS refactoring.The nested structure improves maintainability while preserving selector specificity and all existing styles. The nesting follows SCSS best practices and the data attribute modifiers are correctly placed.
Coverage
✅ Coverage thresholds met! All tests passing. Run |
fixes #1666
Summary by CodeRabbit
Tests
Style
✏️ Tip: You can customize this high-level summary in your review settings.