WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.48k stars 4.18k forks source link

Theme.json - styles.blocks.core/button.spacing.padding doesn't apply if button uses Outline style #35438

Open chrisbellboy opened 3 years ago

chrisbellboy commented 3 years ago

Description

Hey everyone,

Was playing around with the theme.json and noticed that if you set styles.blocks.core/button.spacing.padding.X and styles.blocks.core/button.border.X values in the theme.json - they work fine except when you set the outline style to the button, as this style triggers higher specificity css on the frontend. This only applies to the frontend, as the editor adds even higher specificity.

The only "workaround" I could find was to add !important to the theme.json padding values, but of course... 🤢

Step-by-step reproduction instructions

  1. Add a value to: styles.blocks.core/button.spacing.padding.left
  2. Add two buttons in the editor. Set one to Outline style.
  3. Save and check the frontend

Screenshots, screen recording, code snippet

image

Environment info

Gutenberg 11.6.0

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

richtabor commented 3 years ago

I came here to report this as well.

richtabor commented 3 years ago

Essentially the outline button block style is being omitted from theme.json border.width, border.color and all spacing.padding styles, as the core bundled styles targeting .is-style-outline are too specific for the styles that are being injected from theme.json.

I've tested using the following in theme.json in blocks:

"core/button": {
        "border": {
                "radius": "0",
                "width": "4px",
                "color": "red"
        },
        "spacing": {
                "padding": {
                        "bottom": "1.75rem",
                        "left": "1.5rem",
                        "right": "1.5rem",
                        "top": "1.75rem"
                }
        }
},

Which accurately modifies the spacing on the filled button, but the border width, color and block padding are all being overwritten for the outline style.

Screen Shot 2021-10-26 at 8 51 25 AM
richtabor commented 3 years ago

I've tested that modifying the __experimentalSelector to .wp-block-buttons .wp-block-button__link ensures a theme's theme.json overrides the default styles (solves the border.color and border.width) but then there's still the original size discrepancy for spacing.padding, which was why there's the padding set in the outline style anyhow. 🤔

Screen Shot 2021-10-26 at 9 08 41 AM
richtabor commented 3 years ago

but then there's still the original size discrepancy for spacing.padding, which was why there's the padding set in the outline style anyhow.

This is currently "solved" for core by subtracting the border width from the padding here, but if the border width of the button block has changed from the default 2px theme.json to anything else (or the spacing.padding is changed for the block), this is no longer valid (as seen in the screenshot above).

Thoughts:

A. Have the border.width applied to the button as a whole, but use the block's background color property as the border color for the filled/default block?

I can get it close, using this below, but the buttons are not the same size visually, as the border color of currrentColor is not the button's background color. If it were, this would likely work:

Screen Shot 2021-10-26 at 9 19 14 AM

B. Have the border.width automatically subtracted from the spacing.padding values, for the outline style only?

richtabor commented 3 years ago

I've tested that modifying the experimentalSelector to .wp-block-buttons .wp-block-buttonlink ensures a theme's theme.json overrides the default styles

I also don't know that this is the best way forward.

oandregal commented 3 years ago

Hey, I'd like to grab @jasmussen thoughts on this.

This is what I'm thinking. We don't support block styles variations via theme.json yet (see https://github.com/WordPress/gutenberg/issues/33232). I'd consider the styles coming from theme.json as styles that the block gets in all its variations (or work well when switching between them). For overriding styles for a specific style variation, I'd think the theme wants to create its own CSS until theme.json gains support for them.

Said this, there's something we could do to make it easier for themes to override the outline style variation https://github.com/WordPress/gutenberg/pull/35968 I'm not 100% sold on the idea, though. Unless we find some CSS that core can ship and makes both variations the same height, the themes will still have to enqueue their own CSS for setting the outlined one (so it seems best to collocate border and spacing together in the CSS file).

jasmussen commented 3 years ago

This is indeed a tricky one. Absorbing variations into theme.json is a noble goal that can help surface and solve so many issues like these, and make the theme.json, design tools, and the global styles interfaces all the more resilient for it.

At the same time, there are and likely will always be designs that are not really possible without finessing the CSS a little bit. But it still seems like a good north star to aim for, to work towards letting global styles handle as much as possible.

In the case of the same-height button/outlined buttons, I wonder if box-shadow would be a tool to leverage? We'd need an interface for it, of course, but the benefit is that the shadow doesn't change the footprint of the button at all. The outline property doesn't either, but it doesn't support border-radius.

In any case, this is a CSS challenge that I would love to see a diversity of creative ideas on how to solve. Ultimately we can build anything we want — might as well implement the best idea!

oandregal commented 3 years ago

The ability to respect theme.json styles over the outline variation for the button has been fixed by https://github.com/WordPress/gutenberg/pull/35968

When it comes to variations there's the issue I've shared above. In particular, for the outline one in the button block, I've seen a related report to make the button's height the same across different button statuses (hover, active, etc). I've shared the full use case and some ideas at https://github.com/WordPress/gutenberg/issues/34141#issuecomment-952895572

richtabor commented 3 years ago

In the case of the same-height button/outlined buttons, I wonder if box-shadow would be a tool to leverage? We'd need an interface for it, of course, but the benefit is that the shadow doesn't change the footprint of the button at all. The outline property doesn't either, but it doesn't support border-radius.

That's interesting.

It's tough because if a theme modifies the spacing.padding of the buttons; they'll need to add custom CSS to make them look 1:1 in size (which will need to be updated again if the value changes at any point). Seems like whatever we support in core should be easily modified by theme.json (which most values are) but this one in particular is a remnant of hard-coding styles from Gutenberg. 🤔

jasmussen commented 3 years ago

Seems like whatever we support in core should be easily modified by theme.json

Definitely agreed.

but this one in particular is a remnant of hard-coding styles from Gutenberg

Yes, I think we can probably keep those around for back compat, but I also think that we can find a way to redefine their contents entirely for a theme.json world. An example I like is the Quote style that features a border on the left: this border, its style, width and color, as well as the leftmost padding, those should all be absorbed by theme.json.

It's tough because if a theme modifies the spacing.padding of the buttons; they'll need to add custom CSS to make them look 1:1 in size

Yep, that's why I imagined using box-shadow as an alternative to border, it wouldn't change the dimensions. The idea being that if we have to revisit these anyway, as they are theme.json absorbed, we might as well revisit the base ingredients to solve those foundational problems.

youknowriad commented 5 days ago

If I'm not wrong, we now support register style variations using theme.json like config, so I wonder if we can solve this issue today?