Manage joist related code in scenery/FocusIO

[zepumph]

Promoting REVIEW comments from #832 found in FocusIO.toStateObject:

        // REVIEW: What is joist-related code doing in Scenery? I'd definitely prefer if there is a better way to hook
        // REVIEW: things together. Scenery could expose an API that would allow this behavior?
        // REVIEW: This will HARD-fail out if ever called in most non-sim use cases (e.g. presentations, documentation,
        // REVIEW: or anything that isn't phet-related).

Right now we are safe because this code will only be run in phet-io brand, which is (to my knowledge which will not run for non-sim scenery implementations)

@samreid can you think of a better way to do this? Perhaps this should sit until we work on a11y/phet-io combo.

[samreid]

I'd like to clarify/understand the purpose of having Focus instrumented. Some possibilities:

• For playbacks, so we can see how the user changed focus
• For data stream, so researchers can understand how the user moved focus
• Something else?

Mousing-over an object is not in data stream nor is it in the state--we will need to decide whether focus should be treated differently.

[zepumph]

Now that we have streamlined a11y events into Input with emitters. The above commit accomplished updating much of the above goals. All a11y events now emit to the data stream. Here is an example pretty log.

With any luck perhaps we could add a phetioID in there for readibility.

[zepumph]

In the above commit I stripped FocusIO of all sour code, because really it doesn't need to be in the state, or be able to recreate itself on the downstream side. Instead I'm thinking of Display.focusProperty as a read-only Property used to monitor the current phetioID that is focused. That coupled with the focusIn/Out emitters make for a nice data stream.

On top of that I removed it from state, we don't have mouse moves in state, and that feels similar.

For customization here is what it looks like in studio, not bad!

@samreid please review.

[zepumph]

I also moved the Property under general, but ditched the display sub-tandem. Should we rename focusedPhetioID to phetioID?

[samreid]

Does this mean a11y events can now be recorded and played back?

[zepumph]

Yes. Right now the only interaction I have not seen successfully recorded is the "grab button"

[samreid]

It looks like this code isn't doing anything. Is that correct?

        var phetioIDIndices = [];
        focus.trail.nodes.forEach( function( node, i ) {

          // Don't include the last node, since it is the focused node
          if ( i < focus.trail.nodes.length - 1 ) {

            // If the node was PhET-iO instrumented, include its phetioID instead of its index (because phetioID is more stable)
            if ( node.tandem ) {
              phetioIDIndices.push( node.tandem.phetioID );
            }
            else {
              phetioIDIndices.push( focus.trail.indices[ i ] );
            }
          }
        } );

[zepumph]

It looks like this code isn't doing anything. Is that correct?

It isn't needed for record/playback, but I thought that something was needed for how to display FocusIOs on the data stream. When ever display.focusProperty changes it goes on the data stream.

Instead of including indices which could change, I updated it to only have the instrumented nodes in the hierarchy. I also dropped the unneeded support for null, and updated documentation. I think we are ready to close. Anything else here @samreid?

[samreid]

Does this mean a11y events can now be recorded and played back?

I think the last thing for this issue is to add a checklist item for how to test that a11y events can be recorded and played back. Is there a QA issue or checklist for testing record and playback where it would be appropriate to add this? After that's been added, this issue can be closed.

[zepumph]

@KatieWoe will you please add the following note to the recording section of the QA book, likely in the "Events: Recording Wrapper Test" section:

"If this sim is outfitted with alternative input, then also test keyboard navigation to make sure that those input events work as well."

or something like that.

[KatieWoe]

Done in above commit.
@zepumph does this instruction include screen readers?

[zepumph]

It does not need to. Thanks!