Skip to content

Commit

Permalink
Primer typography update (#418)
Browse files Browse the repository at this point in the history 
* checking in WIP

* rm outdated TODO comment

* updates good example fixture with typography usage

* rm walkGroups, code cleanup

* attempt to fix linting errors

* attempt to fix linting errors

* rm needless disable of primer/typography

* Turn on primer/typography for other files

* Fixing tests

---------

Co-authored-by: Jon Rohan <yes@jonrohan.codes>
  • Loading branch information
@mperrotti @jonrohan
mperrotti and jonrohan committed Sep 11, 2024
1 parent cac16d6 commit 7e5f31c
Show file tree
Hide file tree
Showing 7 changed files with 421 additions and 93 deletions.
1 change: 0 additions & 1 deletion __tests__/__fixtures__/good/example.pcss
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* stylelint-disable primer/borders */
/* stylelint-disable primer/typography */
/* stylelint-disable primer/spacing */

/* CSS for Button */
Expand Down
4 changes: 2 additions & 2 deletions __tests__/__fixtures__/good/example.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
.li {
display: block;
padding: var(--base-size-4) var(--base-size-8);
font-weight: $font-weight-semibold;
font-weight: var(--base-text-weight-semibold);
border-bottom: var(--borderWidth-thin) solid var(--borderColor-muted);

.foo {
font-weight: $font-weight-normal;
font-weight: var(--base-text-weight-normal);
color: var(--fgColor-muted);
}

Expand Down
4 changes: 3 additions & 1 deletion __tests__/__fixtures__/good/example.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,14 @@ export const StyledToast = styled.div`
export const ToastAction = styled.button<SxProp>`
background-color: transparent;
border: 0;
/* stylelint-disable-next-line primer/typography */
font-weight: ${themeGet('fontWeights.bold')};
margin-left: ${themeGet('space.2')};
margin-top: ${themeGet('space.3')};
margin-bottom: ${themeGet('space.3')};
color: ${themeGet('colors.fg.onEmphasis')};
/* stylelint-disable-next-line primer/typography */
font-size: ${themeGet('fontSizes.1')};
font-family: inherit;
outline: none;
Expand Down Expand Up @@ -180,4 +182,4 @@ const Toast = (props: ToastProps) => {
)
}

export default Toast
export default Toast
291 changes: 227 additions & 64 deletions __tests__/typography.js
View file
Original file line number Diff line number Diff line change
@@ -1,67 +1,230 @@
import dedent from 'dedent'
import stylelint from 'stylelint'
import pluginPath from '../plugins/typography.js'
import plugin from '../plugins/typography.js'

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

describe(ruleName, () => {
describe('font-size', () => {
it('reports properties w/o variables', () => {
return stylelint
.lint({
code: dedent`
.x { font-size: 11px; }
.y { font-size: $spacer-3; }
`,
config: configWithOptions(true),
})
.then(data => {
expect(data).toHaveErrored()
expect(data).toHaveWarnings([
`Please use a font-size variable instead of "11px". See https://primer.style/css/utilities/typography. (${ruleName})`,
`Please use a font-size variable instead of "$spacer-3". See https://primer.style/css/utilities/typography. (${ruleName})`,
])
})
})

it('does not report font size variables', () => {
return stylelint
.lint({
code: dedent`
.h1 { font-size: $h1-size; }
.h2 { font-size: $h2-size; }
small { font-size: $font-size-small; }
`,
config: configWithOptions(true),
})
.then(data => {
expect(data).not.toHaveErrored()
expect(data).toHaveWarningsLength(0)
})
})
const plugins = [plugin]
const {
ruleName,
rule: {messages},
} = plugin

it('can fix them', () => {
return stylelint
.lint({
code: dedent`
.x { font-size: 32px; }
`,
config: configWithOptions(true, {verbose: true}),
fix: true,
})
.then(data => {
expect(data).not.toHaveErrored()
expect(data).toHaveWarningsLength(0)
expect(data.output).toEqual(dedent`
.x { font-size: $h1-size; }
`)
})
})
})
// General Tests
testRule({
plugins,
ruleName,
config: [true, {}],
fix: true,
cache: false,
accept: [
// Font sizes
{
code: '.x { font-size: var(--text-title-size-medium); }',
description: 'CSS > Accepts font size variables',
},
// Font weights
{
code: '.x { font-weight: var(--base-text-weight-semibold); }',
description: 'CSS > Accepts base font weight variables',
},
{
code: '.x { font-weight: var(--text-title-weight-medium); }',
description: 'CSS > Accepts functional font weight variables',
},
// Line heights
{
code: '.x { line-height: var(--text-title-lineHeight-medium); }',
description: 'CSS > Accepts line height variables',
},
// Font family
{
code: '.x { font-family: var(--fontStack-system); }',
description: 'CSS > Accepts font stack variables',
},
// Font shorthand
{
code: '.x { font: var(--text-display-shorthand); }',
description: 'CSS > Accepts font shorthand variables',
},
],
reject: [
// Font sizes
{
code: '.x { font-size: 42px; }',
unfixable: true,
message: messages.rejected('42px'),
line: 1,
column: 17,
endColumn: 21,
description: 'CSS > Errors on value not in font size list',
},
{
code: '.x { font-size: 40px; }',
fixed: '.x { font-size: var(--text-display-size); }',
message: messages.rejected('40px', {name: '--text-display-size'}),
line: 1,
column: 17,
endColumn: 21,
description: "CSS > Replaces '40px' with 'var(--text-display-size)'.",
},
{
code: '.x { font-size: 2.5rem; }',
fixed: '.x { font-size: var(--text-display-size); }',
message: messages.rejected('2.5rem', {name: '--text-display-size'}),
line: 1,
column: 17,
endColumn: 23,
description: "CSS > Replaces '2.5rem' with 'var(--text-display-size)'.",
},
{
code: '.x { font-size: 1.25rem; }',
unfixable: true,
message: messages.rejected('1.25rem', [{name: '--text-title-size-medium'}, {name: '--text-subtitle-size'}]),
line: 1,
column: 17,
endColumn: 24,
description: "CSS > Suggests list of variables to replace '1.25rem' with.",
},
// Font weights
{
code: '.x { font-weight: 500; }',
unfixable: true,
message: messages.rejected('500', [{name: '--base-text-weight-medium'}, {name: '--text-display-weight'}]),
line: 1,
column: 19,
endColumn: 22,
description:
"CSS > Errors on font-weight of 500 and suggests '--base-text-weight-medium' or '--text-display-weight'.",
},
{
code: '.x { font-weight: 100; }',
fixed: '.x { font-weight: var(--base-text-weight-light); }',
message: messages.rejected('100', {name: '--base-text-weight-light'}),
line: 1,
column: 19,
endColumn: 22,
description: "CSS > Replaces font-weight less than 300 with 'var(--base-text-weight-light)'.",
},
{
code: '.x { font-weight: 800; }',
unfixable: true,
message: messages.rejected('800', [
{name: '--base-text-weight-semibold'},
{name: '--text-title-weight-large'},
{name: '--text-title-weight-medium'},
{name: '--text-title-weight-small'},
]),
line: 1,
column: 19,
endColumn: 22,
description:
"CSS > Errors on font-weight greater than 600 and suggests '--base-text-weight-semibold', '--text-title-weight-large', '--text-title-weight-medium', or '--text-title-weight-small'.",
},
{
code: '.x { font-weight: lighter; }',
fixed: '.x { font-weight: var(--base-text-weight-light); }',
message: messages.rejected('lighter', {name: '--base-text-weight-light'}),
line: 1,
column: 19,
endColumn: 26,
description: "CSS > Replaces 'lighter' font-weight keyword with 'var(--base-text-weight-light)'.",
},
{
code: '.x { font-weight: bold; }',
unfixable: true,
message: messages.rejected('bold', [
{name: '--base-text-weight-semibold'},
{name: '--text-title-weight-large'},
{name: '--text-title-weight-medium'},
{name: '--text-title-weight-small'},
]),
line: 1,
column: 19,
endColumn: 23,
description:
"CSS > Errors on 'bold' font-weight keyword and suggests '--base-text-weight-semibold', '--text-title-weight-large', '--text-title-weight-medium', or '--text-title-weight-small'.",
},
{
code: '.x { font-weight: bolder; }',
unfixable: true,
message: messages.rejected('bolder', [
{name: '--base-text-weight-semibold'},
{name: '--text-title-weight-large'},
{name: '--text-title-weight-medium'},
{name: '--text-title-weight-small'},
]),
line: 1,
column: 19,
endColumn: 25,
description:
"CSS > Errors on 'bolder' font-weight keyword and suggests '--base-text-weight-semibold', '--text-title-weight-large', '--text-title-weight-medium', or '--text-title-weight-small'.",
},
{
code: '.x { font-weight: normal; }',
unfixable: true,
message: messages.rejected('normal', [
{name: '--base-text-weight-normal'},
{name: '--text-subtitle-weight'},
{name: '--text-body-weight'},
{name: '--text-caption-weight'},
{name: '--text-codeBlock-weight'},
{name: '--text-codeInline-weight'},
]),
line: 1,
column: 19,
endColumn: 25,
description:
"CSS > Errors on 'normal' font-weight keyword and suggests '--base-text-weight-normal', '--text-subtitle-weight', '--text-body-weight', '--text-caption-weight', '--text-codeBlock-weight' or '--text-codeInline-weight'.",
},
// Line heights
{
code: '.x { line-height: 42px; }',
unfixable: true,
message: messages.rejected('42px'),
line: 1,
column: 19,
endColumn: 23,
description: 'CSS > Errors on value not in line height list',
},
{
code: '.x { line-height: 1.4; }',
fixed: '.x { line-height: var(--text-display-lineHeight); }',
message: messages.rejected('1.4', {name: '--text-display-lineHeight'}),
line: 1,
column: 19,
endColumn: 22,
description: "CSS > Replaces '1.4' line-height with 'var(--text-display-lineHeight)'.",
},
{
code: '.x { line-height: 1.5; }',
unfixable: true,
message: messages.rejected('1.5', [
{name: '--text-title-lineHeight-large'},
{name: '--text-title-lineHeight-small'},
{name: '--text-body-lineHeight-large'},
]),
line: 1,
column: 19,
endColumn: 22,
description:
"CSS > Errors on '1.5' line-height and suggests '--text-title-lineHeight-large', '--text-title-lineHeight-small', or '--text-body-lineHeight-large'.",
},
// Font family
{
code: '.x { font-family: Comic Sans; }',
unfixable: true,
message: messages.rejected('Comic Sans'),
line: 1,
column: 19,
endColumn: 29,
description: 'CSS > Errors on value not in font family list',
},
// Font shorthand
{
code: '.x { font: bold 24px/1 sans-serif; }',
unfixable: true,
message: messages.rejected('bold 24px/1 sans-serif'),
line: 1,
column: 12,
endColumn: 34,
description: 'CSS > Errors on hard-coded value not in font shorthand list',
},
],
})
2 changes: 0 additions & 2 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ export default {
'length-zero-no-unit': null,
'selector-max-type': null,
'primer/colors': null,
'primer/typography': null,
'primer/box-shadow': null,
},
},
Expand Down Expand Up @@ -197,7 +196,6 @@ export default {
},
],
// temporarily disabiling Primer plugins while we work on upgrades https://github.com/github/primer/issues/3165
'primer/typography': null,
'primer/box-shadow': null,
},
},
Expand Down
4 changes: 4 additions & 0 deletions plugins/lib/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ export function primitivesVariables(type) {
case 'border':
files.push('functional/size/border.json')
break
case 'typography':
files.push('base/typography/typography.json')
files.push('functional/typography/typography.json')
break
}

for (const file of files) {
Expand Down
Loading

0 comments on commit 7e5f31c

Please sign in to comment.