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

Fixes 11053: focus event causing jumpy scroll effect #11056

Open
wants to merge 1 commit into
base: v5
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 20 additions & 5 deletions packages/react-core/src/components/Dropdown/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ export interface DropdownProps extends MenuProps, OUIAProps {
maxMenuHeight?: string;
/** @beta Flag indicating the first menu item should be focused after opening the dropdown. */
shouldFocusFirstItemOnOpen?: boolean;
/** Flag indicating if scroll on focus of the first menu item should occur. */
shouldPreventScrollOnItemFocus?: boolean;
/** Time in ms to wait before firing the toggles' focus event. Defaults to 0 */
focusTimeoutDelay?: number;
}

const DropdownBase: React.FunctionComponent<DropdownProps> = ({
Expand All @@ -92,6 +96,8 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
menuHeight,
maxMenuHeight,
shouldFocusFirstItemOnOpen = true,
shouldPreventScrollOnItemFocus = true,
focusTimeoutDelay = 0,
...props
}: DropdownProps) => {
const localMenuRef = React.useRef<HTMLDivElement>();
Expand All @@ -114,7 +120,7 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
) {
if (onOpenChangeKeys.includes(event.key)) {
onOpenChange(false);
toggleRef.current?.focus();
toggleRef.current?.focus({ preventScroll: shouldPreventScrollOnItemFocus });
}
}
};
Expand All @@ -126,8 +132,8 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
const firstElement = menuRef?.current?.querySelector(
'li button:not(:disabled),li input:not(:disabled),li a:not([aria-disabled="true"])'
);
firstElement && (firstElement as HTMLElement).focus();
}, 10);
firstElement && (firstElement as HTMLElement).focus({ preventScroll: shouldPreventScrollOnItemFocus });
}, focusTimeoutDelay);
}

// If the event is not on the toggle and onOpenChange callback is provided, close the menu
Expand All @@ -145,7 +151,16 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
window.removeEventListener('keydown', handleMenuKeys);
window.removeEventListener('click', handleClick);
};
}, [isOpen, menuRef, toggleRef, onOpenChange, onOpenChangeKeys]);
}, [
isOpen,
menuRef,
toggleRef,
onOpenChange,
onOpenChangeKeys,
shouldPreventScrollOnItemFocus,
shouldFocusFirstItemOnOpen,
focusTimeoutDelay
]);

const scrollable = maxMenuHeight !== undefined || menuHeight !== undefined || isScrollable;

Expand All @@ -155,7 +170,7 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
ref={menuRef}
onSelect={(event, value) => {
onSelect && onSelect(event, value);
shouldFocusToggleOnSelect && toggleRef.current.focus();
shouldFocusToggleOnSelect && toggleRef.current?.focus();
}}
isPlain={isPlain}
isScrollable={scrollable}
Expand Down
16 changes: 11 additions & 5 deletions packages/react-core/src/components/Menu/MenuContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ export interface MenuContainerProps {
zIndex?: number;
/** Additional properties to pass to the Popper */
popperProps?: MenuPopperProps;
/** Flag indicating if scroll on focus of the first menu item should occur. */
shouldPreventScrollOnItemFocus?: boolean;
/** Time in ms to wait before firing the toggles' focus event. Defaults to 0 */
focusTimeoutDelay?: number;
}

