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

Merged
merged 3 commits into from
Oct 11, 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
4 changes: 2 additions & 2 deletions examples/sites/demos/pc/app/badge/type.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ test('主題样式', async ({ page }) => {
const primaryBadge = badge.nth(5)

await expect(dangerBadge).toHaveClass(/tiny-badge--danger/)
await expect(dangerBadge).toHaveCSS('background-color', 'rgb(246, 111, 106)')
await expect(dangerBadge).toHaveCSS('background-color', 'rgb(242, 48, 48)')
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Old color value 'rgb(246, 111, 106)' still present in other test files

The color rgb(246, 111, 106) is still used in the following files:

  • examples/sites/demos/pc/app/time-line/status.spec.ts

Please update these instances to align with the new design system.

🔗 Analysis chain

Approve change and verify new color value

The updated background color for the danger badge (rgb(242, 48, 48)) is consistent with the PR's refactoring objectives. This change appears to introduce a more vibrant red color.

Please confirm that this new color value aligns with the updated design system and is consistently applied across the application. You may want to run the following command to check for any other occurrences of the old color value:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

rg --type less --type typescript "rgb\(246, 111, 106\)"

Length of output: 229


Script:

#!/bin/bash
# Searching for the old dangerBadge color value in TypeScript files
rg -g "*.ts" "rgb\\(246, 111, 106\\)"

Length of output: 354

await expect(primaryBadge).toHaveClass(/tiny-badge--primary/)
await expect(primaryBadge).toHaveCSS('background-color', 'rgb(25, 25, 25)')
await expect(successBadge).toHaveClass(/tiny-badge--success/)
await expect(successBadge).toHaveCSS('background-color', 'rgb(92, 179, 0)')
await expect(warningBadge).toHaveClass(/tiny-badge--warning/)
await expect(warningBadge).toHaveCSS('background-color', 'rgb(255, 136, 0)')
await expect(infoBadge).toHaveClass(/tiny-badge--info/)
await expect(infoBadge).toHaveCSS('background-color', 'rgb(25, 25, 25)')
await expect(infoBadge).toHaveCSS('background-color', 'rgb(20, 118, 255)')
})
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ import { Link as TinyLink } from '@opentiny/vue'

