Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(button): update button sizes in icon only to have a minimum width #2130

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

booc0mtaco
Copy link
Contributor

@booc0mtaco booc0mtaco commented Feb 12, 2025

Previously, the button styles when icon-only would unset the minimum width to take on a perfectly circular shape. This is fine in default cases, but would cause the button to lose width when used in a flex context, like in DataTable. Instead of unsetting the width for icon-only, add a min-width to each button size when layout is icon-only.

Test Plan:

  • Wrote/updated automated tests
  • CI tests / new tests are not applicable
  • Manually tested my changes, and here are the details:
    • validate no snapshot changes occurred

@booc0mtaco booc0mtaco requested a review from a team February 12, 2025 20:59
Copy link

codecov bot commented Feb 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.13%. Comparing base (717938f) to head (a38aca4).
Report is 1 commits behind head on next.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #2130   +/-   ##
=======================================
  Coverage   97.13%   97.13%           
=======================================
  Files         114      114           
  Lines        2829     2829           
  Branches      764      764           
=======================================
  Hits         2748     2748           
  Misses         77       77           
  Partials        4        4           

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

Copy link

github-actions bot commented Feb 12, 2025

size-limit report 📦

Path Size
components 95.45 KB (0%)
styles 29.87 KB (+0.07% 🔺)

Copy link

@forgeabout forgeabout left a comment

Choose a reason for hiding this comment

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

Just a couple minor comments. Beyond that, I'll leave it up to you whether the changes seen in Chromatic are good.

.gitignore Outdated Show resolved Hide resolved
src/components/Button/Button.module.css Outdated Show resolved Hide resolved
@booc0mtaco booc0mtaco changed the base branch from main to next February 13, 2025 00:40
@booc0mtaco
Copy link
Contributor Author

Just a couple minor comments. Beyond that, I'll leave it up to you whether the changes seen in Chromatic are good.

Turns out those two snapshots had a hidden example of the same issue seen in the sort buttons used in the DataTable :D harder to see since the VERY edge of the X close buttons was close to being trimmed

@booc0mtaco booc0mtaco merged commit 8a312e8 into next Feb 13, 2025
12 checks passed
@booc0mtaco booc0mtaco deleted the aholloway/EDS-1478 branch February 13, 2025 00:43
@booc0mtaco booc0mtaco mentioned this pull request Feb 15, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants