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

Primer border update #404

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
14 changes: 7 additions & 7 deletions __tests__/__fixtures__/good/example.scss
Expand Up @@ -9,15 +9,15 @@
list-style: none;
cursor: pointer;
background: var(--overlay-bgColor);
border: $border-width $border-style var(--borderColor-default);
border-radius: $border-radius;
border: var(--borderWidth-thin) solid var(--borderColor-default);
border-radius: var(--borderRadius-small);
box-shadow: var(--shadow-resting-medium);

.li {
display: block;
padding: var(--base-size-4) var(--base-size-8);
font-weight: $font-weight-semibold;
border-bottom: $border-width $border-style var(--borderColor-muted);
border-bottom: var(--borderWidth-thin) solid var(--borderColor-muted);

.foo {
font-weight: $font-weight-normal;
Expand All @@ -26,13 +26,13 @@

&:last-child {
border-bottom: 0;
border-bottom-right-radius: $border-radius;
border-bottom-left-radius: $border-radius;
border-bottom-right-radius: var(--borderRadius-small);
border-bottom-left-radius: var(--borderRadius-small);
}

&:first-child {
border-top-left-radius: $border-radius;
border-top-right-radius: $border-radius;
border-top-left-radius: var(--borderRadius-small);
border-top-right-radius: var(--borderRadius-small);
}

&:hover {
Expand Down
216 changes: 135 additions & 81 deletions __tests__/borders.js
@@ -1,84 +1,138 @@
import dedent from 'dedent'
import stylelint from 'stylelint'
import borders from '../plugins/borders.js'
import plugin from '../plugins/borders.js'

const ruleName = 'primer/borders'
const configWithOptions = args => ({
plugins: [borders],
rules: {
[ruleName]: args,
},
})

describe(ruleName, () => {
it('reports multiple border properties', () => {
return stylelint
.lint({
code: `
.foo { border: 1px solid gray; }
`,
config: configWithOptions(true),
})
.then(data => {
expect(data).toHaveErrored()
expect(data).toHaveWarnings([
`Please use "$border-width" instead of "1px". See https://primer.style/css/utilities/borders. (${ruleName})`,
`Please use "$border-style" instead of "solid". See https://primer.style/css/utilities/borders. (${ruleName})`,
`Please use a border color variable instead of "gray". See https://primer.style/css/utilities/borders. (${ruleName})`,
])
})
})

it('recognizes function calls as whole tokens', () => {
return stylelint
.lint({
code: `
.foo { border: calc($spacer-2 + var(--derp)) $border-style rgba($border-gray-dark, 50%); }
`,
config: configWithOptions(true),
})
.then(data => {
expect(data).toHaveErrored()
expect(data).toHaveWarnings([
`Please use a border width variable instead of "calc($spacer-2 + var(--derp))". See https://primer.style/css/utilities/borders. (${ruleName})`,
`Please use a border color variable instead of "rgba($border-gray-dark, 50%)". See https://primer.style/css/utilities/borders. (${ruleName})`,
])
})
})

it('allows $border shorthand in border{,-top,-right,-bottom,-left}', () => {
return stylelint
.lint({
code: dedent`
.a { border: $border; }
.b { border-top: $border; }
.c { border-right: $border; }
.d { border-bottom: $border; }
.e { border-left: $border; }
`,
config: configWithOptions(true),
})
.then(data => {
expect(data).not.toHaveErrored()
expect(data).toHaveWarningsLength(0)
})
})
const plugins = [plugin]
const {
ruleName,
rule: {messages},
} = plugin

it('does not report properties with valid border color', () => {
return stylelint
.lint({
code: dedent`
.x { border-color: var(--color-border-primary); }
.y { border-color: var(--color-btn-border-hover); }
.z { border-color: var(--color-diff-deletion-border); }
.a { border-color: var(--color-border); }
.a { border-color: var(--color-accent); }
`,
config: configWithOptions(true),
})
.then(data => {
expect(data).not.toHaveErrored()
expect(data).toHaveWarningsLength(0)
})
})
// General Tests
testRule({
plugins,
ruleName,
config: [true, {}],
fix: true,
cache: false,
accept: [
// Border widths
{
code: '.x { border: var(--borderWidth-thin) solid var(--borderColor-default); }',
description: 'CSS > Accepts border shorthand with variables',
},
{
code: '.x { border-width: var(--borderWidth-thin); }',
description: 'CSS > Accepts border shorthand with variables',
},
{
code: '.x { border-left-width: var(--borderWidth-thin); }',
description: 'CSS > Accepts directional border longhand with variables',
},
{
code: '.x { border-inline-start-width: var(--borderWidth-thin); }',
description: 'CSS > Accepts logical properties directional border longhand with variables',
},
{
code: '.x { border: 0; }',
description: 'CSS > Allows zero values',
},
{
code: '.x { border: inherit; border: initial; border: revert; border: revert-layer; border: unset; }',
description: 'CSS > Allows global values',
},
// Border radii
{
code: '.x { border-radius: var(--borderRadius-medium); }',
description: 'CSS > Accepts border-radius with variables',
},
{
code: '.x { border-radius: var(--borderRadius-large) var(--borderRadius-small); }',
description: 'CSS > Accepts border-radius shorthand with variables',
},
{
code: '.x { border-bottom: var(--borderWidth-thin) solid var(--borderColor-muted); }',
description: 'CSS > Accepts directional border-bottom',
},
// Figure out how to allow `calc()` values
],
reject: [
// Border widths
{
code: '.x { border: 20px; }',
unfixable: true,
message: messages.rejected('20px'),
line: 1,
column: 14,
endColumn: 18,
description: 'CSS > Errors on value not in border width list',
},
{
code: '.x { border: $border-width $border-style var(--borderColor-attention-emphasis, var(--color-attention-emphasis)); }',
unfixable: true,
description: 'CSS > Errors on value not in border width list',
warnings: [
{
message: messages.rejected('$border-width'),
line: 1,
column: 14,
endColumn: 27,
rule: 'primer/spacing',
severity: 'error',
},
{
message: messages.rejected('$border-style'),
line: 1,
column: 28,
endColumn: 41,
rule: 'primer/spacing',
severity: 'error',
},
],
},
{
code: '.x { border: 1px; }',
fixed: '.x { border: var(--borderWidth-thin); }',
message: messages.rejected('1px', {name: '--borderWidth-thin'}),
line: 1,
column: 14,
endColumn: 17,
description: "CSS > Replaces '1px' with 'var(--borderWidth-thin)'.",
},
{
code: '.x { border-width: var(--borderRadius-small); }',
unfixable: true,
message: messages.rejected('var(--borderRadius-small)', undefined, 'border-width'),
line: 1,
column: 24,
endColumn: 44,
description: 'CSS > Does not accept a border radius variable for border width.',
},
// Border radii
{
code: '.x { border-radius: 3px; }',
fixed: '.x { border-radius: var(--borderRadius-small); }',
message: messages.rejected('3px', {name: '--borderRadius-small'}),
line: 1,
column: 21,
endColumn: 24,
description: "CSS > Replaces '3px' with 'var(--borderRadius-small)'.",
},
{
code: '.x { border-radius: 0.1875rem; }',
fixed: '.x { border-radius: var(--borderRadius-small); }',
message: messages.rejected('0.1875rem', {name: '--borderRadius-small'}),
line: 1,
column: 21,
endColumn: 30,
description: "CSS > Replaces '0.1875rem' with 'var(--borderRadius-small)'.",
},
{
code: '.x { border-radius: var(--borderWidth-thin); }',
unfixable: true,
message: messages.rejected('var(--borderWidth-thin)', undefined, 'border-radius'),
line: 1,
column: 25,
endColumn: 43,
description: 'CSS > Does not accept a border width variable for border radius.',
},
],
})
2 changes: 0 additions & 2 deletions index.js
Expand Up @@ -146,7 +146,6 @@ export default {
'length-zero-no-unit': null,
'selector-max-type': null,
'primer/colors': null,
'primer/borders': null,
'primer/typography': null,
'primer/box-shadow': null,
},
Expand Down Expand Up @@ -200,7 +199,6 @@ export default {
],
'css-modules/no-global-scoped-selector': true,
// temporarily disabiling Primer plugins while we work on upgrades https://github.com/github/primer/issues/3165
'primer/borders': null,
'primer/typography': null,
'primer/box-shadow': null,
},
Expand Down