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

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

Open
chrisbellboy opened this issue Oct 7, 2021 · 11 comments
Labels
[Block] Buttons Affects the Buttons Block Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended

Comments

@chrisbellboy
Copy link
Contributor

chrisbellboy commented Oct 7, 2021

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

@carolinan carolinan added Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Block] Buttons Affects the Buttons Block labels Oct 10, 2021
@richtabor
Copy link
Member

I came here to report this as well.

@richtabor
Copy link
Member

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 richtabor added the [Type] Bug An existing feature does not function as intended label Oct 26, 2021
@richtabor
Copy link
Member

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
Copy link
Member

richtabor commented Oct 26, 2021

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
Copy link
Member

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

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

@oandregal
Copy link
Member

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 #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 #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
Copy link
Contributor

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
Copy link
Member

The ability to respect theme.json styles over the outline variation for the button has been fixed by #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 #34141 (comment)

@richtabor
Copy link
Member

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
Copy link
Contributor

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
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Buttons Affects the Buttons Block Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

6 participants