Skip to content

Commit

Permalink
remove accessibleContentDisplayed, #814
Browse files Browse the repository at this point in the history
  • Loading branch information
@zepumph
zepumph committed Jul 14, 2018
1 parent 3c36a1e commit afbe969
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 142 deletions.
1 change: 0 additions & 1 deletion doc/accessibility.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ <h1>Accessible Content in Scenery</h1>
<div class="sublink"><a href="#input">addAccessibleInputListener()</a></div>
<div class="sublink"><a href="#focus">focusable</a></div>
<div class="sublink"><a href="#focus">accessibleVisible</a></div>
<div class="sublink"><a href="#focus">accessibleContentDisplayed</a></div>
<div class="sublink"><a href="#focus">focus()</a></div>
<div class="sublink"><a href="#focus">blur()</a></div>

Expand Down
65 changes: 7 additions & 58 deletions js/accessibility/Accessibility.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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.<Function>} - For accessibility input handling {keyboard/click/HTML form}
this._accessibleInputListeners = [];

Expand Down Expand Up @@ -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();
// }
// }

}
},
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -2265,7 +2214,7 @@ define( function( require ) {
*/
getAccessibleContent: function() {

return this._accessibleContentDisplayed && !!this._tagName;
return !!this._tagName;

},
get accessibleContent() { return this.getAccessibleContent(); },
Expand Down
77 changes: 0 additions & 77 deletions js/accessibility/AccessibilityTests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 );
Expand Down
2 changes: 1 addition & 1 deletion js/accessibility/AccessibilityTree.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions js/accessibility/AccessiblePeer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion js/accessibility/FocusTests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
Expand Down

0 comments on commit afbe969

Please sign in to comment.