Skip to content

Link: Add inline prop to tag links inside a text block #3946

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.

Sign up for GitHub

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

Merged
siddharthkp merged 17 commits into from
Nov 20, 2023
Merged

Link: Add inline prop to tag links inside a text block #3946

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
69c8d46
wip
siddharthkp Nov 15, 2023
7b2f601
cleanup code, add story and test
siddharthkp Nov 15, 2023
3fd90ac
add muted to story
siddharthkp Nov 15, 2023
cf57561
update snapshots
siddharthkp Nov 16, 2023
c472465
Create selfish-beans-cheat.md
siddharthkp Nov 16, 2023
b3a78ea
split inline story into feature and dev
siddharthkp Nov 20, 2023
97951e8
add inline to docs.json
siddharthkp Nov 20, 2023
e2996e6
Add Link e2e snapshots
siddharthkp Nov 20, 2023
324a0a3
oops, left a debug!
siddharthkp Nov 20, 2023
7fc5c26
Merge branch 'main' into inline-link
siddharthkp Nov 20, 2023
42bc84c
omg, remove debug statement
siddharthkp Nov 20, 2023
99da01e
use dev story for tests
siddharthkp Nov 20, 2023
21e5b08
Merge branch 'inline-link' of github.com:primer/react into inline-link
siddharthkp Nov 20, 2023
f764e12
update snapshots for dev tests
siddharthkp Nov 20, 2023
a1b8c3b
can't have hover state with multiple links
siddharthkp Nov 20, 2023
45f6a4e
oops duplicate
siddharthkp Nov 20, 2023
8bb69b7
remove outdated snapshots
siddharthkp Nov 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/selfish-beans-cheat.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Link: Add `inline` prop to tag links inside a text block, underlined with accessibility setting `[data-a11y-link-underlines]`
42 changes: 42 additions & 0 deletions e2e/components/Link.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,46 @@ test.describe('Link', () => {
})
}
})

test.describe('Inline', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-link-features--inline',
globals: {
colorScheme: theme,
},
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`Link.Inline.${theme}.png`)

// Hover state
await page.getByRole('link').hover()
expect(await page.screenshot()).toMatchSnapshot(`Link.Inline.${theme}.hover.png`)

// Focus state
await page.keyboard.press('Tab')
expect(await page.screenshot()).toMatchSnapshot(`Link.Inline.${theme}.focus.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-link-features--inline',
globals: {
colorScheme: theme,
},
})
await expect(page).toHaveNoViolations({
rules: {
'color-contrast': {
enabled: theme !== 'dark_dimmed',
},
},
})
})
})
}
})
})
74 changes: 74 additions & 0 deletions src/Link/Link.features.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,77 @@ export const Underline = () => (
Link
</Link>
)

export const Inline = () => (

@joshblack joshblack Nov 16, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a good candidate for a dev story, similar to what we have for button (I think?). What would you think of having the example in the feature here be a link in a text block and this would be its own dev story?

@siddharthkp siddharthkp Nov 20, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, done!

<div>
<div style={{display: 'flex', flexDirection: 'column'}} data-a11y-link-underlines="true">
[data-a11y-link-underlines=true] (inline always underlines)
<Link href="#">inline: undefined, underline: undefined</Link>
<Link underline={true} href="#">
inline: undefined, underline: true
</Link>
<Link underline={false} href="#">
inline: undefined, underline: false
</Link>
<br />
<Link inline={true} href="#">
inline: true, underline: undefined
</Link>
<Link inline={false} href="#">
inline: false, underline: undefined
</Link>
<br />
<Link inline={true} underline={true} href="#">
inline: true, underline: true
</Link>
<Link inline={true} underline={false} href="#">
inline: true, underline: false
</Link>
<Link inline={false} underline={true} href="#">
inline: false, underline: true
</Link>
<Link inline={false} underline={false} href="#">
inline: false, underline: false
</Link>
<br />
<Link muted={true} inline={true} href="#">
inline: true, muted: true
</Link>
</div>
<br />
<div style={{display: 'flex', flexDirection: 'column'}} data-a11y-link-underlines="false">
[data-a11y-link-underlines=false] (inline does nothing)
<Link href="#">inline: undefined, underline: undefined</Link>
<Link underline={true} href="#">
inline: undefined, underline: true
</Link>
<Link underline={false} href="#">
inline: undefined, underline: false
</Link>
<br />
<Link inline={true} href="#">
inline: true, underline: undefined
</Link>
<Link inline={false} href="#">
inline: false, underline: undefined
</Link>
<br />
<Link inline={true} underline={true} href="#">
inline: true, underline: true
</Link>
<Link inline={true} underline={false} href="#">
inline: true, underline: false
</Link>
<Link inline={false} underline={true} href="#">
inline: false, underline: true
</Link>
<Link inline={false} underline={false} href="#">
inline: false, underline: false
</Link>
<br />
<Link muted={true} inline={true} href="#">
inline: true, muted: true
</Link>
</div>
</div>
)
1 change: 1 addition & 0 deletions src/Link/Link.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Playground.args = {
href: '#',
muted: false,
underline: false,
inline: false,
}

export const Default = () => <Link href="#">Link</Link>
17 changes: 16 additions & 1 deletion src/Link/Link.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ type StyledLinkProps = {
hoverColor?: string
muted?: boolean
underline?: boolean
// Link inside a text block
inline?: boolean
} & SxProp

const hoverColor = system({
Expand All @@ -22,7 +24,19 @@ 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')};

/* By default, Link does not have underline */
text-decoration: none;

/* You can add one by setting underline={true} */
text-decoration: ${props => (props.underline ? 'underline' : undefined)};

/* Inline links (inside a text block), however, should have underline based on accessibility setting set in data-attribute */
/* Note: setting underline={false} does not override this */
[data-a11y-link-underlines='true'] &[data-inline='true'] {
text-decoration: underline;
}

&:hover {
text-decoration: ${props => (props.muted ? 'none' : 'underline')};
${props => (props.hoverColor ? hoverColor : props.muted ? `color: ${get('colors.accent.fg')(props)}` : '')};
Expand Down Expand Up @@ -72,6 +86,7 @@ const Link = forwardRef(({as: Component = 'a', ...props}, forwardedRef) => {
return (
<StyledLink
as={Component}
data-inline={props.inline}
{...props}
// @ts-ignore shh
ref={innerRef}
Expand Down
78 changes: 54 additions & 24 deletions src/Link/__tests__/__snapshots__/Link.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Link applies button styles when rendering a button element 1`] = `
.c0 {
.c1 {
color: #0969da;
-webkit-text-decoration: none;
text-decoration: none;
}

.c0:hover {
[data-a11y-link-underlines='true'] .c0[data-inline='true'] {
-webkit-text-decoration: underline;
text-decoration: underline;
}

.c0:is(button) {
.c1:hover {
-webkit-text-decoration: underline;
text-decoration: underline;
}

.c1:is(button) {
display: inline-block;
padding: 0;
font-size: inherit;
Expand All @@ -30,23 +35,28 @@ exports[`Link applies button styles when rendering a button element 1`] = `
}

