phetsims/vector-addition

questions about allowTouchSnag in VectorCreatorPanelSlot

Closed this issue · 6 comments

VectorCreatorPanelSlot is one of the vector icons in the VectorCreatorPanel (aka vector toolbox) and handles creation of vectors.

In VectorCreatorPanelSlot:

      iconNode.addInputListener( DragListener.createForwardingListener( ( event ) => {
...
      }, { allowTouchSnag: true } ) );

In DragListener constructor:

  function DragListener( options ) {
    var self = this;

    options = _.extend( {
      // {boolean} - If true, unattached touches that move across our node will trigger a press(). This helps sometimes
      // for small draggable objects.
      allowTouchSnag: true,
...

In DragListener.createForwardingListener:

    createForwardingListener: function( down, options ) {

      options = _.extend( {
        allowTouchSnag: false
      }, options );
...

Questions:

(1) Why does createForwardingListener set allowTouchSnag: false? @jonathanolson?

(2) Why does VectorCreatorPanelSlot need to set allowTouchSnag: true? @brandonLi8?

Why does VectorCreatorPanelSlot need to set allowTouchSnag: true?

@pixelzoom I don't recall adding allowTouchSnag: true but I do remember seeing it. I don't even know what "touch snag" is.

Once upon a time there was no VectorCreatorPanelSlot, just a VectorCreatorPanel (which was originally written by @veillette). I decided to create a VectorCreatorPanelSlot to modularize the functionality, and may have just copy and pasted that option.

After investigating the commit history of the VectorCreatorPanelSlot, I found 81fa14f committed by @veillette, which adds the "touch snag" option.

@pixelzoom : allowTouchSnag was set to true such that it is easier to grab the vectors out of the vector toolbox when using an iPad. Alternatively you can dilate the touchBounds but setting allowTouchSnag to true makes it much more forgiving.

Slack discussion about:

(1) Why does createForwardingListener set allowTouchSnag: false? @jonathanolson?

Jonathan Olson:house_with_garden: 3:34 PM
That’s a good question. If more usages are using touch snag, we could change that default? Maybe we should anyways?

Chris Malley 3:35 PM
Was there some reason for defaulting it to something that’s not the default for DragListener?

Jonathan Olson:house_with_garden: 3:37 PM
Likely, they might have both been default false, and then the main one changed to default true. There is no code reason to have them different. I’m not sure about whether it’s more likely to NOT want touch snag for forwarding, but I think that’s not a sufficient reason for not making them match

My questions were answered. The default was changed to true in phetsims/scenery#999, so I was able to remove { allowTouchSnag: true } from VectorCreatorPanelSlot in c69aad4.

Closing.