prefer FocusManager.pdomFocusedNode to document.activeElement, #1284

phetsims · Sep 17, 2021 · 7a00a1c · 7a00a1c
1 parent 93e0285
commit 7a00a1c
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 17 deletions.
diff --git a/js/accessibility/FocusManager.js b/js/accessibility/FocusManager.js
@@ -139,18 +139,21 @@ class FocusManager {
    * @param {Focus|null} value
    */
   static set pdomFocus( value ) {
-    let previousFocus;
-    if ( FocusManager.pdomFocusProperty.value ) {
-      previousFocus = FocusManager.pdomFocusedNode;
-    }
+    if ( FocusManager.pdomFocusProperty.value !== value ) {
+
+      let previousFocus;
+      if ( FocusManager.pdomFocusProperty.value ) {
+        previousFocus = FocusManager.pdomFocusedNode;
+      }
 
-    FocusManager.pdomFocusProperty.value = value;
+      FocusManager.pdomFocusProperty.value = value;
 
-    // if set to null, make sure that the active element is no longer focused
-    if ( previousFocus && !value ) {
+      // if set to null, make sure that the active element is no longer focused
+      if ( previousFocus && !value ) {
 
-      // blur the document.activeElement instead of going through the Node, Node.blur() won't work in cases of DAG
-      document.activeElement.blur();
+        // TODO: Won't this be buggy if document.activeElement is not focused, because then PDOMPointer won't actually get the blur listener, https://github.com/phetsims/scenery/issues/1284
+        previousFocus.blur();
+      }
     }
   }
 

diff --git a/js/accessibility/pdom/PDOMInstance.js b/js/accessibility/pdom/PDOMInstance.js
@@ -315,9 +315,12 @@ class PDOMInstance {
     this.peer.setVisible( this.invisibleCount <= 0 );
 
     // if we hid a parent element, blur focus if active element was an ancestor
-    if ( !this.peer.isVisible() ) {
-      if ( this.peer.primarySibling.contains( document.activeElement ) ) { // still true if activeElement is this primary sibling
-        // NOTE: We don't seem to be able to import normally here
+    if ( !this.peer.isVisible() && scenery.FocusManager.pdomFocusedNode ) {
+      assert && assert( scenery.FocusManager.pdomFocusedNode.pdomInstances.length === 1,
+        'focusable Nodes do not support DAG, and should be connected with an instance if focused.' );
+
+      // NOTE: We don't seem to be able to import normally here
+      if ( scenery.FocusManager.pdomFocusedNode.pdomInstances[ 0 ].trail.containsNode( this.node ) ) {
         scenery.FocusManager.pdomFocus = null;
       }
     }

diff --git a/js/accessibility/pdom/PDOMPeer.js b/js/accessibility/pdom/PDOMPeer.js
@@ -793,10 +793,8 @@ class PDOMPeer {
   blur() {
     assert && assert( this._primarySibling, 'must have a primary sibling to blur' );
 
-    // no op if primary sibling does not have focus
-    if ( document.activeElement === this._primarySibling ) {
-      this._primarySibling.blur();
-    }
+    // no op by the browser if primary sibling does not have focus
+    this._primarySibling.blur();
   }
 
   /**

diff --git a/js/accessibility/pdom/PDOMUtils.js b/js/accessibility/pdom/PDOMUtils.js
@@ -168,6 +168,8 @@ function getNextPreviousFocusable( direction, parentElement ) {
   const parent = parentElement || document.body;
   const linearDOM = getLinearDOMElements( parent );
 
+  // TODO: PhET-iO concerns around using activeElement, for playback, see https://github.com/phetsims/scenery/issues/1284
+  // TODO: why is FocusManager.pdomFocusedNode null here? Perhaps just make activeElement document.body if that is the case, https://github.com/phetsims/scenery/issues/1284
   const activeElement = document.activeElement;
   const activeIndex = linearDOM.indexOf( activeElement );
   const delta = direction === NEXT ? +1 : -1;

diff --git a/js/input/Input.js b/js/input/Input.js
@@ -168,6 +168,7 @@ import EventType from '../../../tandem/js/EventType.js';
 import Tandem from '../../../tandem/js/Tandem.js';
 import NullableIO from '../../../tandem/js/types/NullableIO.js';
 import NumberIO from '../../../tandem/js/types/NumberIO.js';
+import FocusManger from '../accessibility/FocusManager.js';
 import PDOMUtils from '../accessibility/pdom/PDOMUtils.js';
 import Display from '../display/Display.js';
 import scenery from '../scenery.js';
@@ -653,7 +654,7 @@ class Input {
         // Focus is set with DOM API to avoid the performance hit of looking up the Node from trail id.
         if ( relatedTarget ) {
 
-          const focusMovedInCallbacks = this.isTargetUnderPDOM( document.activeElement );
+          const focusMovedInCallbacks = FocusManger.pdomFocusedNode && FocusManger.pdomFocusedNode.getConnectedDisplays().includes( this.display );
           const targetFocusable = PDOMUtils.isElementFocusable( relatedTarget );
           if ( targetFocusable && !focusMovedInCallbacks ) {
             relatedTarget.focus();

diff --git a/js/listeners/SwipeListener.js b/js/listeners/SwipeListener.js
@@ -164,6 +164,8 @@ class SwipeListener {
 
               // send a click event to the active element
               const pdomRoot = document.getElementsByClassName( 'a11y-pdom-root' )[ 0 ];
+
+              // TODO: document.activeElement is not set up to work well with PhET-iO, use FocusManger.pdomFocusedNode instead.
               if ( pdomRoot && pdomRoot.contains( document.activeElement ) ) {
                 document.activeElement.click();
               }