Skip to content

Commit

Permalink
Adapt handling of initial focus in useTabKeyNavigation
Browse files Browse the repository at this point in the history
Ensure that the following initial focus use cases are supported by the
`useTabKeyNavigation` hook:

- When the container itself is focused: The container (a non-interactive
  element) does not remain part of the tab sequence once it loses focus
- When an item in the tab sequence is focused, but it is not the first
  item in the tab sequence: ensure tab sequence is ordered correctly.
  • Loading branch information
lyzadanger committed Mar 28, 2023
1 parent 9c29fa8 commit 943eb1b
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 16 deletions.
78 changes: 73 additions & 5 deletions src/hooks/test/use-tab-key-navigation-test.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,29 @@
import { render } from 'preact';
import { useRef } from 'preact/hooks';
import { useEffect, useRef } from 'preact/hooks';
import { act } from 'preact/test-utils';

import { waitFor } from '../../test-util/wait';
import { useTabKeyNavigation } from '../use-tab-key-navigation';

function Toolbar({ navigationOptions = {} }) {
function Toolbar({
focusContainer = false,
initialFocusIndex = -1,
navigationOptions = {},
}) {
const containerRef = useRef();

useEffect(() => {
if (focusContainer) {
containerRef.current.focus();
} else if (initialFocusIndex >= 0) {
containerRef.current.children[initialFocusIndex].focus();
}
}, [focusContainer, initialFocusIndex]);

useTabKeyNavigation(containerRef, navigationOptions);

return (
<div ref={containerRef} data-testid="toolbar">
<div ref={containerRef} tabIndex={-1} data-testid="toolbar">
<button data-testid="bold">Bold</button>
<button data-testid="italic">Italic</button>
<button data-testid="underline">Underline</button>
Expand All @@ -35,12 +47,23 @@ describe('useTabKeyNavigation', () => {
container.remove();
});

function renderToolbar(options = {}) {
function renderToolbar({
focusContainer = false,
initialFocusIndex = -1,
...options
} = {}) {
// We render the component with Preact directly rather than using Enzyme
// for these tests. Since the `tabIndex` state lives only in the DOM,
// and there are no child components involved, this is more convenient.
act(() => {
render(<Toolbar navigationOptions={options} />, container);
render(
<Toolbar
focusContainer={focusContainer}
initialFocusIndex={initialFocusIndex}
navigationOptions={options}
/>,
container
);
});
return findElementByTestId('toolbar');
}
Expand Down Expand Up @@ -144,6 +167,51 @@ describe('useTabKeyNavigation', () => {
});
});

context('when initial focus is on the container', () => {
it('should move to first item in tab sequence when tab pressed', async () => {
const toolbar = renderToolbar({ focusContainer: true });
await waitFor(() => document.activeElement === toolbar);

pressKey('Tab');
assert.equal(currentItem(), 'Bold');

// Container has been removed from the tab sequence
pressKey({ key: 'Tab', shiftKey: true });
assert.equal(currentItem(), 'Help');
});

it('should not include container in tab sequence', async () => {
const toolbar = renderToolbar({ focusContainer: true });
await waitFor(() => document.activeElement === toolbar);

// Forward to first item in tab sequence
pressKey('Tab');
// Back to last item in tab sequence
pressKey({ key: 'Tab', shiftKey: true });
assert.equal(currentItem(), 'Help');
});
});

context('when initial focus is not on first item in tab sequence', () => {
it('should focus on the next item in the tab sequence when "Tab" pressed', async () => {
const toolbar = renderToolbar({ initialFocusIndex: 1 });
await waitFor(() => document.activeElement === toolbar.children[1]);

assert.equal(currentItem(), 'Italic');

pressKey('Tab');
assert.equal(currentItem(), 'Underline');
});

it('should focus on the previous item in the tab sequence when "shift-Tab" pressed', async () => {
const toolbar = renderToolbar({ initialFocusIndex: 2 });
await waitFor(() => document.activeElement === toolbar.children[2]);

pressKey({ key: 'Tab', shiftKey: true });
assert.equal(currentItem(), 'Italic');
});
});

it('should skip hidden elements', () => {
renderToolbar();
findElementByTestId('bold').focus();
Expand Down
31 changes: 20 additions & 11 deletions src/hooks/use-tab-key-navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,6 @@ function isElementVisible(element: HTMLElement) {
}

export type UseTabKeyNavigationOptions = {
/**
* Whether to focus the first element in the set of matching elements when
* the component is mounted
*/
autofocus?: boolean;

/**
* Don't respond to any keyboard events if not enabled. This allows selective
* enabling by components using this hook, as hook use itself cannot be
Expand All @@ -26,7 +20,7 @@ export type UseTabKeyNavigationOptions = {
enabled?: boolean;

/**
* CSS selector which specifies the elements that navigation moves between
* CSS selector which specifies which elements should be in the tab sequence
*/
selector?: string;
};
Expand All @@ -36,6 +30,9 @@ export type UseTabKeyNavigationOptions = {
* 'Shift-Tab' keys to navigate through interactive descendants. See [1] for
* reference for how keyboard navigation should work within modal dialogs.
*
* Note that this hook does not set initial focus: routing initial focus
* appropriately is the responsibility of the consuming component.
*
* NB: This hook should be removed/disused once we migrate to using native
* <dialog> elements. The hook duplicates some logic in `useArrowKeyNavigation`.
*
Expand Down Expand Up @@ -64,7 +61,6 @@ export function useTabKeyNavigation(
containerRef: RefObject<HTMLElement | undefined>,
{
enabled = true,
autofocus = false,
selector = 'a,button,input,select,textarea',
}: UseTabKeyNavigationOptions = {}
) {
Expand All @@ -81,9 +77,17 @@ export function useTabKeyNavigation(
const elements: HTMLElement[] = Array.from(
container.querySelectorAll(selector)
);
return elements.filter(
const filtered = elements.filter(
el => isElementVisible(el) && !isElementDisabled(el)
);
// Include the container itself in the set of navigable elements if it
// is currently focused. It will not be part of the tab sequence once it
// loses focus. This allows, e.g., a modal container to be focused when
// opened but not be part of the subsequent trapped tab sequence.
if (document.activeElement === container) {
filtered.unshift(container);
}
return filtered;
};

/**
Expand Down Expand Up @@ -148,7 +152,12 @@ export function useTabKeyNavigation(
event.stopPropagation();
};

updateTabIndexes(getNavigableElements(), 0, autofocus);
const elements = getNavigableElements();
// One of the navigable elements may already have focus
const focusedIndex = elements.indexOf(
document.activeElement as HTMLElement
);
updateTabIndexes(elements, focusedIndex);

const listeners = new ListenerCollection();

Expand Down Expand Up @@ -181,5 +190,5 @@ export function useTabKeyNavigation(
listeners.removeAll();
mo.disconnect();
};
}, [autofocus, containerRef, enabled, selector]);
}, [containerRef, enabled, selector]);
}

0 comments on commit 943eb1b

Please sign in to comment.