<style scoped>
.tiny-link {
margin-right: 8px;
margin-right: 48px;
}
</style>
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/link/basic-usage.vue
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ export default {

<style scoped>
.tiny-link {
margin-right: 8px;
margin-right: 48px;
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ const TinyIconFilletExternalLink = iconFilletExternalLink()

<style scoped>
.tiny-link {
margin-right: 8px;
margin-right: 48px;
}
</style>
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/link/custom-icon.vue
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ export default {

<style scoped>
.tiny-link {
margin-right: 8px;
margin-right: 48px;
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ import { Link as TinyLink } from '@opentiny/vue'

<style scoped>
.tiny-link {
margin-right: 8px;
margin-right: 48px;
}
</style>
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/link/dynamic-disable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ export default {

<style scoped>
.tiny-link {
margin-right: 8px;
margin-right: 48px;
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ import { Link as TinyLink } from '@opentiny/vue'

<style scoped>
.tiny-link {
margin-right: 8px;
margin-right: 48px;
}
</style>
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/link/focus-no-underline.vue
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ export default {

<style scoped>
.tiny-link {
margin-right: 8px;
margin-right: 48px;
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ import { Link as TinyLink } from '@opentiny/vue'

<style scoped>
.tiny-link {
margin-right: 8px;
margin-right: 48px;
}
</style>
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/link/link-style.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ export default {

<style scoped>
.tiny-link {
margin-right: 8px;
margin-right: 48px;
}
</style>
3 changes: 3 additions & 0 deletions examples/sites/demos/pc/app/link/size-composition-api.vue
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,7 @@ const TinyIconEdit = iconEdit()
.tiny-custom-del {
margin-left: 2px;
}
.tiny-link {
margin-right: 48px;
}
</style>
3 changes: 3 additions & 0 deletions examples/sites/demos/pc/app/link/size.vue
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,7 @@ export default {
.tiny-custom-del {
margin-left: 2px;
}
.tiny-link {
margin-right: 48px;
}
Comment on lines +40 to +42
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 adjusting the margin for better flexibility.

The addition of margin-right: 48px; to .tiny-link improves spacing between link components. However, this fixed margin might not be ideal for all screen sizes and could potentially conflict with the existing flex layout.

Consider using a more flexible approach:

  1. Use gap property in the flex container instead of individual margins:
.tiny-custom {
  display: flex;
  flex-direction: column;
  gap: 16px; /* Adjust as needed */
}
  1. If you need horizontal spacing for inline usage, consider using CSS custom properties for easier maintenance:
.tiny-link {
  margin-right: var(--tiny-link-margin, 16px);
}

This allows for easy customization and responsiveness by adjusting the --tiny-link-margin variable as needed.

</style>
60 changes: 18 additions & 42 deletions packages/theme/src/badge/index.less
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,23 @@

.@{badge-prefix-cls} {
&__wrapper {
.component-css-vars-badge();
.inject-Badge-vars();
}

display: inline-block;
min-width: var(--ti-badge-size);
height: var(--ti-badge-size);
line-height: var(--ti-badge-line-height);
border-radius: var(--ti-badge-border-radius);
padding: 0 4px;
font-size: var(--ti-badge-font-size);
font-weight: var(--ti-badge-font-weight);
color: var(--ti-badge-text-color);
background-color: var(--ti-badge-bg-color);
min-width: 16px;
height: 16px;
line-height: 16px;
border-radius: var(--tv-Badge-border-radius);
padding: 0 var(--tv-Badge-horizontal-padding);
font-size: var(--tv-Badge-font-size);
font-weight: var(--tv-Badge-font-weight);
color: var(--tv-Badge-text-color);
background-color: var(--tv-Badge-bg-color);
Comment on lines +24 to +32
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 relative units for better responsiveness.

The use of fixed pixel values for min-width, height, and line-height may affect responsiveness on different screen sizes. Consider using relative units like em or rem to enhance scalability across various devices.

vertical-align: baseline;
white-space: nowrap;
text-align: center;
margin-left: var(--ti-badge-margin-left);
border: var(--ti-badge-border) solid #FFFFFF;
border: 1px solid #FFFFFF;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Replace hardcoded border color with a CSS variable for consistency.

The border color is hardcoded as #FFFFFF. To maintain consistency and facilitate theming, it's recommended to use a CSS variable instead.

Apply this diff to define and use a border color variable:

+ // Define the variable in vars.less
+ --tv-Badge-border-color: #FFFFFF;

  // In index.less
- border: 1px solid #FFFFFF;
+ border: 1px solid var(--tv-Badge-border-color);

Committable suggestion was skipped due to low confidence.


&:empty {
display: none;
Expand All @@ -47,39 +46,16 @@

a,
a:hover {
color: var(--ti-badge-text-color);
color: var(--tv-Badge-a-text-color);
text-decoration: none;
}

a.badge {
&:hover,
&:focus {
color: var(--ti-badge-link-hover-text-color);
text-decoration: none;
cursor: pointer;
}
}

a.list-group-item.active > .badge,
.nav-pills > .active > a > .badge {
color: var(--ti-badge-active-text-color);
background-color: var(--ti-badge-active-bg-color);
}

.nav-pills > li > a > .badge {
margin-left: 3px;
}

&--max {
padding: 0 6px;
border-radius: 10px;
}

&--default {
width: 6px;
height: 6px;
min-width: auto;
background-color: var(--ti-badge-bg-color);
background-color: var(--tv-Badge-dot-bg-color);
display: inline-block;
vertical-align: top;
padding: 0;
Expand All @@ -88,22 +64,22 @@
}

&--primary {
background-color: var(--ti-badge-primary-bg-color);
background-color: var(--tv-Badge-primary-bg-color);
}

&--success {
background-color: var(--ti-badge-success-bg-color);
background-color: var(--tv-Badge-success-bg-color);
}

&--warning {
background-color: var(--ti-badge-warning-bg-color);
background-color: var(--tv-Badge-warning-bg-color);
}

&--danger {
background-color: var(--ti-badge-danger-bg-color);
background-color: var(--tv-Badge-danger-bg-color);
}

&--info {
background-color: var(--ti-badge-info-bg-color);
background-color: var(--tv-Badge-info-bg-color);
}
}
45 changes: 27 additions & 18 deletions packages/theme/src/badge/vars.less
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,31 @@
*
*/

.component-css-vars-badge() {
--ti-badge-size: var(--ti-common-font-size-2, 16px);
--ti-badge-font-size: var(--ti-common-font-size-0, 12px);
--ti-badge-line-height: calc(var(--ti-common-line-height-base, 18px) - 4px);
--ti-badge-font-weight: var(--ti-common-font-weight-normal, 400);
--ti-badge-border-radius: var(--ti-common-border-radius-4, 8px);
--ti-badge-text-color: var(--ti-base-color-bg-7, #ffffff);
--ti-badge-bg-color: var(--ti-base-color-error, #f23030);
--ti-badge-link-hover-text-color: var(--ti-common-color-light, #ffffff);
--ti-badge-active-text-color: var(--ti-base-color-brand-6, #191919);
--ti-badge-active-bg-color: var(--ti-common-color-light, #ffffff);
--ti-badge-primary-bg-color: var(--ti-base-color-brand-6, #191919);
--ti-badge-success-bg-color: var(--ti-common-color-success-normal, #5cb300);
--ti-badge-warning-bg-color: var(--ti-common-color-warning-normal, #ff8800);
--ti-badge-danger-bg-color: var(--ti-base-color-error-3, #f66f6a);
--ti-badge-info-bg-color: var(--ti-common-color-info-normal, #191919);
--ti-badge-border: var(--ti-common-border-weight-normal, 1px);
--ti-badge-margin-left: 0;
.inject-Badge-vars() {
// 标记的圆角
--tv-Badge-border-radius: var(--tv-border-radius-round);
// 标记的内边距
--tv-Badge-horizontal-padding: var(--tv-space-base);
// 标记的尺寸
--tv-Badge-font-size: var(--tv-font-size-sm);
// 标记的字重
--tv-Badge-font-weight: var(--tv-font-weight-regular);
// 标记的文本色
--tv-Badge-text-color: var(--tv-color-act-primary-text-white);
// 标记的背景色
--tv-Badge-bg-color: var(--tv-color-act-danger-bg);
// 标记a标签的文本及悬浮文本色
--tv-Badge-a-text-color: var(--tv-color-act-primary-text-white);
// 标记的小圆点背景色
--tv-Badge-dot-bg-color: var(--tv-color-act-danger-bg);
// 标记的主要背景色
--tv-Badge-primary-bg-color: var(--tv-color-bg-primary);
// 标记的成功背景色
--tv-Badge-success-bg-color: var(--tv-color-act-success-bg);
// 标记的警告背景色
--tv-Badge-warning-bg-color: var(--tv-color-act-warning-bg);
// 标记的危险背景色
--tv-Badge-danger-bg-color: var(--tv-color-act-danger-bg);
// 标记的信息背景色
--tv-Badge-info-bg-color: var(--tv-color-act-info-bg);
}
Loading