Skip to content

Commit

Permalink
fix: Fix Dialog focus loss (#682)
Browse files Browse the repository at this point in the history
Closes #677
  • Loading branch information
diegohaz committed Jul 18, 2020
1 parent bbc9531 commit 8ae0da7
Show file tree
Hide file tree
Showing 18 changed files with 305 additions and 61 deletions.
1 change: 1 addition & 0 deletions packages/reakit-playground/src/__deps/reakit-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export default {
"reakit-utils/getActiveElement": require("reakit-utils/getActiveElement"),
"reakit-utils/getDefaultView": require("reakit-utils/getDefaultView"),
"reakit-utils/getDocument": require("reakit-utils/getDocument"),
"reakit-utils/getNextActiveElementOnBlur": require("reakit-utils/getNextActiveElementOnBlur"),
"reakit-utils/hasFocusWithin": require("reakit-utils/hasFocusWithin"),
"reakit-utils/isButton": require("reakit-utils/isButton"),
"reakit-utils/isEmpty": require("reakit-utils/isEmpty"),
Expand Down
1 change: 1 addition & 0 deletions packages/reakit-utils/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
/getActiveElement
/getDefaultView
/getDocument
/getNextActiveElementOnBlur
/hasFocusWithin
/isButton
/isEmpty
Expand Down
23 changes: 23 additions & 0 deletions packages/reakit-utils/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ yarn add reakit-utils
- [getActiveElement](#getactiveelement)
- [getDefaultView](#getdefaultview)
- [getDocument](#getdocument)
- [getNextActiveElementOnBlur](#getnextactiveelementonblur)
- [hasFocusWithin](#hasfocuswithin)
- [isButton](#isbutton)
- [isEmpty](#isempty)
Expand Down Expand Up @@ -258,6 +259,28 @@ Returns `element.ownerDocument || window.document`.

Returns **[Document](https://developer.mozilla.org/docs/Web/API/Document)**

### getNextActiveElementOnBlur

Cross-browser method that returns the next active element (the element that
is receiving focus) after a blur event is dispatched. It receives the blur
event object as the argument.

#### Parameters

- `event` **(React.FocusEvent | [FocusEvent](https://developer.mozilla.org/docs/Web/API/FocusEvent))**

#### Examples

```javascript
import { getNextActiveElementOnBlur } from "reakit-utils";

const element = document.getElementById("id");
element.addEventListener("blur", (event) => {
const nextActiveElement = getNextActiveElementOnBlur(event);
...
});
```

### hasFocusWithin

Checks if `element` has focus.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import * as React from "react";
import { render, focus } from "reakit-test-utils";
import { getNextActiveElementOnBlur } from "../getNextActiveElementOnBlur";

test("getNextActiveElementOnBlur", () => {
const onBlur = jest.fn(getNextActiveElementOnBlur);
const { getByText: text } = render(
<>
<button onBlur={onBlur}>button1</button>
<button>button2</button>
</>
);
focus(text("button1"));
expect(onBlur).not.toHaveBeenCalled();
focus(text("button2"));
expect(onBlur).toHaveReturnedWith(text("button2"));
});
31 changes: 31 additions & 0 deletions packages/reakit-utils/src/getNextActiveElementOnBlur.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import * as React from "react";
import { getActiveElement } from "./getActiveElement";

const isIE11 = typeof window !== "undefined" && "msCrypto" in window;

/**
* Cross-browser method that returns the next active element (the element that
* is receiving focus) after a blur event is dispatched. It receives the blur
* event object as the argument.
*
* @example
* import { getNextActiveElementOnBlur } from "reakit-utils";
*
* const element = document.getElementById("id");
* element.addEventListener("blur", (event) => {
* const nextActiveElement = getNextActiveElementOnBlur(event);
* ...
* });
*/
export function getNextActiveElementOnBlur(
event: React.FocusEvent | FocusEvent
) {
// IE 11 doesn't support event.relatedTarget on blur.
// document.activeElement points the the next active element.
// On modern browsers, document.activeElement points to the current target.
if (isIE11) {
const activeElement = getActiveElement(event.target as Element);
return activeElement as HTMLElement | null;
}
return event.relatedTarget as HTMLElement | null;
}
1 change: 1 addition & 0 deletions packages/reakit-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export * from "./flatten";
export * from "./getActiveElement";
export * from "./getDefaultView";
export * from "./getDocument";
export * from "./getNextActiveElementOnBlur";
export * from "./hasFocusWithin";
export * from "./isButton";
export * from "./isEmpty";
Expand Down
2 changes: 1 addition & 1 deletion packages/reakit/src/Composite/Composite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { fireEvent } from "reakit-utils/fireEvent";
import { fireKeyboardEvent } from "reakit-utils/fireKeyboardEvent";
import { isSelfTarget } from "reakit-utils/isSelfTarget";
import { useLiveRef } from "reakit-utils/useLiveRef";
import { getNextActiveElementOnBlur } from "reakit-utils/getNextActiveElementOnBlur";
import { useTabbable, TabbableOptions, TabbableHTMLProps } from "../Tabbable";
import { useBox } from "../Box/Box";
import { CompositeStateReturn, useCompositeState } from "./CompositeState";
Expand All @@ -19,7 +20,6 @@ import { flatten } from "./__utils/flatten";
import { findFirstEnabledItem } from "./__utils/findFirstEnabledItem";
import { reverse } from "./__utils/reverse";
import { getCurrentId } from "./__utils/getCurrentId";
import { getNextActiveElementOnBlur } from "./__utils/getNextActiveElementOnBlur";
import { findEnabledItemById } from "./__utils/findEnabledItemById";

export type CompositeOptions = TabbableOptions &
Expand Down

This file was deleted.

15 changes: 15 additions & 0 deletions packages/reakit/src/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import { useHideOnClickOutside } from "./__utils/useHideOnClickOutside";
import { useDialogState, DialogStateReturn } from "./DialogState";
import { useDisableHoverOutside } from "./__utils/useDisableHoverOutside";
import { DialogBackdropContext } from "./__utils/DialogBackdropContext";
import { useFocusOnChildUnmount } from "./__utils/useFocusOnChildUnmount";
import { useFocusOnBlur } from "./__utils/useFocusOnBlur";

export type DialogOptions = DisclosureContentOptions &
Pick<
Expand Down Expand Up @@ -117,6 +119,7 @@ export const useDialog = createHook<DialogOptions, DialogHTMLProps>({
{
ref: htmlRef,
onKeyDown: htmlOnKeyDown,
onBlur: htmlOnBlur,
wrapElement: htmlWrapElement,
tabIndex,
...htmlProps
Expand All @@ -127,6 +130,8 @@ export const useDialog = createHook<DialogOptions, DialogHTMLProps>({
const hasBackdrop = backdrop && backdrop === options.baseId;
const disclosure = useDisclosureRef(dialog, options);
const onKeyDownRef = useLiveRef(htmlOnKeyDown);
const onBlurRef = useLiveRef(htmlOnBlur);
const focusOnBlur = useFocusOnBlur(dialog, options);
const { dialogs, visibleModals, wrap } = useNestedDialogs(dialog, options);
// VoiceOver/Safari accepts only one `aria-modal` container, so if there
// are visible child modals, then we don't want to set aria-modal on the
Expand All @@ -135,6 +140,7 @@ export const useDialog = createHook<DialogOptions, DialogHTMLProps>({

usePreventBodyScroll(dialog, options);
useFocusTrap(dialog, visibleModals, options);
useFocusOnChildUnmount(dialog, options);
useFocusOnShow(dialog, dialogs, options);
useFocusOnHide(dialog, disclosure, options);
useHideOnClickOutside(dialog, disclosure, dialogs, options);
Expand All @@ -161,6 +167,14 @@ export const useDialog = createHook<DialogOptions, DialogHTMLProps>({
[options.hideOnEsc, options.hide]
);

const onBlur = React.useCallback(
(event: React.FocusEvent<HTMLElement>) => {
onBlurRef.current?.(event);
focusOnBlur(event);
},
[focusOnBlur]
);

const wrapElement = React.useCallback(
(element: React.ReactNode) => {
element = wrap(element);
Expand All @@ -185,6 +199,7 @@ export const useDialog = createHook<DialogOptions, DialogHTMLProps>({
"aria-modal": modal,
"data-dialog": true,
onKeyDown,
onBlur,
wrapElement,
...htmlProps,
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import * as React from "react";
import { click, focus, render, wait } from "reakit-test-utils";
import DialogWithFocusLoss from "..";

test("open dialog", () => {
const { getByText: text, getByLabelText: label } = render(
<DialogWithFocusLoss />
);
click(text("Open dialog"));
expect(label("Focus loss")).toBeVisible();
expect(text("Blur on click")).toHaveFocus();
});

test("blur on click", () => {
const { getByText: text, getByLabelText: label } = render(
<DialogWithFocusLoss />
);
click(text("Open dialog"));
click(text("Blur on click"));
expect(label("Focus loss")).toBeVisible();
expect(label("Focus loss")).toHaveFocus();
});

test("unmount on focus", async () => {
const { getByText: text, getByLabelText: label } = render(
<DialogWithFocusLoss />
);
click(text("Open dialog"));
focus(text("Unmount on focus"));
expect(label("Focus loss")).toBeVisible();
await wait(expect(label("Focus loss")).toHaveFocus);
});

test("nested blur on click", () => {
const { getByText: text, getByLabelText: label } = render(
<DialogWithFocusLoss />
);
click(text("Open dialog"));
click(text("Open nested dialog"));
click(text("Nested blur on click"));
expect(label("Nested")).toBeVisible();
expect(label("Nested")).toHaveFocus();
});

test("nested unmount on focus", async () => {
const { getByText: text, getByLabelText: label } = render(
<DialogWithFocusLoss />
);
click(text("Open dialog"));
click(text("Open nested dialog"));
focus(text("Nested unmount on focus"));
expect(label("Nested")).toBeVisible();
await wait(expect(label("Nested")).toHaveFocus);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import * as React from "react";
import { Button, ButtonProps } from "reakit/Button";
import { useDialogState, Dialog, DialogDisclosure } from "reakit/Dialog";

function BlurOnClickButton(props: ButtonProps) {
return <Button onClick={(event) => event.currentTarget.blur()} {...props} />;
}

function UnmountOnFocusButton(props: ButtonProps) {
const [mounted, setMounted] = React.useState(true);
if (mounted) {
return <Button onFocus={() => setMounted(!mounted)} {...props} />;
}
return null;
}

function NestedDialog() {
const dialog = useDialogState({ modal: false });
return (
<>
<DialogDisclosure {...dialog}>Open nested dialog</DialogDisclosure>
<Dialog {...dialog} aria-label="Nested">
<BlurOnClickButton>Nested blur on click</BlurOnClickButton>
<UnmountOnFocusButton>Nested unmount on focus</UnmountOnFocusButton>
</Dialog>
</>
);
}

export default function DialogWithFocusLoss() {
const dialog = useDialogState();
return (
<>
<DialogDisclosure {...dialog}>Open dialog</DialogDisclosure>
<Dialog {...dialog} aria-label="Focus loss">
<BlurOnClickButton>Blur on click</BlurOnClickButton>
<UnmountOnFocusButton>Unmount on focus</UnmountOnFocusButton>
<NestedDialog />
</Dialog>
</>
);
}
34 changes: 19 additions & 15 deletions packages/reakit/src/Dialog/__utils/useEventListenerOutside.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ function isDisclosure(target: Element, disclosure: HTMLElement) {
);
}

function isInDocument(target: Element) {
const document = getDocument(target);
if (target.tagName === "HTML") {
return true;
}
return contains(document.body, target);
}

export function useEventListenerOutside(
containerRef: React.RefObject<HTMLElement>,
disclosureRef: React.RefObject<HTMLElement>,
Expand All @@ -40,13 +48,11 @@ export function useEventListenerOutside(
React.useEffect(() => {
if (!shouldListen) return undefined;

const handleEvent = (event: Event) => {
const onEvent = (event: Event) => {
if (!listenerRef.current) return;

const container = containerRef.current;
const disclosure = disclosureRef.current;
const target = event.target as Element;

if (!container) {
warning(
true,
Expand All @@ -55,27 +61,25 @@ export function useEventListenerOutside(
);
return;
}

// Click inside dialog
// When an element is unmounted right after it receives focus, the focus
// event is triggered after that, when the element isn't part of the
// current document anymore. So we ignore it.
if (!isInDocument(target)) return;
// Event inside dialog
if (contains(container, target)) return;

// Click on disclosure
if (disclosure && isDisclosure(target, disclosure)) {
return;
}

// Click inside a nested dialog or focus trap
// Event on disclosure
if (disclosure && isDisclosure(target, disclosure)) return;
// Event inside a nested dialog or focus trap
if (isFocusTrap(target) || nestedDialogs.some(dialogContains(target))) {
return;
}

listenerRef.current(event);
};

const document = getDocument(containerRef.current);
document.addEventListener(eventType, handleEvent, true);
document.addEventListener(eventType, onEvent, true);
return () => {
document.removeEventListener(eventType, handleEvent, true);
document.removeEventListener(eventType, onEvent, true);
};
}, [
containerRef,
Expand Down
Loading

0 comments on commit 8ae0da7

Please sign in to comment.