clean up enabledProperty in Node, factor out PDOM enabled listener an…

…d fix tests, #1100
phetsims · Oct 30, 2020 · 178ca7c · 178ca7c
1 parent 0513e10
commit 178ca7c
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 5 deletions.
diff --git a/js/accessibility/pdom/ParallelDOM.js b/js/accessibility/pdom/ParallelDOM.js
@@ -451,6 +451,10 @@ const ParallelDOM = {
 
         // {A11yBehaviorFunctionDef} - sets the help text of the Node, this most often corresponds to description text.
         this._accessibleHeadingBehavior = DEFAULT_ACCESSIBLE_HEADING_BEHAVIOR;
+
+        // @private - PDOM specific enabled listener
+        this.pdomBoundEnabledListener = this.pdomEnabledListener.bind( this );
+        this.enabledProperty.lazyLink( this.pdomBoundEnabledListener );
       },
 
 
@@ -464,6 +468,9 @@ const ParallelDOM = {
        * @public (scenery-internal)
        */
       disposeAccessibility: function() {
+
+        this.enabledProperty.unlink( this.pdomBoundEnabledListener );
+
         // To prevent memory leaks, we want to clear our order (since otherwise nodes in our order will reference
         // this node).
         this.accessibleOrder = null;
@@ -477,6 +484,16 @@ const ParallelDOM = {
         this.setActiveDescendantAssociations( [] );
       },
 
+      /**
+       * @private
+       * @param {boolean} enabled
+       */
+      pdomEnabledListener: function (enabled) {
+
+        // Mark this Node as disabled in the ParallelDOM
+        this.setAccessibleAttribute( 'aria-disabled', !enabled );
+      },
+
       /**
        * Get whether this Node's primary DOM element currently has focus.
        * @public

diff --git a/js/accessibility/pdom/ParallelDOMTests.js b/js/accessibility/pdom/ParallelDOMTests.js
@@ -895,8 +895,10 @@ QUnit.test( 'ParallelDOM setters/getters', function( assert ) {
 
   // set/get attributes
   let a1Element = getPrimarySiblingElementByNode( a1 );
+  const initialLength = a1.getAccessibleAttributes().length;
   a1.setAccessibleAttribute( 'role', 'switch' );
-  assert.ok( a1.getAccessibleAttributes()[ 0 ].attribute === 'role', 'attribute set' );
+  assert.ok( a1.getAccessibleAttributes().length === initialLength + 1, 'attribute set should only add 1' );
+  assert.ok( a1.getAccessibleAttributes()[ a1.getAccessibleAttributes().length - 1 ].attribute === 'role', 'attribute set' );
   assert.ok( a1Element.getAttribute( 'role' ) === 'switch', 'HTML attribute set' );
   assert.ok( a1.hasAccessibleAttribute( 'role' ), 'should have accessible attribute' );
 

diff --git a/js/nodes/Node.js b/js/nodes/Node.js
@@ -3514,7 +3514,7 @@ inherit( PhetioObject, Node, {
   onEnabledPropertyChange: function( enabled ) {
     this.interruptSubtreeInput();
     this.pickable = enabled;
-    this.opacity = enabled ? 1.0 : this.disabledOpacity;
+    this.opacity = enabled ? 1.0 : this._disabledOpacity;
   },
 
   /**
@@ -3524,10 +3524,10 @@ inherit( PhetioObject, Node, {
    * @param {number} disabledOpacity
    */
   setDisabledOpacity: function( disabledOpacity ) {
-    assert && assert( typeof disabledOpacity === 'number' && Number.isFinite( disabledOpacity ) );
+    assert && assert( typeof disabledOpacity === 'number' && Number.isFinite( disabledOpacity ), 'disabledOpacity should be a finite number' );
+    assert && assert( disabledOpacity <= 1 && disabledOpacity >= 0, 'disabledOpacity should be between 0 and 1' );
 
-    // This is clamped just like opacity, instead of asserted like it was in EnabledNode.
-    this._disabledOpacity = clamp( disabledOpacity, 0, 1 );
+    this._disabledOpacity = disabledOpacity;
   },
   set disabledOpacity( value ) { this.setDisabledOpacity( value ); },