Skip to content

vertically center icons in icon-only buttons #48178

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

Conversation

momesana
Copy link
Contributor

@momesana momesana commented Mar 29, 2024

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Other (about what?)

🔗 Related issue link

fix #48071

💡 Background and solution

Currently the icons on buttons aren't vertically centered. This is a regression of a prior issue prevalent in antd 4, namely #31701. To illustrate this I have created the following side-by-side screenshot, highlighting the affected buttons and the icons in different colors and with a dashed border. The left side shows the alignment as is whereas the right side shows the fixed version:

vertically-center-icon-on-icon-ony-button

An alternative to the applied fix would've been the following code but I chose the former for brevity:

diff --git a/components/button/style/index.ts b/components/button/style/index.ts
index 9b38143ae7..b7b28e774d 100644
--- a/components/button/style/index.ts
+++ b/components/button/style/index.ts
@@ -419,9 +419,13 @@ const genButtonStyle = (token: ButtonToken, prefixCls: string = ''): CSSInterpol
         borderRadius,
 
         [`&${iconOnlyCls}`]: {
+          display: 'inline-flex',
           width: controlHeight,
           paddingInlineStart: 0,
           paddingInlineEnd: 0,
+          [`${componentCls}-icon`]: {
+            margin: 'auto',
+          },
           [`&${componentCls}-round`]: {
             width: 'auto',
           },

📝 Changelog

Language Changelog
🇺🇸 English vertically center icons on icon-only buttons
🇨🇳 Chinese

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link
Contributor

github-actions bot commented Mar 29, 2024

👁 Visual Regression Report for PR #48178 Failed ❌

🎯 Target branch: master (d51d1bc)
📖 View Full Report ↗︎

Image name Expected Actual Diff
auto-complete-certain-category.compact.png master: d51d1bc9297675f7420bc33044683bf90769a0fc current: pr-48178 diff
auto-complete-certain-category.compact.css-var.png master: d51d1bc9297675f7420bc33044683bf90769a0fc current: pr-48178 diff
auto-complete-certain-category.dark.png master: d51d1bc9297675f7420bc33044683bf90769a0fc current: pr-48178 diff
auto-complete-certain-category.dark.css-var.png master: d51d1bc9297675f7420bc33044683bf90769a0fc current: pr-48178 diff
auto-complete-certain-category.default.png master: d51d1bc9297675f7420bc33044683bf90769a0fc current: pr-48178 diff
auto-complete-certain-category.default.css-var.png master: d51d1bc9297675f7420bc33044683bf90769a0fc current: pr-48178 diff
auto-complete-form-debug.compact.png master: d51d1bc9297675f7420bc33044683bf90769a0fc current: pr-48178 diff
auto-complete-form-debug.compact.css-var.png master: d51d1bc9297675f7420bc33044683bf90769a0fc current: pr-48178 diff
auto-complete-form-debug.dark.png master: d51d1bc9297675f7420bc33044683bf90769a0fc current: pr-48178 diff
auto-complete-form-debug.dark.css-var.png master: d51d1bc9297675f7420bc33044683bf90769a0fc current: pr-48178 diff

Check Full Report for details

Copy link
Contributor

github-actions bot commented Mar 29, 2024

Preview is ready

Copy link

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.

Copy link

codecov bot commented Mar 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (82720bc) to head (ad0c05f).
Report is 17 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master    #48178   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          736       736           
  Lines        12683     12690    +7     
  Branches      3326      3329    +3     
=========================================
+ Hits         12683     12690    +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@li-jia-nan
Copy link
Member

the same as:

@momesana
Copy link
Contributor Author

momesana commented Mar 29, 2024

the same as:

ah, I hadn't been able to find the issue on the issue tracker. My bad. Well, hopefully one of the solutions will be merged soon. The other PR is a bit invasive, applying the fix to all styles whereas this PR only applies it to icon-only buttons.
Whatever the solution, it would be nice to see the issue being taken care of.

@ant-design ant-design deleted a comment from github-actions bot Mar 30, 2024
@li-jia-nan
Copy link
Member

@afc163 @MadCcc @zombieJ I think it's ok

@li-jia-nan li-jia-nan requested review from afc163, zombieJ and MadCcc March 30, 2024 14:12
@afc163 afc163 merged commit 18f9be6 into ant-design:master Mar 30, 2024
Copy link
Contributor

🎉 Thank you for your contribution! If you have not yet joined our DingTalk community group, please feel free to join us (when joining, please provide the link to this PR).

🎉 感谢您的贡献!如果您还没有加入钉钉社区群,请扫描下方二维码加入我们(加群时请提供此 PR 链接)。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Button icons with shape "circle" not fully vertically centered (regression of #31701)
3 participants