Should addAccessibleInputListener automatically preventDefault?

[samreid]

In phetsims/circuit-construction-kit-common#307 @jessegreenberg added preventDefault calls to prevent e.g., firefox from navigating pages back. Should this be automatically implemented in addAccessibleInputListener so we don't end up scattering preventDefault everywhere?

[jessegreenberg]

I am afraid that preventing default in addAccessibleInputListener would be too aggressive. For instance, calling event.preventDefault() on all keydown event listeners will also prevent use of tab key to go to the next item and ctrl + r to refresh the page. We only want to preventDefault when specific keys are pressed in the listener.

[jessegreenberg]

Maybe we can factor out common interactions that will require us to prevent default. Like SimpleDragHadler we could have a KeyboardDeleteHandler (we can think of a better name later). In CCK we have

        // on delete or backspace, the focused circuit element should be deleted
        if ( code === Input.KEY_DELETE || code === Input.KEY_BACKSPACE ) {

          // prevent default so 'backspace' and 'delete' don't navigate back a page in Firefox
          event.preventDefault();
          circuit.circuitElements.remove( circuitElement );
        }

Perhaps the API could look like

var myKeyboardDeleteHandler = new KeyboardDeleteHandler( {
  onDelete: function( event ) {
    circuit.circuitElements.remove( circuitElement );
  }
})
myNode.addAccessibleInputListener( myKeyboardDeleteHandler )

And the handler calls preventDefault for the correct keys so it isn't needed in sim code?

[jessegreenberg]

We could end up with a lot of these
KeyboardDeleteHandler
KeyboardSliderHandler
KeyboardDragHandler
KeyboardArrowKeyHandler
KeyboardNumberHandler

I am not sure how I feel about lots of these handlers. But at least the reasons for preventDefault would be isolated and documented in each one.

[samreid]

Perhaps we should see (or anticipate) what patterns come up frequently. Perhaps an abstraction like:

new KeyHandler([Input.KEY_DELETE,Input.KEY_BACKSPACE],{
    preventDefault:true
},function(){ /*callback*/});

would work well? It's difficult to say whether this would be necessary or useful.

[jessegreenberg]

Agreed, though an abstraction does seem useful. I found 44 usages of addAccessibleInputListener, and inspection showed about 1/4 use event.preventDefault() in some way.

[zepumph]

I spoke with @jessegreenberg about this today.

We believe that preventDefault should still be called in the exact locations that it is needed, and not eagerly.

I believe this could be handled by better encapsulation of keyboard input handlers that may be worked on in #1445. Like from #636 (comment)

We are ready to close this issue.