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

fix(button): [button]fix error css var #2312

Merged
merged 4 commits into from
Oct 19, 2024
Merged

fix(button): [button]fix error css var #2312

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
595dd8b
fix(button): fix error css var
shenjunjian Oct 19, 2024
49358cc
fix(button): fix
shenjunjian Oct 19, 2024
89e8edf
fix(button): fix button e2e test
shenjunjian Oct 19, 2024
c318606
fix(button): fix
shenjunjian Oct 19, 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
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/button/basic-usage-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<tiny-layout>
<p>基本按钮</p>
<tiny-row>
<tiny-button type="primary" native-type="submit"> 主要按钮 </tiny-button>
<tiny-button type="primary"> 主要按钮 </tiny-button>
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 Usage of native-type="submit" in Other Components

The native-type="submit" attribute is still present in the following .vue files:

  • examples/sites/demos/pc/app/drawer/footer-slot-composition-api.vue
  • examples/sites/demos/pc/app/drawer/footer-slot.vue
  • examples/sites/demos/mobile-first/app/button/icon.vue
  • examples/sites/demos/mobile-first/app/button/banner.vue
  • examples/sites/demos/mobile-first/app/button/basic-usage.vue

Please ensure that the removal of native-type="submit" is consistently applied across all button components to maintain standardized behavior and prevent unexpected form submission issues.

🔗 Analysis chain

Verify the intended behavior of the primary button

The native-type="submit" attribute has been removed from the primary button. This change alters the button's behavior in form submissions, as it will no longer automatically submit a form when clicked.

Please confirm if this change is intentional and part of the CSS variable fix mentioned in the PR title. If so, ensure that:

  1. This modification doesn't break any existing form submission functionality in the application.
  2. The change is consistent across all relevant components.
  3. Documentation is updated to reflect this change in button behavior, if necessary.

To check for consistency, you can run the following script to find other instances of native-type="submit" in button components:

Consider adding a comment in the code explaining why the native-type="submit" attribute was removed, to provide context for future developers.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other instances of native-type="submit" in button components
rg --type vue 'native-type="submit"' examples/sites/demos/pc/app/button

Length of output: 101

Script:

#!/bin/bash
# Search for other instances of native-type="submit" in .vue button components
rg 'native-type="submit"' examples/sites/demos/pc/app/button --type-add 'vue:*.vue' --type vue

Length of output: 96

Script:

#!/bin/bash
# Search for all instances of native-type="submit" in .vue files across the entire repository
rg 'native-type="submit"' --type-add 'vue:*.vue' --type vue

Length of output: 1089

<tiny-button> 次要按钮 </tiny-button>
<tiny-button type="success"> 成功按钮 </tiny-button>
<tiny-button type="info"> 信息按钮 </tiny-button>
Expand Down
2 changes: 1 addition & 1 deletion examples/sites/demos/pc/app/button/basic-usage.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<tiny-layout>
<p>基本按钮</p>
<tiny-row>
<tiny-button type="primary" native-type="submit"> 主要按钮 </tiny-button>
<tiny-button type="primary"> 主要按钮 </tiny-button>
<tiny-button> 次要按钮 </tiny-button>
<tiny-button type="success"> 成功按钮 </tiny-button>
<tiny-button type="info"> 信息按钮 </tiny-button>
Expand Down
12 changes: 6 additions & 6 deletions examples/sites/demos/pc/app/button/ghost-composition-api.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<template>
<div class="gray-bg">
<tiny-button ghost type="primary" reset-time="0">主要按钮</tiny-button>
<tiny-button ghost reset-time="0">次要按钮</tiny-button>
<tiny-button ghost type="success" reset-time="0">成功按钮</tiny-button>
<tiny-button ghost type="info" reset-time="0">信息按钮</tiny-button>
<tiny-button ghost type="warning" reset-time="0">告警按钮</tiny-button>
<tiny-button ghost type="danger" reset-time="0">危险按钮</tiny-button>
<tiny-button ghost type="primary">主要按钮</tiny-button>
<tiny-button ghost>次要按钮</tiny-button>
<tiny-button ghost type="success">成功按钮</tiny-button>
<tiny-button ghost type="info">信息按钮</tiny-button>
<tiny-button ghost type="warning">告警按钮</tiny-button>
<tiny-button ghost type="danger">危险按钮</tiny-button>
Comment on lines +3 to +8
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 removal of reset-time attribute across the codebase.

The reset-time attribute is still present in numerous button components across multiple .vue files. This indicates that the removal was not applied consistently, which may lead to unexpected behavior in parts of the application.

Recommended Actions:

  1. Review Scope: Ensure that the removal of reset-time is intended to be global. If so, apply the change consistently across all relevant files.
  2. Verify Functionality: Confirm that removing reset-time does not adversely affect button behavior where it remains.
  3. Update Documentation: Reflect these changes accurately in the PR description for better clarity and documentation.
