Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Select/Dropdown/MenuContainer): arrow key handling to focus items #11132

Merged
merged 8 commits into from
Nov 20, 2024
14 changes: 13 additions & 1 deletion packages/react-core/src/components/Dropdown/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import { css } from '@patternfly/react-styles';
import { Menu, MenuContent, MenuProps } from '../Menu';
import { Popper } from '../../helpers/Popper/Popper';
import { useOUIAProps, OUIAProps } from '../../helpers';
import { useOUIAProps, OUIAProps, onToggleArrowKeydownDefault } from '../../helpers';

export interface DropdownPopperProps {
/** Vertical direction of the popper. If enableFlip is set to true, this will set the initial direction before the popper flips. */
Expand Down Expand Up @@ -51,6 +51,8 @@ export interface DropdownProps extends MenuProps, OUIAProps {
onOpenChange?: (isOpen: boolean) => void;
/** Keys that trigger onOpenChange, defaults to tab and escape. It is highly recommended to include Escape in the array, while Tab may be omitted if the menu contains non-menu items that are focusable. */
onOpenChangeKeys?: string[];
/** Callback to override the toggle keydown behavior. By default, when the toggle has focus and the menu is open, pressing the up/down arrow keys will focus a valid non-disabled menu item - the first item for the down arrow key and last item for the up arrow key. */
onToggleKeydown?: (event: KeyboardEvent) => void;
/** Indicates if the menu should be without the outer box-shadow. */
isPlain?: boolean;
/** Indicates if the menu should be scrollable. */
Expand Down Expand Up @@ -85,6 +87,7 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
toggle,
shouldFocusToggleOnSelect = false,
onOpenChange,
onToggleKeydown,
isPlain,
isScrollable,
innerRef,
Expand Down Expand Up @@ -139,6 +142,14 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
toggleRef.current?.focus();
}
}

if (toggleRef.current?.contains(event.target as Node)) {
if (onToggleKeydown) {
onToggleKeydown(event);
} else if (isOpen) {
onToggleArrowKeydownDefault(event, menuRef);
}
}
};

