phetsims / resistance-in-a-wire

"Resistance in a Wire" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/resistance-in-a-wire
GNU General Public License v3.0
1 stars 4 forks source link

Update tandems on keyboardNav panel #103

Closed zepumph closed 7 years ago

zepumph commented 7 years ago

I noticed that many tandems were the same as other components, so RIAW was broken. I fixed the duplications by removing unneeded layout tandems, but there is still some work to be done. (1) Not all texts/icons have tandems, and (2) not all tandems are named appropriately.

  1. When I make all instrumented items on the panel invisible, this is what I still see in instance proxies. image This makes me think that not every text/icon is instrumented.

  2. PhET-iO tries to follow the convention that the tandem name is the same as the variable name like the following snippets:

    
    var shiftTabKeyIcon = ResistanceInAWireKeyboardHelpContent.createShiftPlusIconHBox( new TabKeyNode( {
        minKeyWidth: TEXT_KEY_WIDTH,
        maxKeyWidth: TEXT_KEY_WIDTH,
        tandem: tandem.createTandem( 'shiftTabKeyIcon' )
    } ) );
    
    var shiftPlusTabDescription = new RichText( ResistanceInAWireA11yStrings.shiftTabKeyDescriptionString, _.extend( {
      tandem: tandem.createTandem( 'shiftPlusTabDescription' ),
      maxWidth: TEXT_MAX_WIDTH
    }, DESCRIPTION_OPTIONS ) );

Using the above convention this snippet is correct, but not ideal because these two items should have names that link them a bit better, either both with "Plus" or neither.

@jessegreenberg said he would look into this. Thanks!

zepumph commented 7 years ago

Also, yesterday we talked about only creating these items (which means registering these tandems too) if accessibility is enabled. Otherwise all of these needless tandems will be created even if accessibility isn't enabled, and there is no keyboard nav button.

jessegreenberg commented 7 years ago

In the above commit I made sure that all icons and descriptions have a tandem and that the tandem names follow convention and that icon and description tandem names are similar so that they are linked as suggested in https://github.com/phetsims/resistance-in-a-wire/issues/103#issue-261483987.

jessegreenberg commented 7 years ago

To address https://github.com/phetsims/resistance-in-a-wire/issues/103#issuecomment-333170089, a11y is now enabled by default in this sim since keyboard navigation is nearly ready for deployment.

jessegreenberg commented 7 years ago

I tested in instance-proxies and I think everything is working well. @zepumph could you please review?

zepumph commented 7 years ago

Looks really nice. Thanks for doing that. I'm not sure that we will need these layout tandems ever.

But heck, I'm not really even sure that any of this needs to be instrumented. For now I would say it is good to go, we can always update it later. Thank you!