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: optimize basic variables and switch the overall rounded corner … #2580

Merged
merged 1 commit into from
Nov 30, 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/base/old-theme.less
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@
--tv-border-radius-md: 2px; // 默认
--tv-border-radius-lg: 4px;
--tv-border-radius-round: 999px; // 50% 会造成椭圆,避免使用。 999px是 tiny3的做法。
--tv-border-radius-brand: 2px; // 品牌相关。用于圆角,按钮、分页等场景

/** 5. 边框 **/
--tv-border-width: 1px;
Expand Down
3 changes: 2 additions & 1 deletion packages/theme/src/base/vars.less
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,8 @@
--tv-border-radius-sm: 4px;
--tv-border-radius-md: 6px; // 默认
--tv-border-radius-lg: 8px;
--tv-border-radius-round: 999px; // 50% 会造成椭圆,避免使用。 999px是 tiny3的做法。
--tv-border-radius-round: 999px; // 50% 会造成椭圆,避免使用。
--tv-border-radius-brand: var(--tv-border-radius-round); // 品牌相关。用于圆角,按钮、分页等场景
Comment on lines +403 to +404
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Consider updating border-radius declarations to use the new brand variable

Based on the verification results, we found:

  • Multiple components using hardcoded border-radius: 50% for circular elements (avatars, dots, icons)
  • Several mobile components using hardcoded border-radius: 999px that could benefit from the new variable
  • Limited adoption of --tv-border-radius-brand (only in pager and guide components)

Suggested updates:

  • Keep border-radius: 50% for truly circular elements like avatars and dots
  • Update the following components to use --tv-border-radius-brand:
    • Mobile button component (packages/theme-mobile/src/button/index.less)
    • Mobile switch component (packages/theme-mobile/src/switch/index.less)
    • Mobile radio component (packages/theme-mobile/src/radio/index.less)
    • Mobile numeric component (packages/theme-mobile/src/numeric/index.less)
🔗 Analysis chain

Verify consistent border radius usage across components

Let's ensure the new brand border radius is being applied consistently and there aren't any hardcoded border radius values that should be using this variable instead.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for hardcoded border radius values and current usage of the new variable

# Check for hardcoded border radius values that might need to be replaced
echo "Checking for hardcoded border radius values:"
rg -g '*.{less,css,vue}' '(border-radius:\s*999px|border-radius:\s*50%)'

# Check current usage of the new brand border radius variable
echo -e "\nChecking usage of new brand border radius variable:"
rg -g '*.{less,css,vue}' 'var\(--tv-border-radius-brand\)'

Length of output: 6277