<button
className="c0"
className="c0 c1"
/>
`;

exports[`Link passes href down to link element 1`] = `
.c0 {
.c1 {
color: #0969da;
-webkit-text-decoration: none;
text-decoration: none;
}

.c0:hover {
[data-a11y-link-underlines='true'] .c0[data-inline='true'] {
-webkit-text-decoration: underline;
text-decoration: underline;
}

.c1:hover {
-webkit-text-decoration: underline;
text-decoration: underline;
}

.c0:is(button) {
.c1:is(button) {
display: inline-block;
padding: 0;
font-size: inherit;
Expand All @@ -64,24 +74,29 @@ exports[`Link passes href down to link element 1`] = `
}

<a
className="c0"
className="c0 c1"
href="https://github.com"
/>
`;

exports[`Link renders consistently 1`] = `
.c0 {
.c1 {
color: #0969da;
-webkit-text-decoration: none;
text-decoration: none;
}

.c0:hover {
[data-a11y-link-underlines='true'] .c0[data-inline='true'] {
-webkit-text-decoration: underline;
text-decoration: underline;
}

.c1:hover {
-webkit-text-decoration: underline;
text-decoration: underline;
}

.c0:is(button) {
.c1:is(button) {
display: inline-block;
padding: 0;
font-size: inherit;
Expand All @@ -99,24 +114,29 @@ exports[`Link renders consistently 1`] = `
}

<a
className="c0"
className="c0 c1"
/>
`;

exports[`Link respects hoverColor prop 1`] = `
.c0 {
.c1 {
color: #0969da;
-webkit-text-decoration: none;
text-decoration: none;
}

.c0:hover {
[data-a11y-link-underlines='true'] .c0[data-inline='true'] {
-webkit-text-decoration: underline;
text-decoration: underline;
}

.c1:hover {
-webkit-text-decoration: underline;
text-decoration: underline;
color: #0969da;
}

.c0:is(button) {
.c1:is(button) {
display: inline-block;
padding: 0;
font-size: inherit;
Expand All @@ -134,25 +154,30 @@ exports[`Link respects hoverColor prop 1`] = `
}

<a
className="c0"
className="c0 c1"
/>
`;

exports[`Link respects the "sx" prop when "muted" prop is also passed 1`] = `
.c0 {
.c1 {
color: #656d76;
-webkit-text-decoration: none;
text-decoration: none;
color: #ffffff;
}

.c0:hover {
[data-a11y-link-underlines='true'] .c0[data-inline='true'] {
-webkit-text-decoration: underline;
text-decoration: underline;
}

.c1:hover {
-webkit-text-decoration: none;
text-decoration: none;
color: #0969da;
}

.c0:is(button) {
.c1:is(button) {
display: inline-block;
padding: 0;
font-size: inherit;
Expand All @@ -170,25 +195,30 @@ exports[`Link respects the "sx" prop when "muted" prop is also passed 1`] = `
}

<a
className="c0"
className="c0 c1"
muted={true}
/>
`;

exports[`Link respects the "muted" prop 1`] = `
.c0 {
.c1 {
color: #656d76;
-webkit-text-decoration: none;
text-decoration: none;
}

.c0:hover {
[data-a11y-link-underlines='true'] .c0[data-inline='true'] {
-webkit-text-decoration: underline;
text-decoration: underline;
}

.c1:hover {
-webkit-text-decoration: none;
text-decoration: none;
color: #0969da;
}

.c0:is(button) {
.c1:is(button) {
display: inline-block;
padding: 0;
font-size: inherit;
Expand All @@ -206,7 +236,7 @@ exports[`Link respects the "muted" prop 1`] = `
}

<a
className="c0"
className="c0 c1"
muted={true}
/>
`;
Loading