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.

Sign up for GitHub

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(theme): [carousel] refactor carousel theme vars #2244

Merged
merged 6 commits into from
Oct 14, 2024
Merged

refactor(theme): [carousel] refactor carousel theme vars #2244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
35ff94c
refactor(theme): [carousel] refactor carousel theme vars
zzcr Oct 12, 2024
241ecca
fix: fix e2e error
zzcr Oct 12, 2024
c97a135
refactor(theme): [nav-menu] refactor nav-menu theme vars
zzcr Oct 14, 2024
5be4d5f
fix: fix e2e error
zzcr Oct 14, 2024
907aaa7
fix: fix e2e error
zzcr Oct 14, 2024
67bb36a
fix: fix e2e error
zzcr Oct 14, 2024
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
12 changes: 7 additions & 5 deletions examples/sites/demos/pc/app/carousel/card-show.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ test('轮播卡片', async ({ page }) => {
const preview = page.locator('#card-show')
const btnRight = preview.getByRole('button').nth(1)
await btnRight.click()
await preview
.locator('div')
.filter({ hasText: /^2-11-1-content$/ })
.first()
expect(btnRight).toHaveCSS('fill', 'rgb(194, 194, 194)')

await expect(
preview
.locator('div')
.filter({ hasText: /^2-11-1-content$/ })
.first()
).toBeVisible()
})
14 changes: 7 additions & 7 deletions packages/theme/src/carousel-item/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
@carousel-prefix-cls: ~'@{css-prefix}carousel';

