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(tag): refactor fix lost css for button,tag #2301

Merged
merged 4 commits into from
Oct 19, 2024
Merged

refactor(tag): refactor fix lost css for button,tag #2301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
03f63c1
fix(modal): add button's line-height
shenjunjian Oct 18, 2024
42b90e5
fix(tag): prevents high stretching of tag
shenjunjian Oct 18, 2024
733fa15
fix(alert): fix alert-icon
shenjunjian Oct 18, 2024
e2c394f
fix(tree): fix loading-icon
shenjunjian Oct 18, 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
8 changes: 4 additions & 4 deletions examples/sites/demos/pc/app/tag/size-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<template>
<div class="tiny-tag-demo">
<tiny-tag size="medium"> 中等标签 </tiny-tag>
<tiny-tag> 默认标签 </tiny-tag>
<tiny-tag size="small"> 小型标签 </tiny-tag>
<tiny-tag size="mini"> 超小标签 </tiny-tag>
<tiny-tag size="medium" type="success"> 中等标签 </tiny-tag>
<tiny-tag type="success"> 默认标签 </tiny-tag>
<tiny-tag size="small" type="success"> 小型标签 </tiny-tag>
<tiny-tag size="mini" type="success"> 超小标签 </tiny-tag>
</div>
</template>

Expand Down
8 changes: 4 additions & 4 deletions examples/sites/demos/pc/app/tag/size.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<template>
<div class="tiny-tag-demo">
<tiny-tag size="medium"> 中等标签 </tiny-tag>
<tiny-tag> 默认标签 </tiny-tag>
<tiny-tag size="small"> 小型标签 </tiny-tag>
<tiny-tag size="mini"> 超小标签 </tiny-tag>
<tiny-tag size="medium" type="success"> 中等标签 </tiny-tag>
<tiny-tag type="success"> 默认标签 </tiny-tag>
<tiny-tag size="small" type="success"> 小型标签 </tiny-tag>
<tiny-tag size="mini" type="success"> 超小标签 </tiny-tag>
</div>
</template>

Expand Down
1 change: 1 addition & 0 deletions packages/theme/src/alert/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
font-size: var(--tv-Alert-icon-size);
margin-right: var(--tv-Alert-icon-margin-right);
flex-shrink: 0;
margin-top: 2px;
}

/** content 场景：title/description/opration */
Expand Down
1 change: 1 addition & 0 deletions packages/theme/src/button/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
vertical-align: middle;
font-weight: var(--tv-Button-font-weight);
border: var(--tv-Button-border-width) solid;
line-height: var(--tv-Button-line-height);

/** 1、 圆角场景 */
border-radius: var(--tv-Button-border-radius);
Expand Down
3 changes: 2 additions & 1 deletion packages/theme/src/button/vars.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
--tv-Button-font-weight: var(--tv-font-weight-regular);
// 默认时按钮边框宽度
--tv-Button-border-width: var(--tv-border-width);

// 按钮的行产，
--tv-Button-line-height: var(--tv-border-width);
Comment on lines +19 to +20
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Potential Issue: Line Height Set to Border Width

The variable --tv-Button-line-height is currently set to var(--tv-border-width) (1px). Assigning a border width value to line height is unconventional and may result in text alignment issues within buttons.

Consider the following:

  • Intentional Design? If this assignment is deliberate for a specific design effect, please provide the rationale to ensure it aligns with the overall UI/UX goals.
  • Potential Styling Conflicts: Using border width for line height might cause cramped text or inconsistent button heights across different contexts.
  • Variable Separation: It may be beneficial to define a separate variable for line height to maintain clarity and flexibility in styling.
🔗 Analysis chain

Clarify the purpose of setting line height to border width

The addition of --tv-Button-line-height is consistent with the existing naming conventions. However, setting the line height to the border width (var(--tv-border-width)) is unusual and might lead to unexpected button appearances. This could affect text alignment and overall button height.

Consider the following:

  1. Is this intentional? If so, please explain the reasoning behind this decision.
  2. Have you tested this change with various button sizes and content to ensure it doesn't cause layout issues?
  3. The comment above the variable is in Chinese. For consistency and maintainability, consider translating it to English if that's the primary language used in the project.

To assess the impact of this change, let's check how the --tv-border-width variable is defined and used:

This will help us understand the context and potential implications of using border width as line height.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the definition and usage of --tv-border-width
rg --type less '--tv-border-width'

Length of output: 454

Script:

#!/bin/bash
# Search for the definition and usage of --tv-border-width using the correct rg syntax
rg --type less -e '--tv-border-width'

Length of output: 1442

