Fixes 11053: focus event causing jumpy scroll effect

patternfly · Oct 10, 2024 · 1b3ffd0 · 1b3ffd0
1 parent 6f9fc9a
commit 1b3ffd0
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 33 deletions.
diff --git 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<DropdownProps> = ({
@@ -92,6 +94,7 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
   menuHeight,
   maxMenuHeight,
   shouldFocusFirstItemOnOpen = true,
+  shouldPreventScrollOnItemFocus = true,
   ...props
 }: DropdownProps) => {
   const localMenuRef = React.useRef<HTMLDivElement>();
@@ -114,20 +117,18 @@ const DropdownBase: React.FunctionComponent<DropdownProps> = ({
       ) {
         if (onOpenChangeKeys.includes(event.key)) {
           onOpenChange(false);
-          toggleRef.current?.focus();
+          toggleRef.current?.focus({ preventScroll: shouldPreventScrollOnItemFocus });
         }
       }
     };
 
     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<DropdownProps> = ({
       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
@@ -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<MenuContainerProps> = ({
   onOpenChange,
   zIndex = 9999,
   popperProps,
-  onOpenChangeKeys = ['Escape', 'Tab']
+  onOpenChangeKeys = ['Escape', 'Tab'],
+  shouldPreventScrollOnItemFocus = true
 }: MenuContainerProps) => {
   React.useEffect(() => {
     const handleMenuKeys = (event: KeyboardEvent) => {
@@ -63,20 +66,18 @@ export const MenuContainer: React.FunctionComponent<MenuContainerProps> = ({
       ) {
         if (onOpenChangeKeys.includes(event.key)) {
           onOpenChange(false);
-          toggleRef.current?.focus();
+          toggleRef.current?.focus({ preventScroll: shouldPreventScrollOnItemFocus });
         }
       }
     };
 
     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
@@ -56,6 +56,8 @@ 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;
 }
 
 export const PaginationOptionsMenu: React.FunctionComponent<PaginationOptionsMenuProps> = ({
@@ -80,7 +82,8 @@ export const PaginationOptionsMenu: React.FunctionComponent<PaginationOptionsMen
   toggleTemplate,
   onPerPageSelect = () => null as any,
   containerRef,
-  appendTo
+  appendTo,
+  shouldPreventScrollOnItemFocus = true
 }: PaginationOptionsMenuProps) => {
   const [isOpen, setIsOpen] = React.useState(false);
   const toggleRef = React.useRef<HTMLButtonElement>(null);
@@ -123,18 +126,16 @@ export const PaginationOptionsMenu: React.FunctionComponent<PaginationOptionsMen
       ) {
         if (event.key === 'Escape' || event.key === 'Tab') {
           setIsOpen(false);
-          toggleRef.current?.focus();
+          toggleRef.current?.focus({ preventScroll: shouldPreventScrollOnItemFocus });
         }
       }
     };
 
     const handleClick = (event: MouseEvent) => {
       // 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
@@ -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<SelectProps & OUIAProps> = ({
@@ -99,6 +101,7 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
   menuHeight,
   maxMenuHeight,
   isScrollable,
+  shouldPreventScrollOnItemFocus = true,
   ...props
 }: SelectProps & OUIAProps) => {
   const localMenuRef = React.useRef<HTMLDivElement>();
@@ -121,18 +124,16 @@ const SelectBase: React.FunctionComponent<SelectProps & OUIAProps> = ({
         if (onOpenChangeKeys.includes(event.key)) {
           event.preventDefault();
           onOpenChange(false);
-          toggleRef.current?.focus();
+          toggleRef.current?.focus({ preventScroll: shouldPreventScrollOnItemFocus });
         }
       }
     };
 
     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
@@ -21,6 +21,8 @@ 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;
 }
 
 export const OverflowTab: React.FunctionComponent<OverflowTabProps> = ({
@@ -30,6 +32,7 @@ export const OverflowTab: React.FunctionComponent<OverflowTabProps> = ({
   defaultTitleText = 'More',
   toggleAriaLabel,
   zIndex = 9999,
+  shouldPreventScrollOnItemFocus = true,
   ...props
 }: OverflowTabProps) => {
   const menuRef = React.useRef<HTMLDivElement>();
@@ -75,12 +78,11 @@ export const OverflowTab: React.FunctionComponent<OverflowTabProps> = ({
 
   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 = (