/** 5. 边框 **/
--tv-border-width: 1px;
Expand Down
12 changes: 6 additions & 6 deletions packages/theme/src/date-table/vars.less
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
// 表头文本色
--tv-DateTable-th-text-color: var(--tv-color-text-weaken, #808080);
// 单元格圆角
--tv-DateTable-td-border-radius: var(--tv-border-radius-round, 999px);
--tv-DateTable-td-border-radius: var(--tv-border-radius-brand, 999px);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Update fallback values to match the new design system

The changes switch from --tv-border-radius-round to --tv-border-radius-brand (which is 2px according to the PR description), but the fallback values are still set to 999px. This creates an inconsistent visual appearance when the brand variable is not available.

Apply this change to all affected border-radius variables:

- --tv-DateTable-td-border-radius: var(--tv-border-radius-brand, 999px);
+ --tv-DateTable-td-border-radius: var(--tv-border-radius-brand, 2px);

- --tv-DateTable-border-radius-current-select: var(--tv-border-radius-brand, 999px);
+ --tv-DateTable-border-radius-current-select: var(--tv-border-radius-brand, 2px);

- --tv-DateTable-td-today-border-radius: var(--tv-border-radius-brand, 999px);
+ --tv-DateTable-td-today-border-radius: var(--tv-border-radius-brand, 2px);

- --tv-DateTable-start-date-border-radius: var(--tv-border-radius-brand, 999px);
+ --tv-DateTable-start-date-border-radius: var(--tv-border-radius-brand, 2px);

- --tv-DateTable-cell-border-radius-selected: var(--tv-border-radius-brand, 999px);
+ --tv-DateTable-cell-border-radius-selected: var(--tv-border-radius-brand, 2px);

- --tv-DateTable-td-border-radius-disabled: var(--tv-border-radius-brand, 999px);
+ --tv-DateTable-td-border-radius-disabled: var(--tv-border-radius-brand, 2px);

Also applies to: 36-36, 63-63, 74-74, 76-76, 80-80

// 选中项的圆角
--tv-DateTable-border-radius-current-select: var(--tv-border-radius-round, 999px);
--tv-DateTable-border-radius-current-select: var(--tv-border-radius-brand, 999px);
// 上个月文本色
--tv-DateTable-td-pre-month-text-color: var(--tv-color-text-weaken, #808080);
// 选中日期文本色
Expand All @@ -60,7 +60,7 @@
// 今天的高度
--tv-DateTable-td-today-height: var(--tv-size-height-sm, 28px);
// 今天的圆角
--tv-DateTable-td-today-border-radius: var(--tv-border-radius-round, 999px);
--tv-DateTable-td-today-border-radius: var(--tv-border-radius-brand, 999px);
// 今天的边框色
--tv-DateTable-td-today-circle-border-color: var(--tv-color-border-active, #191919);

Expand All @@ -71,13 +71,13 @@
// 单元格顶部间距
--tv-DateTable-td-span-top: 3px;
// 开始日期圆角
--tv-DateTable-start-date-border-radius: var(--tv-border-radius-round, 999px);
--tv-DateTable-start-date-border-radius: var(--tv-border-radius-brand, 999px);
// 选中日期圆角
--tv-DateTable-cell-border-radius-selected: var(--tv-border-radius-round, 999px);
--tv-DateTable-cell-border-radius-selected: var(--tv-border-radius-brand, 999px);
// 行高
--tv-DateTable-tr-line-height: var(--tv-line-height-number, 1.5);
// 禁用单元格圆角
--tv-DateTable-td-border-radius-disabled: var(--tv-border-radius-round, 999px);
--tv-DateTable-td-border-radius-disabled: var(--tv-border-radius-brand, 999px);
// 星期行高
--tv-DateTable-week-height: var(--tv-size-height-md, 32px);
}
6 changes: 3 additions & 3 deletions packages/theme/src/dropdown/vars.less
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@
// 按钮组时,下拉按钮的左侧内边距
--tv-Dropdown-caret-button-padding-left: var(--tv-space-base, 4px);
// 按钮组时,左侧按钮圆角
--tv-Dropdown-caret-button-border-radius: 0 var(--tv-border-radius-round, 999px) var(--tv-border-radius-round, 999px)
--tv-Dropdown-caret-button-border-radius: 0 var(--tv-border-radius-brand, 999px) var(--tv-border-radius-brand, 999px)
0;
// 按钮组时,右侧按钮圆角
--tv-Dropdown-title-button-border-radius: var(--tv-border-radius-round, 999px) 0 0
var(--tv-border-radius-round, 999px);
--tv-Dropdown-title-button-border-radius: var(--tv-border-radius-brand, 999px) 0 0
var(--tv-border-radius-brand, 999px);
// 按钮组时,文本按钮内边距
--tv-Dropdown-title-button-padding: 0px 0px 0px 24px;
}
2 changes: 1 addition & 1 deletion packages/theme/src/guide/vars.less
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
// 引导框按钮默认样式边框色
--tv-Guide-button-border-color: var(--tv-color-border-secondary);
// 引导框按钮默认样式圆角
--tv-Guide-button-border-radius: var(--tv-border-radius-round);
--tv-Guide-button-border-radius: var(--tv-border-radius-brand);
// 引导框按钮默认样式背景色
--tv-Guide-button-bg-color: var(--tv-color-bg-secondary);
// 引导框按钮默认样式悬浮边框色
Expand Down
14 changes: 7 additions & 7 deletions packages/theme/src/month-table/vars.less
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@
)
0;
// 单元格圆角(选中)
--tv-MonthTable-cell-border-radius: var(--tv-border-radius-round, 999px);
--tv-MonthTable-cell-border-radius: var(--tv-border-radius-brand, 999px);
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 border-radius variable usage detected across components

Several components are still using --tv-border-radius-round directly instead of --tv-border-radius-brand:

  • packages/theme/src/slider/vars.less: Slider handle
  • packages/theme/src/skeleton-item/vars.less: SkeletonItem circle
  • packages/theme/src/switch/vars.less: Switch inner
  • packages/theme/src/radio/vars.less: Radio inner
  • packages/theme/src/image-viewer/vars.less: ImageViewer actions
  • packages/theme/src/button/vars.less: Button round and circle variants
  • packages/theme/src/badge/vars.less: Badge
  • packages/theme/src/calendar-view/vars.less: CalendarView mark

