From 1541c75a7cc7eaf4c903830111cfd5b6d36d4f68 Mon Sep 17 00:00:00 2001 From: zepumph Date: Tue, 23 Jun 2020 12:14:22 -0800 Subject: [PATCH] PDOM listeners to respect Node.pickable, https://github.com/phetsims/scenery/issues/1056 --- js/Checkbox.js | 3 ++- js/ComboBox.js | 1 + js/EnabledNode.js | 3 +++ js/EnabledNodeTests.js | 40 ++++++++++++++++++++++++++++++---------- 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/js/Checkbox.js b/js/Checkbox.js index f5887eaf..b4fbd1f3 100644 --- a/js/Checkbox.js +++ b/js/Checkbox.js @@ -8,6 +8,7 @@ import Action from '../../axon/js/Action.js'; import BooleanProperty from '../../axon/js/BooleanProperty.js'; +import validate from '../../axon/js/validate.js'; import InstanceRegistry from '../../phet-core/js/documentation/InstanceRegistry.js'; import inherit from '../../phet-core/js/inherit.js'; import merge from '../../phet-core/js/merge.js'; @@ -22,7 +23,6 @@ import Tandem from '../../tandem/js/Tandem.js'; import FontAwesomeNode from './FontAwesomeNode.js'; import sun from './sun.js'; import SunConstants from './SunConstants.js'; -import validate from '../../axon/js/validate.js'; // constants const ENABLED_PROPERTY_TANDEM_NAME = 'enabledProperty'; @@ -178,6 +178,7 @@ function Checkbox( content, property, options ) { phetioFeatured: true } ); + // TODO: aria-disabled is covered by EnabledNode, and can be removed once Checkbox uses that mixin. https://github.com/phetsims/sun/issues/585 const enabledListener = function( enabled ) { if ( enabled ) { self.setAccessibleAttribute( 'onclick', '' ); diff --git a/js/ComboBox.js b/js/ComboBox.js index 51b7345b..8a71f0bb 100644 --- a/js/ComboBox.js +++ b/js/ComboBox.js @@ -232,6 +232,7 @@ class ComboBox extends Node { const enabledObserver = enabled => { this.pickable = enabled; this.opacity = enabled ? 1.0 : options.disabledOpacity; + this.button.setAccessibleAttribute( 'aria-disabled', !enabled ); }; this.enabledProperty.link( enabledObserver ); diff --git a/js/EnabledNode.js b/js/EnabledNode.js index 911b63f2..0e8f2129 100644 --- a/js/EnabledNode.js +++ b/js/EnabledNode.js @@ -54,6 +54,9 @@ const EnabledNode = { this.pickable = enabled; this.opacity = enabled ? 1.0 : options.disabledOpacity; + // Mark this Node as disabled in the ParallelDOM + this.setAccessibleAttribute( 'aria-disabled', !enabled ); + // handle cursor by supporting setting back to what the cursor was when component was made disabled. this.cursor = enabled ? cursor : 'default'; }; diff --git a/js/EnabledNodeTests.js b/js/EnabledNodeTests.js index 985da454..8223dd50 100644 --- a/js/EnabledNodeTests.js +++ b/js/EnabledNodeTests.js @@ -7,23 +7,24 @@ */ import BooleanProperty from '../../axon/js/BooleanProperty.js'; -import Node from '../../scenery/js/nodes/Node.js'; import Property from '../../axon/js/Property.js'; +import Display from '../../scenery/js/display/Display.js'; +import Node from '../../scenery/js/nodes/Node.js'; import EnabledNode from './EnabledNode.js'; QUnit.module( 'EnabledNode' ); -QUnit.test( 'EnabledNode', assert => { - - class EnabledNodeClass extends Node { - constructor( options ) { - super( options ); - this.initializeEnabledNode( options ); - } +class EnabledNodeClass extends Node { + constructor( options ) { + super( options ); + this.initializeEnabledNode( options ); } +} + +// mix in enabled component into a Node +EnabledNode.mixInto( EnabledNodeClass ); - // mix in enabled component into a Node - EnabledNode.mixInto( EnabledNodeClass ); +QUnit.test( 'EnabledNode', assert => { let node = new EnabledNodeClass(); @@ -56,6 +57,25 @@ QUnit.test( 'EnabledNode', assert => { assert.ok( myEnabledProperty.changedEmitter.getListenerCount() === defaultListenerCount, 'listener count should match original' ); } ); +QUnit.test( 'EnabledNode with PDOM', assert => { + + const rootNode = new Node( { tagName: 'div' } ); + var display = new Display( rootNode ); // eslint-disable-line + document.body.appendChild( display.domElement ); + + const a11yNode = new EnabledNodeClass( { + tagName: 'p' + } ); + + rootNode.addChild( a11yNode ); + assert.ok( a11yNode.accessibleInstances.length === 1, 'should have an instance when attached to display' ); + assert.ok( a11yNode.accessibleInstances[ 0 ].peer, 'should have a peer' ); + assert.ok( a11yNode.accessibleInstances[ 0 ].peer.primarySibling.getAttribute( 'aria-disabled' ) === 'false', 'should be enabled' ); + a11yNode.enabled = false; + assert.ok( a11yNode.accessibleInstances[ 0 ].peer.primarySibling.getAttribute( 'aria-disabled' ) === 'true', 'should be enabled' ); + testEnabledNode( assert, a11yNode, 'For accessible Node' ); +} ); + /** * Test basic functionality for an object that mixes in EnabledComponent * @param {Object} assert - from QUnit