Skip to content

Commit d1526b5

Browse files
jonrohanmperrotti
jonrohan
and
mperrotti
authored
Primer border update (#404)
* Pairing for today * parsing * progress on border plugin * updates messages, ignores shorthand border color * rm irrelevant TODO comments * Adjust spacing rule for change in primitives import * Rename to utils * Cleanup * changes * Fix tests * Adjust fixtures * Merge branch 'main' into primer_border_update * enable borders on other syntaxes * Fix for border shorthand directional * Fix border transparent * lint fix * comma * Failing test * update description * allows 'dotted' * accept border-bottom-color prop * ignore border-color * simple tests * ignores border-color properties * updates border shorthand regex to match logical properties too * prevents border-radius values from being replaced with border width variables * appease the linter --------- Co-authored-by: Mike Perrotti <[email protected]>
1 parent b3150cc commit d1526b5

File tree

7 files changed

+413
-183
lines changed

7 files changed

+413
-183
lines changed

‎__tests__/__fixtures__/good/example.scss

+7-7
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,15 @@
99
list-style: none;
1010
cursor: pointer;
1111
background: var(--overlay-bgColor);
12-
border: $border-width $border-style var(--borderColor-default);
13-
border-radius: $border-radius;
12+
border: var(--borderWidth-thin) solid var(--borderColor-default);
13+
border-radius: var(--borderRadius-small);
1414
box-shadow: var(--shadow-resting-medium);
1515

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

2222
.foo {
2323
font-weight: $font-weight-normal;
@@ -26,13 +26,13 @@
2626

2727
&:last-child {
2828
border-bottom: 0;
29-
border-bottom-right-radius: $border-radius;
30-
border-bottom-left-radius: $border-radius;
29+
border-bottom-right-radius: var(--borderRadius-small);
30+
border-bottom-left-radius: var(--borderRadius-small);
3131
}
3232

3333
&:first-child {
34-
border-top-left-radius: $border-radius;
35-
border-top-right-radius: $border-radius;
34+
border-top-left-radius: var(--borderRadius-small);
35+
border-top-right-radius: var(--borderRadius-small);
3636
}
3737

3838
&:hover {

‎__tests__/__fixtures__/good/example.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ const StyledButton = styled.button`
1212
padding: var(--base-size-8);
1313
/* stylelint-disable-next-line primer/spacing */
1414
margin: var(--base-size-8) 10px var(--base-size-8) 0px;
15-
border-radius: 3px;
15+
border-radius: var(--borderRadius-small);
1616
color: ${themeGet('colors.fg.onEmphasis')};
1717
border: 0;
1818
background: transparent;

‎__tests__/borders.js

+172-79
Original file line numberDiff line numberDiff line change
@@ -1,84 +1,177 @@
1+
import plugin from '../plugins/borders.js'
12
import dedent from 'dedent'
2-
import stylelint from 'stylelint'
3-
import borders from '../plugins/borders.js'
43

5-
const ruleName = 'primer/borders'
6-
const configWithOptions = args => ({
7-
plugins: [borders],
8-
rules: {
9-
[ruleName]: args,
10-
},
11-
})
12-
13-
describe(ruleName, () => {
14-
it('reports multiple border properties', () => {
15-
return stylelint
16-
.lint({
17-
code: `
18-
.foo { border: 1px solid gray; }
19-
`,
20-
config: configWithOptions(true),
21-
})
22-
.then(data => {
23-
expect(data).toHaveErrored()
24-
expect(data).toHaveWarnings([
25-
`Please use "$border-width" instead of "1px". See https://primer.style/css/utilities/borders. (${ruleName})`,
26-
`Please use "$border-style" instead of "solid". See https://primer.style/css/utilities/borders. (${ruleName})`,
27-
`Please use a border color variable instead of "gray". See https://primer.style/css/utilities/borders. (${ruleName})`,
28-
])
29-
})
30-
})
4+
const plugins = [plugin]
5+
const {
6+
ruleName,
7+
rule: {messages},
8+
} = plugin
319