// --------------------------------------------------- 场景1、圆角--------------------------------------------
// 默认时按钮圆角
--tv-Button-border-radius: var(--tv-border-radius-round); // 默认规范不需要 6px的圆角的效果，默认就是半圆
Expand Down
10 changes: 7 additions & 3 deletions packages/theme/src/tag/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,16 @@
.@{tag-prefix-cls} {
.inject-Tag-vars();

line-height: var(--tv-Tag-line-height);
border: 1px solid;
white-space: nowrap;
display: inline-flex;
align-items: center;
vertical-align: middle;
height: fit-content; // 避免 flex时，高度拉伸
line-height: var(--tv-Tag-line-height);
border: 1px solid;
max-width: 200px; // 新增约束，避免太长的标签
white-space: nowrap;
text-overflow: ellipsis;
overflow: hidden;

/** size 场景 */
.size-mixin(@size) {
Expand Down
2 changes: 1 addition & 1 deletion packages/theme/src/tag/vars.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
// 超小标签水平间距
--tv-Tag-padding-x-mini: var(--tv-space-sm);
// 超小标签垂直间距
--tv-Tag-padding-y-mini: 0;
--tv-Tag-padding-y-mini: 0.5px;
// 超小标签圆角
--tv-Tag-border-radius-mini: var(--tv-border-radius-xs);
// 小型标签字号
Expand Down
12 changes: 1 addition & 11 deletions packages/theme/src/tree/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,19 +429,9 @@
text-overflow: ellipsis;
}

&__loading.circular {
&__loading {
margin-right: 4px;
margin-left: var(--ti-tree-node-label-margin-left);
font-size: var(--ti-tree-node-icon-font-size);

.path {
animation: loading-dash 1.5s ease-in-out infinite;
stroke-dasharray: 90, 150;
stroke-dashoffset: 0;
stroke-width: 2;
stroke: var(--ti-tree-node-loading-icon-color);
stroke-linecap: round;
}
}

& > &__children {
Expand Down
8 changes: 3 additions & 5 deletions packages/vue/src/tree/src/tree-node.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,7 @@
:label="node.id"
:disabled="!!node.disabled"
></tiny-radio>
<svg v-if="node.loading" class="tiny-tree-node__loading tiny-svg circular" viewBox="25 25 50 50">
<circle class="path" cx="50" cy="50" r="24" fill="none"></circle>
</svg>
<icon-loading v-if="node.loading" class="tiny-tree-node__loading" />
<slot name="prefix" :node="node"></slot>
<template v-if="action.type === 'edit' && action.node && action.node === node">
<tiny-input
Expand Down Expand Up @@ -242,7 +240,7 @@ import { renderless, api } from '@opentiny/vue-renderless/tree-node/vue'
import CollapseTransition from '@opentiny/vue-collapse-transition'
import {
iconChevronRight,
iconLoading,
iconLoadingShadow,
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Issues Found: Inconsistent usage of iconLoading detected across the codebase.

Several .vue files still use iconLoading instead of iconLoadingShadow. This inconsistency may lead to visual discrepancies and unexpected behavior.

Affected files include, but are not limited to:

  • packages/vue/src/select/src/mobile-first.vue
  • packages/vue/src/modal/src/pc.vue
  • examples/sites/demos/pc/app/time-line/custom-icon.vue
  • examples/sites/demos/mobile/app/loading/spinner.vue
  • ... [additional files as per the script output]

Please ensure all instances of iconLoading are updated to iconLoadingShadow to maintain consistency.

🔗 Analysis chain

LGTM: Updated loading icon import and registration.

The change from iconLoading to iconLoadingShadow is consistent with the earlier template modification. This update likely introduces a new visual style for the loading icon.

To ensure this change doesn't introduce any unexpected visual regressions, please run the following verification script:

This script will help identify any inconsistencies in the usage of the new iconLoadingShadow across the codebase.

Also applies to: 328-328

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of iconLoadingShadow across the codebase

# Test: Check for any remaining usage of the old iconLoading
echo "Checking for any remaining usage of iconLoading:"
rg "iconLoading[^Shadow]" --type vue

# Test: Verify consistent usage of iconLoadingShadow
echo "Verifying consistent usage of iconLoadingShadow:"
rg "iconLoadingShadow" --type vue

# Test: Check for any potential naming conflicts
echo "Checking for potential naming conflicts:"
rg "IconLoading" --type vue

Length of output: 489

Script:

#!/bin/bash
# Description: Verify the usage of iconLoadingShadow across the codebase

# Test: Check for any remaining usage of the old iconLoading
echo "Checking for any remaining usage of iconLoading:"
rg "iconLoading[^Shadow]" -g "*.vue"

# Test: Verify consistent usage of iconLoadingShadow
echo "Verifying consistent usage of iconLoadingShadow:"
rg "iconLoadingShadow" -g "*.vue"

# Test: Check for any potential naming conflicts
echo "Checking for potential naming conflicts:"
rg "IconLoading" -g "*.vue"

Length of output: 7742

iconArrowBottom,
iconEdit,
iconDel,
Expand Down Expand Up @@ -327,7 +325,7 @@ export default defineComponent({
TinyRadio: Radio,
TinyInput: Input,
IconChevronRight: iconChevronRight(),
IconLoading: iconLoading(),
IconLoading: iconLoadingShadow(),
IconArrowBottom: iconArrowBottom(),
IconEdit: iconEdit(),
IconDel: iconDel(),
Expand Down
Loading