From afbe969faf7bd6aa6087d928dcd6017c2ab706d9 Mon Sep 17 00:00:00 2001 From: zepumph Date: Sat, 14 Jul 2018 12:20:56 -0800 Subject: [PATCH] remove accessibleContentDisplayed, https://github.com/phetsims/scenery/issues/814 --- doc/accessibility.html | 1 - js/accessibility/Accessibility.js | 65 +++------------------- js/accessibility/AccessibilityTests.js | 77 -------------------------- js/accessibility/AccessibilityTree.js | 2 +- js/accessibility/AccessiblePeer.js | 4 -- js/accessibility/FocusTests.js | 1 - 6 files changed, 8 insertions(+), 142 deletions(-) diff --git a/doc/accessibility.html b/doc/accessibility.html index cc6dae88f..7ef4701e3 100644 --- a/doc/accessibility.html +++ b/doc/accessibility.html @@ -175,7 +175,6 @@

Accessible Content in Scenery

addAccessibleInputListener()
focusable
accessibleVisible
-
accessibleContentDisplayed
focus()
blur()
diff --git a/js/accessibility/Accessibility.js b/js/accessibility/Accessibility.js index afbcb23ec..0ec5aa8c5 100644 --- a/js/accessibility/Accessibility.js +++ b/js/accessibility/Accessibility.js @@ -180,7 +180,6 @@ define( function( require ) { 'focusHighlightLayerable', // Flag to determine if the focus highlight node can be layered in the scene graph, see setFocusHighlightLayerable() 'groupFocusHighlight', // Sets the outer focus highlight for this node when a descendant has focus, see setGroupFocusHighlight() 'accessibleVisible', // Sets whether or not the node's DOM element is visible in the parallel DOM, see setAccessibleVisible() - 'accessibleContentDisplayed', // Sets whether or not the accessible content of the node (and its subtree) is displayed, see setAccessibleContentDisplayed() 'focusable', // Sets whether or not the node can receive keyboard focus, see setFocusable() 'accessibleOrder', // Modifies the order of accessible navigation, see setAccessibleOrder() 'accessibleContent', // Sets up accessibility handling (probably don't need to use this), see setAccessibleContent() @@ -352,12 +351,6 @@ define( function( require ) { // http://www.ssbbartgroup.com/blog/how-windows-screen-readers-work-on-the-web/ this._accessibleVisible = true; - // @private {boolean} - Whether or not the accessible content will be visible from the browser and assistive - // technologies. When accessible content is not displayed, the node will not be focusable, and it cannot - // be found by assistive technology with the virtual cursor. Content should almost always be set invisible with - // setAccessibleVisible(), see that function and setAccessibleContentDisplayed() for more information. - this._accessibleContentDisplayed = true; - // @private {Array.} - For accessibility input handling {keyboard/click/HTML form} this._accessibleInputListeners = []; @@ -727,12 +720,12 @@ define( function( require ) { // TODO: this could be setting a11y content twice this.onAccessibleContentChange(); - if ( this._accessibleInstances.length > 0 ) { - for ( var i = 0; i < this._accessibleInstances.length; i++ ) { - var peer = this._accessibleInstances[ i ].peer; - peer.onTagNameChange(); - } - } + // if ( this._accessibleInstances.length > 0 ) { + // for ( var i = 0; i < this._accessibleInstances.length; i++ ) { + // var peer = this._accessibleInstances[ i ].peer; + // peer.onTagNameChange(); + // } + // } } }, @@ -1888,50 +1881,6 @@ define( function( require ) { }, get accessibleVisible() { return this.getAccessibleVisible(); }, - /** - * Sets whether or not the accessible content should be displayed in the DOM. Almost always, setAccessibleVisible - * should be used instead of this function. This should behave exactly like setAccessibleVisible. If removed - * from display, content will be removed from focus order and undiscoverable with the virtual cursor. Sometimes, - * hidden attribute is not handled the same way across screen readers, so this function can be used to - * completely remove the content from the DOM. - * @public - * - * @param {boolean} contentDisplayed - */ - setAccessibleContentDisplayed: function( contentDisplayed ) { - this._accessibleContentDisplayed = contentDisplayed; - - //TODO: we shouldn't need to set our children too, would it be better to have getters keep track of parents' values too??? - // for ( var j = 0; j < this._children.length; j++ ) { - // var child = this._children[ j ]; - // child.setAccessibleContentDisplayed( contentDisplayed ); - // } - - this.onAccessibleContentChange(); - // for ( var i = 0; i < this._accessibleInstances.length; i++ ) { - // var peer = this._accessibleInstances[ i ].peer; - // peer.onAccessibleContentDisplayedChange(); - // } - }, - set accessibleContentDisplayed( contentDisplayed ) { this.setAccessibleContentDisplayed( contentDisplayed ); }, - - /** - * Get whether or not this Node is displayed in the PDOM, NOT just its this._accessibleContentDisplayed boolean value - * @returns {boolean|*} - */ - getAccessibleContentDisplayed: function() { - // {boolean} if any parents are flagged as removed from the accessibility tree, set content to null - var contentDisplayed = this._accessibleContentDisplayed; - for ( var i = 0; i < this._parents.length; i++ ) { - if ( !this._parents[ i ].accessibleContentDisplayed ) { - contentDisplayed = false; - } - } - - return contentDisplayed; - }, - get accessibleContentDisplayed() { return this.getAccessibleContentDisplayed(); }, - /** * Set the value of an input element. Element must be a form element to support the value attribute. The input * value is converted to string since input values are generally string for HTML. @@ -2265,7 +2214,7 @@ define( function( require ) { */ getAccessibleContent: function() { - return this._accessibleContentDisplayed && !!this._tagName; + return !!this._tagName; }, get accessibleContent() { return this.getAccessibleContent(); }, diff --git a/js/accessibility/AccessibilityTests.js b/js/accessibility/AccessibilityTests.js index 47f1d9858..0fc67c398 100644 --- a/js/accessibility/AccessibilityTests.js +++ b/js/accessibility/AccessibilityTests.js @@ -862,72 +862,6 @@ define( function( require ) { assert.ok( getPrimarySiblingElementByNode( b ).tabIndex >= 0, 'set tagName after focusable' ); } ); - QUnit.test( 'accessibleContentDisplayed', function( assert ) { - - var rootNode = new Node(); - var display = new Display( rootNode ); // eslint-disable-line - document.body.appendChild( display.domElement ); - - var e = new Node( { tagName: 'div', innerContent: TEST_LABEL } ); - var f = new Node( { tagName: 'div', innerContent: TEST_LABEL } ); - var g = new Node( { tagName: 'div', innerContent: TEST_LABEL } ); - var h = new Node( { tagName: 'div', innerContent: TEST_LABEL } ); - e.addChild( f ); - f.addChild( g ); - g.addChild( h ); - rootNode.addChild( e ); - - - var existenceCorrect = function( node, shouldExist ) { - if ( shouldExist ) { - return !!node.accessibleInstances[ 0 ]; - } - return node.accessibleInstances.length === 0; - }; - - var assertAccessibleInstanceExistence = function( shouldExist ) { - var message = ' should' + ( shouldExist ? '' : ' not' ) + ' exist'; - assert.ok( existenceCorrect( e, shouldExist ), 'e' + message ); - assert.ok( existenceCorrect( f, shouldExist ), 'f' + message ); - assert.ok( existenceCorrect( g, shouldExist ), 'g' + message ); - assert.ok( existenceCorrect( h, shouldExist ), 'h' + message ); - }; - assertAccessibleInstanceExistence( true ); - - e.accessibleContentDisplayed = false; - assertAccessibleInstanceExistence( false ); - - e.accessibleContentDisplayed = true; - assertAccessibleInstanceExistence( true ); - - g.accessibleContentDisplayed = false; - assert.ok( existenceCorrect( e, true ), 'e should exist' ); - assert.ok( existenceCorrect( f, true ), 'f should exist' ); - assert.ok( existenceCorrect( g, false ), 'g should not exist' ); - assert.ok( existenceCorrect( h, false ), 'h should not exist' ); - - e.accessibleContentDisplayed = true; - assertAccessibleInstanceExistence( true ); - - - var i = new Node(); - f.removeChild( g ); - i.addChild( g ); - f.addChild( i ); - - e.accessibleContentDisplayed = false; - assertAccessibleInstanceExistence( false ); - - // TODO: Do we want to support this? setting accessibleContentDisplayed on a node without accessibleContent, - // TODO: but with children that have accessibleContent? - // i.accessibleContentDisplayed = true; - // assert.ok( existenceCorrect( e, false ), 'e should not exist' ); - // assert.ok( existenceCorrect( f, false ), 'f should not exist' ); - // assert.ok( existenceCorrect( g, true ), 'g should exist' ); - // assert.ok( existenceCorrect( h, true ), 'h should exist' ); - - } ); - QUnit.test( 'Accessibility input listeners', function( assert ) { // create a node @@ -1061,17 +995,6 @@ define( function( require ) { // verify the dom assert.ok( rootDOMElement.children.length === 5, 'children added' ); - rootNode.accessibleContentDisplayed = false; - assert.ok( rootDOMElement.children.length === 0, 'sub tree removed from DOM' ); - assert.ok( dDOMElement.children.length === 0, 'sub tree removed from DOM' ); - - // invalidation should not add content back to the DOM - rootNode.tagName = 'button'; - d.tagName = 'span'; - assert.ok( rootDOMElement.children.length === 0, 'invalidate without addition' ); - - rootNode.accessibleContentDisplayed = true; - // redefine because the dom element references above have become stale rootDOMElement = getPrimarySiblingElementByNode( rootNode ); dDOMElement = getPrimarySiblingElementByNode( d ); diff --git a/js/accessibility/AccessibilityTree.js b/js/accessibility/AccessibilityTree.js index da8558686..8b0d78448 100644 --- a/js/accessibility/AccessibilityTree.js +++ b/js/accessibility/AccessibilityTree.js @@ -359,7 +359,7 @@ define( function( require ) { // If we are accessible ourself, we need to create the instance (so we can provide it to child instances). var instance; var existed = false; - if ( node.accessibleContent && node.accessibleContentDisplayed ) { + if ( node.accessibleContent ) { instance = parentInstance.findChildWithTrail( trail ); if ( instance ) { existed = true; diff --git a/js/accessibility/AccessiblePeer.js b/js/accessibility/AccessiblePeer.js index b183ac2d3..3136d510f 100644 --- a/js/accessibility/AccessiblePeer.js +++ b/js/accessibility/AccessiblePeer.js @@ -315,10 +315,6 @@ define( function( require ) { this.setHasAccessibleContent(); }, - onAccessibleContentDisplayedChange: function() { - - this.setHasAccessibleContent(); - }, setHasAccessibleContent: function() { // no op, this should be handled in the view, and updated here when AccessiblePeer is constructed. diff --git a/js/accessibility/FocusTests.js b/js/accessibility/FocusTests.js index d0bcb0661..e10f690a0 100644 --- a/js/accessibility/FocusTests.js +++ b/js/accessibility/FocusTests.js @@ -9,7 +9,6 @@ define( function( require ) { 'use strict'; // modules - var AccessiblePeer = require( 'SCENERY/accessibility/AccessiblePeer' ); var Display = require( 'SCENERY/display/Display' ); var Node = require( 'SCENERY/nodes/Node' ); var Trail = require( 'SCENERY/util/Trail' );