🔗 Analysis chain

Consistent removal of reset-time attribute, but clarification needed.

The reset-time attribute has been consistently removed from all six button components. This change appears intentional and aligns with the AI-generated summary.

However, I have a few points for consideration:

  1. Could you please clarify the purpose of removing the reset-time attribute? Understanding its impact on button behavior would be helpful.
  2. The PR objectives mention fixing a CSS var error, but this change doesn't directly relate to CSS variables. Could you explain how this change addresses the stated objective?
  3. It would be beneficial to update the PR description to reflect these specific changes for better documentation.

To ensure this change is consistent across the codebase, let's check for any remaining usage of reset-time in button components:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining usage of reset-time in button components
rg --type vue 'reset-time' | grep -i button

Length of output: 71

Script:

#!/bin/bash
# Search for any remaining usage of reset-time in button components within .vue files
rg 'reset-time' --glob '*.vue' | grep -i button

Length of output: 32314

</div>
</template>

Expand Down
41 changes: 2 additions & 39 deletions examples/sites/demos/pc/app/button/ghost.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,66 +11,29 @@ test('幽灵按钮', async ({ page }) => {
await expect(getGhostBtn(0)).toHaveCSS('color', 'rgb(25, 25, 25)')
await expect(getGhostBtn(0)).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
await expect(getGhostBtn(0)).toHaveCSS('border-bottom-color', 'rgb(89, 89, 89)')
await page.waitForTimeout(100)
await getGhostBtn(0).click()
await page.waitForTimeout(100)
await expect(getGhostBtn(0)).toHaveCSS('color', 'rgb(25, 25, 25)')
await expect(getGhostBtn(0)).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
await expect(getGhostBtn(0)).toHaveCSS('border-bottom-color', 'rgb(25, 25, 25)')

// 次要幽灵按钮
await page.waitForTimeout(1000)
await expect(getGhostBtn(1)).toHaveCSS('color', 'rgb(25, 25, 25)')
await expect(getGhostBtn(1)).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
await expect(getGhostBtn(1)).toHaveCSS('border-bottom-color', 'rgb(194, 194, 194)')
await page.waitForTimeout(100)
await getGhostBtn(1).click()
await page.waitForTimeout(100)
await expect(getGhostBtn(1)).toHaveCSS('color', 'rgb(25, 25, 25)')
await expect(getGhostBtn(1)).toHaveCSS('color', 'rgb(89, 89, 89)')
await expect(getGhostBtn(1)).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
await expect(getGhostBtn(1)).toHaveCSS('border-bottom-color', 'rgb(25, 25, 25)')
await expect(getGhostBtn(1)).toHaveCSS('border-bottom-color', 'rgb(128, 128, 128)')

// 成功幽灵按钮
await expect(getGhostBtn(2)).toHaveCSS('color', 'rgb(92, 179, 0)')
await expect(getGhostBtn(2)).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
await expect(getGhostBtn(2)).toHaveCSS('border-bottom-color', 'rgb(218, 242, 187)')
await page.waitForTimeout(100)
await getGhostBtn(2).click()
await page.waitForTimeout(100)
await expect(getGhostBtn(2)).toHaveCSS('color', 'rgb(92, 179, 0)')
await expect(getGhostBtn(2)).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
await expect(getGhostBtn(2)).toHaveCSS('border-bottom-color', 'rgb(92, 179, 0)')

// 信息幽灵按钮
await expect(getGhostBtn(3)).toHaveCSS('color', 'rgb(20, 118, 255)')
await expect(getGhostBtn(3)).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
await expect(getGhostBtn(3)).toHaveCSS('border-bottom-color', 'rgb(222, 236, 255)')
await page.waitForTimeout(100)
await getGhostBtn(3).click()
await page.waitForTimeout(100)
await expect(getGhostBtn(3)).toHaveCSS('color', 'rgb(20, 118, 255)')
await expect(getGhostBtn(3)).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
await expect(getGhostBtn(3)).toHaveCSS('border-bottom-color', 'rgb(20, 118, 255)')

// 告警幽灵按钮
await expect(getGhostBtn(4)).toHaveCSS('color', 'rgb(255, 136, 0)')
await expect(getGhostBtn(4)).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
await expect(getGhostBtn(4)).toHaveCSS('border-bottom-color', 'rgb(255, 235, 209)')
await page.waitForTimeout(100)
await getGhostBtn(4).click()
await page.waitForTimeout(100)
await expect(getGhostBtn(4)).toHaveCSS('color', 'rgb(255, 136, 0)')
await expect(getGhostBtn(4)).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
await expect(getGhostBtn(4)).toHaveCSS('border-bottom-color', 'rgb(255, 136, 0)')

// 危险幽灵按钮
await expect(getGhostBtn(5)).toHaveCSS('color', 'rgb(242, 48, 48)')
await expect(getGhostBtn(5)).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
await expect(getGhostBtn(5)).toHaveCSS('border-bottom-color', 'rgb(252, 226, 224)')
await page.waitForTimeout(100)
await getGhostBtn(5).click()
await page.waitForTimeout(100)
await expect(getGhostBtn(5)).toHaveCSS('color', 'rgb(242, 48, 48)')
await expect(getGhostBtn(5)).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
await expect(getGhostBtn(5)).toHaveCSS('border-bottom-color', 'rgb(242, 48, 48)')
})
12 changes: 6 additions & 6 deletions examples/sites/demos/pc/app/button/ghost.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<template>
<div class="gray-bg">
<tiny-button ghost reset-time="0" type="primary">主要按钮</tiny-button>
<tiny-button ghost reset-time="0">次要按钮</tiny-button>
<tiny-button ghost reset-time="0" type="success">成功按钮</tiny-button>
<tiny-button ghost reset-time="0" type="info">信息按钮</tiny-button>
<tiny-button ghost reset-time="0" type="warning">告警按钮</tiny-button>
<tiny-button ghost reset-time="0" type="danger">危险按钮</tiny-button>
<tiny-button ghost type="primary">主要按钮</tiny-button>
<tiny-button ghost>次要按钮</tiny-button>
<tiny-button ghost type="success">成功按钮</tiny-button>
<tiny-button ghost type="info">信息按钮</tiny-button>
<tiny-button ghost type="warning">告警按钮</tiny-button>
<tiny-button ghost type="danger">危险按钮</tiny-button>
Comment on lines +3 to +8
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 Removal of reset-time Attribute

