[FocusScope] Fix bug where focus is moved to container when it shouldn't be

.yarn/versions/e5d8b2b5.yml

@@ -0,0 +1,13 @@
+releases:
+  "@radix-ui/react-alert-dialog": minor
+  "@radix-ui/react-context-menu": minor
+  "@radix-ui/react-dialog": minor
+  "@radix-ui/react-dropdown-menu": minor
+  "@radix-ui/react-focus-scope": minor
+  "@radix-ui/react-menu": minor
+  "@radix-ui/react-menubar": minor
+  "@radix-ui/react-popover": minor
+  "@radix-ui/react-select": minor
+
+declined:
+  - primitives

cypress/integration/Dialog.spec.ts

@@ -121,6 +121,14 @@ describe('Dialog', () => {
       cy.realPress('Tab');
       cy.findByText('close').should('be.focused');
     });
+
+    it('focuses next element correctly when another element is removed on blur', () => {
+      cy.findByLabelText('enable blur validation').click();
+      cy.findByText('open').click();
+      cy.findByLabelText('My text field').type('hello');
+      cy.realPress('Tab');
+      cy.findByLabelText('My second text field').should('be.focused');
+    });
   });
 
   describe('given a non-modal dialog', () => {

packages/react/dialog/src/Dialog.stories.tsx

@@ -457,8 +457,10 @@ Chromatic.parameters = { chromatic: { disable: false } };
 export const Cypress = () => {
   const [modal, setModal] = React.useState(true);
   const [animated, setAnimated] = React.useState(false);
+  const [blurValidationEnabled, setBlurValidationEnabled] = React.useState(false);
   const [count, setCount] = React.useState(0);
   const [hasDestroyButton, setHasDestroyButton] = React.useState(true);
+  const [invalid, setInvalid] = React.useState(true);
 
   return (
     <>
@@ -474,6 +476,27 @@ export const Cypress = () => {
           >
             <Dialog.Title>title</Dialog.Title>
             <Dialog.Description>description</Dialog.Description>
+            {blurValidationEnabled && (
+              <div>
+                <label style={{ display: 'block' }}>
+                  <span>My text field</span>
+                  <input
+                    type="text"
+                    onBlur={(e) => setInvalid(e.target.value === '')}
+                    aria-describedby={invalid ? 'error-text' : undefined}
+                  />
+                </label>
+                <label style={{ display: 'block' }}>
+                  <span>My second text field</span>
+                  <input type="text" />
+                </label>
+                {invalid && (
+                  <div id="error-text" style={{ color: 'red', fontSize: 12 }}>
+                    This field is required
+                  </div>
+                )}
+              </div>
+            )}
             <Dialog.Close className={closeClass()}>close</Dialog.Close>
             {hasDestroyButton && (
               <div>
@@ -511,6 +534,17 @@ export const Cypress = () => {
 
       <br />
 
+      <label>
+        <input
+          type="checkbox"
+          checked={blurValidationEnabled}
+          onChange={(event) => setBlurValidationEnabled(Boolean(event.target.checked))}
+        />{' '}
+        enable blur validation
+      </label>
+
+      <br />
+
       <label>
         count up{' '}
         <button type="button" onClick={() => setCount((count) => count + 1)}>

packages/react/focus-scope/src/FocusScope.tsx

@@ -109,11 +109,26 @@ const FocusScope = React.forwardRef<FocusScopeElement, FocusScopeProps>((props,
       // back to the document.body. In this case, we move focus to the container
       // to keep focus trapped correctly.
       function handleMutations(mutations: MutationRecord[]) {
-        const focusedElement = document.activeElement as HTMLElement | null;
-        if (focusedElement !== document.body) return;
-        for (const mutation of mutations) {
-          if (mutation.removedNodes.length > 0) focus(container);
-        }
+        // If the activeElement is not the document body, then the focus was not
+        // lost, so we don't need to rescue the focus.  Please note that this is
+        // not perfectly robust, because the active element can still be
+        // document.body even in some cases where the focus was not lost.  For
+        // example, during a blur event, browsers set the activeElement to
+        // document.body just before activeElement may be set to the next
+        // element
+        if (document.activeElement !== document.body) return;
+
+        // If no elements were removed, then we don't need to rescue focus
+        if (noElementsRemoved(mutations)) return;
+
+        // If the last focused element is still in the container, then it wasn't
+        // removed and we don't need to rescue focus
+        const lastFocusedElementStillInContainer = container?.contains(
+          lastFocusedElementRef.current
+        );
+        if (lastFocusedElementStillInContainer) return;
+
+        focus(container);
       }
 
       document.addEventListener('focusin', handleFocusIn);
@@ -342,6 +357,15 @@ function removeLinks(items: HTMLElement[]) {
   return items.filter((item) => item.tagName !== 'A');
 }
 
+function noElementsRemoved(mutations: MutationRecord[]) {
+  for (const mutation of mutations) {
+    if (mutation.removedNodes.length > 0) {
+      return false;
+    }
+  }
+  return true;
+}
+
 const Root = FocusScope;
 
 export {