/**
Expand All @@ -52,7 +56,9 @@ export const MenuContainer: React.FunctionComponent<MenuContainerProps> = ({
onOpenChange,
zIndex = 9999,
popperProps,
onOpenChangeKeys = ['Escape', 'Tab']
onOpenChangeKeys = ['Escape', 'Tab'],
shouldPreventScrollOnItemFocus = true,
focusTimeoutDelay = 0
}: MenuContainerProps) => {
React.useEffect(() => {
const handleMenuKeys = (event: KeyboardEvent) => {
Expand All @@ -63,7 +69,7 @@ export const MenuContainer: React.FunctionComponent<MenuContainerProps> = ({
) {
if (onOpenChangeKeys.includes(event.key)) {
onOpenChange(false);
toggleRef.current?.focus();
toggleRef.current?.focus({ preventScroll: shouldPreventScrollOnItemFocus });
}
}
};
Expand All @@ -75,8 +81,8 @@ export const MenuContainer: React.FunctionComponent<MenuContainerProps> = ({
const firstElement = menuRef?.current?.querySelector(
'li button:not(:disabled),li input:not(:disabled),li a:not([aria-disabled="true"])'
);
firstElement && (firstElement as HTMLElement).focus();
}, 0);
firstElement && (firstElement as HTMLElement).focus({ preventScroll: shouldPreventScrollOnItemFocus });
}, focusTimeoutDelay);
}

// If the event is not on the toggle and onOpenChange callback is provided, close the menu
Expand All @@ -94,7 +100,7 @@ export const MenuContainer: React.FunctionComponent<MenuContainerProps> = ({
window.removeEventListener('keydown', handleMenuKeys);
window.removeEventListener('click', handleClick);
};
}, [isOpen, menuRef, onOpenChange, onOpenChangeKeys, toggleRef]);
}, [focusTimeoutDelay, isOpen, menuRef, onOpenChange, onOpenChangeKeys, shouldPreventScrollOnItemFocus, toggleRef]);

return (
<Popper
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ export interface PaginationOptionsMenuProps extends React.HTMLProps<HTMLDivEleme
containerRef?: React.RefObject<HTMLDivElement>;
/** @beta The container to append the pagination options menu to. Overrides the containerRef prop. */
appendTo?: HTMLElement | (() => HTMLElement) | 'inline';
/** Flag indicating if scroll on focus of the first menu item should occur. */
shouldPreventScrollOnItemFocus?: boolean;
/** Time in ms to wait before firing the toggles' focus event. Defaults to 0 */
focusTimeoutDelay?: number;
}

export const PaginationOptionsMenu: React.FunctionComponent<PaginationOptionsMenuProps> = ({
Expand All @@ -80,7 +84,9 @@ export const PaginationOptionsMenu: React.FunctionComponent<PaginationOptionsMen
toggleTemplate,
onPerPageSelect = () => null as any,
containerRef,
appendTo
appendTo,
shouldPreventScrollOnItemFocus = true,
focusTimeoutDelay = 0
}: PaginationOptionsMenuProps) => {
const [isOpen, setIsOpen] = React.useState(false);
const toggleRef = React.useRef<HTMLButtonElement>(null);
Expand Down Expand Up @@ -123,7 +129,7 @@ export const PaginationOptionsMenu: React.FunctionComponent<PaginationOptionsMen
) {
if (event.key === 'Escape' || event.key === 'Tab') {
setIsOpen(false);
toggleRef.current?.focus();
toggleRef.current?.focus({ preventScroll: shouldPreventScrollOnItemFocus });
}
}
};
Expand All @@ -133,8 +139,8 @@ export const PaginationOptionsMenu: React.FunctionComponent<PaginationOptionsMen
if (isOpen && toggleRef.current?.contains(event.target as Node)) {
setTimeout(() => {
const firstElement = menuRef?.current?.querySelector('li button:not(:disabled)');
firstElement && (firstElement as HTMLElement).focus();
}, 0);
firstElement && (firstElement as HTMLElement).focus({ preventScroll: shouldPreventScrollOnItemFocus });
}, focusTimeoutDelay);
}

// If the event is not on the toggle, close the menu
Expand All @@ -154,7 +160,7 @@ export const PaginationOptionsMenu: React.FunctionComponent<PaginationOptionsMen
window.removeEventListener('keydown', handleMenuKeys);
window.removeEventListener('click', handleClick);
};
}, [isOpen, menuRef]);
}, [focusTimeoutDelay, isOpen, menuRef, shouldPreventScrollOnItemFocus]);

