Skip to content
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.

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

Global focus styles #1744

Merged
merged 69 commits into from
Apr 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
deda0d0
setup base focus styles
langermank Nov 2, 2021
b1ab3ca
add feature stories
langermank Nov 2, 2021
1e6be4a
Merge branch 'main' of https://github.com/primer/css into global-focu…
langermank Nov 9, 2021
cf582b5
add stories for focusable things, delete outline:0
langermank Nov 9, 2021
85f3431
switch back to box-shadow
langermank Nov 9, 2021
b7e5c98
support class
langermank Nov 9, 2021
84aa9d6
stylelint
langermank Nov 9, 2021
9b17a70
fix theme viewer
langermank Nov 9, 2021
ba93d3d
switch back to outline, address feedback
langermank Nov 10, 2021
55e0753
lint
langermank Nov 10, 2021
905a856
Merge branch 'main' into global-focus-styles
langermank Nov 11, 2021
6cca16f
temp stashing stories here
langermank Nov 11, 2021
57ae3b8
Merge branch 'global-focus-styles' of https://github.com/primer/css i…
langermank Nov 16, 2021
c033dcb
Merge branch 'main' of https://github.com/primer/css into global-focu…
langermank Nov 16, 2021
d0425dc
Create giant-bees-talk.md
jonrohan Nov 16, 2021
e38caea
I think we got it!
langermank Nov 17, 2021
2fade4e
address input directly
langermank Nov 17, 2021
d2e21e7
checkbox/radio outline offset
langermank Nov 17, 2021
b84e0b0
lint
langermank Nov 17, 2021
5341273
change actionlist to just use focus
langermank Nov 17, 2021
af58aed
Merge branch 'main' into global-focus-styles
langermank Nov 18, 2021
6844e0d
Merge branch 'main' into global-focus-styles
langermank Nov 18, 2021
59f8d43
Merge branch 'main' into global-focus-styles
jonrohan Nov 19, 2021
751376b
Merge branch 'main' into global-focus-styles
langermank Nov 19, 2021
17a4622
Merge branch 'main' into global-focus-styles
langermank Nov 22, 2021
5fcefff
Merge branch 'main' of https://github.com/primer/css into global-focu…
langermank Dec 14, 2021
168a9f8
merge
langermank Dec 14, 2021
0eb5ffd
Update giant-bees-talk.md
langermank Dec 14, 2021
c9c792f
Merge branch 'global-focus-styles' of https://github.com/primer/css i…
langermank Dec 14, 2021
d570060
address marketing styles
langermank Dec 14, 2021
b703198
Merge branch 'main' of https://github.com/primer/css into global-focu…
langermank Dec 22, 2021
eb3a61c
tabnav focus fix
langermank Dec 22, 2021
771d0dd
reach all buttons
langermank Dec 22, 2021
6770984
attempt windows hc selector
langermank Dec 22, 2021
500d091
Merge branch 'main' into global-focus-styles
jonrohan Jan 12, 2022
db25d10
Stylelint auto-fixes
actions-user Jan 12, 2022
c8d4ba3
Merge branch 'main' into global-focus-styles
langermank Jan 12, 2022
303b5a3
fixes
langermank Jan 12, 2022
8809254
Merge branch 'global-focus-styles' of https://github.com/primer/css i…
langermank Jan 12, 2022
33af9b5
add focus style testing page
langermank Jan 13, 2022
6b77caa
Merge branch 'main' of https://github.com/primer/css into global-focu…
langermank Jan 13, 2022
457a0a7
Stylelint auto-fixes
actions-user Jan 13, 2022
a53a2ad
add href for testing
langermank Jan 14, 2022
53b6c7a
Merge branch 'global-focus-styles' of https://github.com/primer/css i…
langermank Jan 14, 2022
75615c7
remove position relative to fix chrome bug
langermank Jan 14, 2022
592f4a3
fix details scenario
langermank Jan 14, 2022
a7078ae
add offset to WHC
langermank Jan 14, 2022
2b41913
maintain offset specificity in whc
langermank Jan 14, 2022
74fc060
inset tabnav focus
langermank Jan 14, 2022
6381c0a
switch offset to inset
langermank Mar 8, 2022
fb4c581
Merge branch 'main' of https://github.com/primer/css into global-focu…
langermank Mar 8, 2022
7e4b863
fix actionlist focus
langermank Mar 8, 2022
60be9a8
lint
langermank Mar 8, 2022
a900e35
better scoping, handle forms for safari
langermank Mar 18, 2022
0f1ae23
Merge branch 'main' of https://github.com/primer/css into global-focu…
langermank Apr 5, 2022
aa7e158
moving specific styles from dotcom
langermank Apr 11, 2022
ba84876
address autocomplete
langermank Apr 11, 2022
65b92ac
Merge branch 'main' into global-focus-styles
langermank Apr 11, 2022
f84d027
cleanup
langermank Apr 11, 2022
692aea9
Merge branch 'global-focus-styles' of https://github.com/primer/css i…
langermank Apr 11, 2022
dbeb7a8
cleanup
langermank Apr 11, 2022
d19fd5d
selected focus states
langermank Apr 19, 2022
5251269
Merge branch 'main' into global-focus-styles
langermank Apr 19, 2022
fd71b36
adjust marketing focus
langermank Apr 20, 2022
33e9712
use offset instead for marketing
langermank Apr 20, 2022
be2ab1a
Stylelint auto-fixes
actions-user Apr 20, 2022
f2b6102
Merge branch 'main' into global-focus-styles
langermank Apr 20, 2022
a859750
Merge branch 'next_major' into global-focus-styles
langermank Apr 20, 2022
f84874e
fix merge
langermank Apr 20, 2022
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/giant-bees-talk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/css": major
---

Global CSS focus styles
2 changes: 1 addition & 1 deletion docs/src/stories/components/Link/Link.stories.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const LinkTemplate = ({label, variant, href, noUnderline, focusElement, f
<>
<a
href={href}
className={clsx(variant && `${variant}`, noUnderline && 'no-underline', focusAllElements && 'focus')}
className={clsx('Link', variant && `${variant}`, noUnderline && 'no-underline', focusAllElements && 'focus')}
>
{label}
</a>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export default {
layout: 'padded'
},

excludeStories: ['ButtonTemplate'],
excludeStories: ['MarketingButtonTemplate'],
argTypes: {
variant: {
options: [0, 1, 2, 3], // iterator
Expand Down Expand Up @@ -77,7 +77,7 @@ const focusMethod = function getFocus() {
button.focus()
}

export const ButtonTemplate = ({label, variant, disabled, size, animated, focusElement, focusAllElements}) => (
export const MarketingButtonTemplate = ({label, variant, disabled, size, animated, focusElement, focusAllElements}) => (
<>
<button
disabled={disabled}
Expand Down Expand Up @@ -111,7 +111,7 @@ export const ButtonTemplate = ({label, variant, disabled, size, animated, focusE
</>
)

export const Playground = ButtonTemplate.bind({})
export const Playground = MarketingButtonTemplate.bind({})
Playground.args = {
animated: false,
focusElement: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export default {
layout: 'padded'
},

excludeStories: ['LinkTemplate'],
excludeStories: ['MarketingLinkTemplate'],
argTypes: {
size: {
options: [0, 1], // iterator
Expand Down Expand Up @@ -67,7 +67,7 @@ const focusMethod = function getFocus() {
link.focus()
}

export const LinkTemplate = ({label, emphasis, href, size, focusElement, focusAllElements}) => (
export const MarketingLinkTemplate = ({label, emphasis, href, size, focusElement, focusAllElements}) => (
<>
<a
href={href}
Expand Down Expand Up @@ -98,7 +98,7 @@ export const LinkTemplate = ({label, emphasis, href, size, focusElement, focusAl
</>
)

export const Playground = LinkTemplate.bind({})
export const Playground = MarketingLinkTemplate.bind({})
Playground.args = {
label: 'Link label',
href: '/',
Expand Down
88 changes: 88 additions & 0 deletions docs/src/stories/patterns/FocusStyles.stories.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import React from 'react'
import clsx from 'clsx'
import {ButtonTemplate} from '../components/Button/Button.stories.jsx'
import {CheckboxTemplate} from '../components/Forms/Checkbox.stories.jsx'
import {InputTemplate} from '../components/Forms/Input.stories.jsx'
import {SelectTemplate} from '../components/Forms/Select.stories.jsx'
import {TextareaTemplate} from '../components/Forms/Textarea.stories.jsx'
import {LinkTemplate} from '../components/Link/Link.stories.jsx'
import {MarketingButtonTemplate} from '../components/Marketing/MarketingButton.stories.jsx'
import {MarketingLinkTemplate} from '../components/Marketing/MarketingLink.stories.jsx'
import {TabNavTemplate} from '../components/Navigation/TabNav.stories.jsx'
import {TabNavItemTemplate} from '../components/Navigation/TabNavItem.stories.jsx'

export default {
title: 'Patterns/FocusStyles',
layout: 'padded'
}

export const FocusStyles = ({}) => (
<div style={{display: 'flex', flexDirection: 'column', gap: '2rem'}}>
<div style={{display: 'flex', gap: '0.5rem'}}>
<ButtonTemplate variant="btn-primary" label="Primary" />
<ButtonTemplate variant="btn-secondary" label="Secondary" />
<ButtonTemplate variant="btn-outline" label="Outline" />
<ButtonTemplate variant="btn-danger" label="Danger" />
<ButtonTemplate variant="btn-link" label="Link" />
<ButtonTemplate variant="btn-invisible" label="Invisible" />
<ButtonTemplate
variant="btn-octicon"
leadingVisual={`<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" width="16" height="16">
<path d="M8 16c.9 0 1.7-.6 1.9-1.5.1-.3-.1-.5-.4-.5h-3c-.3 0-.5.2-.4.5.2.9 1 1.5 1.9 1.5zM3 5c0-2.8 2.2-5 5-5s5 2.2 5 5v3l1.7 2.6c.2.2.3.5.3.8 0 .8-.7 1.5-1.5 1.5h-11c-.8.1-1.5-.6-1.5-1.4 0-.3.1-.6.3-.8L3 8.1V5z"></path>
</svg>`}
/>
<ButtonTemplate variant="btn-primary" label="Primary" disabled />
<ButtonTemplate variant="btn-secondary" label="Secondary" disabled />
<ButtonTemplate variant="btn-outline" label="Outline" disabled />
<ButtonTemplate variant="btn-danger" label="Danger" disabled />
<ButtonTemplate variant="btn-link" label="Link" disabled />
<ButtonTemplate variant="btn-invisible" label="Invisible" disabled />
</div>
<div style={{display: 'flex', flexDirection: 'column', gap: '1rem'}}>
<CheckboxTemplate label="checkbox" type="checkbox" />
<CheckboxTemplate label="radio" type="radio" />
<InputTemplate label="input" type="text" />
<input className="form-control border-0" placeholder="no border form control"></input>
<SelectTemplate label="select" />
<TextareaTemplate label="textarea" />
<LinkTemplate label="Primer link" href="/" />
<a href="/">Link with no CSS class</a>
<MarketingButtonTemplate label="Marketing Button" />
<MarketingLinkTemplate label="Marketing Link" href="/" />
</div>
<div>
<TabNavTemplate>
<TabNavItemTemplate text="First tab" ariaCurrent="location" href="#url" />
<TabNavItemTemplate text="Second tab" href="#url" />
</TabNavTemplate>
</div>
<div class="BtnGroup">
<a href="/" class="btn-sm btn BtnGroup-item">
One
</a>
<a href="/" class="btn-sm btn BtnGroup-item">
Two
</a>
</div>
<div class="Box faketarget">:target styles</div>
<nav class="UnderlineNav" aria-label="Foo bar">
<div class="UnderlineNav-body">
<a class="UnderlineNav-item" href="#url" aria-current="page">
Item 1
</a>
<a class="UnderlineNav-item" href="#url">
Item 2
</a>
<a class="UnderlineNav-item" href="#url">
Item 3
</a>
<a class="UnderlineNav-item" href="#url">
Item 4
</a>
</div>
<div class="UnderlineNav-actions">
<a class="btn btn-sm">Button</a>
</div>
</nav>
</div>
)
17 changes: 10 additions & 7 deletions src/actionlist/action-list-item.scss
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
// stylelint-disable max-nesting-depth, selector-max-specificity, selector-max-compound-selectors

@mixin focusOutline {
position: relative;
z-index: 1;
outline: none;
box-shadow: 0 0 0 2px var(--color-accent-fg); // this color breaks convention
}

@mixin activeIndicatorLine {
position: absolute;
top: calc(50% - 12px);
Expand Down Expand Up @@ -314,6 +307,16 @@
text-decoration: none;
}

&:focus {
@include focusOutline;

// remove fallback :focus if :focus-visible is supported
&:not(:focus-visible) {
outline: solid 1px transparent;
}
}

// default focus state
&:focus-visible {
@include focusOutline;
}
Expand Down
10 changes: 3 additions & 7 deletions src/autocomplete/autocomplete.scss
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
align-items: center;

&:focus-within {
border-color: var(--color-accent-emphasis);
outline: none;
box-shadow: var(--color-primer-shadow-focus);
border-color: var(--color-accent-fg);

@include focusBoxShadowInset;
}

.form-control {
Expand All @@ -42,10 +42,6 @@
// stylelint-disable-next-line
border: none;
box-shadow: none;

&:focus {
box-shadow: none;
}
}
}

Expand Down
66 changes: 63 additions & 3 deletions src/base/base.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// stylelint-disable selector-max-type
// stylelint-disable selector-max-type, selector-no-qualifying-type
* {
box-sizing: border-box;
}
Expand Down Expand Up @@ -77,10 +77,70 @@ button {
}

details {
summary { cursor: pointer; }
summary {
cursor: pointer;
}

&:not([open]) {
// Set details content hidden by default for browsers that don't do this
> *:not(summary) { display: none !important; }
> *:not(summary) {
display: none !important;
}
}
}

// global focus styles

a,
button,
[role='button'],
input[type='radio'],
input[type='checkbox'] {
transition: 80ms cubic-bezier(0.33, 1, 0.68, 1);
transition-property: color, background-color, box-shadow, border-color;
// fallback :focus state
&:focus {
@include focusOutline;
mperrotti marked this conversation as resolved.
Show resolved Hide resolved

// remove fallback :focus if :focus-visible is supported
&:not(:focus-visible) {
outline: solid 1px transparent;
}
}

// default focus state
&:focus-visible {
@include focusOutline;
}
}

a:not([class]),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the Link component be included here too? The outline-offset: -2px; can sometimes make it hard to read the link text.

image

Also there are probably some cases where a:not([class]) would not be effective, but still desired.. e.g. when a link is shown responsively like <a class="d-none d-md-block">Link</a>. Hard to fix without adding tons of exceptions for utility classes. 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Links are tricky! I changed this to 0px offset as a happy medium, and added that offset to the PCSS Link component. There were a lot of edge cases in dotcom where having a larger offset wasn't desirable, primarily where an additional utility or class was used. I felt like :not() was an appropriate safeguard for now.

input[type='radio'],
input[type='checkbox'] {
&:focus,
&:focus-visible {
outline-offset: 0;
}
}

// for handling focus conditionally
.focus {
@include focusBoxShadowInset;
}

// Windows High Contrast mode
@media (forced-colors: active) {
mperrotti marked this conversation as resolved.
Show resolved Hide resolved
*:focus,
*:focus-visible {
outline: solid 1px transparent;
}

input:not([type='radio'], [type='checkbox']),
textarea,
select {
&:focus,
&:focus-visible {
outline-offset: 2px;
}
}
}
Loading