The reset-time attribute has not been removed consistently from <tiny-button> components across the project. Numerous files still contain instances of :reset-time, which may lead to inconsistent behavior.

Please ensure that the reset-time attribute is either fully deprecated and removed from all relevant components or retained where necessary with proper documentation.

🔗 Analysis chain

Summary: Consistent removal of reset-time attribute from all buttons.

The changes in this file consistently remove the reset-time attribute from all <tiny-button> components while maintaining their other attributes and overall structure. This appears to be part of a larger effort to remove this attribute from buttons across the project.

To ensure completeness of the review:

  1. Could you provide context on why the reset-time attribute was removed and what its purpose was?
  2. Are there any potential side effects or behavioral changes expected from removing this attribute?
  3. Have all other files using <tiny-button> components been updated similarly?

To verify the consistent removal of the reset-time attribute across the project, please run the following script:

This script will help identify any inconsistencies in the removal of the reset-time attribute and find any related documentation that might need updating.

To address the issue with the rg command not recognizing the vue file type, please run the following updated script:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining instances of the reset-time attribute on tiny-button components

# Search for tiny-button components with reset-time attribute
echo "Searching for remaining reset-time attributes on tiny-button components:"
rg --type vue '<tiny-button.*reset-time.*>' -g '!examples/sites/demos/pc/app/button/ghost.vue'

# Search for any mentions of reset-time in relation to buttons in comments or documentation
echo "Searching for mentions of reset-time in relation to buttons:"
rg --type vue 'reset-time.*button|button.*reset-time' -g '!examples/sites/demos/pc/app/button/ghost.vue'

Length of output: 545

Script:

#!/bin/bash
# Description: Check for any remaining instances of the reset-time attribute on tiny-button components

# Search for tiny-button components with reset-time attribute in .vue files
echo "Searching for remaining reset-time attributes on tiny-button components:"
rg '<tiny-button.*reset-time.*>' --glob '*.vue' -g '!examples/sites/demos/pc/app/button/ghost.vue'

# Search for any mentions of reset-time in relation to buttons in comments or documentation within .vue files
echo "Searching for mentions of reset-time in relation to buttons:"
rg 'reset-time.*button|button.*reset-time' --glob '*.vue' -g '!examples/sites/demos/pc/app/button/ghost.vue'

Length of output: 64490

</div>
</template>

Expand Down
6 changes: 3 additions & 3 deletions packages/theme/src/button/vars.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +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);
// 按钮的文本行高
--tv-Button-line-height: var(--tv-line-height-number);
// --------------------------------------------------- 场景1、圆角--------------------------------------------
// 默认时按钮圆角
--tv-Button-border-radius: var(--tv-border-radius-round); // 默认规范不需要 6px的圆角的效果，默认就是半圆
Expand Down Expand Up @@ -181,7 +181,7 @@
--tv-Button-border-color-ghost-default: var(--tv-color-border-ghost);

// primary 主题时按钮幽灵文本色
--tv-Button-text-colorghostn-primary: var(--tv-color-act-primary-text);
--tv-Button-text-color-ghost-primary: var(--tv-color-act-primary-text);
// primary 主题时按钮幽灵边框色
--tv-Button-border-color-ghost-primary: var(--tv-color-act-primary-border-light);

Expand Down
Loading