const handleClick = (event: MouseEvent) => {
Expand All @@ -163,6 +174,7 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
toggleRef,
onOpenChange,
onOpenChangeKeys,
onToggleKeydown,
shouldPreventScrollOnItemFocus,
shouldFocusFirstItemOnOpen,
focusTimeoutDelay
Expand Down
27 changes: 24 additions & 3 deletions packages/react-core/src/components/Menu/MenuContainer.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { Popper } from '../../helpers/Popper/Popper';
import { onToggleArrowKeydownDefault, Popper } from '../../helpers';

export interface MenuPopperProps {
/** Vertical direction of the popper. If enableFlip is set to true, this will set the initial direction before the popper flips. */
Expand All @@ -17,6 +17,7 @@ export interface MenuPopperProps {
/** Flag to prevent the popper from overflowing its container and becoming partially obscured. */
preventOverflow?: boolean;
}

export interface MenuContainerProps {
/** Menu to be rendered */
menu: React.ReactElement<any, string | React.JSXElementConstructor<any>>;
Expand All @@ -33,6 +34,8 @@ export interface MenuContainerProps {
onOpenChange?: (isOpen: boolean) => void;
/** Keys that trigger onOpenChange, defaults to tab and escape. It is highly recommended to include Escape in the array, while Tab may be omitted if the menu contains non-menu items that are focusable. */
onOpenChangeKeys?: string[];
/** Callback to override the toggle keydown behavior. By default, when the toggle has focus and the menu is open, pressing the up/down arrow keys will focus a valid non-disabled menu item - the first item for the down arrow key and last item for the up arrow key. */
onToggleKeydown?: (event: KeyboardEvent) => void;
/** z-index of the dropdown menu */
zIndex?: number;
/** Additional properties to pass to the Popper */
Expand All @@ -55,10 +58,11 @@ export const MenuContainer: React.FunctionComponent<MenuContainerProps> = ({
toggle,
toggleRef,
onOpenChange,
onToggleKeydown,
zIndex = 9999,
popperProps,
onOpenChangeKeys = ['Escape', 'Tab'],
shouldFocusFirstItemOnOpen = true,
shouldFocusFirstItemOnOpen = false,
kmcfaul marked this conversation as resolved.
Show resolved Hide resolved
shouldPreventScrollOnItemFocus = true,
focusTimeoutDelay = 0
}: MenuContainerProps) => {
Expand Down Expand Up @@ -90,6 +94,14 @@ export const MenuContainer: React.FunctionComponent<MenuContainerProps> = ({
toggleRef.current?.focus();
}
}

if (toggleRef.current?.contains(event.target as Node)) {
if (onToggleKeydown) {
onToggleKeydown(event);
} else if (isOpen) {
onToggleArrowKeydownDefault(event, menuRef);
}
}
};

const handleClick = (event: MouseEvent) => {
Expand All @@ -108,7 +120,16 @@ export const MenuContainer: React.FunctionComponent<MenuContainerProps> = ({
window.removeEventListener('keydown', handleMenuKeys);
window.removeEventListener('click', handleClick);
};
}, [focusTimeoutDelay, isOpen, menuRef, onOpenChange, onOpenChangeKeys, shouldPreventScrollOnItemFocus, toggleRef]);
}, [
focusTimeoutDelay,
isOpen,
menuRef,
onOpenChange,
onOpenChangeKeys,
onToggleKeydown,
shouldPreventScrollOnItemFocus,
toggleRef
]);

return (
<Popper
Expand Down
17 changes: 16 additions & 1 deletion packages/react-core/src/components/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import { css } from '@patternfly/react-styles';
import { Menu, MenuContent, MenuProps } from '../Menu';
import { Popper } from '../../helpers/Popper/Popper';
import { getOUIAProps, OUIAProps, getDefaultOUIAId } from '../../helpers';
import { getOUIAProps, OUIAProps, getDefaultOUIAId, onToggleArrowKeydownDefault } from '../../helpers';

export interface SelectPopperProps {
/** Vertical direction of the popper. If enableFlip is set to true, this will set the initial direction before the popper flips. */
Expand Down Expand Up @@ -62,6 +62,10 @@ export interface SelectProps extends MenuProps, OUIAProps {
onOpenChange?: (isOpen: boolean) => void;
/** Keys that trigger onOpenChange, defaults to tab and escape. It is highly recommended to include Escape in the array, while Tab may be omitted if the menu contains non-menu items that are focusable. */
onOpenChangeKeys?: string[];
/** Callback to override the toggle keydown behavior. By default, when the toggle has focus and the menu is open, pressing the up/down arrow keys will focus a valid non-disabled menu item - the first item for the down arrow key and last item for the up arrow key. */
onToggleKeydown?: (event: KeyboardEvent) => void;
/** Indicates that the Select is used as a typeahead (combobox). Focus won't shift to menu items when pressing up/down arrows. */
isTypeahead?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any chance we would want this to be a wider covering variant prop? Ideally we don't want to have this prop and later find we need to also add isCheckbox, isMultiTypeahead, etc. If we go with variant, the prop description can indicate that the prop affects default keyboard behavior based on its value.

What does everyone think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good to me

/** Indicates if the select should be without the outer box-shadow */
isPlain?: boolean;
/** @hide Forwarded ref */
Expand Down Expand Up @@ -95,6 +99,8 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
shouldFocusFirstItemOnOpen = false,
onOpenChange,
onOpenChangeKeys = ['Escape', 'Tab'],
onToggleKeydown,
isTypeahead,
isPlain,
innerRef,
zIndex = 9999,
Expand Down Expand Up @@ -144,6 +150,14 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
toggleRef.current?.focus();
}
}

if (toggleRef.current?.contains(event.target as Node)) {
if (onToggleKeydown) {
onToggleKeydown(event);
} else if (isOpen && !isTypeahead) {
onToggleArrowKeydownDefault(event, menuRef);
}
}
};

const handleClick = (event: MouseEvent) => {
Expand All @@ -168,6 +182,7 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
toggleRef,
onOpenChange,
onOpenChangeKeys,
onToggleKeydown,
shouldPreventScrollOnItemFocus,
shouldFocusFirstItemOnOpen,
focusTimeoutDelay
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ export const SelectMultiTypeahead: React.FunctionComponent = () => {
!isOpen && closeMenu();
}}
toggle={toggle}
shouldFocusFirstItemOnOpen={false}
isTypeahead
>
<SelectList isAriaMultiselectable id="select-multi-typeahead-listbox">
{selectOptions.map((option, index) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ export const SelectMultiTypeaheadCheckbox: React.FunctionComponent = () => {
!isOpen && closeMenu();
}}
toggle={toggle}
shouldFocusFirstItemOnOpen={false}
isTypeahead
>
<SelectList isAriaMultiselectable id="select-multi-typeahead-checkbox-listbox">
{selectOptions.map((option, index) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ export const SelectMultiTypeaheadCreatable: React.FunctionComponent = () => {
!isOpen && closeMenu();
}}
toggle={toggle}
shouldFocusFirstItemOnOpen={false}
isTypeahead
>
<SelectList isAriaMultiselectable id="select-multi-create-typeahead-listbox">
{selectOptions.map((option, index) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ export const SelectTypeahead: React.FunctionComponent = () => {
!isOpen && closeMenu();
}}
toggle={toggle}
shouldFocusFirstItemOnOpen={false}
isTypeahead
>
<SelectList id="select-typeahead-listbox">
{selectOptions.map((option, index) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ export const SelectTypeaheadCreatable: React.FunctionComponent = () => {
!isOpen && closeMenu();
}}
toggle={toggle}
shouldFocusFirstItemOnOpen={false}
isTypeahead
>
<SelectList id="select-create-typeahead-listbox">
{selectOptions.map((option, index) => (
Expand Down
27 changes: 27 additions & 0 deletions packages/react-core/src/helpers/KeyboardHandler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,33 @@ export const setTabIndex = (options: HTMLElement[]) => {
}
};

/**
* This function is used in Dropdown, Select and MenuContainer as a default toggle keydown behavior. When the toggle has focus and the menu is open, pressing the up/down arrow keys will focus a valid non-disabled menu item - the first item for the down arrow key and last item for the up arrow key.
*
* @param event Event triggered by the keyboard
* @param menuRef Menu reference
*/
export const onToggleArrowKeydownDefault = (event: KeyboardEvent, menuRef: React.RefObject<HTMLDivElement>) => {
if (event.key !== 'ArrowDown' && event.key !== 'ArrowUp') {
return;
}

event.preventDefault();

const listItems = Array.from(menuRef.current?.querySelectorAll('li'));
const focusableElements = listItems
.map((li) => li.querySelector('button:not(:disabled),input:not(:disabled),a:not([aria-disabled="true"])'))
.filter((el) => el !== null);

let focusableElement: Element;
if (event.key === 'ArrowDown') {
focusableElement = focusableElements[0];
} else {
focusableElement = focusableElements[focusableElements.length - 1];
}
focusableElement && (focusableElement as HTMLElement).focus();
};

class KeyboardHandler extends React.Component<KeyboardHandlerProps> {
static displayName = 'KeyboardHandler';
static defaultProps: KeyboardHandlerProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ export const MultiTypeaheadSelectBase: React.FunctionComponent<MultiTypeaheadSel
!isOpen && closeMenu();
}}
toggle={toggle}
shouldFocusFirstItemOnOpen={false}
isTypeahead
ref={innerRef}
{...props}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ export const TypeaheadSelectBase: React.FunctionComponent<TypeaheadSelectProps>
!isOpen && closeMenu();
}}
toggle={toggle}
shouldFocusFirstItemOnOpen={false}
isTypeahead
ref={innerRef}
{...props}
>
Expand Down
Loading