SimpleDragHandler should use Pointer.attachedProperty instead of Pointer.dragging

[samreid]

It looks like Pointer.js was changed to use attachedProperty in Feb 2017. SimpleDragHandler has been attaching Pointer.dragging separately. This means that a Pointer dragged by SimpleDragHandler doesn't know that it's attached and hence you can pick up something else with it via swipe to snag. Example and video are here:
phetsims/wave-interference#243

It seems this can be solved by changing SimpleDragHandler to use Pointer.attachedProperty. I'll propose a patch shortly for @jonathanolson to review.

[samreid]

Here's a patch that addressed the symptom in Wave Interference:

Index: scenery/js/input/SimpleDragHandler.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- scenery/js/input/SimpleDragHandler.js	(revision 78fe680521378219178aa415d51ab5340ebc9dc7)
+++ scenery/js/input/SimpleDragHandler.js	(date 1544763242000)
@@ -263,7 +263,7 @@
       sceneryLog && sceneryLog.InputListener && sceneryLog.push();
 
       // set a flag on the pointer so it won't pick up other nodes
-      event.pointer.dragging = true;
+      event.pointer.attachedProperty.set( true );
       event.pointer.cursor = this.options.dragCursor;
       event.pointer.addInputListener( this.dragListener, this.options.attach );
 
@@ -289,7 +289,7 @@
       sceneryLog && sceneryLog.InputListener && sceneryLog.InputListener( 'SimpleDragHandler endDrag' );
       sceneryLog && sceneryLog.InputListener && sceneryLog.push();
 
-      this.pointer.dragging = false;
+      this.pointer.attachedProperty.set( false );
       this.pointer.cursor = null;
       this.pointer.removeInputListener( this.dragListener );
 
@@ -345,7 +345,7 @@
 
       // only start dragging if the pointer isn't dragging anything, we aren't being dragged, and if it's a mouse it's button is down
       if ( !this.dragging &&
-           !event.pointer.dragging &&
+           !event.pointer.attachedProperty.value &&
            event.pointer !== this.lastInterruptedTouchPointer &&
            event.canStartPress() ) {
         this.startDrag( event );
@@ -389,7 +389,7 @@
       this.disposed = true;
 
       if ( this.dragging ) {
-        this.pointer.dragging = false;
+        this.pointer.attachedProperty.value = false;
         this.pointer.cursor = null;
         this.pointer.removeInputListener( this.dragListener );
       }
@@ -421,7 +421,7 @@
 
       return {
         down: function( event ) {
-          if ( !event.pointer.dragging && event.canStartPress() ) {
+          if ( !event.pointer.attachedProperty.value && event.canStartPress() ) {
             down( event );
           }
         },
Index: expression-exchange/js/common/view/CoinTermCreatorNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- expression-exchange/js/common/view/CoinTermCreatorNode.js	(revision 80cbff792ea23bf28704aac233ada15ac339bf34)
+++ expression-exchange/js/common/view/CoinTermCreatorNode.js	(date 1544763184000)
@@ -137,7 +137,7 @@
       down: function( event ) {
 
         // ignore this if already dragging
-        if ( event.pointer.dragging ) { return; }
+        if ( event.pointer.attachedProperty.value ) { return; }
 
         // don't try to start drags with a right mouse button or an attached pointer
         if ( !event.canStartPress() ) { return; }

You can apply it using WebStorm => VCS => Apply Patch.

It seems likely that this bug may be affecting other sims that are using mixed types of drag listeners, so I'll mark this as blocking publication. @jonathanolson can you please take a look?

[samreid]

Note I also patched a usage in CoinTermCreatorNode.

[jonathanolson]

Does setting { attach: true } in SimpleDragHandler resolve this issue?

[samreid]

That works perfectly, thanks! On hold until @KatieWoe tests phetsims/wave-interference#243

[zepumph]

Looks like things over there have been cleaned up, tested, and closed. Marking off hold.

[samreid]

Approved in testing, closing.