Skip to content

Commit

Permalink
fix: Fix disabled events when using the as prop (#736)
Browse files Browse the repository at this point in the history
  • Loading branch information
@diegohaz
diegohaz authored Sep 20, 2020
1 parent b7516c0 commit c594166
Show file tree
Hide file tree
Showing 9 changed files with 314 additions and 101 deletions.
111 changes: 56 additions & 55 deletions packages/reakit/src/Composite/Composite.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ function useKeyboardEventProxy(
const eventHandlerRef = useLiveRef(htmlEventHandler);
return React.useCallback(
(event: React.KeyboardEvent) => {
eventHandlerRef.current?.(event);
if (event.defaultPrevented) return;
if (virtual && canProxyKeyboardEvent(event)) {
const currentElement = currentItem?.ref.current;
if (currentElement) {
Expand All @@ -84,11 +86,9 @@ function useKeyboardEventProxy(
if (event.currentTarget.contains(currentElement)) {
event.stopPropagation();
event.preventDefault();
return;
}
}
}
eventHandlerRef.current?.(event);
},
[virtual, currentItem]
);
Expand Down Expand Up @@ -132,18 +132,20 @@ export const useComposite = createHook<CompositeOptions, CompositeHTMLProps>({
options,
{
ref: htmlRef,
onFocus: htmlOnFocus,
onBlur: htmlOnBlur,
onFocusCapture: htmlOnFocusCapture,
onBlurCapture: htmlOnBlurCapture,
onKeyDown: htmlOnKeyDown,
onKeyUp: htmlOnKeyUp,
onKeyDownCapture: htmlOnKeyDownCapture,
onKeyUpCapture: htmlOnKeyUpCapture,
...htmlProps
}
) {
const ref = React.useRef<HTMLElement>(null);
const currentItem = findEnabledItemById(options.items, options.currentId);
const previousElementRef = React.useRef<HTMLElement | null>(null);
const onFocusRef = useLiveRef(htmlOnFocus);
const onBlurRef = useLiveRef(htmlOnBlur);
const onKeyDownRef = useLiveRef(htmlOnKeyDown);
const onFocusCaptureRef = useLiveRef(htmlOnFocusCapture);
const onBlurCaptureRef = useLiveRef(htmlOnBlurCapture);
// IE 11 doesn't support event.relatedTarget, so we use the active element
// ref instead.
const activeElementRef = isIE11 ? useActiveElementRef(ref) : undefined;
Expand All @@ -165,20 +167,22 @@ export const useComposite = createHook<CompositeOptions, CompositeHTMLProps>({
}
}, [options.unstable_moves, currentItem]);

const onKeyDown = useKeyboardEventProxy(
const onKeyDownCapture = useKeyboardEventProxy(
options.unstable_virtual,
currentItem,
htmlOnKeyDown
htmlOnKeyDownCapture
);

const onKeyUp = useKeyboardEventProxy(
const onKeyUpCapture = useKeyboardEventProxy(
options.unstable_virtual,
currentItem,
htmlOnKeyUp
htmlOnKeyUpCapture
);

const onFocus = React.useCallback(
const onFocusCapture = React.useCallback(
(event: React.FocusEvent) => {
onFocusCaptureRef.current?.(event);
if (event.defaultPrevented) return;
if (options.unstable_virtual) {
const currentElement = currentItem?.ref.current || null;
// IE11 doesn't support event.relatedTarget, so we use the active
Expand All @@ -189,29 +193,26 @@ export const useComposite = createHook<CompositeOptions, CompositeHTMLProps>({
options.items,
previousActiveElement
);
if (isSelfTarget(event) && !previousActiveElementWasItem) {
// This means that the composite element has been focused while the
// composite item has not. For example, by clicking on the
// composite element without touching any item, or by tabbing into
// the composite element. In this case, we want to trigger focus on
// the item, just like it would happen with roving tabindex.
// When it receives focus, the composite item will put focus back
// on the composite element, in which case hasItemWithFocus will be
// true.
onFocusRef.current?.(event);
currentElement?.focus();
return;
}
if (previousActiveElementWasItem) {
// Composite has been focused as a result of an item receiving
// focus. The composite item will move focus back to the composite
// container. In this case, we don't want to propagate this
// additional event nor call the onFocus handler passed to
// <Composite onFocus={...} /> (htmlOnFocus). Unless users add DOM
// event handlers to the composite element directly, this will be
// like this event has never existed.
event.stopPropagation();
return;
if (isSelfTarget(event)) {
if (previousActiveElementWasItem) {
// Composite has been focused as a result of an item receiving
// focus. The composite item will move focus back to the composite
// container. In this case, we don't want to propagate this
// additional event nor call the onFocus handler passed to
// <Composite onFocus={...} />.
event.stopPropagation();
} else {
// This means that the composite element has been focused while
// the composite item has not. For example, by clicking on the
// composite element without touching any item, or by tabbing
// into the composite element. In this case, we want to trigger
// focus on the item, just like it would happen with roving
// tabindex. When it receives focus, the composite item will put
// focus back on the composite element, in which case
// hasItemWithFocus will be true.
onFocusCaptureRef.current?.(event);
currentElement?.focus();
}
}
} else if (isSelfTarget(event)) {
// When the roving tabindex composite gets intentionally focused (for
Expand All @@ -220,7 +221,6 @@ export const useComposite = createHook<CompositeOptions, CompositeHTMLProps>({
// itself is focused).
options.setCurrentId?.(null);
}
onFocusRef.current?.(event);
},
[
options.unstable_virtual,
Expand All @@ -230,8 +230,10 @@ export const useComposite = createHook<CompositeOptions, CompositeHTMLProps>({
]
);

const onBlur = React.useCallback(
const onBlurCapture = React.useCallback(
(event: React.FocusEvent) => {
onBlurCaptureRef.current?.(event);
if (event.defaultPrevented) return;
// When virtual is set to true, we move focus from the composite
// container (this component) to the composite item that is being
// selected. Then we move focus back to the composite container. This
Expand Down Expand Up @@ -286,27 +288,26 @@ export const useComposite = createHook<CompositeOptions, CompositeHTMLProps>({
// We want to ignore intermediate blur events, so we stop its
// propagation and return early so onFocus will not be called.
event.stopPropagation();
return;
}
const targetIsItem = isItem(options.items, event.target);
if (!targetIsItem && currentElement) {
// If target is not a composite item, it may be the composite
// element itself (isSelfTarget) or a tabbable element inside the
// composite widget. This may be triggered by clicking outside the
// composite widget or by tabbing out of it. In either cases we
// want to fire a blur event on the current item.
fireBlurEvent(currentElement, event);
} else {
const targetIsItem = isItem(options.items, event.target);
if (!targetIsItem && currentElement) {
// If target is not a composite item, it may be the composite
// element itself (isSelfTarget) or a tabbable element inside the
// composite widget. This may be triggered by clicking outside
// the composite widget or by tabbing out of it. In either cases
// we want to fire a blur event on the current item.
fireBlurEvent(currentElement, event);
}
}
}
onBlurRef.current?.(event);
},
[options.unstable_virtual, options.items, currentItem]
);

const onMove = React.useMemo(
const onKeyDown = React.useMemo(
() =>
createOnKeyDown({
onKeyDown,
onKeyDown: onKeyDownRef,
stopPropagation: true,
shouldKeyDown: (event) =>
isSelfTarget(event) && options.currentId === null,
Expand Down Expand Up @@ -339,7 +340,6 @@ export const useComposite = createHook<CompositeOptions, CompositeHTMLProps>({
},
}),
[
onKeyDown,
options.currentId,
options.orientation,
options.groups,
Expand All @@ -353,10 +353,11 @@ export const useComposite = createHook<CompositeOptions, CompositeHTMLProps>({
return {
ref: useForkRef(ref, htmlRef),
id: options.baseId,
onFocus,
onBlur,
onKeyDown: onMove,
onKeyUp,
onFocusCapture,
onBlurCapture,
onKeyDownCapture,
onKeyDown,
onKeyUpCapture,
"aria-activedescendant": options.unstable_virtual
? currentItem?.id || undefined
: undefined,
Expand Down
13 changes: 6 additions & 7 deletions packages/reakit/src/Composite/CompositeItem.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export const useCompositeItem = createHook<
ref: htmlRef,
tabIndex: htmlTabIndex = 0,
onFocus: htmlOnFocus,
onBlur: htmlOnBlur,
onBlurCapture: htmlOnBlurCapture,
onKeyDown: htmlOnKeyDown,
onClick: htmlOnClick,
...htmlProps
Expand All @@ -140,7 +140,7 @@ export const useCompositeItem = createHook<
const hasFocusedComposite = React.useRef(false);
const item = useItem(options);
const onFocusRef = useLiveRef(htmlOnFocus);
const onBlurRef = useLiveRef(htmlOnBlur);
const onBlurCaptureRef = useLiveRef(htmlOnBlurCapture);
const onKeyDownRef = useLiveRef(htmlOnKeyDown);
const onClickRef = useLiveRef(htmlOnClick);
const shouldTabIndex =
Expand Down Expand Up @@ -209,18 +209,18 @@ export const useCompositeItem = createHook<
]
);

const onBlur = React.useCallback(
const onBlurCapture = React.useCallback(
(event: React.FocusEvent<HTMLElement>) => {
onBlurCaptureRef.current?.(event);
if (event.defaultPrevented) return;
if (options.unstable_virtual && hasFocusedComposite.current) {
// When hasFocusedComposite is true, composite has been focused right
// after focusing this item. This is an intermediate blur event, so
// we ignore it.
hasFocusedComposite.current = false;
event.preventDefault();
event.stopPropagation();
return;
}
onBlurRef.current?.(event);
},
[options.unstable_virtual]
);
Expand Down Expand Up @@ -249,7 +249,6 @@ export const useCompositeItem = createHook<
() =>
createOnKeyDown({
onKeyDown: onCharacterKeyDown,
stopPropagation: true,
// We don't want to listen to focusable elements inside the composite
// item, such as a CompositeItemWidget.
shouldKeyDown: isSelfTarget,
Expand Down Expand Up @@ -342,7 +341,7 @@ export const useCompositeItem = createHook<
"aria-selected":
options.unstable_virtual && isCurrentItem ? true : undefined,
onFocus,
onBlur,
onBlurCapture,
onKeyDown,
onClick,
...htmlProps,
Expand Down
56 changes: 56 additions & 0 deletions packages/reakit/src/Composite/__examples__/CompositeVirtualAsProp/__tests__/index-test.tsx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import * as React from "react";
import { render, press, screen, axe } from "reakit-test-utils";
import CompositeVirtualAsProp from "..";

test("navigate through composite items", () => {
render(<CompositeVirtualAsProp />);
press.Tab();
press.ArrowDown();
expect(screen.getByLabelText("events")).toMatchInlineSnapshot(`
<ul
aria-label="events"
>
<li>
focus Item 1
</li>
<li>
focus composite - Item 1
</li>
<li>
focus composite
</li>
<li>
keyup composite (Tab)
</li>
<li>
keydown Item 1 (ArrowDown)
</li>
<li>
keydown composite - Item 1 (ArrowDown)
</li>
<li>
blur Item 1
</li>
<li>
blur composite - Item 1
</li>
<li>
focus Item 2
</li>
<li>
focus composite - Item 2
</li>
<li>
keyup Item 2 (ArrowDown)
</li>
<li>
keyup composite - Item 2 (ArrowDown)
</li>
</ul>
`);
});

test("a11y", async () => {
const { baseElement } = render(<CompositeVirtualAsProp />);
expect(await axe(baseElement)).toHaveNoViolations();
});
Loading

0 comments on commit c594166

Please sign in to comment.