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

[Feature Request] Add wp-block class to all the blocks #6639

Closed
srikat opened this issue May 8, 2018 · 24 comments
Closed

[Feature Request] Add wp-block class to all the blocks #6639

srikat opened this issue May 8, 2018 · 24 comments
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Feature] Extensibility The ability to extend blocks or the editing experience [Type] Enhancement A suggestion for improvement.

Comments

@srikat
Copy link

srikat commented May 8, 2018

It would be handy to have wp-block class added to all the blocks in addition to block specific class.

This would be useful if we want to set say, a bottom margin to all the blocks in CSS.

Ex.:

Instead of having to write

p.wp-block-subhead,
.entry-content .wp-block-cover-image,
.wp-block-image,
.entry-content ul.wp-block-gallery,
.wp-block-text-columns,
.entry-content .wp-block-button,
blockquote.wp-block-quote,
.entry-content .wp-block-quote.is-large,
.wp-block-code,
.wp-block-audio,
.entry-content .wp-block-video,
.wp-block-preformatted,
.wp-block-verse,
.wp-block-table,
.wp-block-categories,
.entry-content .wp-block-latest-posts,
.wp-block-embed {
    margin-bottom: 60px;
}

we could simply write

.wp-block {
    margin-bottom: 60px;
}
@mtias mtias added the Needs Decision Needs a decision to be actionable or relevant label May 8, 2018
@aduth
Copy link
Member

aduth commented May 8, 2018

While not as explicit, would the following work?

[class^="wp-block-"] {
    margin-bottom: 60px;
}

@rchipka
Copy link

rchipka commented May 9, 2018

👍

There's no reason why this shouldn't be the case and the sooner we decide on the class name the better.

This would also be useful if Gutenberg decides to take over dynamic CSS injection.

@aduth It should be its own wp-block class (in addition any to other classes) so that block developers can target their blocks using a higher-than-default (but still easily overridable) selector specificity.

@danielbachhuber danielbachhuber added the [Feature] Extensibility The ability to extend blocks or the editing experience label Jun 27, 2018
@gziolo
Copy link
Member

gziolo commented Jul 10, 2018

We offer the filter which allows to modify the default class name:
https://wordpress.org/gutenberg/handbook/extensibility/extending-blocks/#blocks-getblockdefaultclassname

What about using the following instead:

function addBlockCustomClassName( className ) {
    return 'wp-block ' + className;
}

// Adding the filter
wp.hooks.addFilter(
    'blocks.getBlockDefaultClassName',
    'my-plugin/add-block-custom-class-name',
    addBlockCustomClassName
);

@mtias
Copy link
Member

mtias commented Jul 12, 2018

This probably needs a decision for the "try" milestone.

@mcsf
Copy link
Contributor

mcsf commented Jul 13, 2018

I don't feel strongly about adding wp-block on its own, but I do think we should make sure all core blocks do respect the wp-block-foo nomenclature (for instances, Lists are an exception).

@chrisvanpatten
Copy link
Contributor

One question: is the idea that .wp-block is added on all blocks, or only core blocks?

