Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace SimpleDragHandler with DragListener #183

Closed
zepumph opened this issue Oct 11, 2019 · 5 comments
Closed

Replace SimpleDragHandler with DragListener #183

zepumph opened this issue Oct 11, 2019 · 5 comments

Comments

@zepumph
Copy link
Member

zepumph commented Oct 11, 2019

No description provided.

@zepumph
Copy link
Member Author

zepumph commented Nov 6, 2019

This was a simple conversion, and I couldn't see any problems when testing. Closing.

@zepumph zepumph closed this as completed Nov 6, 2019
zepumph added a commit that referenced this issue Nov 6, 2019
@zepumph zepumph reopened this Nov 6, 2019
@zepumph
Copy link
Member Author

zepumph commented Nov 6, 2019

I should also do MoveableDragHandler.

@zepumph
Copy link
Member Author

zepumph commented Nov 6, 2019

While converting the MoveableDragHandler in TracerNode, I got 80% there, but found that DragListener doesn't have a transform setter, it is only set through the options.

I didn't commit anything, but worked on it and have a patch.

I added that setter to DragListener, which seemed to work. I also changed the startDrag to press, but the transform seems off because when I press, the mouse offset is about half of the screen.

Index: projectile-motion/js/common/view/ToolboxPanel.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- projectile-motion/js/common/view/ToolboxPanel.js	(revision 2aacf1d814b3d59550f405488d977a42b8408470)
+++ projectile-motion/js/common/view/ToolboxPanel.js	(date 1573083809192)
@@ -87,7 +87,7 @@
       // coordinates empirically determined to shift tracer to mouse when pulled out of the toolbox
       const initialViewPosition = tracerNode.globalToParentPoint( event.pointer.point ).plusXY( -180, 0 );
       tracer.positionProperty.set( transformProperty.get().viewToModelPosition( initialViewPosition ) );
-      tracerNode.movableDragHandler.startDrag( event );
+      tracerNode.dragListener.press( event );
 
     }, { allowTouchSnag: true } ) );
 
Index: projectile-motion/js/common/view/TracerNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- projectile-motion/js/common/view/TracerNode.js	(revision 2aacf1d814b3d59550f405488d977a42b8408470)
+++ projectile-motion/js/common/view/TracerNode.js	(date 1573083660653)
@@ -12,11 +12,11 @@
   const BooleanProperty = require( 'AXON/BooleanProperty' );
   const Bounds2 = require( 'DOT/Bounds2' );
   const Circle = require( 'SCENERY/nodes/Circle' );
+  const DragListener = require( 'SCENERY/listeners/DragListener' );
   const HBox = require( 'SCENERY/nodes/HBox' );
   const inherit = require( 'PHET_CORE/inherit' );
   const MathSymbols = require( 'SCENERY_PHET/MathSymbols' );
   const merge = require( 'PHET_CORE/merge' );
-  const MovableDragHandler = require( 'SCENERY_PHET/input/MovableDragHandler' );
   const Node = require( 'SCENERY/nodes/Node' );
   const Path = require( 'SCENERY/nodes/Path' );
   const projectileMotion = require( 'PROJECTILE_MOTION/projectileMotion' );
@@ -146,14 +146,17 @@
       { fill: 'gray' }
     );
 
+    const dragBoundsProperty = new Property( screenView.visibleBoundsProperty.get().shiftedX( dragBoundsShift ) );
+
     // @public so events can be forwarded to it by ToolboxPanel
-    this.movableDragHandler = new MovableDragHandler( tracer.positionProperty, {
+    this.dragListener = new DragListener( {
+      locationProperty: tracer.positionProperty,
       modelViewTransform: transformProperty.get(),
-      dragBounds: screenView.visibleBoundsProperty.get().shiftedX( dragBoundsShift ),
-      startDrag: function( event ) {
+      dragBoundsProperty: dragBoundsProperty,
+      startDrag: event => {
         self.isUserControlledProperty.set( true );
       },
-      endDrag: function() {
+      endDrag: () => {
         self.isUserControlledProperty.set( false );
       },
       tandem: options.tandem.createTandem( 'dragHandler' )
@@ -244,14 +247,14 @@
 
     // Observe changes in the modelViewTransform and update/adjust positions accordingly
     transformProperty.link( function( transform ) {
-      self.movableDragHandler.setModelViewTransform( transform );
-      self.movableDragHandler.setDragBounds( transform.viewToModelBounds( screenView.visibleBoundsProperty.get().shiftedX( dragBoundsShift ) ) );
+      self.dragListener.transform = transform;
+      dragBoundsProperty.value = ( transform.viewToModelBounds( screenView.visibleBoundsProperty.get().shiftedX( dragBoundsShift ) ) );
       updatePosition( tracer.positionProperty.get() );
     } );
 
     // Observe changes in the visible bounds and update drag bounds and adjust positions accordingly
     screenView.visibleBoundsProperty.link( function( bounds ) {
-      self.movableDragHandler.setDragBounds( transformProperty.get().viewToModelBounds( screenView.visibleBoundsProperty.get().shiftedX( dragBoundsShift ) ) );
+      dragBoundsProperty.value = ( transformProperty.get().viewToModelBounds( screenView.visibleBoundsProperty.get().shiftedX( dragBoundsShift ) ) );
       updatePosition( tracer.positionProperty.get() );
     } );
 
@@ -275,7 +278,7 @@
     Node.call( this, options );
 
     // When dragging, move the tracer tool
-    this.addInputListener( this.movableDragHandler );
+    this.addInputListener( this.dragListener );
 
     // visibility of the tracer
     tracer.isActiveProperty.link( function( active ) {
Index: scenery/js/listeners/DragListener.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- scenery/js/listeners/DragListener.js	(revision 141b9830f0659a2e5a67ecc3c30cfe80052a7ce4)
+++ scenery/js/listeners/DragListener.js	(date 1573083660648)
@@ -632,6 +632,30 @@
     },
     get dragBounds() { return this.getDragBounds(); },
 
+    /**
+     * Sets the drag transform of the listener.
+     * @public
+     *
+     * @param {Bounds2} transform
+     */
+    setTransform: function( transform ) {
+      assert && assert( transform instanceof Transform3 );
+
+      this._transform = transform;
+    },
+    set transform( transform ) { this.setTransform( transform ); },
+
+    /**
+     * Returns the transform of the listener.
+     * @public
+     *
+     * @returns {Transform3}
+     */
+    getTransform: function() {
+      return this._transform;
+    },
+    get transform() { return this.getTransform(); },
+
     /**
      * Interrupts the listener, releasing it (canceling behavior).
      * @public

@zepumph
Copy link
Member Author

zepumph commented Nov 8, 2019

After getting some assistance from @jessegreenberg and @jonathanolson, we got the DragListener conversion working.

The main issue is that we wanted DragListener to handle to offset for us, instead of manually having that code in the forwarding listener in the toolbox. That can be covered by offsetLocation.

There is another bug that came out of this, and I will make an issue in scenery for that.

@zepumph
Copy link
Member Author

zepumph commented Nov 8, 2019

Everything for this repo seems to be working now, see phetsims/scenery#1014 for the bug report. Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant