From 1b3ffd02b523ac663ea42bf60166635e710af638 Mon Sep 17 00:00:00 2001 From: Andrew Dewar Date: Thu, 10 Oct 2024 15:20:10 +0100 Subject: [PATCH] Fixes 11053: focus event causing jumpy scroll effect --- .../src/components/Dropdown/Dropdown.tsx | 17 +++++++++-------- .../src/components/Menu/MenuContainer.tsx | 17 +++++++++-------- .../Pagination/PaginationOptionsMenu.tsx | 13 +++++++------ .../react-core/src/components/Select/Select.tsx | 11 ++++++----- .../src/components/Tabs/OverflowTab.tsx | 14 ++++++++------ 5 files changed, 39 insertions(+), 33 deletions(-) diff --git a/packages/react-core/src/components/Dropdown/Dropdown.tsx b/packages/react-core/src/components/Dropdown/Dropdown.tsx index 2aaa1c38b44..06ed9fb8e43 100644 --- a/packages/react-core/src/components/Dropdown/Dropdown.tsx +++ b/packages/react-core/src/components/Dropdown/Dropdown.tsx @@ -71,6 +71,8 @@ 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; } const DropdownBase: React.FunctionComponent = ({ @@ -92,6 +94,7 @@ const DropdownBase: React.FunctionComponent = ({ menuHeight, maxMenuHeight, shouldFocusFirstItemOnOpen = true, + shouldPreventScrollOnItemFocus = true, ...props }: DropdownProps) => { const localMenuRef = React.useRef(); @@ -114,7 +117,7 @@ const DropdownBase: React.FunctionComponent = ({ ) { if (onOpenChangeKeys.includes(event.key)) { onOpenChange(false); - toggleRef.current?.focus(); + toggleRef.current?.focus({ preventScroll: shouldPreventScrollOnItemFocus }); } } }; @@ -122,12 +125,10 @@ const DropdownBase: React.FunctionComponent = ({ const handleClick = (event: MouseEvent) => { // toggle was opened, focus on first menu item if (isOpen && shouldFocusFirstItemOnOpen && toggleRef.current?.contains(event.target as Node)) { - setTimeout(() => { - 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); + const firstElement = menuRef?.current?.querySelector( + 'li button:not(:disabled),li input:not(:disabled),li a:not([aria-disabled="true"])' + ); + firstElement && (firstElement as HTMLElement).focus({ preventScroll: shouldPreventScrollOnItemFocus }); } // If the event is not on the toggle and onOpenChange callback is provided, close the menu @@ -155,7 +156,7 @@ const DropdownBase: React.FunctionComponent = ({ ref={menuRef} onSelect={(event, value) => { onSelect && onSelect(event, value); - shouldFocusToggleOnSelect && toggleRef.current.focus(); + shouldFocusToggleOnSelect && toggleRef.current?.focus(); }} isPlain={isPlain} isScrollable={scrollable} diff --git a/packages/react-core/src/components/Menu/MenuContainer.tsx b/packages/react-core/src/components/Menu/MenuContainer.tsx index 6208fb02222..327b0b58013 100644 --- a/packages/react-core/src/components/Menu/MenuContainer.tsx +++ b/packages/react-core/src/components/Menu/MenuContainer.tsx @@ -37,6 +37,8 @@ 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; } /** @@ -52,7 +54,8 @@ export const MenuContainer: React.FunctionComponent = ({ onOpenChange, zIndex = 9999, popperProps, - onOpenChangeKeys = ['Escape', 'Tab'] + onOpenChangeKeys = ['Escape', 'Tab'], + shouldPreventScrollOnItemFocus = true }: MenuContainerProps) => { React.useEffect(() => { const handleMenuKeys = (event: KeyboardEvent) => { @@ -63,7 +66,7 @@ export const MenuContainer: React.FunctionComponent = ({ ) { if (onOpenChangeKeys.includes(event.key)) { onOpenChange(false); - toggleRef.current?.focus(); + toggleRef.current?.focus({ preventScroll: shouldPreventScrollOnItemFocus }); } } }; @@ -71,12 +74,10 @@ export const MenuContainer: React.FunctionComponent = ({ const handleClick = (event: MouseEvent) => { // toggle was opened, focus on first menu item if (isOpen && toggleRef.current?.contains(event.target as Node)) { - setTimeout(() => { - 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); + const firstElement = menuRef?.current?.querySelector( + 'li button:not(:disabled),li input:not(:disabled),li a:not([aria-disabled="true"])' + ); + firstElement && (firstElement as HTMLElement).focus({ preventScroll: shouldPreventScrollOnItemFocus }); } // If the event is not on the toggle and onOpenChange callback is provided, close the menu diff --git a/packages/react-core/src/components/Pagination/PaginationOptionsMenu.tsx b/packages/react-core/src/components/Pagination/PaginationOptionsMenu.tsx index cdc8967434e..6ef34905915 100644 --- a/packages/react-core/src/components/Pagination/PaginationOptionsMenu.tsx +++ b/packages/react-core/src/components/Pagination/PaginationOptionsMenu.tsx @@ -56,6 +56,8 @@ export interface PaginationOptionsMenuProps extends React.HTMLProps; /** @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; } export const PaginationOptionsMenu: React.FunctionComponent = ({ @@ -80,7 +82,8 @@ export const PaginationOptionsMenu: React.FunctionComponent null as any, containerRef, - appendTo + appendTo, + shouldPreventScrollOnItemFocus = true }: PaginationOptionsMenuProps) => { const [isOpen, setIsOpen] = React.useState(false); const toggleRef = React.useRef(null); @@ -123,7 +126,7 @@ export const PaginationOptionsMenu: React.FunctionComponent { // Focus the first non-disabled menu item on toggle 'click' 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); + const firstElement = menuRef?.current?.querySelector('li button:not(:disabled)'); + firstElement && (firstElement as HTMLElement).focus({ preventScroll: shouldPreventScrollOnItemFocus }); } // If the event is not on the toggle, close the menu diff --git a/packages/react-core/src/components/Select/Select.tsx b/packages/react-core/src/components/Select/Select.tsx index 21179505802..d05dcb4e4bf 100644 --- a/packages/react-core/src/components/Select/Select.tsx +++ b/packages/react-core/src/components/Select/Select.tsx @@ -78,6 +78,8 @@ 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; } const SelectBase: React.FunctionComponent = ({ @@ -99,6 +101,7 @@ const SelectBase: React.FunctionComponent = ({ menuHeight, maxMenuHeight, isScrollable, + shouldPreventScrollOnItemFocus = true, ...props }: SelectProps & OUIAProps) => { const localMenuRef = React.useRef(); @@ -121,7 +124,7 @@ const SelectBase: React.FunctionComponent = ({ if (onOpenChangeKeys.includes(event.key)) { event.preventDefault(); onOpenChange(false); - toggleRef.current?.focus(); + toggleRef.current?.focus({ preventScroll: shouldPreventScrollOnItemFocus }); } } }; @@ -129,10 +132,8 @@ const SelectBase: React.FunctionComponent = ({ const handleClick = (event: MouseEvent) => { // toggle was opened, focus on first menu item 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); + const firstElement = menuRef?.current?.querySelector('li button:not(:disabled),li input:not(:disabled)'); + firstElement && (firstElement as HTMLElement).focus({ preventScroll: shouldPreventScrollOnItemFocus }); } // If the event is not on the toggle and onOpenChange callback is provided, close the menu diff --git a/packages/react-core/src/components/Tabs/OverflowTab.tsx b/packages/react-core/src/components/Tabs/OverflowTab.tsx index 34d42a11e24..b3a1348fdc8 100644 --- a/packages/react-core/src/components/Tabs/OverflowTab.tsx +++ b/packages/react-core/src/components/Tabs/OverflowTab.tsx @@ -21,6 +21,8 @@ export interface OverflowTabProps extends React.HTMLProps { 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; } export const OverflowTab: React.FunctionComponent = ({ @@ -30,6 +32,7 @@ export const OverflowTab: React.FunctionComponent = ({ defaultTitleText = 'More', toggleAriaLabel, zIndex = 9999, + shouldPreventScrollOnItemFocus = true, ...props }: OverflowTabProps) => { const menuRef = React.useRef(); @@ -75,12 +78,11 @@ export const OverflowTab: React.FunctionComponent = ({ const toggleMenu = () => { setIsExpanded((prevIsExpanded) => !prevIsExpanded); - setTimeout(() => { - if (menuRef?.current) { - const firstElement = menuRef.current.querySelector('li > button,input:not(:disabled)'); - firstElement && (firstElement as HTMLElement).focus(); - } - }, 0); + + if (menuRef?.current) { + const firstElement = menuRef.current.querySelector('li > button,input:not(:disabled)'); + firstElement && (firstElement as HTMLElement).focus({ preventScroll: shouldPreventScrollOnItemFocus }); + } }; const overflowTab = (