fix(theme): fix error css vars #2338
Conversation
WalkthroughThe pull request includes modifications to several LESS stylesheets, primarily focusing on correcting variable references and enhancing the styling of various components. Key changes involve fixing syntax errors in variable declarations, refining table component styles for different sizes and states, and updating color properties in the search and select dropdown components. The adjustments ensure proper rendering and consistent styling across the application, without introducing significant changes to the overall structure or logic. Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
![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 (4)
packages/theme/src/cascader-panel/vars.less (1)
29-29: Correct CSS variable reference and consider refactoringThe fix properly references the
--tv-space-xlvariable, which is correct and consistent with the previous change.Consider refactoring to avoid repetition:
--tv-CascaderPanel-node-label-padding: var(--tv-space-xl, 16px); --tv-CascaderPanel-node-label-padding-right: var(--tv-CascaderPanel-node-label-padding); --tv-CascaderPanel-node-label-padding-left: var(--tv-CascaderPanel-node-label-padding);This approach would centralize the padding value and make future updates easier.
packages/theme/src/base/reset.less (1)
34-34: Approved: CSS variable syntax corrected.The change from
----tv-color-text-activeto--tv-color-text-activefixes the CSS variable syntax, which is correct and consistent with other files in this PR.Consider adding a comment explaining why
!importantis necessary here, as it's generally best to avoid using it unless absolutely required:.tiny-hl-query-node { // Use !important to ensure consistent highlighting across all contexts color: var(--tv-color-text-active) !important; }packages/theme/src/grid/table.less (2)
927-929: Good catch: Fixed CSS variable syntaxThis change corrects the syntax for the CSS variable
--tv-Grid-success-border-color. Using two dashes instead of three ensures that the variable will be properly recognized and applied.Consider adding a comment explaining the significance of this color, e.g., "// Indicates a successfully validated dirty cell".
Line range hint
1-1284: Overall: Well-structured and comprehensive grid stylesThis LESS file provides a robust set of styles for a complex grid/table component. The minor fix to the CSS variable syntax improves the code quality. The file demonstrates good organization, covers various scenarios and states, and should provide a solid foundation for the grid component's appearance and behavior.
Consider adding more comments throughout the file to explain the purpose of different sections and complex selectors. This would improve maintainability for future developers working on this stylesheet.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- packages/theme/src/base/reset.less (1 hunks)
- packages/theme/src/cascader-panel/vars.less (1 hunks)
- packages/theme/src/grid/table.less (1 hunks)
- packages/theme/src/search/index.less (1 hunks)
- packages/theme/src/select-dropdown/vars.less (1 hunks)
🧰 Additional context used
🔇 Additional comments (8)
packages/theme/src/cascader-panel/vars.less (2)
27-27: Correct CSS variable referenceThe fix properly references the
--tv-space-xlvariable. This correction is crucial for ensuring the correct padding is applied to the CascaderPanel node labels.
27-29: Summary of changes and their impactThese corrections to the CSS variable references are crucial for the proper styling of the CascaderPanel component. They align with similar fixes across other files mentioned in the PR summary, indicating a systematic resolution of incorrect variable references.
To ensure the effectiveness of these changes:
Run the following script to check for any remaining incorrect CSS variable references in the theme directory:
This will help verify that all similar issues have been addressed across the theme files.
packages/theme/src/select-dropdown/vars.less (1)
21-21: Correct CSS variable referenceThe change from
var(---tv-color-text-secondary, #595959)tovar(--tv-color-text-secondary, #595959)is correct. This fixes the CSS custom property reference, ensuring the proper variable is used for the select dropdown tip text color.To ensure consistency across the codebase, let's verify if there are any remaining instances of incorrectly formatted CSS variable references:
If this script returns any results, those instances should also be corrected.
✅ Verification successful
All CSS variable references correctly formatted
No instances of incorrectly formatted CSS variable references found beyond the reviewed change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for CSS variables with more than two leading dashes # Test: Search for CSS var() calls with more than two leading dashes rg -g '*.{css,less,scss,vue}' 'var\(-{3,}[a-zA-Z0-9-]+\)'Length of output: 59
Script:
#!/bin/bash # Description: Check for CSS var() calls with more than two leading dashes # Test: Search for var() with three or more dashes in CSS, LESS, SCSS, and Vue files rg -g '*.{css,less,scss,vue}' 'var\(-{3,}[a-zA-Z0-9-]+\)'Length of output: 59
packages/theme/src/search/index.less (1)
195-195: Approved: Corrected CSS variable for selector text colorThis change improves the clarity and correctness of the styling for the search selector text color. The previous value appeared to be an erroneous concatenation of two different CSS variables, which could have led to unexpected styling behavior. The new value
var(--tv-Search-selector-text)is a properly named, single CSS variable that accurately represents the intended styling property.This modification enhances code readability and maintains consistency in the theme's variable naming convention.
packages/theme/src/grid/table.less (4)
Line range hint
1-926: LGTM: Well-structured and comprehensive stylesThe first part of this LESS file demonstrates a well-organized approach to styling a complex grid/table component. It covers various scenarios, states, and responsive designs, which is commendable.
Line range hint
930-1135: LGTM: Comprehensive styling for form elements and grid componentsThis section provides detailed styling for various form inputs, filters, and grid components. The code is well-organized and covers different states and interactions.
Line range hint
1136-1270: LGTM: Well-defined styles for interactive elementsThis section provides comprehensive styling for column anchors, operation columns, and row validation. The code is well-organized and covers various interactive states, enhancing the user experience.
Line range hint
1271-1284: LGTM: Clear and concise fullscreen stylesThe fullscreen styles are well-defined and should provide a good user experience when the table is displayed in fullscreen mode.
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
Release Notes
New Features
Bug Fixes
Improvements