Refs #2 Adds beginning outline of our scss coding standards #3

kmuncie · 2017-07-18T21:07:48Z

Cleaned up a few things and tried to isolate from any one project example

jthomerson

Please make the changes noted in the review. Once we have all those changes reviewed, there will also be one sweeping change that needs to be made to line-wrap all your paragraphs, but we will hold off on that until after we get the actual review done so that the re-reviews are easier.

jthomerson · 2017-12-01T14:04:30Z

scss-coding-standards.md

+
+Sass (file extension .scss) is a language which compiles to CSS. It is a powerful tool in the writing of CSS but does not absolve you as a developer from the code which is output. You must understand what good CSS looks like before you can effectively write Sass.
+
+Below are some guiding principles and rules when writting styling code.


s/writting/writing/

jthomerson · 2017-12-01T14:05:33Z

scss-coding-standards.md

+* >3 selectors (when compiled)
+* ID's or !important
+* element.class
+* z-index declarations


Please format all markdown lists where they start with a three-space indent as is done in our other markdown files in this repo.

jthomerson · 2017-12-01T14:06:40Z

scss-coding-standards.md

+Some basic [code smells](https://en.wikipedia.org/wiki/Code_smell) might include:
+
+* >3 selectors (when compiled)
+* ID's or !important


no apostrophe ... the ID is not possessive or a contraction

jthomerson · 2017-12-01T14:07:53Z

scss-coding-standards.md

+
+### Naming
+
+Class and ID names should be **camel case (first word lower-case) and dashes used sparingly**. Example: `.classNameThatsPrettyLong`


rather than re-defining what camel case means here, just link the words "camel case" to the glossary

I also think "and dashes used sparingly" should be separated into its own paragraph rather than being in the same one that has the camelCase rule. That paragraph can explain acceptable and unacceptable usecases for hyphens. (also better to call them hyphens than dashes since "dash" is ambiguous)

Rewrote naming section to simplify

jthomerson · 2017-12-01T14:10:34Z

scss-coding-standards.md

+
+Class and ID names should be **camel case (first word lower-case) and dashes used sparingly**. Example: `.classNameThatsPrettyLong`
+
+Variables should in ["kebab-case" style](https://en.wikipedia.org/wiki/Letter_case#Special_case_styles) and feature a logical structure which


Rather than linking externally to a definition of a case, this should be added to the other definitions of cases in our glossary. The "kebab-case" (no need for the word "style" here) can be linked to the glossary.

Rewrote naming section to simplify, not even mentioning kebab-case any longer

jthomerson · 2017-12-01T14:25:45Z

scss-coding-standards.md

+```
+.mainContainer {
+   .nestedClassOne {
+     color: Tomato;


You have an indentation error here.

jthomerson · 2017-12-01T14:27:25Z

scss-coding-standards.md

+         color: firebrick;
+      }
+   }
+   // ----------------------


For example, I would have expected a blank line above this comment line ...

I have a strong personal opinion on this so perhaps we can discuss further

jthomerson · 2017-12-01T14:28:22Z

scss-coding-standards.md

+   }
+   .oneThing,
+   .secondThing,
+   .thirdThing {


I didn't see anything in the documentation about this ... lists of selectors separated by commas. Should they always be on their own line, or would .oneThing, .secondThing, .thirdThing { ever be valid?

jthomerson · 2017-12-01T14:29:04Z

scss-coding-standards.md

+}
+```
+
+Most **RTL** rules should be written so as to be independent of the page direction. We have mixed content in many places so any major component should get a direction set on its outermost container. Example:


Most people may not be familiar with the term RTL, so please s/RTL/right-to-left (RTL)/ here, and then you can use RTL after that as much as needed.

"We have mixed content in many places" may not make sense in this public forum ... it's an internal thing. Please explain it in a more neutral way, and clarify what "mixed content" means.

jthomerson · 2017-12-01T14:30:19Z

scss-coding-standards.md

+}
+```
+
+An example where this method is used extensively is in *_synopsis.scss*. Because synopsis is an element that can inherit the direction of the site language *or* the direction of the mixed content language (depending on where it appears on the page), its behavior relies only on the language direction explicitly specified on the synopsis container itself.


You can't have references to an internal piece of code in this public repo.

Also, let's hyphenate mixed-content so it reads more clearly.

pbredenberg · 2018-02-08T22:48:55Z

It would not be terrible for this to get merged sometime soon. kthanksbye

acemuncie

Please take a look at the changes in this review. Based on our other documentation, I've tried to note where the voice should change from "we" to imperative "you".

acemuncie · 2018-10-30T14:01:55Z

scss-coding-standards.md

@@ -0,0 +1,139 @@
+# Styling Code Standards
+
+Sass (file extension .scss) is a language which compiles to CSS. It is a powerful tool in the writing of CSS but does not absolve you as a


Needs additional wording - language that compiles what to CSS?

acemuncie · 2018-10-30T14:10:13Z

scss-coding-standards.md

+
+## General Standards
+
+Unless there is a compelling reason to use an ID we should always default to using classes [[1]](#references) for styling.


What about:
Unless there is a compelling reason to use an ID, you should default to classes first for styling [1].

acemuncie · 2018-10-30T14:42:14Z

scss-coding-standards.md

+
+Unless there is a compelling reason to use an ID we should always default to using classes [[1]](#references) for styling.
+
+Code smells [[2]](#references) to watch out for might include:


What about:
Be wary of these code smells[2]:

acemuncie · 2018-10-30T14:44:28Z

scss-coding-standards.md

+
+Code smells [[2]](#references) to watch out for might include:
+
+   * >3 selectors (when compiled)


What about spelling out > to More than 3 selectors? Otherwise, > needs to be escaped.

acemuncie · 2018-10-30T14:45:45Z

scss-coding-standards.md

+Code smells [[2]](#references) to watch out for might include:
+
+   * >3 selectors (when compiled)
+   * Using an ID or !important


!important should be inline code

acemuncie · 2018-10-30T15:24:18Z

scss-coding-standards.md

+
+### Blank Lines
+
+Follow our blank lines [[8]](#references) standards. There should always be a blank line seperating top level selectors.


s/seperating/separating/

s/top level/top-level/

acemuncie · 2018-10-30T15:25:34Z

scss-coding-standards.md

+
+Follow our blank lines [[8]](#references) standards. There should always be a blank line seperating top level selectors.
+
+In most cases multiple selectors in a single block should each be listed on a new line.


Needs comma after In most cases

acemuncie · 2018-10-30T15:42:50Z

scss-coding-standards.md

+   * `$color-link--visited`
+   * `.pm-mediaGroup-top--mobile`
+   * `.pm-mediaGroup-top--desktop`
+   * `$color-primButtonBg--hover`


The above three examples seem like internal names.

acemuncie · 2018-10-30T15:49:53Z

scss-coding-standards.md

+```
+### References
+
+[1] [http://oli.jp/2011/ids/](http://oli.jp/2011/ids/)


When viewing the document, these references run together in one line. A linebreak is needed after each reference.

acemuncie · 2018-10-30T15:56:28Z

scss-coding-standards.md

+
+### Directional Content
+
+Often styling needs to consider if the immediate context is being displayed in a left-to-right (LTR) or right-to-left (RTL) manner,


Can we add some context for when this is needed? Something like:
When content with different writing directions is included on the same page...

jthomerson requested changes Dec 1, 2017

View reviewed changes

onebytegone assigned kmuncie Sep 27, 2018

Refs silvermine#2 Adds beginning outline of our scss coding standards

564c910

kmuncie force-pushed the add_stylingStandards branch from f907bed to 564c910 Compare October 11, 2018 20:59

acemuncie suggested changes Oct 30, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refs #2 Adds beginning outline of our scss coding standards #3

Refs #2 Adds beginning outline of our scss coding standards #3

kmuncie commented Jul 18, 2017

jthomerson left a comment

jthomerson Dec 1, 2017

kmuncie Oct 9, 2018

jthomerson Dec 1, 2017

kmuncie Oct 9, 2018

jthomerson Dec 1, 2017

kmuncie Oct 9, 2018

jthomerson Dec 1, 2017

jthomerson Dec 1, 2017

kmuncie Oct 10, 2018

jthomerson Dec 1, 2017

kmuncie Oct 10, 2018

jthomerson Dec 1, 2017

jthomerson Dec 1, 2017

kmuncie Oct 11, 2018

jthomerson Dec 1, 2017

kmuncie Oct 11, 2018

jthomerson Dec 1, 2017

jthomerson Dec 1, 2017

jthomerson Dec 1, 2017

jthomerson Dec 1, 2017

pbredenberg commented Feb 8, 2018

acemuncie left a comment

acemuncie Oct 30, 2018

acemuncie Oct 30, 2018

acemuncie Oct 30, 2018

acemuncie Oct 30, 2018

acemuncie Oct 30, 2018

acemuncie Oct 30, 2018

acemuncie Oct 30, 2018

acemuncie Oct 30, 2018

acemuncie Oct 30, 2018

acemuncie Oct 30, 2018


		Sass (file extension .scss) is a language which compiles to CSS. It is a powerful tool in the writing of CSS but does not absolve you as a developer from the code which is output. You must understand what good CSS looks like before you can effectively write Sass.

		Below are some guiding principles and rules when writting styling code.


		### Naming

		Class and ID names should be camel case (first word lower-case) and dashes used sparingly. Example: `.classNameThatsPrettyLong`


		Class and ID names should be camel case (first word lower-case) and dashes used sparingly. Example: `.classNameThatsPrettyLong`

		Variables should in ["kebab-case" style](https://en.wikipedia.org/wiki/Letter_case#Special_case_styles) and feature a logical structure which

		@@ -0,0 +1,139 @@
		# Styling Code Standards

		Sass (file extension .scss) is a language which compiles to CSS. It is a powerful tool in the writing of CSS but does not absolve you as a


		## General Standards

		Unless there is a compelling reason to use an ID we should always default to using classes [[1]](#references) for styling.


		Unless there is a compelling reason to use an ID we should always default to using classes [[1]](#references) for styling.

		Code smells [[2]](#references) to watch out for might include:


		Code smells [[2]](#references) to watch out for might include:

		* >3 selectors (when compiled)


		### Blank Lines

		Follow our blank lines [[8]](#references) standards. There should always be a blank line seperating top level selectors.


		Follow our blank lines [[8]](#references) standards. There should always be a blank line seperating top level selectors.

		In most cases multiple selectors in a single block should each be listed on a new line.


		### Directional Content

		Often styling needs to consider if the immediate context is being displayed in a left-to-right (LTR) or right-to-left (RTL) manner,

Refs #2 Adds beginning outline of our scss coding standards #3

Are you sure you want to change the base?

Refs #2 Adds beginning outline of our scss coding standards #3

Conversation

kmuncie commented Jul 18, 2017

jthomerson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pbredenberg commented Feb 8, 2018

acemuncie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment