Do not close overlays on scroll (#3742)

Author: devongovett

packages/@react-aria/overlays/src/useCloseOnScroll.ts

@@ -30,7 +30,7 @@ export function useCloseOnScroll(opts: CloseOnScrollOptions) {
   let {triggerRef, isOpen, onClose} = opts;
 
   useEffect(() => {
-    if (!isOpen) {
+    if (!isOpen || onClose === null) {
       return;
     }
 

packages/@react-aria/overlays/src/useOverlayPosition.ts

@@ -179,7 +179,7 @@ export function useOverlayPosition(props: AriaPositionProps): PositionAria {
   useCloseOnScroll({
     triggerRef: targetRef,
     isOpen,
-    onClose: onClose ? close : undefined
+    onClose: onClose && close
   });
 
   return {

packages/@react-aria/overlays/src/usePopover.ts

@@ -88,7 +88,8 @@ export function usePopover(props: AriaPopoverProps, state: OverlayTriggerState):
     ...otherProps,
     targetRef: triggerRef,
     overlayRef: popoverRef,
-    isOpen: state.isOpen
+    isOpen: state.isOpen,
+    onClose: null
   });
 
   // Delay preventing scroll until popover is positioned to avoid extra scroll padding.

packages/@react-spectrum/menu/test/MenuTrigger.test.js

@@ -370,26 +370,6 @@ describe('MenuTrigger', function () {
       act(() => {jest.runAllTimers();}); // FocusScope raf
     }
 
-    it.each`
-      Name             | Component      | props
-      ${'MenuTrigger'} | ${MenuTrigger} | ${{onOpenChange}}
-    `('$Name closes the menu upon trigger body scroll', function ({Component, props}) {
-      tree = renderComponent(Component, props);
-      let button = tree.getByRole('button');
-      triggerPress(button);
-      act(() => {jest.runAllTimers();});
-
-      let menu = tree.getByRole('menu');
-      expect(menu).toBeTruthy();
-
-      let scrollable = tree.getByTestId('scrollable');
-      fireEvent.scroll(scrollable);
-      act(() => {jest.runAllTimers();}); // FocusScope useLayoutEffect cleanup
-      act(() => {jest.runAllTimers();}); // FocusScope raf
-      expect(menu).not.toBeInTheDocument();
-      expect(document.activeElement).toBe(button);
-    });
-
     // Can't figure out why this isn't working for the v2 component
     it.each`
       Name             | Component      | props

packages/@react-spectrum/picker/test/Picker.test.js

@@ -586,48 +586,6 @@ describe('Picker', function () {
       expect(document.activeElement).toBe(picker);
     });
 
-    it('closes on scroll on a parent element', function () {
-      let onOpenChange = jest.fn();
-      let {getByRole, getByTestId, queryByRole} = render(
-        <Provider theme={theme}>
-          <div data-testid="scrollable">
-            <Picker label="Test" onOpenChange={onOpenChange}>
-              <Item>One</Item>
-              <Item>Two</Item>
-              <Item>Three</Item>
-            </Picker>
-          </div>
-        </Provider>
-      );
-
-      expect(queryByRole('listbox')).toBeNull();
-
-      let picker = getByRole('button');
-      triggerPress(picker);
-      act(() => jest.runAllTimers());
-
-      let listbox = getByRole('listbox');
-      expect(listbox).toBeVisible();
-      expect(onOpenChange).toBeCalledTimes(1);
-      expect(onOpenChange).toHaveBeenCalledWith(true);
-      expect(picker).toHaveAttribute('aria-expanded', 'true');
-      expect(picker).toHaveAttribute('aria-controls', listbox.id);
-
-      let scrollable = getByTestId('scrollable');
-      fireEvent.scroll(scrollable);
-      act(() => jest.runAllTimers());
-
-      expect(listbox).not.toBeInTheDocument();
-      expect(picker).toHaveAttribute('aria-expanded', 'false');
-      expect(picker).not.toHaveAttribute('aria-controls');
-      expect(onOpenChange).toBeCalledTimes(2);
-      expect(onOpenChange).toHaveBeenCalledWith(false);
-
-      // run restore focus rAF
-      act(() => jest.runAllTimers());
-      expect(document.activeElement).toBe(picker);
-    });
-
     it('tabs to the next element after the trigger and closes the menu', function () {
       let onOpenChange = jest.fn();
       let {getByRole, getByTestId} = render(