Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(old-theme): [dropdown,grid,tree] modify incorrect css vars #2551

Merged
merged 1 commit into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/theme/src/dropdown/index.less
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
padding: var(--tv-Dropdown-title-button-padding);
border-right: none;
border-radius: var(--tv-Dropdown-title-button-border-radius);
min-width: auto;
}

.@{dropdown-prefix-cls}__caret-button {
Expand Down
2 changes: 1 addition & 1 deletion packages/theme/src/grid/vars.less
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
// 表格激活态背景色
--tv-Grid-bg-color-active: var(--tv-color-bg-active);
// 表格悬浮背景色
--tv-Grid-bg-color-hover: var(--tv-color-bg);
--tv-Grid-bg-color-hover: var(--tv-color-bg-hover);
// 表格禁用背景色
--tv-Grid-bg-color-disabled: var(--tv-color-bg-disabled);
// 表格选中背景色
Expand Down
2 changes: 1 addition & 1 deletion packages/theme/src/tree/vars.less
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
// 选中节点背景色
--tv-Tree-node-content-current-bg-color: var(--tv-color-bg-secondary, #ffffff);
// 节点悬浮背景色
--tv-Tree-node-content-hover-bg-color: var(--tv-color-bg, #f5f5f5);
--tv-Tree-node-content-hover-bg-color: var(--tv-color-bg-hover, #f5f5f5);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Inconsistent hover background color variables need standardization

Several components are still using --tv-color-bg for hover states instead of the semantically correct --tv-color-bg-hover:

  • packages/theme/src/transfer/vars.less: --tv-Transfer-panel-item-hover-bg-color
  • packages/theme/src/pager/vars.less: --tv-Pager-page-item-bg-color-hover
  • packages/theme/src/drop-times/vars.less: --tv-DropTimes-item-select-hover-bg-color
  • packages/theme/src/cascader-node/vars.less: --tv-CascaderNode-selectable-hover-bg-color
  • packages/theme/src/button/vars.less: --tv-Button-bg-color-only-icon-ontext-hover

While this PR correctly updates the Tree component, similar standardization is needed across these other components to maintain consistency in hover state styling.

🔗 Analysis chain

Verify the hover state consistency across components

The change from --tv-color-bg to --tv-color-bg-hover aligns with similar updates in the grid component and provides a more semantically correct variable for hover states.

Let's verify this change is consistent across related components:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for hover background color variables across components
# to ensure consistent usage of --tv-color-bg-hover

# Search for hover background color variables
rg --type=less "hover-bg-color.*tv-color-bg" packages/theme/src/

# Search for any remaining usage of --tv-color-bg in hover contexts
rg --type=less "hover.*tv-color-bg[^-]" packages/theme/src/

Length of output: 3722

// 节点字号
--tv-Tree-node-label-font-size: var(--tv-font-size-default, 14px);
// 节点内容左边距
Expand Down
Loading