From dc954525bcce10e544f6f4007a5c6b2664840954 Mon Sep 17 00:00:00 2001 From: Jonathan Olson Date: Tue, 17 Dec 2019 13:03:37 -0700 Subject: [PATCH] Adding documentation and PressListener more comprehensive disposal, see https://github.com/phetsims/scenery/issues/131 and https://github.com/phetsims/sun/issues/372 --- js/listeners/DragListener.js | 9 ++++++++- js/listeners/FireListener.js | 3 ++- js/listeners/PressListener.js | 7 ++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/js/listeners/DragListener.js b/js/listeners/DragListener.js index b6e624392..06d572ebd 100644 --- a/js/listeners/DragListener.js +++ b/js/listeners/DragListener.js @@ -60,6 +60,11 @@ * } * } ) * + * It's completely fine to use one DragListener with multiple objects, however this isn't done as much since specifying + * locationProperty only works with ONE model position Property (so if things are backed by the same Property it would + * be fine). Doing things based on modelPoint/modelDelta/etc. should be completely fine using one listener with + * multiple nodes. The typical pattern IS creating one DragListener per draggable view Node. + * * @author Jonathan Olson */ @@ -96,7 +101,9 @@ define( require => { options = merge( { // {Property.|null} - If provided, it will be synchronized with the drag location in the model coordinate - // frame (applying any provided transforms as needed). + // frame (applying any provided transforms as needed). Typically, DURING a drag this Property should not be + // modified externally (as the next drag event will probably undo the change), but it's completely fine to modify + // this Property at any other time. locationProperty: null, // {Function|null} - Called as start( event: {Event}, listener: {DragListener} ) when the drag is started. diff --git a/js/listeners/FireListener.js b/js/listeners/FireListener.js index e492fb7ea..85a495e70 100644 --- a/js/listeners/FireListener.js +++ b/js/listeners/FireListener.js @@ -1,7 +1,8 @@ // Copyright 2017-2019, University of Colorado Boulder /** - * A listener for common button usage, providing the fire() method/callback and helpful properties. + * A listener for common button usage, providing the fire() method/callback and helpful properties. NOTE that it doesn't + * need to be an actual button (or look like a button), this is useful whenever that type of "fire" behavior is helpful. * * For example usage, see scenery/examples/input.html. Usually you can just pass a fire callback and things work. * diff --git a/js/listeners/PressListener.js b/js/listeners/PressListener.js index 5c2d518aa..9e461d77e 100644 --- a/js/listeners/PressListener.js +++ b/js/listeners/PressListener.js @@ -810,9 +810,14 @@ define( require => { } !this.isHoveringProperty.isDisposed && this.isHoveringProperty.unlink( this._isHighlightedListener ); + this.overPointers.dispose(); + this.isPressedProperty.dispose(); + this.isOverProperty.dispose(); + this.isHoveringProperty.dispose(); + this.isHighlightedProperty.dispose(); + this.isFocusedProperty.dispose(); this.a11yClickingProperty.dispose(); this.looksPressedProperty.dispose(); - this._pressAction.dispose(); this._releaseAction.dispose();