const renderItems = () =>
perPageOptions.map(({ value, title }) => (
Expand Down
23 changes: 19 additions & 4 deletions packages/react-core/src/components/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ export interface SelectProps extends MenuProps, OUIAProps {
maxMenuHeight?: string;
/** Indicates if the select menu should be scrollable */
isScrollable?: boolean;
/** Flag indicating if scroll on focus of the first menu item should occur. */
shouldPreventScrollOnItemFocus?: boolean;
/** Time in ms to wait before firing the toggles' focus event. Defaults to 0 */
focusTimeoutDelay?: number;
}

const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
Expand All @@ -99,6 +103,8 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
menuHeight,
maxMenuHeight,
isScrollable,
shouldPreventScrollOnItemFocus = true,
focusTimeoutDelay = 0,
...props
}: SelectProps & OUIAProps) => {
const localMenuRef = React.useRef<HTMLDivElement>();
Expand All @@ -121,7 +127,7 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
if (onOpenChangeKeys.includes(event.key)) {
event.preventDefault();
onOpenChange(false);
toggleRef.current?.focus();
toggleRef.current?.focus({ preventScroll: shouldPreventScrollOnItemFocus });
}
}
};
Expand All @@ -131,8 +137,8 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
if (isOpen && shouldFocusFirstItemOnOpen && toggleRef.current?.contains(event.target as Node)) {
setTimeout(() => {
const firstElement = menuRef?.current?.querySelector('li button:not(:disabled),li input:not(:disabled)');
firstElement && (firstElement as HTMLElement).focus();
}, 10);
firstElement && (firstElement as HTMLElement).focus({ preventScroll: shouldPreventScrollOnItemFocus });
}, focusTimeoutDelay);
}

// If the event is not on the toggle and onOpenChange callback is provided, close the menu
Expand All @@ -150,7 +156,16 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
window.removeEventListener('keydown', handleMenuKeys);
window.removeEventListener('click', handleClick);
};
}, [isOpen, menuRef, toggleRef, onOpenChange, onOpenChangeKeys]);
}, [
isOpen,
menuRef,
toggleRef,
onOpenChange,
onOpenChangeKeys,
shouldPreventScrollOnItemFocus,
shouldFocusFirstItemOnOpen,
focusTimeoutDelay
]);

const menu = (
<Menu
Expand Down
10 changes: 8 additions & 2 deletions packages/react-core/src/components/Tabs/OverflowTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ export interface OverflowTabProps extends React.HTMLProps<HTMLLIElement> {
toggleAriaLabel?: string;
/** z-index of the overflow tab */
zIndex?: number;
/** Flag indicating if scroll on focus of the first menu item should occur. */
shouldPreventScrollOnItemFocus?: boolean;
/** Time in ms to wait before firing the toggles' focus event. Defaults to 0 */
focusTimeoutDelay?: number;
}

export const OverflowTab: React.FunctionComponent<OverflowTabProps> = ({
Expand All @@ -30,6 +34,8 @@ export const OverflowTab: React.FunctionComponent<OverflowTabProps> = ({
defaultTitleText = 'More',
toggleAriaLabel,
zIndex = 9999,
shouldPreventScrollOnItemFocus = true,
focusTimeoutDelay = 0,
...props
}: OverflowTabProps) => {
const menuRef = React.useRef<HTMLDivElement>();
Expand Down Expand Up @@ -78,9 +84,9 @@ export const OverflowTab: React.FunctionComponent<OverflowTabProps> = ({
setTimeout(() => {
if (menuRef?.current) {
const firstElement = menuRef.current.querySelector('li > button,input:not(:disabled)');
firstElement && (firstElement as HTMLElement).focus();
firstElement && (firstElement as HTMLElement).focus({ preventScroll: shouldPreventScrollOnItemFocus });
}
}, 0);
}, focusTimeoutDelay);
};

const overflowTab = (
Expand Down