Skip to content
This repository has been archived by the owner on Dec 11, 2021. It is now read-only.

Style Guide: Initial commit of SCSS style guide #112

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 123 additions & 0 deletions style-guides/scss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
<script>{
"title": "Chassis SCSS Style Guide"
}</script>
Copy link
Contributor

Choose a reason for hiding this comment

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

This does nothing unless we either move this to contribute.jquery.org or start using a jquery-wp-content website. Either way in this repo it should be removed.


This page outlines the style guide for SCSS for the jQuery Chassis project.
Copy link
Contributor

Choose a reason for hiding this comment

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

this sounds a little awkward to me maybe change to.
This page outlines the SCSS style guide for the Chassis project


## Comments

SCSS comments should follow the valid CSS comment guidelines outlined in the jQuery contribute CSS style guide.
Copy link
Contributor

Choose a reason for hiding this comment

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

We actually decided against this a while back we should use which ever style makes sense. Comments which will lose context when converted to css. ( like variables ) should use // so that they will not end up in the final css with no context or rules near them that make any sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

Id like to see this cover support comments specificly. I think we should use something like jquery/contribute.jquery.org#95 (comment) see table at bottom of comment and discussion that follows.

Copy link
Contributor

Choose a reason for hiding this comment

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

As I recall, the meeting on 05-05-2015, we decided everything was going to be valid css comments, thus issue #78 was created to update the existing repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

There was no discussion about this on that date in the meeting or in any meeting on any date actually ( at least this year i didn't check last year but this came up much after that ). See http://irc.jquery.org/%23jquery-meeting/default_%23jquery-meeting_20150505.log.html

The discussion on this actually took place in the Typography PR here #57 (comment) and you will see that the conclusion before merging was that we should use whatever makes sense which was no change from what was in the pr / we were already doing.

As for the PR that updated the comments. It does not look like that or the issue it fixes, was ever reviewed or commented on by any one before it was merged / fixed and both reference their reason as the same meeting you reference above which contains no discussion of this. ( There was also no discussion about it in #css-chassis at any point that i can find in the logs. )

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it was my mistake mistake then, my notes on that meeting for that day mentioned converting comments to valid css comments, which must have been come from the Typography PR discussion. I remembered the first half of the Typography PR discussion about comments and wanting to align as closely as possible to valid CSS, but not the second half. Since you and @MichaelArestad did discuss other uses cases for different comments, perhaps it would be good to bring it up in the next meeting, so the group can iron out the details on how we want to handle comments for @kristyjy to incorporate into the Style Guide.

As for the PR on comments, I had that item written down on the agenda for meetings asking the community to review it for a few weeks weeks before it was actually merged in. However if we decide we want to go with different commenting rules for SCSS than for CSS, then we can definitely set up an issue to make everything compliant with this completed style guide.


```scss
/* Good single line comment */

/*
* Good example of a multi-line comment.
* If your comment takes up multiple lines, please do this.
*/
```

## Cascading

Avoid cascading as often as possible and follow the `BEM` naming conventions. If cascading is necessary avoid cascading selectors that are more than 3 selectors long.
Copy link
Contributor

Choose a reason for hiding this comment

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

BEM forbids cascading completely as we are going to use it so there should never be any cascading.


```scss
/*
* Example without BEM using cascading
* (Please avoid this)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are going to use bem this should be more then a kind suggestion there should never be any cascading.

*/
.login {
.submit {
.disabled {

}
}
}

/* Good example using BEM */
.login {
/* Block */
&__submit {
/* Element */
&--disabled {
/* Modifier */
}
}
}
```

## Rule Order

To keep CSS consistant and easy to navigate please use the following order for each rule:

1. scoped variables
Copy link
Contributor

Choose a reason for hiding this comment

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

There should not be any variables directly in the scss

2. @extend
3. @include
4. regular styles
- Sort declarations based on type such as padding/margin, position/top/right/bottom/left etc.
Copy link
Contributor

Choose a reason for hiding this comment

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

we use css-comb for property sorting.

5. pseudo classes and elements
6. media queries
Copy link
Contributor

Choose a reason for hiding this comment

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

From what i'v always seen it seems like general convention is to put media queries last

7. nested selectors / BEM Elements / BEM Modifiers

## Breakpoints

Breakpoints should be named using variables. They should be identified by relative size not by a specific device size. This way the values are easily configurable for the user.
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the discussions we had breakpoints will all be predefined so this seems like it should just be. Use the predefined breakpoints.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry just read next section never mind


```scss
/* Yes */
$breakpoint-xs: 400px;
$breakpoint-sm: 768px;
$breakpoint-md: 920px;
$breakpoint-lg: 1200px;

/* No */
$iphone: 320px;
$ipad: 768px;
```

## Media Queries

Media Queries should always use the predefined breakpoints mentioned above. When adding multiple media queries to a selector sort them from smallest to largest.

```scss
.button {
font-weight: 100;
@media (min-width: $breakpoint-sm) {
font-weight: normal;
}
@media (min-width: $breakpoint-md) {
font-weight: bold;
}
}
```

## Mobile First

All responsive styles should be written using mobile first design. In the example below the initial definition of .button's font size is for small devices and then using a min width media query the originial style is overwritten for larger devices.

```scss
.button {
font-size: $small-font-size;
@media (min-width: $breakpoint-md) {
font-size: $medium-font-size;
}
@media (min-width: $breakpoint-lg) {
font-size: $large-font-size;
}
}
```

## @extend / Placeholder Selectors

In general using @extend should be avoided. When using @extend placeholder selectors should be used in favor of regular selectors. [This article](http://www.smashingmagazine.com/2015/05/extending-in-sass-without-mess/) goes over some tips and explains good use cases for @extend. Mixins are also a great way to avoid @extend and will work inside media queries unlike @extend.

## Magic Numbers

[Magic Numbers](https://css-tricks.com/magic-numbers-in-css/) should be avoided but if used should include a comment stating what it is for and why it's needed.
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like it should be in the CSS style guide not the SCSS style guide since its not specific to scss


## Accessibility

- Define :focus, :active and :hover for hover states on links and buttons
- :active and :focus should also be styled for form elements such as input, textarea and select
- Use unitless definitions for line-height to better support users who increase their font size
- Colors selected should have a high enough contrast for color blind users
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like it should probably be in the CSS style guide not scss this is not scss specific. That said im all for the section but think if we are going to have it it should include things like. This should probably also cover common CSS accessibility pitfalls. Properly hiding elements from a screenreader but showing to visual users, hiding elements from visual users but keeping accessible to screen readers. Hiding from both. And proper use of display none vs visibility hidden vs opacity 0 and how this impacts screen readers. Making sure you css does not break proper tab order ( also plays into properly hiding )

Those are the things that come to mind immediately. @dylanb i'm sure has some other ones i'm not thinking of.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call. Might be good to open up an issue on that repo to include the accessibility items we want in the CSS guide.