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

Update components CONTROL_HEIGHT config value to 40 pixels #67484

Closed
2 of 6 tasks
afercia opened this issue Dec 2, 2024 · 4 comments
Closed
2 of 6 tasks

Update components CONTROL_HEIGHT config value to 40 pixels #67484

afercia opened this issue Dec 2, 2024 · 4 comments
Labels
[Package] Components /packages/components [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Dec 2, 2024

Description

Discovered while investigating #67425

To my understanding, some components that use CSS-in-JS rely on some CSS values stored in packages/components/src/utils/config-values.js.
It appears the height value in this config is still 36 pixels, while the new default size should be 40 pixels.

const CONTROL_HEIGHT = '36px';

Notice also all the controlHeight* variants are based on 36 pixels.

One of the places where the old 36 pixels value is used is for the ItemGroup > Item component, which can be optionally rendered as a button by the means of the prop as. When rendered as a button or as a link, it uses this unstyledButton style where there's no height value set. Still, all the padding values are based on the 36 pixels value.

I guess all these calculations should use the new 40 pixels height, not sure whether the best option would be allowing to pass the __next40pxDefaultSize flag to the styles calculations or switching directly to new hard-coded values.

Cc @WordPress/gutenberg-components @t-hamano

Step-by-step reproduction instructions

N/A

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes

Please confirm which theme type you used for testing.

  • Block
  • Classic
  • Hybrid (e.g. classic with theme.json)
  • Not sure
@afercia afercia added [Package] Components /packages/components [Type] Bug An existing feature does not function as intended labels Dec 2, 2024
@tyxla
Copy link
Member

tyxla commented Dec 2, 2024

I think @mirka planned to address this as part of #46741, which is in progress, so I'd personally close this issue in favor of #46741.

@afercia
Copy link
Contributor Author

afercia commented Dec 2, 2024

Oh thanks for pointing me at the existing issue. I see it;s from Dec 22, 2022 and still no movement 🙁 No objections to close this one, in that case I will report my findings on #46741

@tyxla
Copy link
Member

tyxla commented Dec 2, 2024

Oh thanks for pointing me at the existing issue. I see it;s from Dec 22, 2022 and still no movement 🙁 No objections to close this one, in that case I will report my findings on #46741

Right, I agree it took a while, but for what it's worth, @mirka is working on one of the last pieces (#65751) and it's almost there.

Feel free to leave this issue open, but IMHO it's unnecessary because in #46741 we have a tracking item specifically for what you reported:

Clean up usage of $button-size SCSS variable and CONFIG.controlHeight CSS-in-JS variable

@afercia
Copy link
Contributor Author

afercia commented Dec 3, 2024

Closing in favor of #46741

@afercia afercia closed this as completed Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

2 participants