Skip to content

Commit

Permalink
Fixes 11053: focus event causing jumpy scroll effect
Browse files Browse the repository at this point in the history
  • Loading branch information
Andrewgdewar committed Oct 10, 2024
1 parent 6f9fc9a commit 7289988
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 21 deletions.
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

0 comments on commit 7289988

Please sign in to comment.