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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 8 commits into from
Nov 20, 2024
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0fe487a
feat(Select): default arrow key handling to focus items
adamviktora Oct 23, 2024
039e095
feat(Dropdown): default arrow key handling to focus items
adamviktora Oct 25, 2024
fd712a7
feat(MenuContainer): default arrow key handling to focus items
adamviktora Oct 25, 2024
fdbc8b3
fix: invoke callback only when toggle is focused and menu opened
adamviktora Oct 29, 2024
513c6f5
fix: query selector, refactor common functionality
adamviktora Nov 12, 2024
cdd6f0f
feat: use general onToggleKeydown instead of onToggleArrowKeydown
adamviktora Nov 12, 2024
de44721
refactor(onToggleArrowKeydownDefault): don't use :has() selector
adamviktora Nov 12, 2024
901e606
fix: remove isTypeahead prop in favor of variant
adamviktora Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions packages/react-core/src/components/Dropdown/Dropdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
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[];
/** Custom callback to override the default behaviour when pressing up/down arrows. Default is focusing the menu items (first item on arrow down, last item on arrow up). */
onArrowUpDownKeyDown?: (event: KeyboardEvent) => void;
adamviktora marked this conversation as resolved.
Show resolved Hide resolved
/** 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,
onArrowUpDownKeyDown,
isPlain,
isScrollable,
innerRef,
Expand Down Expand Up @@ -127,6 +130,23 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
}, [isOpen]);

React.useEffect(() => {
const onArrowUpDownKeyDownDefault = (event: KeyboardEvent) => {
event.preventDefault();

let listItem: HTMLLIElement;
if (event.key === 'ArrowDown') {
listItem = menuRef.current?.querySelector('li');
} else {
const allItems = menuRef.current?.querySelectorAll('li');
listItem = allItems ? allItems[allItems.length - 1] : null;
}

const focusableElement = listItem?.querySelector(
'button:not(:disabled),input:not(:disabled),a:not([aria-disabled="true"])'
);
focusableElement && (focusableElement as HTMLElement).focus();
};

const handleMenuKeys = (event: KeyboardEvent) => {
// Close the menu on tab or escape if onOpenChange is provided
if (
Expand All @@ -139,6 +159,14 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
toggleRef.current?.focus();
}
}

if (event.key === 'ArrowDown' || event.key === 'ArrowUp') {
adamviktora marked this conversation as resolved.
Show resolved Hide resolved
if (onArrowUpDownKeyDown) {
onArrowUpDownKeyDown(event);
} else {
onArrowUpDownKeyDownDefault(event);
}
}
};

const handleClick = (event: MouseEvent) => {
Expand All @@ -163,6 +191,7 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
toggleRef,
onOpenChange,
onOpenChangeKeys,
onArrowUpDownKeyDown,
shouldPreventScrollOnItemFocus,
shouldFocusFirstItemOnOpen,
focusTimeoutDelay
Expand Down
42 changes: 40 additions & 2 deletions packages/react-core/src/components/Menu/MenuContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
/** Custom callback to override the default behaviour when pressing up/down arrows. Default is focusing the menu items (first item on arrow down, last item on arrow up). */
onArrowUpDownKeyDown?: (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,
onArrowUpDownKeyDown,
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 All @@ -79,6 +83,23 @@ export const MenuContainer: React.FunctionComponent<MenuContainerProps> = ({
}, [isOpen]);

React.useEffect(() => {
const onArrowUpDownKeyDownDefault = (event: KeyboardEvent) => {
event.preventDefault();

let listItem: HTMLLIElement;
if (event.key === 'ArrowDown') {
listItem = menuRef.current?.querySelector('li');
} else {
const allItems = menuRef.current?.querySelectorAll('li');
listItem = allItems ? allItems[allItems.length - 1] : null;
}

const focusableElement = listItem?.querySelector(
'button:not(:disabled),input:not(:disabled),a:not([aria-disabled="true"])'
);
focusableElement && (focusableElement as HTMLElement).focus();
};

const handleMenuKeys = (event: KeyboardEvent) => {
// Close the menu on tab or escape if onOpenChange is provided
if (
Expand All @@ -90,6 +111,14 @@ export const MenuContainer: React.FunctionComponent<MenuContainerProps> = ({
toggleRef.current?.focus();
}
}

if (event.key === 'ArrowDown' || event.key === 'ArrowUp') {
if (onArrowUpDownKeyDown) {
onArrowUpDownKeyDown(event);
} else {
onArrowUpDownKeyDownDefault(event);
}
}
};

const handleClick = (event: MouseEvent) => {
Expand All @@ -108,7 +137,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,
onArrowUpDownKeyDown,
shouldPreventScrollOnItemFocus,
toggleRef
]);

return (
<Popper
Expand Down
30 changes: 30 additions & 0 deletions packages/react-core/src/components/Select/Select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
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[];
/** Custom callback to override the default behaviour when pressing up/down arrows. Default is focusing the menu items (first item on arrow down, last item on arrow up). */
onArrowUpDownKeyDown?: (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.

@tlabaj @thatblindgeye

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'],
onArrowUpDownKeyDown,
isTypeahead,
isPlain,
innerRef,
zIndex = 9999,
Expand Down Expand Up @@ -131,6 +137,21 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
}, [isOpen]);

React.useEffect(() => {
const onArrowUpDownKeyDownDefault = (event: KeyboardEvent) => {
event.preventDefault();

let listItem: HTMLLIElement;
if (event.key === 'ArrowDown') {
listItem = menuRef.current?.querySelector('li');
} else {
const allItems = menuRef.current?.querySelectorAll('li');
listItem = allItems ? allItems[allItems.length - 1] : null;
}

const focusableElement = listItem?.querySelector('button:not(:disabled),input:not(:disabled)');
kmcfaul marked this conversation as resolved.
Show resolved Hide resolved
focusableElement && (focusableElement as HTMLElement).focus();
};

const handleMenuKeys = (event: KeyboardEvent) => {
// Close the menu on tab or escape if onOpenChange is provided
if (
Expand All @@ -144,6 +165,14 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
toggleRef.current?.focus();
}
}

if (event.key === 'ArrowDown' || event.key === 'ArrowUp') {
if (onArrowUpDownKeyDown) {
onArrowUpDownKeyDown(event);
} else if (!isTypeahead) {
onArrowUpDownKeyDownDefault(event);
}
}
};

const handleClick = (event: MouseEvent) => {
Expand All @@ -168,6 +197,7 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
toggleRef,
onOpenChange,
onOpenChangeKeys,
onArrowUpDownKeyDown,
shouldPreventScrollOnItemFocus,
shouldFocusFirstItemOnOpen,
focusTimeoutDelay
Expand Down
2 changes: 1 addition & 1 deletion packages/react-core/src/components/Select/examples/SelectMultiTypeahead.tsx
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion packages/react-core/src/components/Select/examples/SelectMultiTypeaheadCheckbox.tsx
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion packages/react-core/src/components/Select/examples/SelectMultiTypeaheadCreatable.tsx
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion packages/react-core/src/components/Select/examples/SelectTypeahead.tsx
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion packages/react-core/src/components/Select/examples/SelectTypeaheadCreatable.tsx
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion packages/react-templates/src/components/Select/MultiTypeaheadSelect.tsx
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion packages/react-templates/src/components/Select/TypeaheadSelect.tsx
View file
Open in desktop
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