WordPress / gutenberg

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

CSS coding standards: naming conventions #11689

Open afercia opened 5 years ago

afercia commented 5 years ago

The CSS naming convention used in Gutenberg doesn't meet the current WordPress CSS coding standards. Quoting from https://make.wordpress.org/core/handbook/best-practices/coding-standards/css/

Similar to the WordPress PHP Coding Standards for file names, use lowercase and separate words with hyphens when naming selectors. Avoid camelcase and underscores.

Specifically, the underscore is not allowed. Either the WordPress CSS coding standards need to be changed (which is up to the 6 WordPress project leads) or the naming convention used in Gutenberg needs to be changed.

I'd like to note I've personally raised this issue a few times in the last months during the #core-editor team chats on Slack, but no action has been taken.

afercia commented 5 years ago

Gutenberg CSS coding guidelines: https://github.com/WordPress/gutenberg/blob/4156bbadbdcb07c2f93d38839220c089d12358e3/docs/reference/coding-guidelines.md#css

chrisvanpatten commented 5 years ago

One person's opinion: I think this is a case where the coding standards should be updated to allow underscores. Underscores in CSS class names are becoming incredibly common, through BEM and similar naming conventions, and I think only good can come from adapting these conventions. They make CSS class names and selectors more readable and help to better delineate their purpose and scope.

chrisvanpatten commented 5 years ago

I'd also note that I interpret "Avoid camelcase and underscores" as a recommendation, not a hard requirement. To that extent, I think Gutenberg's use of underscores is technically permissible, even if it's not encouraged.

If the core standards aim to truly prohibit underscores, it should be phrased as "Do not use camelcase and underscores" so the intention is less ambiguous.

afercia commented 5 years ago

For clarity: I'm not opposed to update the current standards 🙂 I do strongly feel the mismatch need to be addressed before the release though. I'd say any software should be released meeting the standards the software established for itself.

If the core standards aim to truly prohibit underscores, it should be phrased as "Do not use camelcase and underscores" so the intention is less ambiguous.

Interpretation 🙂To my knowledge, core doesn't use underscores.

chrisvanpatten commented 5 years ago

Thanks @afercia. Re-reading my comments they may have come off a little rougher than I intended; sorry about that!

What would be the process for considering updating these standards? I know there are Core JS and PHP teams who tend to handle recommendations for those standards, but it's not clear what the process would be for CSS. The design team, perhaps…?

afercia commented 5 years ago

It does say "use lowercase and separate words with hyphens when naming selectors" and not "prefer", right?

afercia commented 5 years ago

What would be the process

To my knowledge, technical important decisions like coding standards should be made by the 6 project leads.

chrisvanpatten commented 5 years ago

Fair enough. I know the JS team has collectively updated the JS standards during this process but I guess CSS doesn't technically have a team so it might be helpful to get some weigh-in from the leads on this (cc @pento? sorry…)

afercia commented 5 years ago

One more relevant point to clarify: as Gutenberg introduces this new "BEM-like" naming convention, does that mean that from the 5.0 release on this new convention can be used anywhere in WordPress?

afercia commented 5 years ago

Fair enough. I know the JS team has collectively updated the JS standards during this process

Only partially :) There's the need to update other parts, but that's out of the scope of this issue.

chrisvanpatten commented 5 years ago

If yes: I foresee problems in the consistency of the WordPress admin CSS

I think this is inevitable in any kind of update/transition of the standards, but personally I think it would be great to see this updated across WordPress. CSS selectors in Core are often confusingly named, occasionally misspelled (.hndle, for instance), inconsistently applied (IDs in some places, classes in other places), etc.

We could continue to include those existing classes, but also append new/better classes and update core CSS to reference the newer/better selectors. That would be a great opportunity for improvement.

If no: is having a "double convention", one for legacy core and one for Gutenberg, desirable?

I don't know if I'd say desirable necessarily, but I don't think it's necessarily bad, either. Any code that would be directly merged into WordPress core SVN (instead of being pulled and built from npm, as it is now) should obviously use core standards, but for the code that will live primarily in GitHub I don't think it's a problem to use augmented/modified standards.

As long as the code here is consistent with itself, if it will be developed in a separate repository it shouldn't be too bad to use its own system.

afercia commented 5 years ago

@chrisvanpatten thanks for sharing 🙂Again, I've opened this issue not to discuss technical details. The main purpose here is outlining the need for documentation, and considering how to properly communicate changes to the standards (if any) to the developers community.

Let's put it this way: as a new contributor to core, the day after 5.0 is released I'll see there are 2 different CSS naming conventions. Am I allowed to use the BEM-like syntax outside of Gutenberg? Yes? No? Maybe? Confusion is great. This needs a decision and documentation.

Also, in https://github.com/WordPress/gutenberg/issues/11677#issuecomment-437533203 @timelsass makes a good point:

