Skip to content

Button: Fix incorrect padding with text and right icon#71464

Merged
aduth merged 6 commits intoWordPress:trunkfrom
epeicher:fix/button-padding-icon-right
Sep 23, 2025
Merged

Button: Fix incorrect padding with text and right icon#71464
aduth merged 6 commits intoWordPress:trunkfrom
epeicher:fix/button-padding-icon-right

Conversation

@epeicher
Copy link
Copy Markdown
Contributor

@epeicher epeicher commented Sep 2, 2025

What?

Closes #60734

This PR fixes the right padding when the button has text and an icon positioned on the right. This is required to compensate the extra padding that the icon adds. This is already working for the icon positioned on the left, but the right position of the icon was not taken into account.

Why?

Because when the button has text and an icon positioned on the right, there is more padding on the right of the button than on the left.

How?

Adding an additional css class named has-icon-right that is only added when the icon is positioned on the right. That class will apply the correct padding in the css file.

Testing Instructions

  1. Apply this patch and run storybook by executing npm run storybook:dev
  2. Once storybook opens, finds a button component
  3. Select primary, so it's easier to see the background
  4. On the Controls tab on the bottom, find the icon prop and select an icon, e.g. wordpress
  5. On the iconPosition prop below, select right
  6. Alternatively, you can add the following queryParams to the URL ?path=/story/components-button--primary&args=icon:wordpress;iconPosition:right
  7. Check the left and right padding on the bottom are similar

Testing Instructions for Keyboard

Screenshots or screencast

Before After
CleanShot 2025-09-02 at 17 53 22@2x CleanShot 2025-09-02 at 17 53 26@2x

@epeicher epeicher requested a review from ajitbohra as a code owner September 2, 2025 16:00
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 2, 2025 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: epeicher <epeicher@git.wordpress.org>
Co-authored-by: aduth <aduth@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: johnhooks <bitmachina@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Sep 2, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 2, 2025

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @epeicher! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended [Package] Components /packages/components labels Sep 3, 2025
@mirka mirka requested a review from a team September 19, 2025 09:05
aduth
aduth approved these changes Sep 19, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

I wondered if there might be a way to style this without an additional class†, but I think this is a simple solution consistent with the existing style arrangement for left icon + text. LGTM 👍

† The original styles prior to #50277 targeted svg elements, so technically we could do something like .button:has(svg:last-child) { ... }, but targeting svg broadly doesn't feel like it provides as much guarantees as here with icon and icon positioning.

@aduth
Copy link
Copy Markdown
Member

aduth commented Sep 19, 2025

It looks like the failing Unit Tests are true failures pointing to some snapshots that need to be updated. Can you update those?

@epeicher
Copy link
Copy Markdown
Contributor Author

epeicher commented Sep 22, 2025

I wondered if there might be a way to style this without an additional class†, but I think this is a simple solution consistent with the existing style arrangement for left icon + text. LGTM 👍

† The original styles prior to #50277 targeted svg elements, so technically we could do something like .button:has(svg:last-child) { ... }, but targeting svg broadly doesn't feel like it provides as much guarantees as here with icon and icon positioning.

Thanks @aduth for the review! I agree with this, it is a simple solution consistent with the existing code, and targeting svg broadly could be more fragile

It looks like the failing Unit Tests are true failures pointing to some snapshots that need to be updated. Can you update those?

Thanks for letting me know! I didn't realise of that, I have updated the snapshots by running npm run test:unit:update and now running npm run test:unit works locally for me 👍

@aduth aduth merged commit 5ca68bc into WordPress:trunk Sep 23, 2025
68 checks passed
@aduth
Copy link
Copy Markdown
Member

aduth commented Sep 23, 2025

Thanks for the updates! I had to fight a bit with some merge conflicts and flakey tests after adding a changelog note, but finally got this merged today 😄

@github-actions github-actions bot added this to the Gutenberg 21.8 milestone Sep 23, 2025
@epeicher epeicher deleted the fix/button-padding-icon-right branch September 24, 2025 08:49
@hambai hambai mentioned this pull request Oct 8, 2025
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduth aduth aduth approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

Assignees

No one assigned

Labels

First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Package] Components /packages/components [Type] Bug An existing feature does not function as intended

Projects

None yet

Milestone

Gutenberg 21.8

Development

Successfully merging this pull request may close these issues.

The Button component has incorrect padding with text and icon position right

3 participants

@epeicher @aduth @t-hamano