Stylelint update #968

shawnbot · 2019-10-31T19:22:56Z

Our new and improved stylelint config introduces some new rules, and these rules demand fealty! Here's my workflow for rectifying the new linting errors, one rule at a time:

Enable the rule in stylelint.config.js.
Run npx stylelint-only <rule> -- --fix src to autofix as many values as possible
Run npx stylelint-disable <rule> -- src to add stylelint-disable-next-line comments above offending declarations.
Run npx stylelint-only <rule> -- src to see if there are any remaining violations, and fix those manually. (Most of these were because of extra lines being added after comments by stylelint-disable).
Run git add -p, skip any changes that are undesirable (such as color: $tooltip-text-color outside of tooltips), then git checkout -- src to throw out the changes we don't want.

I've added stylelint-only and stylelint-disable as dev dependencies so that we can use them more readily for focused linting and/or refactors.

The other thing I've done here is added some new "functional" variables to cover common patterns:

$bg-white: $white "fixes" 32 instances of `background-color: $white'
$text-white: $white fixes 24 instances of color: $white
$text-black: $black fixes a handful of instances of color: $black
$border-white: $white fixes a handful of border colors with $white
$border-color-button: rgba($black, 0.2) replaces a bunch of static instances of button border colors.

I've also added TODO deprecation comments (per #946) that state our intent to deprecate those variables in #925 (v14.0.0). I suppose we should add a new field to our deprecations data for variables too, huh?

…white

…ers)

vercel · 2019-10-31T19:23:04Z

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/primer/primer-css/pc0873qne
🌍 Preview: https://primer-css-git-stylelint-update.primer.now.sh

shawnbot · 2019-10-31T20:50:22Z

src/alerts/flash.scss

@@ -87,7 +95,9 @@
 // FIXME deprecate this


This might be a good candidate for a TODO@14.0.0 comment (see: #946).

src/base/normalize.scss

shawnbot · 2019-10-31T21:13:36Z

src/buttons/button.scss

@@ -15,7 +17,9 @@
  background-repeat: repeat-x;
  background-position: -1px -1px;
  background-size: 110% 110%;
-  border: 1px solid transparentize($black, 0.8);
+  // stylelint-disable-next-line primer/borders
+  border: $border-width $border-style transparentize($black, 0.8);


For consistency, I think we should use rgba() here rather than transparentize() (which decreases the alpha channel by the provided amount):

Suggested change

border: $border-width $border-style transparentize($black, 0.8);

border: $border-width $border-style rgba($black, 0.2);

Of course, at that point we might as well switch over to using $border-black-fade / $black-fade-15, which has an alpha of 15%? 🤔 Here are 15%, 20%, and 30% for comparison:

RGBA SCSS Image

$border-black-fade rgba(27,31,35,0.15)

rgba($black, .2) (current) rgba(27,31,35,0.20)

$black-fade-30 rgba(27,31,35,0.30)

FYI @simurai I found all our transparent $black values and dropped them in our spreadsheet to look at later. At the very least, I think we should standardize on using rgba($color, $alpha) rather than transparentize($color, $alpha-offset).

I've introduced a variable, $border-color-button: rgba($black, 0.2) in 7fd97bf and replaced all of the (equivalent) instances in c5c4f15.

At the very least, I think we should standardize on using rgba($color, $alpha) rather than transparentize($color, $alpha-offset).

Yeah, I find rgba($black, 0.2) much easier to read. Maybe because transparentize($black, 0.8) requires you to do some math first. I guess the only time we would need transparentize() is when we want to make a color that already is semi-transparent a little more transparent. But that's maybe a rare case we don't have to worry about.

shawnbot · 2019-10-31T22:12:37Z

stylelint.config.js

  syntax: 'scss',
  rules: {
+    'scss/dollar-variable-default': [true, {ignore: 'local'}],


This rule enforces the inclusion of !default in all of our global variables.

…arketing vars

shawnbot · 2019-10-31T22:25:38Z

Here is a list of new tasks for #925 when we're ready to merge this:

PRIMER_VERSION=14.0.0 npx stylelint src --quiet

src/marketing/support/variables.scss
 42:1  ✖  Unresolved TODO comment: "remove $marketingSpacers (unused)" (expected to be resolved in "14.0.0")   primer-css/TODO
 44:1  ✖  Unresolved TODO comment: "remove $allSpacers (unused)" (expected to be resolved in "14.0.0")         primer-css/TODO

src/support/variables/colors.scss
  5:1  ✖  Unresolved TODO comment: "remove $status-pending (unused)" (expected to be resolved in "14.0.0")          primer-css/TODO
 10:1  ✖  Unresolved TODO comment: "remove $repo-private-text, $repo-private-bg, and $repo-private-icon" (expected  primer-css/TODO
          to be resolved in "14.0.0")                                                                                              
 16:1  ✖  Unresolved TODO comment: "remove $highlight-yellow (unused)" (expected to be resolved in "14.0.0")        primer-css/TODO

src/support/variables/misc.scss
 29:1  ✖  Unresolved TODO comment: "move $exploregrid-item-border-radius to github/github" (expected to be          primer-css/TODO
          resolved in "14.0.0")                                                                                                    
 32:1  ✖  Unresolved TODO comment: "remove $stats-switcher-py (unused)" (expected to be resolved in "14.0.0")       primer-css/TODO
 37:1  ✖  Unresolved TODO comment: "remove $stats-viewport-height (unused)" (expected to be resolved in "14.0.0")   primer-css/TODO
 40:1  ✖  Unresolved TODO comment: "move $min_tab_size, $max_tab_size to github/github" (expected to be resolved    primer-css/TODO
          in "14.0.0")

simurai

Looks good. 👍

Since this touches a lot of files... do you wanna go ahead and already merge it into release-14.0.0? Then when making new changes we can branch off from release-14.0.0 instead of master to avoid merge conflicts.

src/base/normalize.scss

simurai

🚢

shawnbot added 20 commits October 30, 2019 21:25

bump stylelint-config-primer@9, install stylelint-only

ab379c9

enable primer/colors in stylelint.config.js

66436c6

autofix primer/colors

49d1196

$white + $black -> $text-<color>

34c9665

add $bg-black, $bg-black-fade, $text-white, $text-black, and $border-…

323bc8c

…white

move $black and $white to the top

13b4847

use $bg-black-fade instead of $black-fade-50

72e012e

install stylelint-disable

121b850

break up background properties in <select> + status indicators (spinn…

d350d4b

…ers)

add stylelint-disable comments for primer/colors

c46c7d1

enable primer/borders

b52a7aa

npx stylelint-only primer/borders -- --fix src

6af4ab1

npx stylelint-disable primer/borders -- src

6559cb3

autofix + disable primer/spacing

009705d

autofix some font-weight values

4d5b4c6

enable primer/typography

263dc4f

fix errant empty line

f425cd6

add stylelint-disable comments for primer/typography

268c384

enable primer/box-shadow

fef6e91

add stylelint-disabl comments for primer/box-shadow

4ac0c7f

shawnbot changed the base branch from master to release-14.0.0 October 31, 2019 19:23

shawnbot requested a review from simurai October 31, 2019 20:46

shawnbot marked this pull request as ready for review October 31, 2019 20:49

shawnbot commented Oct 31, 2019

View reviewed changes

src/base/normalize.scss Outdated Show resolved Hide resolved

shawnbot commented Oct 31, 2019

View reviewed changes

shawnbot added 3 commits October 31, 2019 14:17

add !default to $tooltip-background-color

9df0fb8

use $bg-black and $text-black in tooltips

b2e1f74

move grays up, add $border-color-button

7fd97bf

shawnbot added 2 commits October 31, 2019 15:10

add deprecation TODO comments, reorganize misc variables, add !default

5cf3ba8

enforce scss/dollar-variable-default specifically

8177332

vercel bot deployed to staging October 31, 2019 22:10 View deployment

shawnbot commented Oct 31, 2019

View reviewed changes

shawnbot added 3 commits October 31, 2019 15:20

add !default in marketing/support/variables; plan removal of unused m…

8b0f3ee

…arketing vars

move $display-values from utilities to support/variables/layout

ce49044

move $edges from utilities to support/variables/layout

0d4fe91

vercel bot deployed to staging October 31, 2019 22:24 View deployment

simurai approved these changes Nov 1, 2019

View reviewed changes

src/base/normalize.scss Outdated Show resolved Hide resolved

shawnbot mentioned this pull request Nov 1, 2019

Add variable deprecation data and tests #971

Merged

move stylelint-disable to devDeps

d4f87e4

vercel bot deployed to staging November 1, 2019 18:17 View deployment

shawnbot added the Tag: Internal label Nov 1, 2019

shawnbot requested review from BinaryMuse and simurai November 1, 2019 20:15

shawnbot mentioned this pull request Nov 1, 2019

Upgrade stylelint-config-primer, lint all the things #960

Closed

6 tasks

simurai approved these changes Nov 4, 2019

View reviewed changes

use $text-black instead of $text-dark-gray

46c60dc

vercel bot deployed to staging November 4, 2019 22:37 View deployment

shawnbot merged commit c539fcd into release-14.0.0 Nov 4, 2019

shawnbot deleted the stylelint-update branch November 4, 2019 22:47

shawnbot mentioned this pull request Nov 5, 2019

Release 14.0.0 #925

Merged

19 tasks

github-actions bot mentioned this pull request Nov 28, 2022

Link Checker Report ronna/MERL-Center-public#20

Open

This was referenced Dec 25, 2022

Link Checker Report ronna/MERL-Center-public#60

Open

Link Checker Report ronna/MERL-Center-public#64

Open

Link Checker Report ronna/MERL-Center-public#68

Open

github-actions bot mentioned this pull request Jan 7, 2023

Link Checker Report ronna/MERL-Center-public#86

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stylelint update #968

Stylelint update #968

shawnbot commented Oct 31, 2019 •

edited

Loading

vercel bot commented Oct 31, 2019 •

edited

Loading

shawnbot Oct 31, 2019

shawnbot Oct 31, 2019

shawnbot Oct 31, 2019

shawnbot Nov 1, 2019

simurai Nov 4, 2019

shawnbot Oct 31, 2019

shawnbot commented Oct 31, 2019

simurai left a comment

simurai left a comment

	border: $border-width $border-style transparentize($black, 0.8);
	border: $border-width $border-style rgba($black, 0.2);

RGBA	SCSS	Image
`$border-black-fade`	`rgba(27,31,35,0.15)`

`rgba($black, .2)` (current)	`rgba(27,31,35,0.20)`

`$black-fade-30`	`rgba(27,31,35,0.30)`

Stylelint update #968

Stylelint update #968

Conversation

shawnbot commented Oct 31, 2019 • edited Loading

vercel bot commented Oct 31, 2019 • edited Loading

shawnbot Oct 31, 2019

Choose a reason for hiding this comment

shawnbot Oct 31, 2019

Choose a reason for hiding this comment

shawnbot Oct 31, 2019

Choose a reason for hiding this comment

shawnbot Nov 1, 2019

Choose a reason for hiding this comment

simurai Nov 4, 2019

Choose a reason for hiding this comment

shawnbot Oct 31, 2019

Choose a reason for hiding this comment

shawnbot commented Oct 31, 2019

simurai left a comment

Choose a reason for hiding this comment

simurai left a comment

Choose a reason for hiding this comment

shawnbot commented Oct 31, 2019 •

edited

Loading

vercel bot commented Oct 31, 2019 •

edited

Loading