refactor(theme): [input] refactor textarea theme vars #2281
Conversation
WalkthroughThe changes in this pull request involve a comprehensive update to the CSS variable naming conventions within the textarea component's styles. The prefix for CSS variables has been changed from Changes
Possibly related PRs
Suggested labels
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 (
|
![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
🧹 Outside diff range and nitpick comments (1)
packages/theme/src/textarea/vars.less (1)
41-61: LGTM: Consistent update to padding and count-related variablesThe changes to padding and count-related variables are consistent with the new naming convention, using the
--tv-prefix and PascalCase for "Textarea". The shortening of some variable names (e.g.,padding-verticaltopadding-y) aligns with common CSS naming conventions and improves readability.One minor suggestion for consistency:
- Consider changing
--tv-Textarea-padding-rightand--tv-Textarea-padding-bottomto--tv-Textarea-padding-rand--tv-Textarea-padding-brespectively, to match the shorthand used forpadding-yandpadding-x.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/theme/src/textarea/index.less (3 hunks)
- packages/theme/src/textarea/vars.less (1 hunks)
🧰 Additional context used
🔇 Additional comments (14)
packages/theme/src/textarea/vars.less (5)
13-13: LGTM: Mixin name change improves consistencyThe mixin name change from
.component-css-vars-textarea()to.inject-Textarea-vars()aligns better with a consistent naming convention, likely matching other components in the system. This change enhances code readability and maintainability.
15-21: LGTM: Consistent update to basic style variablesThe changes to text color, background color, and font size variables are consistent and follow the new naming convention. The use of
--tv-prefix and PascalCase for "Textarea" improves readability while maintaining the existing theming system.
23-31: LGTM: Consistent update to dimension and border variablesThe changes to height, border radius, and border color variables are consistent with the new naming convention. The use of
--tv-prefix and PascalCase for "Textarea" enhances code consistency while maintaining the existing theming structure.
33-39: LGTM: Consistent update to state-specific variablesThe changes to placeholder, disabled, and exceed state variables are consistent with the new naming convention. The use of
--tv-prefix and PascalCase for "Textarea" improves code consistency while maintaining the existing theming structure for different states.
Line range hint
1-61: Summary: Successful refactoring of textarea theme variablesThe changes in this file consistently update the CSS variable naming convention for the textarea component. The refactoring:
- Changes the prefix from
--ti-to--tv-- Uses PascalCase for "Textarea" in variable names
- Updates the mixin name to
.inject-Textarea-vars()- Maintains the existing theming system and functionality
These changes improve code consistency and readability while preserving the component's styling capabilities. Great job on this refactoring effort!
packages/theme/src/textarea/index.less (9)
29-29: LGTM: CSS variable prefix updated.The font-size CSS variable has been updated to use the new
--tv-prefix, which is consistent with the overall refactoring of CSS variable names. This change improves the specificity and clarity of the variable naming convention.
40-40: LGTM: Border-radius CSS variables updated.The border-radius CSS variables have been consistently updated to use the new
--tv-prefix. This change aligns with the overall refactoring effort and maintains the existing styling while improving variable naming consistency.Also applies to: 52-52
81-81: LGTM: Exceed state CSS variables updated consistently.All CSS variables related to the exceed state of the textarea (border color and text color) have been updated to use the new
--tv-prefix. This change is consistent with the overall refactoring and maintains the existing styling.Also applies to: 85-85, 88-88
131-131: LGTM: Hover and focus state CSS variables updated consistently.All CSS variables related to the hover and focus states of the textarea (border colors) have been updated to use the new
--tv-prefix. This change is consistent with the overall refactoring and maintains the existing styling.Also applies to: 135-135, 139-139
Line range hint
1-280: Summary: Comprehensive update of CSS variable names and mixin injection.This pull request successfully refactors the textarea component's styles by:
- Updating all CSS variable names from the
--ti-prefix to the--tv-prefix.- Changing the mixin name from
.component-css-vars-textarea()to.inject-Textarea-vars().These changes improve consistency and specificity in the naming convention, aligning with the Tiny Vue project standards. The overall structure and functionality of the styles remain intact, ensuring that this refactoring should not introduce any breaking changes.
To ensure the refactoring is complete and correct:
- Verify that all
--ti-prefixed variables have been replaced.- Check that the new
.inject-Textarea-vars()mixin is properly implemented and used.- Conduct thorough testing of the textarea component to confirm that all styles are applied correctly, especially for different states (disabled, exceed, hover, focus) and sub-components (inner textarea, count element).
Run the following script to ensure all
--ti-prefixes have been replaced:#!/bin/bash # Check for any remaining --ti- prefixes in the textarea styles rg --type less "\-\-ti\-" packages/theme/src/textarea/
94-96: LGTM: Count-related CSS variables updated consistently.All CSS variables related to the count element of the textarea have been updated to use the new
--tv-prefix. This includes color, background, font size, positioning, and border radius. The changes are consistent with the overall refactoring and maintain the existing styling.Note: There's a slight change in the width calculation on line 103, which might affect the layout.
To ensure the count element's layout remains correct, please verify the rendered styles:
#!/bin/bash # Verify the count element styling and layout rg --type less "input__count" packages/theme/src/textarea/Also applies to: 101-105, 108-108
56-58: LGTM: Disabled state CSS variables updated consistently.All CSS variables related to the disabled state of the textarea (background color, border color, text color, and placeholder color) have been updated to use the new
--tv-prefix. This change is consistent with the overall refactoring and maintains the existing styling.To ensure the disabled state styling remains intact, please verify the rendered styles:
Also applies to: 60-60, 64-64, 67-67, 71-71, 75-75
23-23: LGTM: Mixin renamed for consistency.The mixin has been renamed from
.component-css-vars-textarea()to.inject-Textarea-vars(), which appears to be part of a larger refactoring effort. The new name is more descriptive and follows a consistent naming pattern.To ensure the refactoring is complete, please verify the implementation of the new mixin:
✅ Verification successful
Verified: The old mixin
.component-css-vars-textarea()has been successfully removed from the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the implementation of the new mixin rg --type less "inject-Textarea-vars\(\)" packages/theme/src/Length of output: 193
Script:
#!/bin/bash # Verify that the old mixin is no longer used rg --type less "\.component-css-vars-textarea\(\)" packages/theme/src/Length of output: 72
114-124: LGTM: Inner textarea CSS variables updated consistently.All CSS variables related to the inner textarea have been updated to use the new
--tv-prefix. This includes height, colors, border, padding, and font size. The changes are consistent with the overall refactoring and maintain the existing styling.Note: There's a new calculation for the minimum height on line 116 using a CSS variable, which provides more flexibility in theming.
To ensure the textarea's layout and styling remain correct, please verify the rendered styles:
Also applies to: 128-128
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/theme/src/textarea/vars.less (2)
15-63: Consider standardizing variable naming to kebab-caseThe CSS custom properties use mixed casing, such as
--tv-Textarea-text-color. It's generally recommended to use kebab-case and lowercase letters for CSS custom properties for consistency and readability. Consider updating variable names to--tv-textarea-text-colorfor consistency with standard CSS conventions.
63-63: Use unitless line-height for better scalabilityLine 63 sets the
--tv-Textarea-count-line-heightto the value of--tv-Textarea-count-font-size:--tv-Textarea-count-line-height: var(--tv-Textarea-count-font-size);Consider using a unitless number for
line-heightto enhance scalability across different font sizes and ensure consistent spacing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/theme/src/textarea/index.less (5 hunks)
- packages/theme/src/textarea/vars.less (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/theme/src/textarea/index.less
🧰 Additional context used
🔇 Additional comments (1)
packages/theme/src/textarea/vars.less (1)
13-63: Variables successfully updated to new naming conventionThe variables have been consistently updated to use the new
--tv-prefix, and the method name has been changed to.inject-Textarea-vars(), aligning with the new theming system.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Chores