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.

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

ButtonGroup: Fix button and icon button styling when tooltips are used on them (#4210) #4251

Closed
wants to merge 11 commits into from

Conversation

broccolinisoup
Copy link
Member

@broccolinisoup broccolinisoup commented Feb 11, 2024

Closes #4129

Reintroducing #4210 since it is reverted due to random VRT tests failings - I removed the threshold setting in Playwright since the default one is less strict.

Changelog

New

Changed

Updated Button group CSS to make sure the icon buttons and buttons are rendered as expected when they have tooltips (both v1 and v2) on them

Removed

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

…d on them (#4210)

* fix button group styling for tooltip and tooltip v2

* Add changeset

* take the a tag into account when styling

* add e2e tests for the buttons groups that use tooltip

* test(vrt): update snapshots

---------

Co-authored-by: broccolinisoup <broccolinisoup@users.noreply.github.com>
Copy link

changeset-bot bot commented Feb 11, 2024

🦋 Changeset detected

Latest commit: f440bec

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Feb 11, 2024

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 113.64 KB (+0.07% 🔺)
packages/react/dist/browser.umd.js 114.34 KB (+0.11% 🔺)

Copy link
Member

@joshblack joshblack left a comment

Choose a reason for hiding this comment

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

Seems good from a quick pass, is it still cutting it off by 1px like you were noticing before? 👀

@broccolinisoup
Copy link
Member Author

Seems good from a quick pass, is it still cutting it off by 1px like you were noticing before? 👀

I think it is all gone now, removing the strict threshold worked 👍🏻

@broccolinisoup
Copy link
Member Author

closing this in favour of #4371

@broccolinisoup broccolinisoup deleted the re-introduce-button-group-tooltip-fix branch April 5, 2024 02:47
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.

Using <Tooltip> within <ButtonGroup> affects styling
3 participants