32-
it('recognizes function calls as whole tokens', () => {
33-
return stylelint
34-
.lint({
35-
code: `
36-
.foo { border: calc($spacer-2 + var(--derp)) $border-style rgba($border-gray-dark, 50%); }
37-
`,
38-
config: configWithOptions(true),
39-
})
40-
.then(data => {
41-
expect(data).toHaveErrored()
42-
expect(data).toHaveWarnings([
43-
`Please use a border width variable instead of "calc($spacer-2 + var(--derp))". See https://primer.style/css/utilities/borders. (${ruleName})`,
44-
`Please use a border color variable instead of "rgba($border-gray-dark, 50%)". See https://primer.style/css/utilities/borders. (${ruleName})`,
45-
])
46-
})
47-
})
48-
49-
it('allows $border shorthand in border{,-top,-right,-bottom,-left}', () => {
50-
return stylelint
51-
.lint({
52-
code: dedent`
53-
.a { border: $border; }
54-
.b { border-top: $border; }
55-
.c { border-right: $border; }
56-
.d { border-bottom: $border; }
57-
.e { border-left: $border; }
58-
`,
59-
config: configWithOptions(true),
60-
})
61-
.then(data => {
62-
expect(data).not.toHaveErrored()
63-
expect(data).toHaveWarningsLength(0)
64-
})
65-
})
10+
// General Tests
11+
testRule({
12+
plugins,
13+
ruleName,
14+
config: [true, {}],
15+
fix: true,
16+
cache: false,
17+
accept: [
18+
// Border widths
19+
{
20+
code: '.x { border: var(--borderWidth-thin) solid var(--borderColor-default); }',
21+
description: 'CSS > Accepts border shorthand with variables',
22+
},
23+
{
24+
code: '.x { border-width: var(--borderWidth-thin); }',
25+
description: 'CSS > Accepts border shorthand with variables',
26+
},
27+
{
28+
code: '.x { border-left-width: var(--borderWidth-thin); }',
29+
description: 'CSS > Accepts directional border longhand with variables',
30+
},
31+
{
32+
code: '.x { border-inline-start-width: var(--borderWidth-thin); }',
33+
description: 'CSS > Accepts logical properties directional border longhand with variables',
34+
},
35+
{
36+
code: '.x { border: 0; }',
37+
description: 'CSS > Allows zero values',
38+
},
39+
{
40+
code: '.x { border: inherit; border: initial; border: revert; border: revert-layer; border: unset; }',
41+
description: 'CSS > Allows global values',
42+
},
43+
// Border radii
44+
{
45+
code: '.x { border-radius: var(--borderRadius-medium); }',
46+
description: 'CSS > Accepts border-radius with variables',
47+
},
48+
{
49+
code: '.x { border-radius: var(--borderRadius-large) var(--borderRadius-small); }',
50+
description: 'CSS > Accepts border-radius shorthand with variables',
51+
},
52+
{
53+
code: '.x { border-bottom: var(--borderWidth-thin) solid var(--borderColor-muted); }',
54+
description: 'CSS > Accepts directional border-bottom',
55+
},
56+
{
57+
code: '.x { border: var(--borderWidth-thin) solid transparent; }',
58+
description: 'CSS > Accepts transparent colors',
59+
},
60+
{
61+
code: '.x { border-color: red; }',
62+
description: 'CSS > Ignores border-color prop',
63+
},
64+
// Figure out how to allow `calc()` values
65+
],
66+
reject: [
67+
// Border widths
68+
{
69+
code: '.x { border: 20px; }',
70+
unfixable: true,
71+
message: messages.rejected('20px'),
72+
line: 1,
73+
column: 14,
74+
endColumn: 18,
75+
description: 'CSS > Errors on value not in border width list',
76+
},
77+
{
78+
code: '.x { border: $border-width $border-style var(--borderColor-attention-emphasis, var(--color-attention-emphasis)); }',
79+
unfixable: true,
80+
description: 'CSS > Errors on value not in border width list',
81+
warnings: [
82+
{
83+
message: messages.rejected('$border-width'),
84+
line: 1,
85+
column: 14,
86+
endColumn: 27,
87+
rule: 'primer/spacing',
88+
severity: 'error',
89+
},
90+
{
91+
message: messages.rejected('$border-style'),
92+
line: 1,
93+
column: 28,
94+
endColumn: 41,
95+
rule: 'primer/spacing',
96+
severity: 'error',
97+
},
98+
],
99+
},
100+
{
101+
code: '.x { border: 1px; }',
102+
fixed: '.x { border: var(--borderWidth-thin); }',
103+
message: messages.rejected('1px', {name: '--borderWidth-thin'}),
104+
line: 1,
105+
column: 14,
106+
endColumn: 17,
107+
description: "CSS > Replaces '1px' with 'var(--borderWidth-thin)'.",
108+
},
109+
{
110+
code: '.x { border-width: var(--borderRadius-small); }',
111+
unfixable: true,
112+
message: messages.rejected('var(--borderRadius-small)', undefined, 'border-width'),
113+
line: 1,
114+
column: 24,
115+
endColumn: 44,
116+
description: 'CSS > Does not accept a border radius variable for border width.',
117+
},
118+
// Border radii
119+
{
120+
code: '.x { border-radius: 3px; }',
121+
fixed: '.x { border-radius: var(--borderRadius-small); }',
122+
message: messages.rejected('3px', {name: '--borderRadius-small'}),
123+
line: 1,
124+
column: 21,
125+
endColumn: 24,
126+
description: "CSS > Replaces '3px' with 'var(--borderRadius-small)'.",
127+
},
128+
{
129+
code: '.x { border-radius: 0.1875rem; }',
130+
fixed: '.x { border-radius: var(--borderRadius-small); }',
131+
message: messages.rejected('0.1875rem', {name: '--borderRadius-small'}),
132+
line: 1,
133+
column: 21,
134+
endColumn: 30,
135+
description: "CSS > Replaces '0.1875rem' with 'var(--borderRadius-small)'.",
136+
},
137+
{
138+
code: '.x { border-radius: var(--borderWidth-thin); }',
139+
unfixable: true,
140+
message: messages.rejected('var(--borderWidth-thin)', undefined, 'border-radius'),
141+
line: 1,
142+
column: 25,
143+
endColumn: 43,
144+
description: 'CSS > Does not accept a border width variable for border radius.',
145+
},
146+
{
147+
code: '.x { border-radius: 1px; }',
148+
unfixable: true,
149+
message: messages.rejected('1px', undefined, 'border-radius'),
150+
line: 1,
151+
column: 21,
152+
endColumn: 24,
153+
description: 'CSS > Does not autofix 1px to borderWidth-thin variable.',
154+
},
155+
],
156+
})
66157

