Adding a bunch of comments explaining things so that i don't lose my place

Author: snowystinger

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

@@ -229,6 +229,20 @@ function useFocusContainment(scopeRef: RefObject<HTMLElement[]>, contain: boolea
     };
 
     let onFocus = (e) => {
+      // I think this onFocus is stealing the focus away from in DialogTrigger test,
+      // the dialog is closed, at which point focus should be restored to the trigger,
+      // it would appear that
+      /**
+       * dialog is blurred (this is the only blur called that makes it to document)
+       * dialog is focused (caught from local element listener)
+       * dialog is focused (global)
+       * button is focused (global)
+       * dialog is focused (local)
+       * dialog is focused (global)
+       *
+       * it's fired on so many things so fast that i don't think onFocus actually fires on the button?
+       */
+
       // If a focus event occurs outside the active scope (e.g. user tabs from browser location bar),
       // restore focus to the previously focused node or the first tabbable element in the active scope.
       let isInAnyScope = isElementInAnyScope(e.target, scopes);
@@ -245,7 +259,6 @@ function useFocusContainment(scopeRef: RefObject<HTMLElement[]>, contain: boolea
     };
 
     let onBlur = (e) => {
-
       let isInAnyScope = isElementInAnyScope(e.relatedTarget, scopes);
 
       if (!isInAnyScope) {
@@ -259,6 +272,9 @@ function useFocusContainment(scopeRef: RefObject<HTMLElement[]>, contain: boolea
     };
 
     document.addEventListener('keydown', onKeyDown, false);
+    /**
+     * removing the two global listeners fixes dialog trigger, but breaks FocusScope, Dialog containment, and Popover containment
+     */
     document.addEventListener('focusin', onFocus, false);
     scope.forEach(element => element.addEventListener('focusin', onFocus, false));
     scope.forEach(element => element.addEventListener('focusout', onBlur, false));

packages/@react-aria/focus/src/focusSafely.ts

@@ -18,7 +18,7 @@ import {getInteractionModality} from '@react-aria/interactions';
  * as page scrolling and screen reader issues with CSS transitions.
  */
 export function focusSafely(element: HTMLElement) {
-  // If the user is interating with a virtual cursor, e.g. screen reader, then
+  // If the user is interacting with a virtual cursor, e.g. screen reader, then
   // wait until after any animated transitions that are currently occurring on
   // the page before shifting focus. This avoids issues with VoiceOver on iOS
   // causing the page to scroll when moving focus if the element is transitioning

packages/@react-aria/interactions/src/useFocusVisible.ts

@@ -134,7 +134,12 @@ function setupGlobalFocusEvents() {
   // Register focus events on the window so they are sure to happen
   // before React's event listeners (registered on the document).
   window.addEventListener('focus', handleFocusEvent, true);
-  window.addEventListener('blur', handleWindowBlur, true);
+  window.addEventListener('blur', handleWindowBlur, true); // reverting this fixes the Picker test that's failing
+  /**
+   * it's happening because as a capture, it's getting a window blur when it shouldn't, when the spectrum-Menu is focused
+   * I assume because of the blur on the trigger?
+   * also, this is getting fired a LOT
+   */
 
   if (typeof PointerEvent !== 'undefined') {
     document.addEventListener('pointerdown', handlePointerEvent, true);

packages/@react-spectrum/dialog/test/DialogTrigger.test.js

@@ -225,12 +225,12 @@ describe('DialogTrigger', function () {
     expect(tray).toBeVisible();
   });
 
-  // this one fails
+  // see notes in focus scope, fails in .only mode as well
   it('should restore focus to the trigger when the dialog is closed', async function () {
     let {getByRole} = render(
       <Provider theme={theme}>
         <DialogTrigger>
-          <ActionButton onFocus={() => console.log('focused trigger')}>Trigger</ActionButton>
+          <ActionButton>Trigger</ActionButton>
           <Dialog>contents</Dialog>
         </DialogTrigger>
       </Provider>
@@ -264,11 +264,9 @@ describe('DialogTrigger', function () {
       expect(dialog).not.toBeInTheDocument();
     }); // wait for animation
 
-    console.log('just before fail');
     expect(document.activeElement).toBe(button);
   });
 
-  // this one fails
   it('should restore focus to the trigger when the dialog is closed from a hidden dismiss button', async function () {
     let {getByRole} = render(
       <Provider theme={theme}>
@@ -308,7 +306,6 @@ describe('DialogTrigger', function () {
     expect(document.activeElement).toBe(button);
   });
 
-  // fails
   it('should set aria-hidden on parent providers on mount and remove on unmount', async function () {
     let rootProviderRef = React.createRef();
     let {getByRole} = render(
@@ -349,7 +346,6 @@ describe('DialogTrigger', function () {
     expect(rootProviderRef.current.UNSAFE_getDOMNode()).not.toHaveAttribute('aria-hidden');
   });
 
-  // fails
   it('can be controlled', async function () {
     function Test({isOpen, onOpenChange}) {
       return (
@@ -412,7 +408,6 @@ describe('DialogTrigger', function () {
     }); // wait for animation
   });
 
-  // fails
   it('can be uncontrolled with defaultOpen', async function () {
     function Test({defaultOpen, onOpenChange}) {
       return (
@@ -451,7 +446,6 @@ describe('DialogTrigger', function () {
     }); // wait for animation
   });
 
-  // fails
   it('can be closed by buttons the user adds', async function () {
     function Test({defaultOpen, onOpenChange}) {
       return (
@@ -491,7 +485,6 @@ describe('DialogTrigger', function () {
     }); // wait for animation
   });
 
-  // fails
   it('can be closed by dismiss button in dialog', async function () {
     function Test({defaultOpen, onOpenChange}) {
       return (
@@ -531,7 +524,6 @@ describe('DialogTrigger', function () {
     }); // wait for animation
   });
 
-  // fails
   it('dismissable modals can be closed by clicking outside the dialog', async function () {
     function Test({defaultOpen, onOpenChange}) {
       return (
@@ -570,7 +562,6 @@ describe('DialogTrigger', function () {
     }); // wait for animation
   });
 
-  // fails
   it('non dismissable modals cannot be closed by clicking outside the dialog', async function () {
     function Test({defaultOpen, onOpenChange}) {
       return (
@@ -603,7 +594,6 @@ describe('DialogTrigger', function () {
     expect(onOpenChange).toHaveBeenCalledTimes(0);
   });
 
-  // fails
   it('mobile type modals should be closable by clicking outside the modal', async function () {
     matchMedia.useMediaQuery('(max-width: 700px)');
     function Test({defaultOpen, onOpenChange}) {
@@ -643,7 +633,6 @@ describe('DialogTrigger', function () {
     }); // wait for animation
   });
 
-  // fails
   it('non-modals can be closed by clicking outside the dialog regardless of isDismissable', async function () {
     function Test({defaultOpen, onOpenChange}) {
       return (
@@ -682,7 +671,6 @@ describe('DialogTrigger', function () {
     }); // wait for animation
   });
 
-  // fails
   it('disable closing dialog via escape key', async function () {
     let {getByRole} = render(
       <Provider theme={theme}>