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
feat: Text Button tokens #47870
base: feature
Are you sure you want to change the base?
feat: Text Button tokens #47870
Conversation
Run & review this pull request in StackBlitz Codeflow. |
👁 Visual Regression Report for PR #47870 Passed ✅
🎊 Congrats! No visual-regression diff found. |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature #47870 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 743 743
Lines 12828 12828
Branches 3364 3364
=========================================
Hits 12828 12828 ☔ View full report in Codecov by Sentry. |
@@ -342,14 +342,16 @@ const genLinkButtonStyle: GenerateStyle<ButtonToken, CSSObject> = (token) => ({ | |||
|
|||
// Type: Text | |||
const genTextButtonStyle: GenerateStyle<ButtonToken, CSSObject> = (token) => ({ | |||
color: token.textColor, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
textColor 和 colorText 是啥关系?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
textColor
是<Button type="text">Text Button</Button>
文本按钮自定义的文本颜色。colorText
是主题中默认的一级文本色。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这两个命名太容易混淆了。得想办法区分开
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
可以把 textColor 改为 textBtnColor,相应的文本按钮颜色 textBtnHoverColor、textBtnActiveColor,但是对比其它类型按钮多了 Btn,不统一了。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
感觉把 colorText 改为 colorTextPrimary 最恰当,与 colorTextSecondary、colorTextTertiary 都是一致的,但是会 breaking change 了。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
textBtnColor 确实好一点。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
只改 textBtnColor 吗?比如已有的 textHoverBg 保持不变吗?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
textTypeColor texTypeHoverBg 如何
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
textHoverBg 是在 5.13.0 版本添加的 #46936 ,应该也不能改的...
- 已有的文本按钮 token:textHoverBg
- 添加的文本按钮 token:textColor、textHoverColor、textActiveColor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不改了吧,text 这里虽然看起来很奇怪但是语义上代表 “text” 类型的按钮
[中文版模板 / Chinese template]
🤔 This is a ...
🔗 Related issue link
close: #47647
💡 Background and solution
📝 Changelog
textColor
,textHoverColor
andtextActiveColor
tokens.textColor
、textHoverColor
和textActiveColor
三个 token。☑️ Self-Check before Merge