Skip to content

Commit

Permalink
feat: improve focus selector perf with focus-visible and focus-within…
Browse files Browse the repository at this point in the history
… polyfill (#24154)

* feat: focus-visible polyfill

Adds a polyfill for `:focus-visible` that is built on top of Keyborg
that serves as a replacement for `useKeyboardNavAttribute`

* focus-within polyfill

* use focusout

* changefiles

* update md

* remove unused

* fix typings

* fix link vr tests

* fix link vr tests

* PR suggestions

* update changefile

* assign ref in effect

* add doics

* update md

* PR suggestions
  • Loading branch information
ling1726 authored Aug 3, 2022
1 parent f11b365 commit 71590ea
Show file tree
Hide file tree
Showing 23 changed files with 310 additions and 19 deletions.
12 changes: 4 additions & 8 deletions apps/vr-tests-react-components/src/stories/Link.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@ storiesOf('Link Converged - Rendered as anchor', module)
.hover('.fui-Link')
.snapshot('hover', { cropTo: '.testWrapper' })
// This needs to be added so that the focus outline is shown correctly
.executeScript(
"document.getElementsByClassName('fui-FluentProvider')[0].setAttribute('data-keyboard-nav', true)",
)
.executeScript("document.getElementsByClassName('fui-Link')[0].classList.add('fui-focus-visible')")
.focus('.fui-Link')
.snapshot('focused', { cropTo: '.testWrapper' })
.executeScript("document.getElementsByClassName('fui-FluentProvider')[0].removeAttribute('data-keyboard-nav')")
.executeScript("document.getElementsByClassName('fui-Link')[0].classList.remove('fui-focus-visible')")
.mouseDown('.fui-Link')
.snapshot('pressed', { cropTo: '.testWrapper' })
.mouseUp('.fui-Link')
Expand Down Expand Up @@ -96,12 +94,10 @@ storiesOf('Link Converged - Rendered as button', module)
.hover('.fui-Link')
.snapshot('hover', { cropTo: '.testWrapper' })
// This needs to be added so that the focus outline is shown correctly
.executeScript(
"document.getElementsByClassName('fui-FluentProvider')[0].setAttribute('data-keyboard-nav', true)",
)
.executeScript("document.getElementsByClassName('fui-Link')[0].classList.add('fui-focus-visible')")
.focus('.fui-Link')
.snapshot('focused', { cropTo: '.testWrapper' })
.executeScript("document.getElementsByClassName('fui-FluentProvider')[0].removeAttribute('data-keyboard-nav')")
.executeScript("document.getElementsByClassName('fui-Link')[0].classList.remove('fui-focus-visible')")
.mouseDown('.fui-Link')
.snapshot('pressed', { cropTo: '.testWrapper' })
.mouseUp('.fui-Link')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "refactor: use `useFocusWithin` hook for :focus-within styles",
"packageName": "@fluentui/react-checkbox",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "fix: use `useFocusVisible` hook for :focus-visible styles",
"packageName": "@fluentui/react-portal",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "fix: use `useFocusVisible` hook for :focus-visible styles",
"packageName": "@fluentui/react-provider",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "refactor: use `useFocusWithin` hook for :focus-within styles",
"packageName": "@fluentui/react-radio",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "refactor: use `useFocusWithin` hook for :focus-within styles",
"packageName": "@fluentui/react-slider",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "refactor: use `useFocusWithin` hook for :focus-within styles",
"packageName": "@fluentui/react-switch",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "feat: create `:focus-visible` and `:focus-within` polyfills",
"packageName": "@fluentui/react-tabster",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
CircleFilled,
} from '@fluentui/react-icons';
import { Label } from '@fluentui/react-label';
import { useFocusWithin } from '@fluentui/react-tabster';

/**
* Create the state required to render Checkbox.
Expand Down Expand Up @@ -69,7 +70,10 @@ export const useCheckbox_unstable = (props: CheckboxProps, ref: React.Ref<HTMLIn
},
root: resolveShorthand(props.root, {
required: true,
defaultProps: nativeProps.root,
defaultProps: {
ref: useFocusWithin<HTMLSpanElement>(),
...nativeProps.root,
},
}),
input: resolveShorthand(props.input, {
required: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import {
useThemeClassName_unstable as useThemeClassName,
useFluent_unstable as useFluent,
} from '@fluentui/react-shared-contexts';
import { useKeyboardNavAttribute } from '@fluentui/react-tabster';
import { makeStyles, mergeClasses } from '@griffel/react';
import { useFocusVisible } from '@fluentui/react-tabster';

export type UsePortalMountNodeOptions = {
/**
Expand All @@ -26,6 +26,7 @@ const useStyles = makeStyles({
*/
export const usePortalMountNode = (options: UsePortalMountNodeOptions): HTMLElement | null => {
const { targetDocument, dir } = useFluent();
const focusVisibleRef = useFocusVisible<HTMLDivElement>() as React.MutableRefObject<HTMLElement | null>;

const classes = useStyles();
const themeClassName = useThemeClassName();
Expand All @@ -49,15 +50,14 @@ export const usePortalMountNode = (options: UsePortalMountNodeOptions): HTMLElem

element.classList.add(...classesToApply);
element.setAttribute('dir', dir);
focusVisibleRef.current = element;

return () => {
element.classList.remove(...classesToApply);
element.removeAttribute('dir');
};
}
}, [element, className, dir]);