I imagine only core blocks (you wouldn't really be able to enforce it on 3rd party blocks) but either way I like the idea.

@wpscholar
Copy link
Member

At present, we are enforcing a specific class naming convention for third-party blocks unless they specifically opt out. I think it would be reasonable to enforce the .wp-block class unless the third-party has opted out and wants to use their own class names.

@noisysocks
Copy link
Member

This came up in a weekly #core-editor chat (link). We decided to punt on this for now because:

  • [class^="wp-block-"] is an OK workaround.
  • Blocks don't necessarily have wrapper elements, e.g. <!-- wp:foo/bar -->hello world<!-- /wp:foo/bar --> is a valid block.
  • It's easier to add a wp-block class later than it is to remove it later.

@noisysocks noisysocks removed the Needs Decision Needs a decision to be actionable or relevant label Jul 18, 2018
@hatsumatsu
Copy link

@noisysocks

  • The suggested workaround [class^="wp-block-"] also targets inner block elements like wp-block-group__inner-container.

  • Adding a custom classname via the filter blocks.getBlockDefaultClassName does not alter the class attributes of "blocks" like Heading, Paragraph and Image...

From a user perspective it's very hard to understand what prevents adding a class of wp-block to all blocks by default.

@noisysocks
Copy link
Member

What do you think, @gziolo? It sounds like the existing solutions we have here aren't good enough.

@gziolo
Copy link
Member

gziolo commented Aug 30, 2019

What do you think, @gziolo? It sounds like the existing solutions we have here aren't good enough.

I think there are still blocks which don't have any class name provided by design like Paragraph, Heading or List blocks (all of those with support set to false). As far as I know, it was a conscious decision to keep HTML output as clean as possible. @mtias or @youknowriad should have the best idea of whether such core change is feasible. You can always use one of the PHP filters which operates on block's content and inject such class to all blocks during render phase if necessary.

@paaljoachim
Copy link
Contributor

Isabel @tellthemachines perhaps this might be an issue for you to take a closer look at?

@skorasaurus skorasaurus added [Type] Enhancement A suggestion for improvement. CSS Styling Related to editor and front end styles, CSS-specific issues. labels Mar 23, 2021
@youknowriad
Copy link
Contributor

As shared by @gziolo not adding this class to all blocks has been a conscious decision, some blocks also don't have wrappers (shortcodes, embeds)...

I think for the use-cases mentioned here, our approach lately have been to add settings to theme.json to define things like block margins, layout... and avoid CSS as much as possible.

I'm going to close this issue for now as for me the ship has sailed here.

@jaclyntan
Copy link

jaclyntan commented Jun 18, 2021

Hello, can we please reopen this issue? I do not feel this is adequately resolved using just[class^="wp-block-"].

Not only is this very limited in scope, this workaround excludes blocks like headings and paragraphs which by default do not get any gutenberg specific classes at all. This can be very problematic if people are using custom blocks and interspersing them with core blocks in their themes (like I am right now).

I would say the most common use case for wanting a common class across all gutenberg blocks is to contain them in a container wrapper. Currently you can wrap each block with the'render_block' filter but this also becomes problematic, because certain block specific styles are very targeted.

For example, if I used a button within a columns block, and wrapped each block using the 'render_block' filter, alignment styles are not applied by default because Gutenberg styles are applied based on specific hierarchy:

.wp-block-buttons > .wp-block-button.wp-block-button__width-100 {
  margin-right: 0;
  width: 100%;
}

As you can see, if a custom wrapper is applied to gutenberg blocks by a developer, the default css for alignment will not be applied at all because it is based on a depth hierarchy. This means that we need to go in and manually test each and every core block to see if any styles conflict!!!! This is very inefficient and I would prefer to be able to target blocks specifically rather than whitelist each and every block for each different project. This is why I would like to see a default class be implemented at the top-most level of each core block.

Also, I am using ACF blocks, so honestly using a JS based filter is not an ideal solution.

Now I realise that adding a common class to headings and paragraphs may need some manual css finessing by the developer in a columns block. However adding a default class would be vastly more appealing than the current set up because it would mean less work and time spent testing each and every block.

Also bear in mind that adding a single class doesn't even affect the front-end at all so if this class isn't used it won't affect the site's display. This addition is purely to make developer's lives easier.

Thanks for your considering and I appreciate the work being done on this. But please, from a developer and even user perspective (asking admins to use groups can be burdensome and confusing to the writer) this workflow needs to be improved.

@youknowriad
Copy link
Contributor

I would say the most common use case for wanting a common class across all gutenberg blocks is to contain them in a container wrapper. Currently you can wrap each block with the'render_block' filter but this also becomes problematic, because certain block specific styles are very targeted.

I agree that alignments inside containers in the biggest use-case for this. This is something we solved using the "layout" config in theme.json by making sure theme authors don't have to provide this kind of CSS at all.

@jaclyntan
Copy link

I would say the most common use case for wanting a common class across all gutenberg blocks is to contain them in a container wrapper. Currently you can wrap each block with the'render_block' filter but this also becomes problematic, because certain block specific styles are very targeted.

I agree that alignments inside containers in the biggest use-case for this. This is something we solved using the "layout" config in theme.json by making sure theme authors don't have to provide this kind of CSS at all.

I see, I was not aware of a theme.json. I do like the concept of it but I can see this getting wildly complex very quickly. Is there a way to set global settings that affect all core blocks?

eg. I would like to set max-width:1000px and margin: 50px auto to all elements from gutenberg including headings/paragraphs/all blocks that don't have a wp class by default. Is this possible using theme.json?

@youknowriad
Copy link
Contributor

There is for the max-width yes but not for the margin yet (it will come). Expect a dev note (post detailing all of that) very soon on make/core as theme.json and layout are shipping in WP 5.8.

@rchipka
Copy link

rchipka commented Jun 18, 2021

What about responsive settings for max-width, margin, and padding for all blocks?

@jaclyntan
Copy link

jaclyntan commented Jun 18, 2021

What about responsive settings for max-width, margin, and padding for all blocks?

Exactly....this is why I would prefer a single class linking all the core blocks together. It allows for the greatest flexibility; CSS in it's purest form instead of nested json values.

Also keep in mind that custom styles that are applied to blocks may be less than ideal in columns. eg. if you applied margin:50px auto to a heading, that would need to be changed when it is used in a column.

To be honest I think a single class will solve this issue in a much cleaner and more manageable way but I will await the documentation for theme.json. :)

