Skip to content

Commit

Permalink
Revert "Support link underline preference" (#3839)
Browse files Browse the repository at this point in the history
* Revert "Support link underline preference (#3720)"

This reverts commit 521b02f.

* chore: add changeset

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
  • Loading branch information
joshblack and joshblack committed Oct 18, 2023
1 parent 4dbc972 commit d463d54
Show file tree
Hide file tree
Showing 30 changed files with 61 additions and 191 deletions.
5 changes: 5 additions & 0 deletions .changeset/fair-radios-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

Restore Link underline preference to original behavior
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
14 changes: 0 additions & 14 deletions e2e/components/BranchName.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,5 @@ 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: 0 additions & 14 deletions e2e/components/Breadcrumbs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,5 @@ 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: 0 additions & 42 deletions e2e/components/Link.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,6 @@ 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 @@ -99,20 +85,6 @@ 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 @@ -155,19 +127,5 @@ 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: 0 additions & 14 deletions e2e/components/LinkButton.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,20 +137,6 @@ 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: 0 additions & 1 deletion src/ActionList/LinkItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ 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: 0 additions & 7 deletions src/BaseStyles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,6 @@ 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: var(--prefers-link-underlines, underline);
text-decoration: none;
${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: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
}
<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: var(--prefers-link-underlines, underline);
text-decoration: none;
&: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: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
}
.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: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
}
.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: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
}
.c0:hover {
Expand Down
6 changes: 2 additions & 4 deletions src/Button/__tests__/__snapshots__/Button.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1512,13 +1512,11 @@ 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: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
}
.c0:hover {
Expand Down
6 changes: 0 additions & 6 deletions src/Button/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,6 @@ 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: 0 additions & 1 deletion src/Heading/Heading.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ 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 pc-Heading"
className="c0"
/>
`;
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' : 'var(--prefers-link-underlines, underline)')};
text-decoration: ${props => (props.underline ? 'underline' : 'none')};
&:hover {
text-decoration: ${props => (props.muted ? 'var(--prefers-link-underlines, underline)' : 'underline')};
text-decoration: ${props => (props.muted ? 'none' : '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: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
}
.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: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
}
.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: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
}
.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: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
}
.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: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
color: #ffffff;
}
.c0:hover {
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
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: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
}
.c0:hover {
-webkit-text-decoration: var(--prefers-link-underlines,underline);
text-decoration: var(--prefers-link-underlines,underline);
-webkit-text-decoration: none;
text-decoration: none;
color: #0969da;
}
Expand Down
Loading

0 comments on commit d463d54

Please sign in to comment.