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

Blockbase: Button block does not respect paddings defined in Global Styles #5856

Closed
iamtakashi opened this issue Apr 12, 2022 · 8 comments
Closed
Assignees

Comments

@iamtakashi
Copy link
Contributor

Steps to replicate

  • Add a button on a page.
  • Go to Global Styles and open the layout panel for Button block
  • Change the paddings
  • See the button not changing its size

Result

The button stays the default style. The border-radius setting work as expected, but not the paddings. If I edit the paddings on the button individually, it works but not with Global Styles.

Screenshot 2022-04-12 at 22 59 37

Expected

The padding in the button changes. I also tested with Twenty Twenty Two, and the result was expected.

@pbking
Copy link
Contributor

pbking commented Apr 15, 2022

This was on purpose. Button padding control is leveraged differently in Blockbase in order to apply the opinion to buttons that aren't button blocks (form buttons, etc). Padding is actually controled via custom properties.

A better Blockbase solution would probably be to disable the user-controlled padding customizations for Blockbase themes. (We can do that right?)

@iamtakashi
Copy link
Contributor Author

@pbking, thanks for the info.

This was on purpose. Button padding control is leveraged differently in Blockbase in order to apply the opinion to buttons that aren't button blocks (form buttons, etc).

Twenty Twenty-Two seems to allow the padding specified in Global Styles to be also applied to a button in the comment form, for example, though? There is a difference in line-height, but both the buttons have the same custom style specified in Global Styles. What am I missing? Why can't Blockbase do the same thing as TT2 does?

Screenshot 2022-04-19 at 15 03 29

@pbking
Copy link
Contributor

pbking commented Apr 19, 2022

Why can't Blockbase do the same thing as TT2 does?

Things have progressed since Blockbase was born; it was very much a bridge to cover the things that we weren't able to do otherwise. However, that did set us down a rather specific path. Blockbase is quite opinionated in implementation with the facade of openness. Additionally Blockbase allows for more options for button states (hover most specifically) that TwentyTwentyTwo doesn't cover.

I think the tradeoff (for Blockbase) was "Let's have more options and better control over what a THEME can do to opinionatedly control how buttons look at the expense of a user".

I think eventually we'll be able to take out the button "ponyfill" stuff we have in Blockbase, but probably not until we have button state style control in Gutenberg. Doing so (removing the button ponyfill CSS) may involve changing all of the children at the same time too and could be rather disruptive to themes outside of our control. Or maybe it would be a clean refactor I can't say for sure.

@iamtakashi
Copy link
Contributor Author

@pbking, thank you for the detailed answer. Much appreciated.

Do you know any Gutenberg ticket to tackle the state style control? That seems to be something we need to raise if there isn't one yet.

@madhusudhand
Copy link
Member

@pbking I created a small PR related to this. I guess it can be part of target GB version 6.0.
I would also need to test child themes. Can you help me understand impact areas around this?

Related issues: #5847

@pbking
Copy link
Contributor

pbking commented Apr 21, 2022

@madhusudhand I left comments on that branch #5901

@jeffikus
Copy link
Contributor

@pbking after launching Pendant on wpcom this week once we resolve the headstart issues, could you take a look at this please?

@madhusudhand madhusudhand assigned pbking and unassigned madhusudhand Jul 12, 2022
@pbking
Copy link
Contributor

pbking commented Jul 20, 2022

Closing: Work merged in #6167.

@pbking pbking closed this as completed Jul 20, 2022
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