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

Docs: Clarify "CSS Naming" coding guidelines #14556

Merged
merged 2 commits into from Apr 3, 2019

Conversation

@aduth
Copy link
Member

commented Mar 21, 2019

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 approved these changes Mar 22, 2019

Copy link
Member

left a comment

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

This comment has been minimized.

Copy link
Member Author

commented Mar 25, 2019

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

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

(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

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2019

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

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

@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

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

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

@afercia

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

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

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

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

1 check passed

Travis CI - Pull Request Build Passed
Details

@aduth aduth deleted the update/coding-guidelines-class-names branch Apr 3, 2019

@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
You can’t perform that action at this time.