@rchipka
Copy link

rchipka commented Jun 18, 2021

And responsive variations on other things like font size?

This theme.json concept might quickly become a kluge.

In order to concisely and naturally represent the full feature-set of CSS you need something like... well, CSS.

@youknowriad
Copy link
Contributor

Gutenberg doesn't have built-in responsive controls yet, discussions and iterations on that aspect are yet to come but the global direction is more toward "intrinsic web design" rather than "responsive web design"

@rchipka
Copy link

rchipka commented Jun 18, 2021

Ideally design follows a system, but ultimately comes from the imagination- unbounded by technical limitations.

CSS has evolved to accommodate a designer's wildest imaginations in a standardized way... it's quite a big wheel to reinvent.

@mtias
Copy link
Member

mtias commented Jun 22, 2021

theme.json is not meant to replace CSS but to absorb a lot of structural and appearance concerns as a block-first API that is more agnostic of the environment a block is rendered (for example, mobile native that lacks CSS, etc). It doesn't cascade in the same way that CSS does and handles composition in a way that is more tied to specific block semantics (like a nested block or template areas). The result should be a theme.css file can become much leaner and manageable.

Regarding the specific issue of a generic class, it'd be fine to explore a filter that just appends wp-block server side and we could consider it for inclusion in core. (theme.json might allow disabling this class, or even changing its base name, for example.) It is important, however, that this is not done at serialization time but at rendering time. We don't want to pollute the source of <p>Text</p> with <p class="wp-block">Text</p> if we can avoid it.

@jaclyntan
Copy link

jaclyntan commented Jun 22, 2021

Regarding the specific issue of a generic class, it'd be fine to explore a filter that just appends wp-block server side and we could consider it for inclusion in core. (theme.json might allow disabling this class, or even changing its base name, for example.) It is important, however, that this is not done at serialization time but at rendering time. We don't want to pollute the source of <p>Text</p> with <p class="wp-block">Text</p> if we can avoid it.

I think this sounds like a good solution, although it would be nice if it was able to be serialized too (or any other custom class a developer may want to add to a block using the filter). I understand that the resulting markup is intended to be kept as clean as possible, but considering that paragraphs and headings are considered separate blocks in the editor, I think it makes sense for them to be treated the same in the resulting markup too. Paragraph and heading tags are by default block level elements so I don't think it is strange to give them their own class if they are generated by Gutenberg specifically. But that is just my opinion. An option to toggle the wp-block class in theme.json would be a nice middle ground. Thank you for considering this, it will be really helpful to theme development if implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Feature] Extensibility The ability to extend blocks or the editing experience [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests