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

[Focus states] Use :focus-visible over :focus for focus-ring state. #8054

Merged
merged 5 commits into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/pink-students-compare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': minor
---

Update focus states to be present on :focus-visible rather than :focus
2 changes: 1 addition & 1 deletion polaris-react/src/components/ActionList/ActionList.scss
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
background-color: var(--p-surface-pressed);
}

&:focus:not(:active) {
&:focus-visible:not(:active) {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
outline: var(--p-border-width-3) solid transparent;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
background-color: var(--p-surface-pressed);
}

&:focus:not(:active) {
&:focus-visible:not(:active) {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
}
Expand Down
4 changes: 2 additions & 2 deletions polaris-react/src/components/Banner/Banner.scss
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@
text-decoration: underline;
}

&:focus > .Text {
&:focus-visible > .Text {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include plain-button-backdrop;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
Expand Down Expand Up @@ -269,7 +269,7 @@
letter-spacing: initial;
color: var(--p-text);

&:focus {
&:focus-visible {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
}
Expand Down
4 changes: 2 additions & 2 deletions polaris-react/src/components/Breadcrumbs/Breadcrumbs.scss
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@
}
}

&:focus {
&:focus-visible {
outline: none;
}

&:focus:not(:active) {
&:focus-visible:not(:active) {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
}
Expand Down
6 changes: 3 additions & 3 deletions polaris-react/src/components/Button/Button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@
box-shadow: none;
}

&:focus {
&:focus-visible {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include no-focus-ring;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
Expand All @@ -303,7 +303,7 @@
}
}

&:focus:not(:active) {
&:focus-visible:not(:active) {
> .Content {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
Expand Down Expand Up @@ -574,7 +574,7 @@
}
}

&:focus {
&:focus-visible {
box-shadow: 0 0 0 var(--p-border-width-1) currentColor;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
Expand Down
2 changes: 1 addition & 1 deletion polaris-react/src/components/Checkbox/Checkbox.scss
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include visually-hidden;

&.keyFocused {
&:focus-visible {
+ .Backdrop {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
Expand Down
15 changes: 1 addition & 14 deletions polaris-react/src/components/Checkbox/Checkbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React, {
forwardRef,
useRef,
useImperativeHandle,
useState,
useContext,
} from 'react';
import {MinusMinor, TickSmallMinor} from '@shopify/polaris-icons';
Expand All @@ -13,7 +12,7 @@ import {useUniqueId} from '../../utilities/unique-id';
import {Choice, helpTextID} from '../Choice';
import {errorTextID} from '../InlineError';
import {Icon} from '../Icon';
import {Error, CheckboxHandles, Key} from '../../types';
import type {Error, CheckboxHandles} from '../../types';
import {WithinListboxContext} from '../../utilities/listbox/context';

import styles from './Checkbox.scss';
Expand Down Expand Up @@ -76,7 +75,6 @@ export const Checkbox = forwardRef<CheckboxHandles, CheckboxProps>(
setTrue: handleMouseOver,
setFalse: handleMouseOut,
} = useToggle(false);
const [keyFocused, setKeyFocused] = useState(false);
const isWithinListbox = useContext(WithinListboxContext);

useImperativeHandle(ref, () => ({
Expand All @@ -89,15 +87,6 @@ export const Checkbox = forwardRef<CheckboxHandles, CheckboxProps>(

const handleBlur = () => {
onBlur && onBlur();
setKeyFocused(false);
};

const handleKeyUp = (event: React.KeyboardEvent) => {
const {keyCode} = event;

if (keyCode === Key.Space || keyCode === Key.Tab) {
!keyFocused && setKeyFocused(true);
}
};

const handleOnClick = () => {
Expand Down Expand Up @@ -142,7 +131,6 @@ export const Checkbox = forwardRef<CheckboxHandles, CheckboxProps>(
const inputClassName = classNames(
styles.Input,
isIndeterminate && styles['Input-indeterminate'],
keyFocused && styles.keyFocused,
);

return (
Expand Down Expand Up @@ -170,7 +158,6 @@ export const Checkbox = forwardRef<CheckboxHandles, CheckboxProps>(
onChange={noop}
onClick={handleOnClick}
onFocus={onFocus}
onKeyUp={handleKeyUp}
aria-invalid={error != null}
aria-controls={ariaControls}
aria-describedby={ariaDescribedBy}
Expand Down
26 changes: 0 additions & 26 deletions polaris-react/src/components/Checkbox/tests/Checkbox.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React, {AllHTMLAttributes} from 'react';
import {mountWithApp} from 'tests/utilities';

import {Key} from '../../../types';
import {Checkbox} from '../Checkbox';

describe('<Checkbox />', () => {
Expand Down Expand Up @@ -276,31 +275,6 @@ describe('<Checkbox />', () => {
});
});
});

describe('Focus className', () => {
it('on keyUp adds a keyFocused class to the input', () => {
const checkbox = mountWithApp(<Checkbox label="Checkbox" />);

checkbox.find('input')!.trigger('onKeyUp', {
keyCode: Key.Space,
});

expect(checkbox).toContainReactComponent('input', {
className: 'Input keyFocused',
});
});

it('on change does not add a keyFocused class to the input', () => {
const checkbox = mountWithApp(<Checkbox label="Checkbox" />);
const checkboxInput = checkbox.find('input');
checkboxInput!.trigger('onChange', {
currentTarget: checkboxInput!.domNode as HTMLInputElement,
});
expect(checkbox).not.toContainReactComponent('input', {
className: 'Input keyFocused',
});
});
});
});

function noop() {}
2 changes: 1 addition & 1 deletion polaris-react/src/components/DataTable/DataTable.scss
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@
}
}

&:focus:not(:active) {
&:focus-visible:not(:active) {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');

Expand Down
2 changes: 1 addition & 1 deletion polaris-react/src/components/DatePicker/DatePicker.scss
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring;

&:focus:not(:active) {
&:focus-visible:not(:active) {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
}
Expand Down
2 changes: 1 addition & 1 deletion polaris-react/src/components/Filters/Filters.scss
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ $list-filters-footer-height: 70px;
outline: var(--p-border-width-1) solid transparent;
}

&:focus:not(:active) {
&:focus-visible:not(:active) {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
background: var(--p-surface-pressed);
}

&:focus:not(:active) {
&:focus-visible:not(:active) {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
}
Expand Down
17 changes: 8 additions & 9 deletions polaris-react/src/components/Navigation/Navigation.scss
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,12 @@ $disabled-fade: 0.6;
@include focus-ring;

&:hover,
&.keyFocused {
&:focus-visible {
background-color: var(--p-background-hovered);
color: var(--p-text-primary-hovered);
}

.keyFocused {
:focus-visible {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
}
Expand Down Expand Up @@ -394,7 +394,7 @@ $disabled-fade: 0.6;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring;

&:focus {
&:focus-visible {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
}
Expand Down Expand Up @@ -443,8 +443,7 @@ $disabled-fade: 0.6;
color: var(--p-text-subdued);
}

// stylelint-disable-next-line selector-max-class -- generated by polaris-migrator DO NOT COPY
&.keyFocused {
&:focus-visible {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
color: var(--p-text);
Expand Down Expand Up @@ -482,8 +481,8 @@ $disabled-fade: 0.6;
&:hover {
color: var(--p-text-primary-hovered);
}
// stylelint-disable-next-line selector-max-class -- generated by polaris-migrator DO NOT COPY
&.keyFocused {

&:focus-visible {
color: var(--p-text-primary);
}

Expand Down Expand Up @@ -610,7 +609,7 @@ $disabled-fade: 0.6;
@include recolor-icon($filter-color: var(--p-filter-icon));
}

&:focus {
&:focus-visible {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
}
Expand Down Expand Up @@ -653,7 +652,7 @@ $disabled-fade: 0.6;
color: var(--p-text-primary);
}

&:focus {
&:focus-visible {
outline: none;
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,7 @@ export function WithVariousStatesAndSecondaryElements() {
{
url: '/admin/products',
disabled: false,
selected: true,
label: 'Selected sub item',
},
{
Expand Down
6 changes: 3 additions & 3 deletions polaris-react/src/components/Navigation/_variables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ $nav-animation-variables: (
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring;

&.keyFocused {
&:focus-visible {
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
@include focus-ring($style: 'focused');
background: var(--p-background-hovered);
Expand Down Expand Up @@ -127,7 +127,7 @@ $nav-animation-variables: (
}

.Item:hover &,
.Item.keyFocused & {
.Item:focus-visible & {
// stylelint-disable -- generated by polaris-migrator DO NOT COPY
@include recolor-icon(
$fill-color: var(--p-icon),
Expand All @@ -142,7 +142,7 @@ $nav-animation-variables: (
.subNavigationActive:hover &,
.Item-child-active &,
.Item-child-active:hover &,
.Item-selected.keyFocused & {
.Item-selected:focus-visible & {
// stylelint-disable -- generated by polaris-migrator DO NOT COPY
@include recolor-icon(
$fill-color: var(--p-action-primary),
Expand Down
Loading