there aren't really any publicly declared scss coding standards dictated for people to follow (at least none that I came across)

Gutenberg introduces an intensive usage of scss. Before Gutenberg, it was used only for the alternate color schemes in the admin. This reinforces the need for new documentation and updated coding standards. What are the rules to follow when writing scss? This needs to be documented and published on the Make core handbook.

chrisvanpatten commented 5 years ago

With respect I think a bit of technical discussion is helpful to make the case for why changing the standards is beneficial (it is, in part, a decision that will have a technical impact) but I do appreciate that the next step here is getting a core developer/project lead to weigh in :)

timelsass commented 5 years ago

I would love seeing the entire admin area take on this naming convention globally, and would definitely fit the scope of being a 5.0.0 major version bump as a lot of plugins do things with the admin css so they would need to be prepared. That feeling aside - WordPress code in EVERY language it uses ie PHP/JS/CSS is very un-uniform, and seems that each major feature added adopts it's own way of doing things -- despite the project already having clearly defined standards. Sometimes it's important to update to new standards as languages have progressed, but that should include tasking the time to refactor and maintain the old codebase instead of letting it become stagnant and unmanageable. In the very least having a roadmap and course of action to adopt the new standards in the existing codebase. Otherwise there's really no point in having any sort of publicly declared coding standards in WordPress as a whole because everyone just defines their own and either updates the standards by loosening the strictness of what is or isn't allowed. A naming convention discussion should have been one of the first discussions before code was even laid out, especially since it was the plan all along to introduce entirely new concepts. Now it's just an after thought, and makes it hard for new contributors to understand the codebase as a whole, especially with a lot of the backwards compat and individual coding styles that have made it up to be what it is today.

I agree with what @afercia has said, but also think @chrisvanpatten makes some good points too. Cross project standards DO make sense, and I do have many repos which contain different standards. It just becomes a tricky thing to balance out IMO when talking about the "WordPress Coding Standards" - as that to me means the all inclusive distributed package, WordPress. I will admit - I seldom have made commits, or even tickets on wp.org because svn and trac are not part of my standard workflow. I have no problem hopping in leaving comments in github - because I'm in here everyday for other thing :).
I think part of Gutenberg's increase productivity is not just from using more modern development practices (which clearly speed up development), but it has made contributing in various ways much more accessible to the larger development communities that have always existed outside of the wp.org ecosystem that don't generally want to spend their time going out of their normal workflows to discuss change or contribute to things. I think this was a major leap for WP.

I hope core adopts not only some of the naming conventions and coding styles implemented in Gutenberg globally, but also the workflow that makes it easier for more people to contribute to discussions, mockups, ideas, and general direction. All of these things are continuously complained about in the WP community, but continuing to support legacy ways can be harmful to productivity. I don't think it's that people don't want to contribute, help, or think Gutenberg is bad as some of the more controversial topics have been floating around - it's just more of the lack of accessibility to get connected with the information about it, and knowing where and how you can help without having to spend a week learning how to use svn, figuring out how to view code that's in trunk, understanding where in trac to go, how to reference issues, what to make tickets for, etc. I think at one point I did some theme reviews, and I honestly still don't have much of a clue how to go back to finding out which ones I gave feedback on. Perhaps it's just a culmination of these things that had prevented the planning and discussions on these sort of things to happen earlier on, but hopefully a standard for allowing cross-project standards becomes a thing while still having a roadmap to the future to have all projects eventually migrate to the new adopted standards set. It would make WP development much more enjoyable to sit down and view the entire application as a whole, and know that I could create a full fledged JS theme - 100% WP compliant, have the customizer settings globally for the site, and a wysiwyg editor all at my disposal to create more interactive interfaces and controls in and provide new immersive experiences for users.

I think the discussion needs to be way bigger - not just SCSS coding standards because it's much more than that which Gutenberg changes and sets as a standard.

Just a few major changes and decisions that Gutenberg really had to do:

There's plenty of other example in gutenberg of where it is changing the WP architecture, for the better, and hopefully core can adopt more of what it has done so far, and a better WordPress will come out of it all, SCSS coding standards and all!

PS sry 4.length

talldan commented 5 years ago

I think there's a fairly clear delineation in the standard that should be used when implementing CSS.

Was the design considered in a way that promotes reusable UI elements, and is the interface going to be implemented using a system that follows that methodology? (e.g. React or others)

afercia commented 5 years ago

@talldan sure :) The original point of this issue is the following:

Also, the following point still stands:

there aren't really any publicly declared scss coding standards (edit: or guidelines) dictated for people to follow (at least none that I came across)

youknowriad commented 3 years ago

Looks like this issue might be something to consider for the #core-css meetings. cc @ryelle (update the docs in core about the CSS conventions)