-
Notifications
You must be signed in to change notification settings - Fork 294
fix(grid): [grid] update dark theme #3123
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
WalkthroughThe pull request updates the CSS styling for several grid components. Hardcoded background colors in Vue component files and LESS theme files have been replaced with CSS variables, resulting in more dynamic theming. Additionally, tests have been updated to reflect the new color values. No changes were made to any exported or public entities. Changes
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
ERR_PNPM_OPTIONAL_DEPS_REQUIRE_PROD_DEPS Optional dependencies cannot be installed without production dependencies Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 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 (
|
WalkthroughThis pull request updates the dark theme for the grid component in the Changes
|
WalkthroughThis pull request updates the dark theme for the grid component in the Changes
|
Walkthrough:This PR updates the dark theme for the grid component by replacing hardcoded background colors with CSS variables for better theme management and consistency. It also updates the corresponding test cases to reflect these changes. Changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/theme/src/grid/vars.less (5)
142-143
: Dark Theme Filter Input Background Variable Added
The new CSS variable--tv-Grid-filter-input-bg-color
is introduced to define the filter input’s background. Please verify that usingvar(--tv-color-bg-secondary)
aligns with your dark theme design for the grid filters.
183-186
: Review Input Padding Variable Comments
There appears to be a mismatch between the comments and the variable names for input padding. The comment on line 183 indicates a vertical padding dimension while the variable--tv-Grid-input-padding-x
(typically representing horizontal padding) is used. Likewise, the comment on line 185 indicating horizontal padding precedes the use of--tv-Grid-input-padding-y
(typically vertical). Please confirm the intended semantics and update either the comments or the variable names accordingly for clarity.
191-192
: Typo in Input Font Size Comment
The comment on line 191 reads “内置输入框框字体大小” which seems to include an extra “框”. For clarity and consistency, consider revising it to “内置输入框字体大小”.
207-210
: Mismatch in Button Property Comment
The comment on line 209 incorrectly refers to button text color, while the declared variable--tv-Grid-button-border-radius
actually sets the button's border radius. Please update the comment to accurately reflect that this variable controls the button’s border radius.
215-220
: Dark Theme Variables for Select and Loading Mask
The updated variables for the select dropdown (--tv-Grid-select-option-bg-color
and--tv-Grid-select-option-bg-color-hover
) as well as the loading mask (--tv-Grid-loading-mask-bg-color
) correctly incorporate dark theme values. Ensure that these values consistently align with your overall dark theme palette for the grid components.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
examples/sites/demos/pc/app/grid/custom-style/class-name-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/grid/custom-style/class-name.spec.js
(1 hunks)examples/sites/demos/pc/app/grid/custom-style/class-name.vue
(1 hunks)examples/sites/demos/pc/app/grid/editor/active-strictly-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/grid/editor/active-strictly.vue
(1 hunks)examples/sites/demos/pc/app/grid/editor/editor-bg-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/grid/editor/editor-bg.vue
(1 hunks)examples/sites/demos/pc/app/grid/loading/grid-custom-loading-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/grid/loading/grid-custom-loading.vue
(1 hunks)packages/theme/src/grid/filter.less
(8 hunks)packages/theme/src/grid/loading.less
(1 hunks)packages/theme/src/grid/menu.less
(1 hunks)packages/theme/src/grid/table.less
(2 hunks)packages/theme/src/grid/toolbar.less
(1 hunks)packages/theme/src/grid/vars.less
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (20)
examples/sites/demos/pc/app/grid/custom-style/class-name.vue (1)
87-87
: Updated background styling for better dark theme compatibilityThe change from using
background
tobackground-color
with a more saturated blue color (#2db7f5) improves visibility in dark theme. This is in line with the PR's objective of updating the grid's dark theme.examples/sites/demos/pc/app/grid/custom-style/class-name-composition-api.vue (1)
78-78
: Consistent styling update across API variantsThe same background color update applied to the Options API version has been correctly applied here to the Composition API version, ensuring visual consistency across implementation styles.
examples/sites/demos/pc/app/grid/custom-style/class-name.spec.js (1)
6-6
: Test updated to match new styling valuesThe test expectation has been properly updated to check for the new background color value (rgb(45, 183, 245)), which is the RGB equivalent of the #2db7f5 hex color used in the components.
examples/sites/demos/pc/app/grid/loading/grid-custom-loading-composition-api.vue (1)
73-73
: Improved theming with CSS variablesReplacing the hardcoded background color with a CSS variable (
--tv-color-bg-active-dark
) enhances theme customization capabilities, particularly for dark themes. This change is aligned with modern CSS practices and supports the PR's goal of updating the dark theme.examples/sites/demos/pc/app/grid/editor/active-strictly.vue (1)
88-92
: Replace Hardcoded Color with CSS Variable in Disabled Cell Styling
Changing the background color in the.disable-cell-edit
class from a fixed value tovar(--tv-color-bg-disabled)
ensures that the component now inherits the dynamic theming provided by the dark theme update. This enhances customization and consistency across the grid components.packages/theme/src/grid/loading.less (1)
27-27
: Utilize CSS Variable for Loading Mask Background
The update from a hardcoded RGBA color tovar(--tv-Grid-loading-mask-bg-color)
in the loading mask styling improves the flexibility and maintainability of the theme. Please verify that this CSS variable is defined appropriately in the project's global theme settings.examples/sites/demos/pc/app/grid/editor/active-strictly-composition-api.vue (1)
80-83
: Adopt CSS Variable for Disabled Cell Background
Updating the.disable-cell-edit
class with the CSS variablevar(--tv-color-bg-disabled)
aligns this component with the new dark theme standards and ensures a unified styling approach across grid editors.examples/sites/demos/pc/app/grid/editor/editor-bg.vue (1)
84-87
: Standardize Disabled Cell Styling Using CSS Variable
Changing the background color of the.disable-cell-edit
class tovar(--tv-color-bg-disabled)
promotes consistency within the grid’s theming. This refactor supports centralized control over theme colors, making future adjustments easier.examples/sites/demos/pc/app/grid/editor/editor-bg-composition-api.vue (1)
75-78
: Enhance Theming Flexibility with CSS Variable
Switching the background color property in the.disable-cell-edit
style to usevar(--tv-color-bg-disabled)
meets the dark theme update objective. This change contributes to a more dynamic, customizable interface for grid components.examples/sites/demos/pc/app/grid/loading/grid-custom-loading.vue (1)
86-86
: Updated Loading Background Styling
The direct RGBA color has been replaced with the CSS variablevar(--tv-color-bg-active-dark)
, which supports dynamic theming for the dark grid mode. Please verify that this variable is properly defined in the theme’s LESS files.packages/theme/src/grid/menu.less (1)
28-28
: Applied CSS Variable for Grid Menu Background
The hardcoded background color is now replaced withvar(--tv-Grid-bg-color)
, enhancing theme flexibility across the grid menu component. Please ensure that this variable’s value is set appropriately in the theme variables file.packages/theme/src/grid/toolbar.less (1)
60-60
: Updated Loading Mask Background
The background color for the loading mask is now defined via the CSS variablevar(--tv-Grid-loading-mask-bg-color)
, aligning it with the dynamic theming approach. Confirm that the variable is updated to match the dark theme requirements.packages/theme/src/grid/table.less (1)
433-439
: Refined Row Hover Highlight Behavior
The selector has been updated to use.row__highlight &-body__row:not(.row__selected):hover
, which prevents the hover effect from overriding the styling of selected rows. This enhances clarity and user experience.packages/theme/src/grid/filter.less (7)
65-69
: Enhanced Filter Body Styling
The filter body now uses a fixed10px
padding and the background color is set viavar(--tv-Grid-bg-color)
, ensuring consistency with the overall dark theme.
71-93
: Improved Filter Input Styles
All inputs (excluding radio types) within the filter panel now leverage CSS variables for properties such as border, border-radius, color, background color, and padding. The hover, focus, and active states are also explicitly defined, contributing to a consistent and refined theme.
95-112
: Standardized Filter Button Styling
The button styles are now standardized using theme-specific variables for font weight, border, background, and padding. This update guarantees a uniform appearance across the grid’s filter component.
159-163
: Consistent Empty State Text Color
The text color for the filter empty state is updated to usevar(--tv-Grid-text-color)
, ensuring that the styling remains consistent with the dark theme design.
277-286
: Updated Filter Input Field Background
The filter option input field now sets its background usingvar(--tv-Grid-filter-input-bg-color)
, along with standardized border, padding, and height. This change helps integrate the component seamlessly with the dark theme.
187-203
: Refined Checkbox Icon Styling in Filter Options
Within the filter options, the checkbox icons have been improved: the first path is set to transparent by default, and when checked (&.is-checked
), the first path fills withvar(--tv-Grid-checkbox-icon-color-selected)
while the last path becomes white. Additionally, the hover state is updated usingvar(--tv-Grid-checkbox-icon-color-hover)
. These changes ensure interactive feedback aligns with the overall theming.
312-318
: Consistent Filter Date Item Styling
The filter date items now employ flex properties for alignment and use the theme’s text color (viavar(--tv-Grid-text-color)
). This minor adjustment enhances readability and ensures consistency with the dark theme.
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