a11y sprint

[pixelzoom]

This issue is to aggregate comments, thoughts, TODOs, commits, etc. related to adding a11y to color-vision.

[pixelzoom]

Tuesday 5/8/18 notes:

• we implemented accessibleOrder for SingleBulbScreen
• OnOffSwitch identified as a common-code component that needs to be instrumented

Notes from that activity:

• accessibleOrder removes things from the default order and prepends them to the order. In practice, it would be useful to be able to append and insert before/after a specific sibling.

• Play Area and Control Panel should be generalized to allow more than 2 groupings. Consider remove "Panel" from Control Panel, since it has loaded visual connotations.

[pixelzoom]

Thursday morning 5/10/18, we added the 1 of each h1, h2 and h3 element to the Single-Bulb screen.

Notes from this experience:

Every ScreenView needs a SceneSummaryNode added to it, not handled in joist.ScreenView. And it must be first in the accessibleOrder. So if you change the accessibleOrder, you have to remember to put the SceneSummaryNode first. Additional developer responsibility, boilerplate.

Using scenery to write HTML5 in SceneSummaryNode is clumsy/indirect.

Standardize format of a11y design docs. In Color Vision: A11y Design Main, “Heading Outline” and “Focus Order” in parallel columns is confusing, looks like they are related. Molecules and Light design doc is different, MB thinks this is clearer (colors for sections, etc.)

Forming the ScreenView’s innerContent (e.g. “RGB Bulbs Screen”) requires creating and getting a pattern string from *A11yString, and getting the screen title from translated strings. StringUtils.fillIn to complete. Screen title was previously imported only by Screen, now needs to be known by both Screen and ScreenView. Joist should do this automatically? MB: Screen (instead of ScreenView) could insert H1 wrapper for screen title? SR: Screen could set this._view.labelContent = “title” after calling createView.

Standardize punctuation and casing. E.g. “RGB Bulbs Screen” vs “RGB Bulbs screen”.

Switching between “accessibility” and “a11y” is confusing. E.g. scenery-phet/accessibility vs ColorVisionA11yStrings.js. Let’s pick one and stick with it.

Convention for string keys for scene summary, similar to convention for screen names.

Boilerplate added by SceneSummaryNode: “This is an interactive sim. It changes as you play with it. Each screen has a Play Area and Control Panel.” Not true that each screen has a Play Area and Control Panel, and no verification.

Why is Accessibility.appendDescription:false the default? Describing scene summary that is more than one paragraph is awkward, impossible if more than 2 paragraphs. E.g. in SingleBulbScreenView:

var sceneSummaryNode = new SceneSummaryNode( singleBulbSceneSummaryString, {
  appendDescription: true,
  descriptionTagName: 'p',
  descriptionContent: '...'
} );

The relationship/structure of regions/headings and focusable elements is unclear/confusing.

When adding heading elements via options, you have to worry about whether they will wrap interactive elements. E.g. in SingleBulbScreenView, we had to use labelTagName instead of tagName because WavelengthSlider contains interactive elements:

var bulbColorSlider = new WavelengthSlider( model.flashlightWavelengthProperty, {
  ...
  // a11y
  labelTagName: 'h3',
  labelContent: lightSourceColorControlString
} );

[samreid]

Using scenery to write HTML5 in SceneSummaryNode is clumsy/indirect.

For example, we saw OhmsLawSceneSummaryNode, which has code like this:

    var rightNowParagraphNode = new Node( { tagName: 'p', innerContent: rightNowString } );

    // list outlining the values for this sim
    var valueListNode = new Node( { tagName: 'ul' } );
    var valueVoltageItemNode = new Node( { tagName: 'li' } );
    var valueResistanceItemNode = new Node( { tagName: 'li' } );
    var valueCurrentItemNode = new Node( { tagName: 'li' } );
    valueListNode.children = [ valueVoltageItemNode, valueResistanceItemNode, valueCurrentItemNode ];

    var sliderParagraphNode = new Node( { tagName: 'p', innerContent: summaryLookForSlidersString } );
    var shortcutParagraphNode = new Node( { tagName: 'p', innerContent: checkOutShortcutsString } );

    // add all children to this node, ordering the accessible content
    content.addChild( summaryNode );
    content.addChild( rightNowParagraphNode );
    content.addChild( valueListNode );
    content.addChild( sliderParagraphNode );
    content.addChild( shortcutParagraphNode );

We've had a few conversations (including but not limited to https://github.com/phetsims/a11y-research/issues/99 ) about using a more direct approach for writing the HTML instead of encoding it through scenery nodes. At this afternoon's meeting, @jonathanolson and @pixelzoom advocated for a higher abstraction instead of writing HTML directly, which sounds like the best long-term plan. In the end, scenery Nodes will still code the accessible HTML, but we envision a more appropriate higher-level API for it. (I'm not sure if/how it would change the OhmsLawSceneSummaryNode example though).

[pixelzoom]

I've labeled this issue as "in progress" because the sim should not be published from master while in this state. In retrospect, I feel that we should have done this working in a branch. We accomplished our goals for the a11y sprint. But the sim is very far from being instrumented, and (I hope) PhET's a11y API will evolve significantly as the result of the sprint. My recommendation would be to remove the little bit of instrumentation that we've done, or possibly move it to a branch.

If we do decide to roll back... Factoring the "thought bubbles" out into PerceivedColorNode is a valuable change that is independent of a11y and should be preserved.

Labeling for developer meeting to discuss.

[pixelzoom]

Files to revert:

package.json SingleBulbScreen.js SingleBulbScreenView.js ColorVisionScreenView.js (then reintroduce PerceivedColorNode)

Files to delete:

color-vision_a11y_view.html ColorVisionA11yStrings.js

[pixelzoom]

I decided to preserve the a11y sprint work in branch "a11y-sprint" (see #124), and then removed the a11y instrumentation from master.

[samreid]

It seems it should be up to @jessegreenberg @zepumph @mbarlow12 and @emily-phet to decide how to proceed with this issue.

[pixelzoom]

I would think the responsible developer would be involved too. If that's going to be the case with a11y, then I'd like to discuss passing off responsibility for this sim to @jessegreenberg @zepumph or @mbarlow12.

[samreid]

Of course, @pixelzoom should help decide where to go with this issue. My apologies.

[pixelzoom]

To clarify... I wrote the Java version of this sim, but I was not involved in the HTML5 port. I inherited responsibility for the sim when the HTML5 developer left the project. So if there are going to be significant changes for a11y, it would probably make sense to transfer responsibility to the developer who will be making those changes.

[pixelzoom]

Removed in-progress label because there's nothing to prevent publishing from master; all changes are in the 'a11y-sprint' branch.

[ariel-phet]

@pixelzoom chances are when this sim comes up for a11y work, you will be the one to do the work. But with competing priorities it is unclear when that work will be done (since the sprint revealed that are some underlying subtle design issues with the sim to be addressed, most notably the play/pause interaction is a bit funky).

That being said if the a11y work needs to be prioritized, and you are not available for it, we will revisit the question of the responsible dev.

[zepumph]

Unassigning until further color-vision a11y work occurs.