.@{carousel-prefix-cls} {
.component-css-vars-carousel-item();
.inject-CarouselItem-vars();

&__item,
&__mask {
Expand All @@ -35,17 +35,17 @@
.item-title {
position: absolute;
bottom: 0;
height: var(--ti-carousel-item-title-height);
height: var(--tv-CarouselItem-title-height);
width: 100%;
line-height: var(--ti-carousel-item-title-height);
color: var(--ti-carousel-item-title-text-color);
line-height: var(--tv-CarouselItem-title-height);
color: var(--tv-CarouselItem-title-text-color);
text-align: left;
background: var(--ti-carousel-item-title-bg-color);
background: var(--tv-CarouselItem-title-bg-color);
opacity: 0.3;

span {
padding: 0 16px;
font-size: var(--ti-carousel-item-title-span-font-size);
font-size: var(--tv-CarouselItem-title-span-font-size);
width: 80%;
display: inline-block;
text-overflow: ellipsis;
Expand Down Expand Up @@ -86,7 +86,7 @@

&__mask {
width: 100%;
background-color: var(--ti-carousel-mask-bg-color);
background-color: var(--tv-CarouselItem-mask-bg-color);
opacity: 0.24;
transition: 0.2s;
}
Expand Down
12 changes: 6 additions & 6 deletions packages/theme/src/carousel-item/vars.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@
*
*/

.component-css-vars-carousel-item() {
--ti-carousel-item-title-height: var(--ti-common-size-height-medium, 40px);
--ti-carousel-item-title-text-color: var(--ti-common-color-light, #ffffff);
--ti-carousel-item-title-bg-color: var(--ti-common-color-bg-primary, #191919);
--ti-carousel-item-title-span-font-size: var(--ti-common-font-size-base, 14px);
--ti-carousel-mask-bg-color: var(--ti-common-color-light, #ffffff);
.inject-CarouselItem-vars() {
--tv-CarouselItem-title-height: var(--tv-size-height-lg);
--tv-CarouselItem-title-text-color: var(--tv-color-text-white);
--tv-CarouselItem-title-bg-color: var(--tv-color-bg-primary);
--tv-CarouselItem-title-span-font-size: var(--tv-font-size-md);
--tv-CarouselItem-mask-bg-color: var(--tv-color-bg-secondary);
}
101 changes: 50 additions & 51 deletions packages/theme/src/carousel/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
@svg-prefix-cls: ~'@{css-prefix}svg';

.@{carousel-prefix-cls} {
.component-css-vars-carousel();
.inject-Carousel-vars();

overflow: hidden;
position: relative;
Expand All @@ -38,68 +38,74 @@
outline: 0;
padding: 0;
margin: 0;
height: var(--ti-carousel-arrow-height);
width: var(--ti-carousel-arrow-width);
height: var(--tv-Carousel-arrow-height);
width: var(--tv-Carousel-arrow-width);
cursor: pointer;
transition: 0.3s;
border-radius: 50%;
box-shadow: var(--ti-carousel-arrow-box-shadow);
background-color: var(--ti-carousel-arrow-bg-color);
color: var(--ti-carousel-indicator-active-text-color);
background-color: var(--tv-Carousel-arrow-bg-color);
color: var(--tv-Carousel-indicator-active-text-color);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential inconsistency in CSS variable usage for arrow text color

The color property for .@{carousel-prefix-cls}__arrow is set to var(--tv-Carousel-indicator-active-text-color), which is an indicator variable. It might be more appropriate to use an arrow-specific variable for consistency.

Consider updating the variable:

- color: var(--tv-Carousel-indicator-active-text-color);
+ color: var(--tv-Carousel-arrow-text-color);

Ensure that --tv-Carousel-arrow-text-color is defined in your vars.less file.

Committable suggestion was skipped due to low confidence.

position: absolute;
top: 50%;
z-index: 3;
transform: translateY(-50%);
text-align: center;
font-size: var(--ti-carousel-arrow-font-size);
font-size: var(--tv-Carousel-arrow-font-size);
display: flex;
align-items: center;
justify-content: center;
opacity: var(--ti-carousel-opacity);
opacity: 0.5;
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 arrow opacity

The opacity property is set to a fixed value 0.5. To maintain consistency and allow for theme customization, consider using a CSS variable.

Suggestion:

- opacity: 0.5;
+ opacity: var(--tv-Carousel-arrow-opacity);

Define --tv-Carousel-arrow-opacity in your vars.less file to allow for theme customization.

📝 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
opacity: 0.5;
opacity: var(--tv-Carousel-arrow-opacity);


.@{svg-prefix-cls} {
fill: var(--ti-carousel-indicator-active-text-color);
fill: var(--tv-Carousel-indicator-active-text-color);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Inconsistent use of CSS variables for SVG fill color

The fill property of .@{svg-prefix-cls} is set to var(--tv-Carousel-indicator-active-text-color), an indicator variable. In other places (lines 72~, 81~), the fill color uses var(--tv-Carousel-arrow-svg-color). For consistency, consider using the arrow-specific variable here as well.

Consider updating the fill color:

- fill: var(--tv-Carousel-indicator-active-text-color);
+ fill: var(--tv-Carousel-arrow-svg-color);
📝 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
fill: var(--tv-Carousel-indicator-active-text-color);
fill: var(--tv-Carousel-arrow-svg-color);


&:hover {
fill: var(--ti-carousel-indicator-hover-text-color);
fill: var(--tv-Carousel-indicator-hover-text-color);
}
}

&.@{carousel-prefix-cls}__arrow-left {
left: 16px;
color: var(--ti-carousel-arrow-text-color);
background-color: var(--ti-carousel-arrow-left-bg-color);
background-color: var(--tv-Carousel-arrow-left-bg-color);

svg {
fill: var(--tv-Carousel-arrow-svg-color);
}
}

&.@{carousel-prefix-cls}__arrow-right {
right: 16px;
color: var(--ti-carousel-arrow-text-color);
background-color: var(--ti-carousel-arrow-right-bg-color);
background-color: var(--tv-Carousel-arrow-right-bg-color);

svg {
fill: var(--tv-Carousel-arrow-svg-color);
}
}

&.@{carousel-prefix-cls}__arrow-top {
@apply top-0;
@apply ~'left-1/2';
@apply ~'-translate-x-2/4';
top: 0;
left: 50%;
transform: translateX(-50%);
}

&.@{carousel-prefix-cls}__arrow-bottom {
@apply bottom-0;
@apply ~'left-1/2';
bottom: 0;
left: -50%;
top: unset;
@apply ~'-translate-x-2/4';
transform: translateX(-50%);
}

&.@{carousel-prefix-cls}__arrow-disabled {
background-color: var(--ti-carousel-arrow-disabled-bg-color);
background-color: var(--tv-Carousel-arrow-disabled-bg-color);
cursor: not-allowed;
.@{svg-prefix-cls} {
fill: var(--ti-carousel-arrow-disabled-color);
fill: var(--tv-Carousel-arrow-disabled-color);
}
}

&:not(.@{carousel-prefix-cls}__arrow-disabled):hover {
background-color: var(--ti-carousel-arrow-hover-bg-color);
color: var(--ti-carousel-arrow-hover-text-color);
background-color: var(--tv-Carousel-arrow-hover-bg-color);
color: var(--tv-Carousel-arrow-hover-text-color);
}

i {
Expand All @@ -117,45 +123,45 @@
margin: 0 0 16px;
padding: 0 4px;
z-index: 2;
background: var(--ti-carousel-indicators-bg-color);
border-radius: var(--ti-carousel-indicators-border-radius);
height: var(--ti-carousel-indicators-height);
background: none;
border-radius: var(--tv-Carousel-indicators-border-radius);
height: var(--tv-Carousel-indicators-height);

.@{carousel-prefix-cls}__indicator {
display: inline-block;
background-color: transparent;
padding: var(--ti-carousel-indicator-padding-vertical) var(--ti-carousel-indicator-padding-horizontal);
margin-right: var(--ti-carousel-indicator-margin-right);
padding: 0;
margin-right: 12px;
cursor: pointer;

&:last-child {
margin-right: 0;
}

&:hover button {
opacity: var(--ti-carousel-hover-opacity);
opacity: 0.8;
}

&.is-active {
width: var(--ti-carousel-indicator-active-width);
background-color: var(--ti-carousel-indicator-active-background-color);
border-radius: var(--ti-carousel-indicator-active-border-radius);
transition: var(--ti-carousel-indicator-active-transition);
width: var(--tv-Carousel-indicator-active-width);
background-color: var(--tv-Carousel-indicator-active-background-color);
border-radius: var(--tv-Carousel-indicator-active-border-radius);
transition: none;
}

&.is-active button {
transition-property: var(--ti-carousel-indicator-active-transition-property);
transition-property: none;
transition-timing-function: cubic-bezier(0.16, 0.75, 0.5, 1);
border-radius: var(--ti-carousel-indicator-active-border-radius);
background-color: var(--ti-carousel-indicator-active-text-color);
width: var(--ti-carousel-indicator-active-button-width);
border-radius: var(--tv-Carousel-indicator-active-border-radius);
background-color: var(--tv-Carousel-indicator-active-text-color);
width: var(--tv-Carousel-indicator-active-button-width);
}

.@{carousel-prefix-cls}__button {
display: block;
width: var(--ti-carousel-indicator-button-width);
height: var(--ti-carousel-indicator-button-height);
background-color: var(--ti-carousel-indicator-button-bg-color);
width: 8px;
height: 8px;
background-color: var(--tv-Carousel-indicator-button-bg-color);
border: none;
outline: 0;
padding: 0;
Expand All @@ -166,13 +172,6 @@
}
}

.@{carousel-prefix-cls}__button._radius {
height: 8px;
width: 8px;
border-radius: 50%;
background-color: var(--ti-carousel-indicators-radius-bg-color);
}

&.@{carousel-prefix-cls}__indicators-outside {
bottom: 26px;
text-align: center;
Expand All @@ -182,7 +181,7 @@
background: transparent;

button {
background-color: var(--ti-carousel-outside-button-bg-color);
background-color: var(--tv-Carousel-outside-button-bg-color);
}

.@{carousel-prefix-cls}__indicator {
Expand All @@ -191,7 +190,7 @@
}

&.is-active button {
background-color: var(--ti-carousel-outside-button-active-bg-color);
background-color: var(--tv-Carousel-outside-button-active-bg-color);
}
}
}
Expand All @@ -204,7 +203,7 @@

.@{carousel-prefix-cls}__button {
padding: 2px 18px;
font-size: var(--ti-carousel-labels-button-font-size);
font-size: var(--tv-Carousel-labels-button-font-size);
}

.@{carousel-prefix-cls}__indicator {
Expand Down
80 changes: 27 additions & 53 deletions packages/theme/src/carousel/vars.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,77 +10,51 @@
*
*/

.component-css-vars-carousel() {
.inject-Carousel-vars() {
// 箭头按钮背景高度
--ti-carousel-arrow-height: var(--ti-common-size-8x, 32px);
--tv-Carousel-arrow-height: 32px;
// 箭头按钮背景宽度
--ti-carousel-arrow-width: var(--ti-common-size-8x, 32px);
--tv-Carousel-arrow-width: 32px;
// 左右箭头的字号
--ti-carousel-arrow-font-size: var(--ti-common-font-size-2, 16px);
--tv-Carousel-arrow-font-size: var(--tv-font-size-lg);
// 左右箭头按钮悬浮背景色
--ti-carousel-arrow-hover-bg-color: #E6E6E6;
--tv-Carousel-arrow-hover-bg-color: var(--tv-color-bg-hover-3);
// 左右箭头按钮背景色
--ti-carousel-arrow-bg-color: #F0F0F0;
// 左右箭头阴影(hide)
--ti-carousel-arrow-box-shadow: var(--ti-common-shadow-none, none);
--tv-Carousel-arrow-bg-color: var(--tv-color-bg-header);
// 箭头正常颜色
--ti-carousel-arrow-text-color: var(--ti-common-color-text-weaken, #808080);
// 箭头点击瞬间文本色(hide)
--ti-carousel-arrow-active-text-color: var(--ti-common-color-light, #ffffff);
// 指示器盒子的背景色
--ti-carousel-indicators-bg-color: none;
--tv-Carousel-arrow-svg-color: var(--tv-color-icon);
// 指示器盒子的圆角大小
--ti-carousel-indicators-border-radius: 13px;
--tv-Carousel-indicators-border-radius: 13px;
// 指示器盒子的高度
--ti-carousel-indicators-height: 8px;
// 指示器圆角的背景色(hide)
--ti-carousel-indicators-radius-bg-color: rgba(0, 0, 0, 0.3);
--tv-Carousel-indicators-height: 8px;
// 幻灯片内的当前指示器图标色
--ti-carousel-indicator-active-text-color: var(--ti-common-color-icon-normal, #808080);
--tv-Carousel-indicator-active-text-color: var(--tv-color-icon);
// 幻灯片内的悬浮指示器图标色
--ti-carousel-indicator-hover-text-color: var(--ti-common-color-primary-normal, #191919);
// 指示器按钮宽度
--ti-carousel-indicator-button-width: var(--ti-common-size-2x, 8px);
// 指示器按钮高度
--ti-carousel-indicator-button-height: var(--ti-common-size-2x, 8px);
--tv-Carousel-indicator-hover-text-color: var(--tv-color-icon-hover);
// 非当前指示器背景色
--ti-carousel-indicator-button-bg-color: #e6e6e6;
// 指示器项右外边距
--ti-carousel-indicator-margin-right: var(--ti-common-space-3x, 12px);
--tv-Carousel-indicator-button-bg-color: var(--tv-color-bg-gray-2);
// 当前指示器右边距
--ti-carousel-indicator-active-width: var(--ti-common-size-3x, 12px);
// 指示器项垂直内边距(hide)
--ti-carousel-indicator-padding-vertical: var(--ti-common-space-0, 0px);
// 指示器项水平内边距
--ti-carousel-indicator-padding-horizontal: var(--ti-common-space-0, 0px);
// 当前指示器动画属性(hide)
--ti-carousel-indicator-active-transition-property: none;
--tv-Carousel-indicator-active-width: 12px;
// 当前指示器背景色
--ti-carousel-indicator-active-background-color: var(--ti-common-color-primary-active, #595959);
--tv-Carousel-indicator-active-background-color: var(--tv-color-bg-hover-primary);
// 当前指示器的宽度
--ti-carousel-indicator-active-button-width: var(--ti-common-size-3x, 12px);
--tv-Carousel-indicator-active-button-width: 12px;
// 当前指示器圆角
--ti-carousel-indicator-active-border-radius: var(--ti-common-space-base, 4px);
// 幻灯片外的指示器动画(hide)
--ti-carousel-indicator-active-transition: none;
--tv-Carousel-indicator-active-border-radius: var(--tv-border-radius-sm);
// 幻灯片外的指示器默认背景色
--ti-carousel-outside-button-bg-color: var(--ti-common-color-dark, #000);
--tv-Carousel-outside-button-bg-color: #000;
// 幻灯片外的当前指示器背景色
--ti-carousel-outside-button-active-bg-color: var(--ti-common-color-text-secondary, #595959);
// 标签按钮的字号(hide)
--ti-carousel-labels-button-font-size: var(--ti-common-font-size-base, 14px);
// 正常透明度颜色
--ti-carousel-opacity: 0.5;
// 幻灯片悬浮时的透明度(hide)
--ti-carousel-hover-opacity: 0.8;
// 箭头悬浮文本颜色(hide)
--ti-carousel-arrow-hover-text-color: var(--ti-common-color-text-primary, #191919);
--tv-Carousel-outside-button-active-bg-color: var(--tv-color-bg-hover-primary);
// 标签按钮的字号
--tv-Carousel-labels-button-font-size: var(--tv-font-size-md);
// 箭头悬浮文本颜色
--tv-Carousel-arrow-hover-text-color: var(--tv-color-icon-hover);
// 左侧箭头背景色
--ti-carousel-arrow-left-bg-color: var(--ti-common-color-bg-normal, #f5f5f5);
--tv-Carousel-arrow-left-bg-color: var(--tv-color-bg);
// 右侧箭头背景色
--ti-carousel-arrow-right-bg-color: var(--ti-common-color-bg-normal, #f5f5f5);
--tv-Carousel-arrow-right-bg-color: var(--tv-color-bg);
// 按钮禁用色
--ti-carousel-arrow-disabled-bg-color: var(--ti-common-color-bg-disabled, #f0f0f0);
--tv-Carousel-arrow-disabled-bg-color: var(--tv-color-bg-disabled);
// 按钮禁用色
--ti-carousel-arrow-disabled-color: var(--ti-common-color-primary-disabled-text, #c2c2c2);
}
--tv-Carousel-arrow-disabled-color: var(--tv-color-icon-disabled);
}
Loading