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

Add flex-shrink: 0 by default toIconButtons #5373

Open
iansan5653 opened this issue Dec 6, 2024 · 1 comment · May be fixed by #5467
Open

Add flex-shrink: 0 by default toIconButtons #5373

iansan5653 opened this issue Dec 6, 2024 · 1 comment · May be fixed by #5467

Comments

@iansan5653
Copy link
Contributor

iansan5653 commented Dec 6, 2024

Something I've noticed frequently when working with Primer components in flex layouts is that because of the way icon buttons are laid out (using width instead of padding to create the spacing around the sides) they are happy to shrink down to a thin rectangle if a flex layout asks them to. This is frequently a very subtle bug for two reasons: first, shrinking typically requires extra long content in the first place to cause the flex layout to need more space. And also, if button is variant=invisible as IconButtons often are, the shrinking is only apparent on hover.

IconButton should always be a square, so there's no risk to just adding flex-shrink: 0 by default to prevent this.

Example (https://github.com/github/copilot-productivity/issues/3222) note how this copy button shrinks down to a rectangle when the code here is long enough to make the flex layout try to shrink it:

Screen.Recording.2024-12-06.at.9.46.08.AM.mov
@Srishtihere
Copy link

Srishtihere commented Dec 9, 2024

hi can I work on this issue ?

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

Successfully merging a pull request may close this issue.

5 participants