Skip to content

Conversation

@sebmarkbage
Copy link
Collaborator

The panel icons are quite small. Especially compared to the equivalent buttons elsewhere in Chrome DevTools that otherwise use the same icons. This makes them a little bigger to make them similar size to our other button icons.

They were also a bit off center. This centers them as well.

Before:

Screenshot 2025-09-26 at 4 23 15 PM

After:

Screenshot 2025-09-26 at 4 22 57 PM

@sebmarkbage sebmarkbage requested a review from eps1lon September 26, 2025 20:25
@meta-cla meta-cla bot added the CLA Signed label Sep 26, 2025
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Sep 26, 2025
};

const panelIcons = '0 -960 960 820';
const panelIcons = '96 -864 768 768';
Copy link
Collaborator

@eps1lon eps1lon Sep 26, 2025

Choose a reason for hiding this comment

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

The AI model I used told me 144 -816 672 672 is the viewbox that fits the path perfectly which I used in #34517.

The original viewbox was directly from the Material icons which didn't fit the path perfectly so it became off-center depending on the font size.

Did you check the alignment when increasing/decreasing base browser font size via settings?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea. Seems fine. The font size doesn't matter. Doesn't change. That's up to the button to align itself. If the button comes out of alignment then that's a different issue.

But I mean alignment of the rest of the row is messed up because things like the checkboxes should be buttons etc.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yea. Seems fine. The font size doesn't matter. Doesn't change. That's up to the button to align itself. If the button comes out of alignment then that's a different issue.

Well, it does matter if the path is not centered in viewbox. But seems like it still is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea I'm not sure where the 820 box came from because all the materialize design ones have a 960 height for the 24px versions which puts them center:

https://fonts.google.com/icons?selected=Material+Symbols+Outlined:close:FILL@0;wght@400;GRAD@0;opsz@24&icon.size=24&icon.color=%231f1f1f

Then I just applied equal sizing to make it a little larger while still always fitting in our viewbox. Seems to work well for all of them.

Really it's weird that we use 24px basis icons and then resize them to 16px. We should probably use 16px basis icon to begin with or maybe use 24px icons everywhere without having to scale them and fix the padding of the buttons instead.

@sebmarkbage sebmarkbage merged commit 09d3cd8 into facebook:main Sep 28, 2025
247 checks passed
EugeneChoi4 pushed a commit to EugeneChoi4/react that referenced this pull request Sep 29, 2025
The panel icons are quite small. Especially compared to the equivalent
buttons elsewhere in Chrome DevTools that otherwise use the same icons.
This makes them a little bigger to make them similar size to our other
button icons.

They were also a bit off center. This centers them as well.

Before:

<img width="409" height="426" alt="Screenshot 2025-09-26 at 4 23 15 PM"
src="https://github.com/user-attachments/assets/4a5de032-e316-44ed-9424-8bccce00f0cd"
/>

After:

<img width="519" height="388" alt="Screenshot 2025-09-26 at 4 22 57 PM"
src="https://github.com/user-attachments/assets/1763e522-5683-4fac-a913-27910a30a039"
/>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants