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

Default style class being saved to block markup. #20706

Open
dougwollison opened this issue Mar 8, 2020 · 3 comments · May be fixed by #28984
Open

Default style class being saved to block markup. #20706

dougwollison opened this issue Mar 8, 2020 · 3 comments · May be fixed by #28984
Labels
[Feature] Theme Style Variations Related to style variations provided by block themes [Type] Bug An existing feature does not function as intended

Comments

@dougwollison
Copy link
Contributor

Describe the bug
If a block supports styles and one registered as default is clicked, the class "is-style-default" (or whatever the name of the style is) is applied to the element. This is contrary to the documentation on registering block styles:

By adding isDefault: true you can mark the registered style variation as the one that is recognized as active when no custom class name is provided. It also means that there will be no custom class name added to the HTML output for the style that is marked as default.

To reproduce
Steps to reproduce the behavior:

  1. Select a block that as styles registered.
  2. Select the default style, either immediately or after switching off from another style.
  3. Inspect the element, the class name is-style-default will be added.

Expected behavior
The style class should be removed entirely if the registered style is designated as the default.

I'm currently working around this by adding a saveProps filter that removes the class, but it currently only removes the literal is-style-default class.

Screenshots
https://imgur.com/NHBtS8M

@jorgefilipecosta
Copy link
Member

Hi @dougwollison, thank you for reporting this issue. I was able to replicate this issue in WordPress 5.3.2 without the Gutenberg plugin.
If we select a non-default style, as soon as the default style is selected the class is added.

@jorgefilipecosta jorgefilipecosta added [Feature] Theme Style Variations Related to style variations provided by block themes [Type] Bug An existing feature does not function as intended labels Mar 9, 2020
@Baelx Baelx linked a pull request Feb 13, 2021 that will close this issue
5 tasks
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Feb 13, 2021
@talldan
Copy link
Contributor

talldan commented Feb 15, 2021

This issue is a bit deeper than it looks. This is something that was tried before - #22266

But it had to be reverted (#23548), the change caused a bug for plugins that might register a new default (#23211).

I think the idea of how default styles work needs a bit of a rethink. It doesn't really make sense for themes or plugins to provide additional defaults over the default that is already provided by core. What does it mean to have many defaults?

An example of this is the button block, which has a default 'fill' style, but there are no is-style-fill styles in the stylesheet. If a theme or plugin makes something else the default, what happens to is-style-fill? It does nothing. This becomes more complex when you consider there might be multiple competing plugins attempting to register defaults 😓

My feeling is that the best option might be deprecating is-default option. The editor could instead provide a default automatically that represents an absence of styles.

@Baelx
Copy link
Contributor

Baelx commented Feb 15, 2021

Great points, @talldan. Given this, I also feel that is-default should be deprecated. We can't get around the fact that plugins and themes will sometimes want to add their own default styles to blocks. All the while, I think having a default option in the BlockStyles panel could work(as a way to remove all is-style- classes). Being able to dynamically set isDefault is the problematic part, as you mention.

As far as implementation goes, it could be something like:

  1. When a block is registered with the styles argument, a "Default" or "Regular" style is added automatically as the first option BlockStyles panel. As mentioned above, it serves only to bring the block back to having no is-style- class.
  2. The ability to set the "isDefault" option is removed and plugin and theme makers can provide default styles to blocks as css. They'll need to do this anyway in their css, but having a sort of "hot potato" isDefault option adds a layer of complexity that doesn't seem sustainable.

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Theme Style Variations Related to style variations provided by block themes [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants