WordPress / gutenberg

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

Documentation: Add coding guidelines enforcing consistency in class names #8027

Open samikeijonen opened 6 years ago

samikeijonen commented 6 years ago

Gutenberg tries to follow BEM naming convention which is a good thing. However there are inconsistencies here and there.

Button block have good example with class names wp-block-button and wp-block-button__link:

<div class="wp-block-button aligncenter">
   <a class="wp-block-button__link" href="https://themebeans.com">Center Aligned Button</a>
</div>

Then again for example columns have:

<div class="wp-block-columns has-2-columns">
   <div class="wp-block-column">
      <p>Cras justo odio, dapibus ac facilisis in, egestas eget quam. Sed posuere consectetur est at lobortis. Nullam id dolor id nibh ultricies vehicula ut id elit. Donec sed odio dui. Nulla vitae elit libero, a pharetra augue.</p>
   </div>

   <div class="wp-block-column">
      <p>Cras justo odio, dapibus ac facilisis in, egestas eget quam. Sed posuere consectetur est at lobortis. Nullam id dolor id nibh ultricies vehicula ut id elit. Donec sed odio dui. Nulla vitae elit libero, a pharetra augue.</p>
   </div>
</div>

With BEM naming classes should probably be wp-block-columns and wp-block-columns__column.

Inconsistencies with has-* and is-* classes.

For example column block have good has-2-columns class. But pretty much all the others like Latest post grid or galleries the class is columns-2. It should be has-2-columns.

Conclusion

I can check all the class names and update this ticket later but it would be super nice to have consistent class names before the launch.

justintadlock commented 6 years ago

Here's a couple of extra notes:


The <figcaption> element for embeds doesn't have a class. If going with BEM, that should be:

<figure class="wp-block-embed">
    ...
    <figcaption class="wp-block-embed__caption">
</figure>

The gallery block uses:

<ul class="wp-block-gallery">

    <li class="blocks-gallery-item"></li>
</ul>

Should be:

<ul class="wp-block-gallery">

    <li class="wp-block-gallery__item"></li>
</ul>

Whether using BEM or any other naming system, having a consistent naming system for classes will help theme authors.

ZebulanStanphill commented 6 years ago

Individual columns are now blocks (Column blocks nested in a Columns block), so the class names are correct in that instance.

samikeijonen commented 5 years ago

@mtias Is this still something I should work on? I think class names are still a little bit all over the place.

youknowriad commented 5 years ago

To clarify our strategy:

Gutenberg uses BEM like syntax for the components the backend. The frontend classNames (typically the classNames uses in save functions) do not have a strict convention yet.

Worth noting that there's a backward compatibility challenge if we were to change some of these.

samikeijonen commented 5 years ago

Yep, I was referring to front-end. I'm OK leaving as it sits now if we can filter all the class names outputted in the front-end. Can we?

youknowriad commented 5 years ago

If we can filter all the class names outputted in the front-end. Can we?

We have a php filter that can filter the HTML output of all blocks. I don't know if that's what answers your question.

jorgefilipecosta commented 5 years ago

Removing good first issue label, as this involves some knowledge of the code base and the creation of deprecations.

samikeijonen commented 5 years ago

I was hoping that changing classes would not involve deprecation system (lot's of code for minor change). I think this is on the horizon.

gziolo commented 5 years ago

@aduth - what's the current status of this one?

aduth commented 5 years ago

@aduth - what's the current status of this one?

I'm not really clear on what the intended action item from this issue is intended to be. It would help to have this clarified.

If I were asked what it should be, it would be about adding explicit coding guideline around how class names are assigned for core blocks. This seems to generally exist in the form of wp-block-[block name] and BEM-like __description suffixes as appropriate for descendent elements. A guideline should be added to the existing CSS Naming guidelines.

A separate task could then to be to enforce this via the introduction of some new lint rules.

I doubt at this point that the existing inconsistencies could be corrected, at least not in a way which doesn't still include them for backwards-compatibility as was done in #14420.

gziolo commented 5 years ago

This seems to generally exist in the form of wp-block-[block name] and BEM-like __description suffixes as appropriate for descendent elements. A guideline should be added to the existing CSS Naming guidelines.

I agree that it would be a great start. Let's re-shape this issue to reflect it.

I doubt at this point that the existing inconsistencies could be corrected, at least not in a way which doesn't still include them for backwards-compatibility as was done in #14420.

That's a fair point. It is also a very complex task to support all different ways those class names can be provided to JSX components. It would be great to have ESLint support but I'm afraid that the efforts required to make it friendly for contributors would be enormous.

paaljoachim commented 3 years ago

I believe that Marcus @mkaz and Isabel @tellthemachines might be able to take a look at this issue.

gziolo commented 3 years ago

It would be good to have more detailed guidelines. Sometimes it isn't clear if proposals like https://github.com/WordPress/gutenberg/pull/29014 should be applied or not.

stylelint rules might help here on the CSS side of things but in practice they are hard to enable, see work from @rafaelgalani in https://github.com/WordPress/gutenberg/pull/28954 for reference.

mburridge commented 1 year ago

Is any action needed here on the documentation side? If so, what?

mburridge commented 1 year ago

@gziolo you added the [Type] Developer Documentation label to this issue. Do any updates need to be made to the handbook here? If so can you guide us as to what needs doing. Thanks.