fix(4233): Fix useRestoreFocus logic (#4353)

Don't restore focus if focus never entered the scope. Fixes combobox reopening when using focus as a trigger

Author: boutahlilsoufiane

packages/@react-aria/focus/src/FocusScope.tsx

@@ -568,7 +568,9 @@ function useRestoreFocus(scopeRef: RefObject<Element[]>, restoreFocus: boolean,
     let onFocus = () => {
       // If focusing an element in a child scope of the currently active scope, the child becomes active.
       // Moving out of the active scope to an ancestor is not allowed.
-      if (!activeScope || isAncestorScope(activeScope, scopeRef)) {
+      if ((!activeScope || isAncestorScope(activeScope, scopeRef)) &&             
+      isElementInScope(document.activeElement, scopeRef.current)
+      ) {
         activeScope = scopeRef;
       }
     };

packages/@react-aria/focus/stories/FocusScope.stories.tsx

@@ -12,7 +12,7 @@
 
 import {FocusScope} from '../';
 import {Meta} from '@storybook/react';
-import React, {ReactNode, useState} from 'react';
+import React, {ReactNode, useEffect, useState} from 'react';
 import ReactDOM from 'react-dom';
 
 const dialogsRoot = 'dialogsRoot';
@@ -149,6 +149,41 @@ function FocusableFirstInScopeExample() {
   );
 }
 
+function IgnoreRestoreFocusExample() {
+  const [display, setDisplay] = useState(false);
+  useEffect(() => {
+    let handleKeyDown = (e) => {
+      if (e.key === 'Escape') {
+        setDisplay(false);
+      }
+    };
+    document.body.addEventListener('keyup', handleKeyDown);
+    return () => {
+      document.body.removeEventListener('keyup', handleKeyDown);
+    };
+  }, []);
+
+  return (
+    <div>
+      <button type="button" onClick={() => setDisplay(state => !state)}>
+        {display ? 'Close dialog 1' : 'Open dialog 1'}
+      </button>
+      <button type="button" onClick={() => setDisplay(state => !state)}>
+        {display ? 'Close dialog 2' : 'Open dialog 2'}
+      </button>
+      {display &&
+        <FocusScope restoreFocus>
+          <div role="dialog">
+            <input  />
+            <input  />
+            <input  />
+          </div>
+        </FocusScope>
+      }
+    </div>
+  );
+}
+
 export const KeyboardNavigation = {
   render: Template,
   args: {isPortaled: false}
@@ -169,6 +204,10 @@ export const KeyboardNavigationInsidePortalNoContain = {
   args: {isPortaled: true, contain: false}
 };
 
+export const IgnoreRestoreFocus = {
+  render: () => <IgnoreRestoreFocusExample />
+};
+
 export const FocusableFirstInScope = {
   render: () => <FocusableFirstInScopeExample />
 };

packages/@react-aria/focus/test/FocusScope.test.js

@@ -16,7 +16,7 @@ import {DialogContainer} from '@react-spectrum/dialog';
 import {FocusScope, useFocusManager} from '../';
 import {focusScopeTree} from '../src/FocusScope';
 import {Provider} from '@react-spectrum/provider';
-import React, {useState} from 'react';
+import React, {useEffect, useState} from 'react';
 import ReactDOM from 'react-dom';
 import {Example as StorybookExample} from '../stories/FocusScope.stories';
 import userEvent from '@testing-library/user-event';
@@ -693,6 +693,70 @@ describe('FocusScope', function () {
         await waitFor(() => expect(document.activeElement).toBe(focusable));
       });
     });
+
+    it('should not not restore focus when active element is outside the scope', function () {
+      function Test() {
+        const [display, setDisplay] = useState(false);
+        useEffect(() => {
+          let handleKeyDown = (e) => {
+            if (e.key === 'Escape') {
+              setDisplay(false);
+            }
+          };
+          document.body.addEventListener('keyup', handleKeyDown);
+          return () => {
+            document.body.removeEventListener('keyup', handleKeyDown);
+          };
+        }, []);
+
+        return (
+          <div>
+            <button
+              data-testid="button1"
+              type="button"
+              onClick={() => setDisplay((state) => !state)}>
+              {display ? 'Close dialog' : 'Open dialog'}
+            </button>
+            <button
+              data-testid="button2"
+              type="button"
+              onClick={() => setDisplay((state) => !state)}>
+              {display ? 'Close dialog' : 'Open dialog'}
+            </button>{' '}
+            {display && (
+              <FocusScope restoreFocus>
+                <input data-testid="input1" />
+              </FocusScope>
+            )}
+          </div>
+        );
+      }
+
+      let {getByTestId} = render(<Test />);
+      let button1 = getByTestId('button1');
+      let button2 = getByTestId('button2');
+      userEvent.click(button1);
+      act(() => {jest.runAllTimers();});
+      expect(document.activeElement).toBe(button1);
+      let input1 = getByTestId('input1');
+      expect(input1).toBeVisible();
+
+      userEvent.click(button2);
+      act(() => {jest.runAllTimers();});
+      expect(document.activeElement).toBe(button2);
+      expect(input1).not.toBeInTheDocument();
+
+      userEvent.click(button1);
+      act(() => {jest.runAllTimers();});
+      input1 = getByTestId('input1');
+      expect(input1).toBeVisible();
+      userEvent.tab();
+      fireEvent.keyDown(document.activeElement, {key: 'Escape'});
+      fireEvent.keyUp(document.activeElement, {key: 'Escape'});
+      act(() => {jest.runAllTimers();});
+      expect(document.activeElement).toBe(button2);
+      expect(input1).not.toBeInTheDocument();
+    });
   });
 
   describe('auto focus', function () {