doc link and addListener management, #156

phetsims · Aug 31, 2019 · f000b15 · f000b15
1 parent 5dfd678
commit f000b15
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 4 deletions.
diff --git a/js/curve-fitting/model/Point.js b/js/curve-fitting/model/Point.js
@@ -46,6 +46,7 @@ define( require => {
       this.animation = null;
 
       // @public {Property.<boolean>} a Property that reflects whether the point is in the graph
+      // No dispose needed because Point owns all dependencies
       this.isInsideGraphProperty = new DerivedProperty(
         [ this.positionProperty ],
         position => CurveFittingConstants.GRAPH_BACKGROUND_MODEL_BOUNDS.containsPoint( position ),
@@ -96,7 +97,7 @@ define( require => {
           this.animation.endedEmitter.removeListener( onAnimationEnd );
           this.animation = null;
         };
-        this.animation.endedEmitter.addListener( onAnimationEnd );
+        this.animation.endedEmitter.addListener( onAnimationEnd ); // removed when animation ends
 
         this.animation.start();
       }

diff --git a/js/curve-fitting/view/CurveFittingScreenView.js b/js/curve-fitting/view/CurveFittingScreenView.js
@@ -105,7 +105,7 @@ define( require => {
       );
 
       // create the panel that serves as an opaque background for the equationAccordionBox; see #126
-      // no dispose necessary because equationAccordionBox is present for the lifetime of the sim
+      // no dispose or removeListener necessary because equationAccordionBox is present for the lifetime of the sim
       const graphEquationBackground = new Rectangle( 0, 0, 0, 0, 0, 0, {
         cornerRadius: CurveFittingConstants.PANEL_CORNER_RADIUS,
         fill: 'white'

diff --git a/js/curve-fitting/view/CurveNode.js b/js/curve-fitting/view/CurveNode.js
@@ -50,7 +50,7 @@ define( require => {
         }
       };
 
-      // unlinks unnecessary because this CurveNode is always present for the lifetime of the simulation
+      // unlink and removeListener unnecessary because this CurveNode is always present for the lifetime of the simulation
       curveVisibleProperty.link( updateCurve );
       curve.orderProperty.link( updateCurve );
       curve.updateCurveEmitter.addListener( updateCurve );

diff --git a/js/curve-fitting/view/PointNode.js b/js/curve-fitting/view/PointNode.js
@@ -354,6 +354,7 @@ define( require => {
         deltaTextLabel.center = deltaTextBackground.center;
       }
 
+      // must be unlinked in dispose
       point.deltaProperty.link( updateDelta );
 
       /**
@@ -386,6 +387,7 @@ define( require => {
         valueTextLabel.center = valueTextBackground.center;
       }
 
+      // these require unlinkAttribute in dispose
       const valueBackgroundHandle = valuesVisibleProperty.linkAttribute( valueTextBackground, 'visible' );
       const valueTextHandle = valuesVisibleProperty.linkAttribute( valueTextLabel, 'visible' );
       const deltaBackgroundHandle = valuesVisibleProperty.linkAttribute( deltaTextBackground, 'visible' );
@@ -408,6 +410,7 @@ define( require => {
         }
       }
 
+      // must be unlinked in dispose
       residualsVisibleProperty.link( updateErrorBarsBasedOnResidualsVisibility );
 
       // point halo
@@ -432,6 +435,7 @@ define( require => {
         updateDelta();
       }
 
+      // must be unlinked in dispose
       point.positionProperty.link( centerPositionListener );
 
       // @private

diff --git a/js/curve-fitting/view/ResidualsNode.js b/js/curve-fitting/view/ResidualsNode.js
@@ -58,7 +58,7 @@ define( require => {
         }
       };
 
-      // no dispose necessary: present for the lifetime of the simulation
+      // unlink and removeListener necessary: present for the lifetime of the simulation
       residualsVisibleProperty.link( updateResiduals );
       curve.orderProperty.link( updateResiduals );
       curve.updateCurveEmitter.addListener( updateResiduals );