(useKeyboardNavAttribute() as React.MutableRefObject<HTMLElement>).current = element!;
}, [className, dir, element, focusVisibleRef]);

React.useEffect(() => {
return () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useKeyboardNavAttribute } from '@fluentui/react-tabster';
import { useFocusVisible } from '@fluentui/react-tabster';
import {
ThemeContext_unstable as ThemeContext,
useFluent_unstable as useFluent,
Expand Down Expand Up @@ -58,7 +58,7 @@ export const useFluentProvider_unstable = (
root: getNativeElementProps('div', {
...props,
dir,
ref: useMergedRefs(ref, useKeyboardNavAttribute()),
ref: useMergedRefs(ref, useFocusVisible<HTMLDivElement>()),
}),
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Label } from '@fluentui/react-label';
import { getPartitionedNativeProps, resolveShorthand, useId, useMergedEventCallbacks } from '@fluentui/react-utilities';
import { RadioGroupContext } from '../../contexts/RadioGroupContext';
import { useContextSelector } from '@fluentui/react-context-selector';
import { useFocusWithin } from '@fluentui/react-tabster';
import type { RadioProps, RadioState } from './Radio.types';

/**
Expand Down Expand Up @@ -41,7 +42,10 @@ export const useRadio_unstable = (props: RadioProps, ref: React.Ref<HTMLInputEle

const root = resolveShorthand(props.root, {
required: true,
defaultProps: nativeProps.root,
defaultProps: {
ref: useFocusWithin<HTMLSpanElement>(),
...nativeProps.root,
},
});

const input = resolveShorthand(props.input, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import { getPartitionedNativeProps, resolveShorthand, useId } from '@fluentui/react-utilities';
import { useSliderState_unstable } from './useSliderState';
import { SliderProps, SliderState } from './Slider.types';
import { useFocusWithin } from '@fluentui/react-tabster';

export const useSlider_unstable = (props: SliderProps, ref: React.Ref<HTMLInputElement>): SliderState => {
const nativeProps = getPartitionedNativeProps({
Expand Down Expand Up @@ -34,6 +35,7 @@ export const useSlider_unstable = (props: SliderProps, ref: React.Ref<HTMLInputE
root: resolveShorthand(root, {
required: true,
defaultProps: {
ref: useFocusWithin<HTMLDivElement>(),
...nativeProps.root,
},
}),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';
import { CircleFilled } from '@fluentui/react-icons';
import { Label } from '@fluentui/react-label';
import { useFocusWithin } from '@fluentui/react-tabster';
import { getPartitionedNativeProps, resolveShorthand, useId, useMergedEventCallbacks } from '@fluentui/react-utilities';
import type { SwitchProps, SwitchState } from './Switch.types';

Expand All @@ -25,7 +26,7 @@ export const useSwitch_unstable = (props: SwitchProps, ref: React.Ref<HTMLInputE
const id = useId('switch-', nativeProps.primary.id);

const root = resolveShorthand(props.root, {
defaultProps: nativeProps.root,
defaultProps: { ref: useFocusWithin<HTMLDivElement>(), ...nativeProps.root },
required: true,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
```ts

import type { GriffelStyle } from '@griffel/react';
import * as React_2 from 'react';
import type { RefObject } from 'react';
import { Types } from 'tabster';

Expand Down Expand Up @@ -66,6 +67,12 @@ export const useFocusFinders: () => {
findPrevFocusable: (currentElement: HTMLElement, options?: Pick<Types.FindNextProps, 'container'>) => HTMLElement | null | undefined;
};

// @public (undocumented)
export function useFocusVisible<TElement extends HTMLElement = HTMLElement>(): React_2.RefObject<TElement>;

// @public
export function useFocusWithin<TElement extends HTMLElement = HTMLElement>(): React_2.RefObject<TElement>;

// @public
export function useKeyboardNavAttribute<E extends HTMLElement>(): RefObject<E>;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
export const KEYBOARD_NAV_ATTRIBUTE = 'data-keyboard-nav' as const;
export const KEYBOARD_NAV_SELECTOR = `:global([${KEYBOARD_NAV_ATTRIBUTE}])` as const;
export const FOCUS_VISIBLE_CLASS = 'fui-focus-visible';
export const FOCUS_WITHIN_CLASS = 'fui-focus-within';
export const defaultOptions = {
style: {},
selector: 'focus',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { KEYBOARD_NAV_SELECTOR, defaultOptions } from './constants';
import { FOCUS_VISIBLE_CLASS, defaultOptions, FOCUS_WITHIN_CLASS } from './constants';
import type { GriffelStyle } from '@griffel/react';

export interface CreateCustomFocusIndicatorStyleOptions {
Expand All @@ -19,5 +19,19 @@ export const createCustomFocusIndicatorStyle = (
':focus': {
outlineStyle: 'none',
},
[`${KEYBOARD_NAV_SELECTOR} :${selector}`]: style,
':focus-visible': {
outlineStyle: 'none',
},
// Remove the `.fui-FluentProvider` global selector once Griffel supports chained global styles
// https://github.com/microsoft/griffel/issues/178
...(selector === 'focus' && {
[`:global(.fui-FluentProvider)`]: {
[`& .${FOCUS_VISIBLE_CLASS}`]: style,
},
}),
...(selector === 'focus-within' && {
[`:global(.fui-FluentProvider)`]: {
[`& .${FOCUS_WITHIN_CLASS}:${selector}`]: style,
},
}),
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import { KEYBORG_FOCUSIN, KeyborgFocusInEvent, createKeyborg, disposeKeyborg } from 'keyborg';
import { FOCUS_VISIBLE_CLASS } from './constants';

/**
* Because `addEventListener` type override falls back to 2nd definition (evt name is unknown string literal)
* evt is being typed as a base class of MouseEvent -> `Event`.
* This type is used to override `listener` calls to make TS happy
*/
type ListenerOverride = (evt: Event) => void;

type FocusVisibleState = {
/**
* Current element with focus visible in state
*/
current: HTMLElement | undefined;
};

type HTMLElementWithFocusVisibleScope = {
focusVisible: boolean | undefined;
} & HTMLElement;

export function applyFocusVisiblePolyfill(scope: HTMLElement, win: Window): () => void {
if (alreadyInScope(scope)) {
// Focus visible polyfill already applied at this scope
return () => undefined;
}

const state: FocusVisibleState = {
current: undefined,
};

const keyborg = createKeyborg(win);

// When navigation mode changes remove the focus-visible selector
keyborg.subscribe(isNavigatingWithKeyboard => {
if (!isNavigatingWithKeyboard && state.current) {
removeFocusVisibleClass(state.current);
state.current = undefined;
}
});

// Keyborg's focusin event is delegated so it's only registered once on the window
// and contains metadata about the focus event
const keyborgListener = (e: KeyborgFocusInEvent) => {
if (state.current) {
removeFocusVisibleClass(state.current);
state.current = undefined;
}

if (keyborg.isNavigatingWithKeyboard() && isHTMLElement(e.target) && e.target) {
// Griffel can't create chained global styles so use the parent element for now
state.current = e.target;
applyFocusVisibleClass(state.current);
}
};

// Make sure that when focus leaves the scope, the focus visible class is removed
const blurListener = (e: FocusEvent) => {
if (!e.relatedTarget || (isHTMLElement(e.relatedTarget) && !scope.contains(e.relatedTarget))) {
if (state.current) {
removeFocusVisibleClass(state.current);
state.current = undefined;
}
}
};

scope.addEventListener(KEYBORG_FOCUSIN, keyborgListener as ListenerOverride);
scope.addEventListener('focusout', blurListener);
(scope as HTMLElementWithFocusVisibleScope).focusVisible = true;

// Return disposer
return () => {
scope.removeEventListener(KEYBORG_FOCUSIN, keyborgListener as ListenerOverride);
scope.removeEventListener('focusout', blurListener);
delete (scope as HTMLElementWithFocusVisibleScope).focusVisible;
disposeKeyborg(keyborg);
};
}

function applyFocusVisibleClass(el: HTMLElement) {
el.classList.add(FOCUS_VISIBLE_CLASS);
}

function removeFocusVisibleClass(el: HTMLElement) {
el.classList.remove(FOCUS_VISIBLE_CLASS);
}

function isHTMLElement(target: EventTarget | null): target is HTMLElement {
if (!target) {
return false;
}
return Boolean(target && typeof target === 'object' && 'classList' in target && 'contains' in target);
}

function alreadyInScope(el: HTMLElement | null | undefined): boolean {
if (!el) {
return false;
}

if ((el as HTMLElementWithFocusVisibleScope).focusVisible) {
return true;
}

return alreadyInScope(el?.parentElement);
}
Loading

0 comments on commit 71590ea

Please sign in to comment.