Skip to content

Commit

Permalink
Support link underline preference (#3720)
Browse files Browse the repository at this point in the history
* adds Prose component and stories

* adds tests and makes some minor changes

* Create happy-rivers-attend.md

* handle link underline preferences

* adds visual regression testing

* fixes storybook ID in Prose docs data

* updates changeset with changed components data

* rm Prose component

* test(vrt): update snapshots

* test(e2e): separate storybook global args by semicolon instead of ampersand

* test(vrt): update snapshots

* update snapshots

---------

Co-authored-by: joshblack <joshblack@users.noreply.github.com>
  • Loading branch information
mperrotti and joshblack authored Oct 7, 2023
1 parent 01b5f79 commit 521b02f
Show file tree
Hide file tree
Showing 30 changed files with 198 additions and 56 deletions.
7 changes: 7 additions & 0 deletions .changeset/happy-rivers-attend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@primer/react": minor
---

Updates link styles to support underline link preferences.

<!-- Changed components: ActionList, BranchName, Breadcrumbs, Button, Button2, Heading, Link -->
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 14 additions & 0 deletions e2e/components/BranchName.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,19 @@ test.describe('BranchName', () => {
})
})
}

test.describe('with forced underlines', () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-branchname--default',
globals: {
colorScheme: 'light',
prefersLinkUnderlines: 'true',
},
})

expect(await page.screenshot()).toMatchSnapshot('BranchName.Default.light.forcedUnderlines.png')
})
})
})
})
14 changes: 14 additions & 0 deletions e2e/components/Breadcrumbs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,19 @@ test.describe('Breadcrumbs', () => {
})
})
}

test.describe('with forced underlines', () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-breadcrumbs--default',
globals: {
colorScheme: 'light',
prefersLinkUnderlines: 'true',
},
})

expect(await page.screenshot()).toMatchSnapshot('Breadcrumbs.Default.light.forcedUnderlines.png')
})
})
})
})
42 changes: 42 additions & 0 deletions e2e/components/Link.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,20 @@ test.describe('Link', () => {
})
})
}

test.describe('with forced underlines', () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-link--default',
globals: {
colorScheme: 'light',
prefersLinkUnderlines: 'true',
},
})

expect(await page.screenshot()).toMatchSnapshot('Link.Default.light.forcedUnderlines.png')
})
})
})

test.describe('Muted', () => {
Expand Down Expand Up @@ -85,6 +99,20 @@ test.describe('Link', () => {
})
})
}

test.describe('with forced underlines', () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-link-features--muted',
globals: {
colorScheme: 'light',
prefersLinkUnderlines: 'true',
},
})

expect(await page.screenshot()).toMatchSnapshot('Link.Muted.light.forcedUnderlines.png')
})
})
})

test.describe('Underline', () => {
Expand Down Expand Up @@ -127,5 +155,19 @@ test.describe('Link', () => {
})
})
}

test.describe('with forced underlines', () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-link-features--underline',
globals: {
colorScheme: 'light',
prefersLinkUnderlines: 'true',
},
})

expect(await page.screenshot()).toMatchSnapshot('Link.Underline.light.forcedUnderlines.png')
})
})
})
})
14 changes: 14 additions & 0 deletions e2e/components/LinkButton.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,20 @@ test.describe('LinkButton', () => {
})
})
}

test.describe('with forced underlines', () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-linkbutton-features--invisible',
globals: {
colorScheme: 'light',
prefersLinkUnderlines: 'true',
},
})

expect(await page.screenshot()).toMatchSnapshot('LinkButton.Invisible.light.forcedUnderlines.png')
})
})
})