67-
it('does not report properties with valid border color', () => {
68-
return stylelint
69-
.lint({
70-
code: dedent`
71-
.x { border-color: var(--color-border-primary); }
72-
.y { border-color: var(--color-btn-border-hover); }
73-
.z { border-color: var(--color-diff-deletion-border); }
74-
.a { border-color: var(--color-border); }
75-
.a { border-color: var(--color-accent); }
76-
`,
77-
config: configWithOptions(true),
78-
})
79-
.then(data => {
80-
expect(data).not.toHaveErrored()
81-
expect(data).toHaveWarningsLength(0)
82-
})
83-
})
158+
// Styled Syntax Specific Tests
159+
testRule({
160+
plugins,
161+
ruleName,
162+
customSyntax: 'postcss-styled-syntax',
163+
codeFilename: 'example.tsx',
164+
config: [true, {}],
165+
fix: true,
166+
cache: false,
167+
accept: [
168+
{
169+
code: dedent`
170+
export const IconContainer = styled(Box)\`
171+
border-radius: \${themeGet('radii.2')};
172+
\`
173+
`,
174+
description: 'TSX > Ignores themeGet.',
175+
},
176+
],
84177
})

‎index.js

-2
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,6 @@ export default {
146146
'length-zero-no-unit': null,
147147
'selector-max-type': null,
148148
'primer/colors': null,
149-
'primer/borders': null,
150149
'primer/typography': null,
151150
'primer/box-shadow': null,
152151
},
@@ -200,7 +199,6 @@ export default {
200199
],
201200
'css-modules/no-global-scoped-selector': true,
202201
// temporarily disabiling Primer plugins while we work on upgrades https://github.com/github/primer/issues/3165
203-
'primer/borders': null,
204202
'primer/typography': null,
205203
'primer/box-shadow': null,
206204
},

0 commit comments

Comments
 (0)