Skip to content

Docs: Clarify "CSS Naming" coding guidelines#14556

Merged
aduth merged 2 commits into
masterfrom
update/coding-guidelines-class-names
Apr 3, 2019
Merged

Docs: Clarify "CSS Naming" coding guidelines#14556
aduth merged 2 commits into
masterfrom
update/coding-guidelines-class-names

Conversation

@aduth

@aduth aduth commented Mar 21, 2019

Copy link
Copy Markdown
Member

This pull request aims to improve the readability and accuracy of the "Naming" text within the Coding Guidelines CSS section, toward several ends:

  • The previous text was specific to "editor" components, and did not account for components defined outside this package (e.g. the components package). The convention was still being enforced, but the guidelines were not supportive of the pattern.
  • Provide better real code examples, including a full component implementation based on a real example (the @wordpress/component Notice component).
  • More strongly assert that class names should never be used outside the component's own directory (it is meant to be implied by the rule itself, but was never strictly mentioned)

@gziolo gziolo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example included makes it much easier to understand. It might be also a good idea to start with an example before getting into all the conventions explained in depth. Maybe it could be moved after the second paragraph before explaining is-* class names which aren't included in the example.

@aduth

aduth commented Mar 25, 2019

Copy link
Copy Markdown
Member Author

It might be also a good idea to start with an example before getting into all the conventions explained in depth.

I noticed this as well, and worried that having the example in the midst of all the text might make it harder to realize there is more to the rule, especially considering that the example has a few paragraphs of its own to explain what's going on. If it's desirable, I think I'd suggest doing it by some combination of (a) having the example be as minimal as possible to avoid the disruptive reading flow and (b) perhaps inline or removing the complementing explaining paragraphs.

@gziolo

gziolo commented Mar 26, 2019

Copy link
Copy Markdown
Member

(a) having the example be as minimal as possible to avoid the disruptive reading flow and (b) perhaps inline or removing the complementing explaining paragraphs.

I think it's fine to keep all explanations included in this PR. Those CSS naming rules should be documented as WordPress developers have different expectations learned from the patterns used in the past. Another way of tackling it is by building an example as new rules introduced:

export default function Notice( { children } ) {
 	return (
 		<div className="components-notice">
 	 	 	{ children }
 	 	</div>
 	);
}

then

export default function Notice( { children } ) {
 	return (
 		<div className="components-notice">
 	 	 	<div className="components-notice__content">
 	 	 	 	{ children }
 	 	 	</div>
 	 	</div>
 	);
}

and so on.

@afercia

afercia commented Mar 30, 2019

Copy link
Copy Markdown
Member

Once this is defined and approved, I'd strongly suggest to add a dedicated section to the CSS coding standards handbook page: https://make.wordpress.org/core/handbook/best-practices/coding-standards/css/

Gutenberg is part of core and its standards need to be documented in the official core handbook too.

@aduth

aduth commented Apr 1, 2019

Copy link
Copy Markdown
Member Author

@afercia Do you know the process I could follow for starting this conversation? It's a bit unclear to me as well how cleanly these guidelines translate as far as being highly tied to components and packages , processes which occur exclusively in this repository.

@aduth

aduth commented Apr 1, 2019

Copy link
Copy Markdown
Member Author

@gziolo I pushed a commit which rearranges the text to include examples closer to where the specific guideline is prescribed.

@afercia

afercia commented Apr 1, 2019

Copy link
Copy Markdown
Member

processes which occur exclusively in this repository.

@aduth I'd tend to think this is the point 🙂I see many in the Gutenberg team tend to consider "this repository" as something separated by core. To me, it's part of core and it will be more and more in the future.

As for all the core related things, dev chat would be a good place where to start a conversation.

@aduth

aduth commented Apr 1, 2019

Copy link
Copy Markdown
Member Author

To me, it's part of core and it will be more and more in the future.

To be clear, I don't dispute this. In the literal sense, Gutenberg exists today as a separate repository from the rest of core, but I too operate on the assumption that all of WordPress' source is set to converge in the future. The latter part of my previous comment was not to imply that it's not subject to participation in the same core development guidelines, but rather to frame the request for logistical advice in mind of a larger conversation around componentized UI patterns.

@aduth aduth merged commit b56842f into master Apr 3, 2019
@aduth aduth deleted the update/coding-guidelines-class-names branch April 3, 2019 14:01
@youknowriad youknowriad added this to the 5.5 (Gutenberg) milestone Apr 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Developer Documentation Documentation for developers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants