Skip to content

Commit

Permalink
fix: Fix Dialog initial focus (#433)
Browse files Browse the repository at this point in the history
  • Loading branch information
diegohaz authored Sep 14, 2019
1 parent df639c6 commit a0916c7
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 29 deletions.
4 changes: 3 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ module.exports = {
files: ["**/*.ts", "**/*.tsx"],
parser: "@typescript-eslint/parser",
parserOptions: {
project: "./tsconfig.json"
project: "./tsconfig.json",
// TODO: Temporary fix https://github.com/typescript-eslint/typescript-eslint/issues/890
createDefaultProgram: true
},
plugins: ["@typescript-eslint"],
rules: {
Expand Down
23 changes: 23 additions & 0 deletions packages/reakit-utils/src/tabbable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,26 @@ export function focusPreviousTabbableIn<T extends Element>(
previousTabbable.focus();
}
}

function defaultIsActive(element: Element) {
return document.activeElement === element;
}

type ForceFocusOptions = FocusOptions & {
isActive?: typeof defaultIsActive;
};

export function forceFocus(
element: HTMLElement,
{ isActive = defaultIsActive, preventScroll }: ForceFocusOptions = {}
) {
if (isActive(element)) return -1;

element.focus({ preventScroll });

if (isActive(element)) return -1;

return requestAnimationFrame(() => {
element.focus({ preventScroll });
});
}
58 changes: 58 additions & 0 deletions packages/reakit/src/Dialog/__tests__/index-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,64 @@ test("focus a given element when dialog opens and initialFocusRef is passed in",
expect(button2).toHaveFocus();
});

test("focus a given element when dialog opens and initial focus has been manually set using React.useEffect", () => {
const Test = () => {
const dialog = useDialogState();
const ref = React.useRef<HTMLButtonElement>(null);

React.useEffect(() => {
if (dialog.visible && ref.current) {
ref.current.focus();
}
}, [dialog.visible]);

return (
<>
<DialogDisclosure {...dialog}>disclosure</DialogDisclosure>
<Dialog {...dialog} aria-label="dialog">
<button>button1</button>
<button ref={ref}>button2</button>
</Dialog>
</>
);
};
const { getByText } = render(<Test />);
const disclosure = getByText("disclosure");
const button2 = getByText("button2");
expect(document.body).toHaveFocus();
fireEvent.click(disclosure);
expect(button2).toHaveFocus();
});

test("focus a given element when dialog opens and initial focus has been manually set using autoFocus", () => {
const Test = () => {
const dialog = useDialogState();
return (
<>
<DialogDisclosure {...dialog}>disclosure</DialogDisclosure>
<Dialog {...dialog} aria-label="dialog">
{props =>
dialog.visible && (
<div {...props}>
<button>button1</button>
{/* eslint-disable-next-line jsx-a11y/no-autofocus */}
<button autoFocus>button2</button>
</div>
)
}
</Dialog>
</>
);
};
const { getByText } = render(<Test />);
const disclosure = getByText("disclosure");
expect(document.body).toHaveFocus();
expect(() => getByText("button2")).toThrow();
fireEvent.click(disclosure);
const button2 = getByText("button2");
expect(button2).toHaveFocus();
});

test("focus dialog itself if there is no tabbable descendant element", () => {
const Test = () => {
const dialog = useDialogState();
Expand Down
36 changes: 14 additions & 22 deletions packages/reakit/src/Dialog/__utils/useDisableHoverOutside.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,18 @@ export function useDisableHoverOutside(
nestedDialogs: Array<React.RefObject<HTMLElement>>,
options: DialogOptions
) {
useEventListenerOutside(
portalRef,
{ current: null },
nestedDialogs,
"mouseover",
event => {
event.stopPropagation();
event.preventDefault();
},
options.visible && options.modal
);
useEventListenerOutside(
portalRef,
{ current: null },
nestedDialogs,
"mouseout",
event => {
event.stopPropagation();
event.preventDefault();
},
options.visible && options.modal
);
const useEvent = (eventType: string) =>
useEventListenerOutside(
portalRef,
{ current: null },
nestedDialogs,
eventType,
event => {
event.stopPropagation();
event.preventDefault();
},
options.visible && options.modal
);
useEvent("mouseover");
useEvent("mouseout");
}
4 changes: 2 additions & 2 deletions packages/reakit/src/Dialog/__utils/useFocusOnHide.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from "react";
import { useUpdateEffect } from "reakit-utils/useUpdateEffect";
import { warning } from "reakit-utils/warning";
import { isTabbable } from "reakit-utils/tabbable";
import { isTabbable, forceFocus } from "reakit-utils/tabbable";
import { DialogOptions } from "../Dialog";

export function useFocusOnHide(
Expand Down Expand Up @@ -33,7 +33,7 @@ export function useFocusOnHide(
(disclosureRefs.current && disclosureRefs.current[0]);

if (finalFocusRef) {
finalFocusRef.focus();
forceFocus(finalFocusRef);
} else {
warning(
true,
Expand Down
9 changes: 5 additions & 4 deletions packages/reakit/src/Dialog/__utils/useFocusOnShow.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from "react";
import { useUpdateEffect } from "reakit-utils/useUpdateEffect";
import { warning } from "reakit-utils/warning";
import { getFirstTabbableIn } from "reakit-utils/tabbable";
import { getFirstTabbableIn, forceFocus } from "reakit-utils/tabbable";
import { DialogOptions } from "../Dialog";

export function useFocusOnShow(
Expand Down Expand Up @@ -37,16 +37,17 @@ export function useFocusOnShow(
initialFocusRef.current.focus({ preventScroll: true });
} else {
const tabbable = getFirstTabbableIn(dialog, true);
const isActive = () => dialog.contains(document.activeElement);
if (tabbable) {
tabbable.focus({ preventScroll: true });
forceFocus(tabbable, { preventScroll: true, isActive });
} else {
dialog.focus({ preventScroll: true });
forceFocus(dialog, { preventScroll: true, isActive });
warning(
dialog.tabIndex === undefined || dialog.tabIndex < 0,
"Dialog",
"It's recommended to have at least one tabbable element inside dialog. The dialog element has been automatically focused.",
"If this is the intended behavior, pass `tabIndex={0}` to the dialog element to disable this warning.",
"See https://reakit.io/docs/dialog"
"See https://reakit.io/docs/dialog/#initial-focus"
);
}
}
Expand Down

0 comments on commit a0916c7

Please sign in to comment.