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

refactor(time-spinner): [date-picker] roll back some styles for time-spinner #2383

Merged
merged 3 commits into from
Oct 24, 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
6 changes: 1 addition & 5 deletions packages/theme/src/time-panel/index.less
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,20 @@

&__header-input {
margin: var(--tv-TimePanel-header-margin);

input {
height: 28px;
}
}

&__header-title {
display: flex;
justify-content: space-around;
color: var(--tv-TimePanel-header-text-color);
margin-top: 8px;
align-items: center;
}

&__content {
font-size: 0;
position: relative;
overflow: hidden;
padding-bottom: var(--tv-TimePanel-content-padding-bottom);

&:after,
&:before {
Expand Down
6 changes: 4 additions & 2 deletions packages/theme/src/time-panel/vars.less
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
// 面板外间距
--tv-TimePanel-margin: var(--tv-space-sm, 4px) 0;
// 面板内间距
--tv-TimePanel-padding: 0 var(--tv-space-xl, 16px);
--tv-TimePanel-padding: 0 14px;
// 面板背景色
--tv-TimePanel-bg-color: var(--tv-color-bg-secondary, #ffffff);
// 面板圆角
Expand All @@ -29,9 +29,11 @@
--tv-TimePanel-border-color-divider: var(--tv-color-border-divider, #f0f0f0);

// 头部外间距
--tv-TimePanel-header-margin: var(--tv-space-lg, 12px) 0;
--tv-TimePanel-header-margin: var(--tv-space-xl, 16px) 2px 24px 2px;
// 头部文本色
--tv-TimePanel-header-text-color: var(--tv-color-text-weaken, #808080);
// 内容区底部内间距
--tv-TimePanel-content-padding-bottom: var(--tv-space-lg, 12px);

// 按钮高度
--tv-TimePanel-btn-height: var(--tv-size-height-sm, 28px);
Expand Down
22 changes: 13 additions & 9 deletions packages/theme/src/time-spinner/index.less
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
}

.@{scrollbar-prefix-cls}__wrap:not(.@{scrollbar-prefix-cls}__wrap--hidden-default) {
padding-bottom: 12px;
padding-bottom: 15px;
}

&.is-arrow {
Expand All @@ -43,10 +43,7 @@
.@{time-spinner-prefix-cls}__list {
&::after,
&::before {
content: '';
display: block;
width: 100%;
height: 52px;
transform: translateY(-32px);
}
}

Expand All @@ -56,6 +53,11 @@
}
}

.@{time-spinner-prefix-cls}__list {
transform: translateY(-6px);
margin: var(--tv-TimeSpinner-list-margin);
}

&:last-child {
.@{time-spinner-prefix-cls}__list {
border-right: 0;
Expand All @@ -64,7 +66,8 @@
}

&__arrow {
padding: var(--tv-TimeSpinner-arrow-padding);
height: 30px;
line-height: 30px;
position: absolute;
left: 0;
width: 100%;
Expand All @@ -82,11 +85,11 @@
}

&.@{css-prefix}icon-arrow-up {
top: 12px;
top: 10px;
}

&.@{css-prefix}icon-arrow-down {
bottom: 12px;
bottom: 10px;
}

&.@{input-prefix-cls} {
Expand All @@ -110,7 +113,7 @@
content: '';
display: block;
width: 100%;
height: 4px;
height: 80px;
}
}

Expand All @@ -122,6 +125,7 @@
line-height: var(--tv-TimeSpinner-line-height);
color: var(--tv-TimeSpinner-text-color);
border-radius: var(--tv-TimeSpinner-item-border-radius);
width: 52px;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using a CSS variable for item width.

For better maintainability and consistency, consider moving the explicit width to a CSS variable.

-  width: 52px;
+  width: var(--tv-TimeSpinner-item-width, 52px);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
width: 52px;
width: var(--tv-TimeSpinner-item-width, 52px);


&:hover:not(.disabled):not(.active) {
background: var(--tv-TimeSpinner-item-bg-color-hover);
Expand Down
7 changes: 3 additions & 4 deletions packages/theme/src/time-spinner/vars.less
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@
// 图标尺寸
--tv-TimeSpinner-icon-size: var(--tv-icon-size, 16px);

// 列表外间距
--tv-TimeSpinner-list-margin: 10px 0 30px 0;
// 时间项外间距
--tv-TimeSpinner-item-margin: var(--tv-space-lg, 12px) var(--tv-space-md, 8px) 0 var(--tv-space-md, 8px);
--tv-TimeSpinner-item-margin: var(--tv-space-lg, 12px) 10px 0 10px;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider maintaining theme adaptability.

The change from var(--tv-space-md, 8px) to static 10px for left/right margins reduces the component's ability to adapt to different themes and spacing scales. Consider keeping the variable-based approach:

--- --tv-TimeSpinner-item-margin: var(--tv-space-lg, 12px) 10px 0 10px;
+++ --tv-TimeSpinner-item-margin: var(--tv-space-lg, 12px) var(--tv-space-md, 8px) 0 var(--tv-space-md, 8px);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
--tv-TimeSpinner-item-margin: var(--tv-space-lg, 12px) 10px 0 10px;
--tv-TimeSpinner-item-margin: var(--tv-space-lg, 12px) var(--tv-space-md, 8px) 0 var(--tv-space-md, 8px);

// 时间项内间距
--tv-TimeSpinner-item-padding: 3.5px 0;
// 时间项圆角
Expand All @@ -41,7 +43,4 @@
--tv-TimeSpinner-item-bg-color-active: var(--tv-color-bg-active-emphasize, #deecff);
// 时间项背景色(禁用)
--tv-TimeSpinner-item-bg-color-disabled: var(--tv-color-bg-disabled, #f0f0f0);

// 箭头内间距
--tv-TimeSpinner-arrow-padding: calc((var(--tv-size-height-sm) - var(--tv-TimeSpinner-icon-size)) / 2) 0;
}
Loading