fix(select): Fix undesired scrolling when activating menu (#795)

* fix(select): Adjust menu focus to prevent undesirable page scrolling * test(select): Add test to ensure menu activation doesn't scroll the page
Workday · Jul 23, 2020 · 9f9a4f6 · 9f9a4f6
1 parent 7acb125
commit 9f9a4f6
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 11 deletions.
diff --git a/cypress/integration/SelectLabs.spec.ts b/cypress/integration/SelectLabs.spec.ts
@@ -680,4 +680,31 @@ describe('Select', () => {
       }
     );
   });
+
+  context(`given the "Portal Test" story is rendered`, () => {
+    beforeEach(() => {
+      h.stories.load('Testing|React/Labs/Select', 'Portal Test');
+    });
+
+    context(
+      'when the page is scrolled to the bottom and the bottommost select button is clicked',
+      () => {
+        beforeEach(() => {
+          cy.scrollTo('bottom');
+          cy.window()
+            .its('scrollY')
+            .as('originalWindowScrollY');
+          cy.findByLabelText('Label (Bottom)').click();
+        });
+
+        context('the page', () => {
+          it('should not scroll', () => {
+            cy.window().then($window => {
+              cy.get('@originalWindowScrollY').should('equal', $window.scrollY);
+            });
+          });
+        });
+      }
+    );
+  });
 });
diff --git a/modules/_labs/select/react/lib/SelectMenu.tsx b/modules/_labs/select/react/lib/SelectMenu.tsx
@@ -188,8 +188,10 @@ const MenuList = styled('ul')<Pick<SelectProps, 'error' | 'theme'>>(
   })
 );
 
-const generatePopperOptions = (props: Pick<SelectMenuProps, 'isFlipped' | 'shouldAutoFlip'>) => {
-  const {isFlipped, shouldAutoFlip} = props;
+const generatePopperOptions = (
+  props: Pick<SelectMenuProps, 'isFlipped' | 'menuRef' | 'shouldAutoFlip' | 'shouldAutoFocus'>
+) => {
+  const {isFlipped, menuRef, shouldAutoFlip, shouldAutoFocus} = props;
 
   let fallbackPlacements: Placement[] = [];
   if (shouldAutoFlip) {
@@ -230,6 +232,14 @@ const generatePopperOptions = (props: Pick<SelectMenuProps, 'isFlipped' | 'shoul
 
   return {
     modifiers,
+    // TODO: Consider using a more general-purpose focus function here rather
+    // than relying on Popper's onFirstUpdate for better control over how
+    // focus is managed
+    onFirstUpdate: () => {
+      if (shouldAutoFocus && menuRef && menuRef.current) {
+        menuRef.current.focus();
+      }
+    },
   };
 };
 
@@ -260,14 +270,6 @@ const SelectMenu = (props: SelectMenuProps) => {
     }
   }, [buttonRef, isHidden]);
 
-  useLayoutEffect(() => {
-    if (shouldAutoFocus) {
-      menuRef?.current?.focus();
-    }
-    // Only focus on mount, so omit dependencies
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
-
   useLayoutEffect(() => {
     handleWidthChange();
   }, [handleWidthChange]);
@@ -311,7 +313,9 @@ const SelectMenu = (props: SelectMenuProps) => {
       anchorElement={buttonRef}
       popperOptions={generatePopperOptions({
         isFlipped,
+        menuRef,
         shouldAutoFlip,
+        shouldAutoFocus,
       })}
       ref={popupRef}
     >

diff --git a/modules/_labs/select/react/stories/stories_testing.tsx b/modules/_labs/select/react/stories/stories_testing.tsx
@@ -117,7 +117,7 @@ export const PortalTest = () => (
     </Container>
     <Container>
       <p>This Select is forced to display its menu upwards since it's at the bottom the page.</p>
-      <FormField label="Label" inputId="select-last" error={FormField.ErrorType.Error}>
+      <FormField label="Label (Bottom)" inputId="select-bottom" error={FormField.ErrorType.Error}>
         {controlComponent(<Select name="contact" options={options} />)}
       </FormField>
     </Container>