phetsims / balloons-and-static-electricity

"Balloons and Static Electricity" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/balloons-and-static-electricity
GNU General Public License v3.0
6 stars 10 forks source link

BalloonNode's accessibleDragNode behaves differently for keydown/up and blur #425

Closed zepumph closed 6 years ago

zepumph commented 6 years ago

Found while working on https://github.com/phetsims/scenery-phet/issues/368 Take a look at this code:

    accessibleDragNode.addAccessibleInputListener( {
      keydown: function( event ) {
        if ( event.keyCode === KeyboardUtil.KEY_ENTER ) {
          releasedWithEnter = true;
          releaseBalloon();
        }
      },
      keyup: function( event ) {
        // release  on keyup of spacebar so that we don't pick up the balloon again when we release the spacebar
        // and trigger a click event - escape could be added to either keyup or keydown listeners
        if ( event.keyCode === KeyboardUtil.KEY_SPACE || event.keyCode === KeyboardUtil.KEY_ESCAPE ) {
          releaseBalloon();
        }
      }
//...
      blur: function( event ) {
        if ( !self.movingToFront ) {
          endDragListener();

          // the draggable node should no longer be focusable
          accessibleDragNode.accessibleVisible = false;

          self.dragNodeBlurredEmitter.emit();

        }
      }
    } );

Shouldn't the blur event interrupt the drag also? Maybe I don't understand this fully. What do you think @jessegreenberg?

zepumph commented 6 years ago

Note: releaseBalloon() calls the interrupt on the drag handler, but endDragListener() does not.

jessegreenberg commented 6 years ago

I added some documentation and renamed a function to hopefully improve clarity. The blur event should interrupt keyboard dragging, but it already did/does. Prior to https://github.com/phetsims/scenery-phet/issues/368, the KeyboardDragListener was reset on the blur event. Now Accessibility.interruptAccessibleInput is doing this for us on blur. @zepumph does this seem OK?

zepumph commented 6 years ago

Yes it makes sense now, thanks!