test.describe('Large', () => {
Expand Down
2 changes: 1 addition & 1 deletion e2e/test-helpers/storybook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export async function visit(page: Page, options: Options) {
let params = ''
for (const [key, value] of Object.entries(globals)) {
if (params !== '') {
params += '&'
params += ';'
}
params += `${key}:${value}`
}
Expand Down
1 change: 1 addition & 0 deletions src/ActionList/LinkItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const LinkItem = React.forwardRef(({sx = {}, active, as: Component, ...pr

// inherit Item styles
color: 'inherit',
textDecoration: 'none',
'&:hover': {color: 'inherit', textDecoration: 'none'},
}

Expand Down
7 changes: 7 additions & 0 deletions src/BaseStyles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ const GlobalStyle = createGlobalStyle<{colorScheme?: 'light' | 'dark'}>`
details-dialog:focus:not(:focus-visible):not(.focus-visible) {
outline: none;
}
/* Used to fake conditional styles using a technique by Lea Verou: https://lea.verou.me/blog/2020/10/the-var-space-hack-to-toggle-multiple-values-with-one-custom-property/ */
/* We have to use a zero-width space character (\u200B) as the value instead of a regular whitespace character because styled-components strips out properties that just have a whitespace value. */
:root {--prefers-link-underlines: \u200B;}
[data-a11y-link-underlines='true'] {
--prefers-link-underlines: initial;
}
`

const Base = styled.div<SystemTypographyProps & SystemCommonProps>`
Expand Down
2 changes: 1 addition & 1 deletion src/BranchName/BranchName.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const BranchName = styled.a<SxProp>`
color: ${get('colors.accent.fg')};
background-color: ${get('colors.accent.subtle')};
border-radius: ${get('radii.2')};
text-decoration: none;
text-decoration: var(--prefers-link-underlines, underline);
${sx};
`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ exports[`BranchName renders consistently 1`] = `
color: #0969da;
background-color: #ddf4ff;
border-radius: 6px;
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
}
<a
Expand Down
2 changes: 1 addition & 1 deletion src/Breadcrumbs/Breadcrumbs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const BreadcrumbsItem = styled.a.attrs<StyledBreadcrumbsItemProps>(props => ({
color: ${get('colors.accent.fg')};
display: inline-block;
font-size: ${get('fontSizes.1')};
text-decoration: none;
text-decoration: var(--prefers-link-underlines, underline);
&:hover {
text-decoration: underline;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ exports[`Breadcrumbs.Item adds activeClassName={SELECTED_CLASS} when it gets a "
color: #0969da;
display: inline-block;
font-size: 14px;
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
}
.c0:hover {
Expand All @@ -32,8 +32,8 @@ exports[`Breadcrumbs.Item renders consistently 1`] = `
color: #0969da;
display: inline-block;
font-size: 14px;
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
}
.c0:hover {
Expand All @@ -57,8 +57,8 @@ exports[`Breadcrumbs.Item respects the "selected" prop 1`] = `
color: #0969da;
display: inline-block;
font-size: 14px;
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
}
.c0:hover {
Expand Down
6 changes: 4 additions & 2 deletions src/Button/__tests__/__snapshots__/Button.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1511,11 +1511,13 @@ exports[`Button styles invisible button appropriately 1`] = `
display: -webkit-inline-flex;
display: -ms-inline-flexbox;
display: inline-flex;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
}
.c0[href]:hover {
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
}
.c0:hover {
Expand Down
6 changes: 6 additions & 0 deletions src/Button/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,12 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
backgroundColor: 'transparent',
borderColor: 'transparent',
boxShadow: 'none',
'&[href]': {
textDecoration: 'var(--prefers-link-underlines, underline)',
'&:hover': {
textDecoration: 'var(--prefers-link-underlines, underline)',
},
},
'&:hover:not([disabled])': {
backgroundColor: 'btn.hoverBg',
},
Expand Down
1 change: 1 addition & 0 deletions src/Heading/Heading.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const Heading = forwardRef(({as: Component = 'h2', ...props}, forwardedRef) => {
return (
<StyledHeading
as={Component}
className="pc-Heading"
{...props}
// @ts-ignore shh
ref={innerRef}
Expand Down
2 changes: 1 addition & 1 deletion src/Heading/__tests__/__snapshots__/Heading.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ exports[`Heading renders consistently 1`] = `
}
<h2
className="c0"
className="c0 pc-Heading"
/>
`;
4 changes: 2 additions & 2 deletions src/Link/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ const hoverColor = system({

const StyledLink = styled.a<StyledLinkProps>`
color: ${props => (props.muted ? get('colors.fg.muted')(props) : get('colors.accent.fg')(props))};
text-decoration: ${props => (props.underline ? 'underline' : 'none')};
text-decoration: ${props => (props.underline ? 'underline' : 'var(--prefers-link-underlines, underline)')};
&:hover {
text-decoration: ${props => (props.muted ? 'none' : 'underline')};
text-decoration: ${props => (props.muted ? 'var(--prefers-link-underlines, underline)' : 'underline')};
${props => (props.hoverColor ? hoverColor : props.muted ? `color: ${get('colors.accent.fg')(props)}` : '')};
}
&:is(button) {
Expand Down
32 changes: 16 additions & 16 deletions src/Link/__tests__/__snapshots__/Link.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
exports[`Link applies button styles when rendering a button element 1`] = `
.c0 {
color: #0969da;
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
}
.c0:hover {
Expand Down Expand Up @@ -37,8 +37,8 @@ exports[`Link applies button styles when rendering a button element 1`] = `
exports[`Link passes href down to link element 1`] = `
.c0 {
color: #0969da;
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
}
.c0:hover {
Expand Down Expand Up @@ -72,8 +72,8 @@ exports[`Link passes href down to link element 1`] = `
exports[`Link renders consistently 1`] = `
.c0 {
color: #0969da;
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
}
.c0:hover {
Expand Down Expand Up @@ -106,8 +106,8 @@ exports[`Link renders consistently 1`] = `
exports[`Link respects hoverColor prop 1`] = `
.c0 {
color: #0969da;
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
}
.c0:hover {
Expand Down Expand Up @@ -141,14 +141,14 @@ exports[`Link respects hoverColor prop 1`] = `
exports[`Link respects the "sx" prop when "muted" prop is also passed 1`] = `
.c0 {
color: #656d76;
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
color: #ffffff;
}
.c0:hover {
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
color: #0969da;
}
Expand Down Expand Up @@ -178,13 +178,13 @@ exports[`Link respects the "sx" prop when "muted" prop is also passed 1`] = `
exports[`Link respects the "muted" prop 1`] = `
.c0 {
color: #656d76;
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
}
.c0:hover {
-webkit-text-decoration: none;
text-decoration: none;
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
color: #0969da;
}
Expand Down
Loading

0 comments on commit 521b02f

Please sign in to comment.