For consistency with the MonthTable changes and the broader styling update, these components should also use --tv-border-radius-brand instead of directly referencing --tv-border-radius-round.

🔗 Analysis chain

LGTM! Verify visual consistency across states

The change from --tv-border-radius-round to --tv-border-radius-brand aligns with the PR objectives. The fallback value ensures backward compatibility.

Let's verify this change is consistent across the component library:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining usage of the old border-radius variable
rg --type=less "tv-border-radius-round" packages/theme/src/

Length of output: 1358

// 单元格圆角(开始月份)
--tv-MonthTable-cell-border-radius-start: var(--tv-border-radius-round, 999px) 0 0
var(--tv-border-radius-round, 999px);
--tv-MonthTable-cell-border-radius-start: var(--tv-border-radius-brand, 999px) 0 0
var(--tv-border-radius-brand, 999px);
// 单元格圆角(结束月份)
--tv-MonthTable-cell-border-radius-end: 0 var(--tv-border-radius-round, 999px) var(--tv-border-radius-round, 999px) 0;
--tv-MonthTable-cell-border-radius-end: 0 var(--tv-border-radius-brand, 999px) var(--tv-border-radius-brand, 999px) 0;
// 选中月份每行第一个单元格的圆角
--tv-MonthTable-cell-border-radius-first: var(--tv-border-radius-round, 999px) 0 0
var(--tv-border-radius-round, 999px);
--tv-MonthTable-cell-border-radius-first: var(--tv-border-radius-brand, 999px) 0 0
var(--tv-border-radius-brand, 999px);
// 选中月份每行最后一个单元格的圆角
--tv-MonthTable-cell-border-radius-last: 0 var(--tv-border-radius-round, 999px) var(--tv-border-radius-round, 999px) 0;
--tv-MonthTable-cell-border-radius-last: 0 var(--tv-border-radius-brand, 999px) var(--tv-border-radius-brand, 999px) 0;

// 单元格边框色(选中)
--tv-MonthTable-cell-border-color-today: var(--tv-color-border-active, #191919);
Expand Down
2 changes: 1 addition & 1 deletion packages/theme/src/pager/vars.less
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
// 分页页码项默认悬浮边框色
--tv-Pager-page-item-border-color-hover: transparent;
// 分页页码项项圆角
--tv-Pager-page-item-border-radius: var(--tv-border-radius-round);
--tv-Pager-page-item-border-radius: var(--tv-border-radius-brand);
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistency found in border radius styling across components

Several components still use --tv-border-radius-round directly instead of --tv-border-radius-brand:

  • Switch: Inner border radius in switch/vars.less
  • Slider: Handle border radius in slider/vars.less
  • SkeletonItem: Circle border radius in skeleton-item/vars.less
  • Radio: Inner border radius in radio/vars.less
  • ImageViewer: Actions border radius in image-viewer/vars.less
  • Button: Multiple border radius variables in button/vars.less
  • Badge: Border radius in badge/vars.less
  • CalendarView: Mark border radius in calendar-view/vars.less

This change should be part of a broader effort to standardize border radius styling using --tv-border-radius-brand. The current mix of direct round and brand usage could lead to inconsistent styling.

🔗 Analysis chain

LGTM! Verify consistency across components.

The change from --tv-border-radius-round to --tv-border-radius-brand aligns with the PR objective to switch the overall rounded corner style of the component library.

Let's verify that this change is consistent with other components:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining usage of --tv-border-radius-round
# and confirm consistent usage of --tv-border-radius-brand across components

echo "Checking for any remaining --tv-border-radius-round usage..."
rg --type=less "border-radius-round" packages/theme/src/

echo "Verifying consistent --tv-border-radius-brand usage..."
rg --type=less "border-radius-brand" packages/theme/src/

Length of output: 4237

// 分页页码悬浮字重
--tv-Pager-page-item-font-weight-hover: var(--tv-font-weight-bold);
// 分页页码项最小宽度
Expand Down
2 changes: 1 addition & 1 deletion packages/theme/src/time-spinner/vars.less
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
// 时间项内间距
--tv-TimeSpinner-item-padding: 3.5px 0;
// 时间项圆角
--tv-TimeSpinner-item-border-radius: var(--tv-border-radius-round, 999px);
--tv-TimeSpinner-item-border-radius: var(--tv-border-radius-brand, 999px);
// 时间项背景色(悬浮)
--tv-TimeSpinner-item-bg-color-hover: var(--tv-color-bg-hover-1);
// 时间项背景色(选中/激活)
Expand Down
Loading