From 77876685bfc146adbe41f7e70b459d156c93a75d Mon Sep 17 00:00:00 2001
From: denz1994 <denz1994@gmail.com>
Date: Fri, 21 Dec 2018 09:13:43 -0500
Subject: [PATCH] Added REVIEW comments regarding unlink()
 https://github.com/phetsims/fractions-common/issues/29

---
 js/building/model/ShapePiece.js        | 1 +
 js/building/view/NumberPieceNode.js    | 2 ++
 js/building/view/ShapePieceNode.js     | 1 +
 js/game/view/FractionChallengeNode.js  | 2 ++
 js/intro/view/EqualityLabScreenView.js | 2 ++
 js/lab/view/LabShapePanel.js           | 1 +
 js/matcher/view/LevelNode.js           | 5 +++++
 js/matcher/view/LevelsContainerNode.js | 1 +
 8 files changed, 15 insertions(+)

diff --git a/js/building/model/ShapePiece.js b/js/building/model/ShapePiece.js
index 8005f46..cbeb719 100644
--- a/js/building/model/ShapePiece.js
+++ b/js/building/model/ShapePiece.js
@@ -87,6 +87,7 @@ define( require => {
       this.dampedHarmonicTimeElapsed = 0;
       this.trueTargetRotation = 0;
 
+      // REVIEW: Does this property need an unlink?
       this.isUserControlledProperty.link( isUserControlled => {
         if ( isUserControlled ) {
           this.shadowProperty.value = 1;
diff --git a/js/building/view/NumberPieceNode.js b/js/building/view/NumberPieceNode.js
index 180f7f4..9dd0b30 100644
--- a/js/building/view/NumberPieceNode.js
+++ b/js/building/view/NumberPieceNode.js
@@ -100,6 +100,8 @@ define( require => {
           options.dropListener && options.dropListener( wasTouch );
         }
       } );
+
+      // REVIEW: Does this need an unlink?
       this.dragListener.isUserControlledProperty.link( controlled => {
         numberPiece.isUserControlledProperty.value = controlled;
       } );
diff --git a/js/building/view/ShapePieceNode.js b/js/building/view/ShapePieceNode.js
index e3c319b..3ed9028 100644
--- a/js/building/view/ShapePieceNode.js
+++ b/js/building/view/ShapePieceNode.js
@@ -146,6 +146,7 @@ define( require => {
           options.dropListener && options.dropListener( wasTouch );
         }
       } );
+      // REVIEW: Does this need an unlink?
       this.dragListener.isUserControlledProperty.link( controlled => {
         shapePiece.isUserControlledProperty.value = controlled;
       } );
diff --git a/js/game/view/FractionChallengeNode.js b/js/game/view/FractionChallengeNode.js
index 87ed316..18c8e0d 100644
--- a/js/game/view/FractionChallengeNode.js
+++ b/js/game/view/FractionChallengeNode.js
@@ -125,6 +125,8 @@ define( require => {
       this.levelCompleteListener = score => {
         this.levelCompleteNode.visible = score === this.challenge.targets.length;
       };
+
+      // REVIEW: Does this listener need an unlink?
       this.challenge.scoreProperty.link( this.levelCompleteListener );
 
       // layout
diff --git a/js/intro/view/EqualityLabScreenView.js b/js/intro/view/EqualityLabScreenView.js
index 574cb5a..9a292f7 100644
--- a/js/intro/view/EqualityLabScreenView.js
+++ b/js/intro/view/EqualityLabScreenView.js
@@ -91,6 +91,7 @@ define( require => {
           beakerIcon
         ]
       } );
+      // REVIEW: Does this need an unlink?
       model.representationProperty.link( representation => {
         circularIcon.visible = representation === IntroRepresentation.CIRCLE;
         horizontalIcon.visible = representation === IntroRepresentation.HORIZONTAL_BAR;
@@ -159,6 +160,7 @@ define( require => {
       multipliedFractionNode.centerY = bottomAlignY;
       multiplierSpinner.centerY = bottomAlignY;
 
+      // REVIEW: Does this need an unlink?
       model.representationProperty.link( () => {
         this.viewContainer.right = this.representationPanel.right;
       } );
diff --git a/js/lab/view/LabShapePanel.js b/js/lab/view/LabShapePanel.js
index 406c255..f41ff8a 100644
--- a/js/lab/view/LabShapePanel.js
+++ b/js/lab/view/LabShapePanel.js
@@ -100,6 +100,7 @@ define( require => {
         boxContainer
       ];
 
+      // REVIEW: Does this need an unlink?
       this.representationProperty.link( representation => {
         this.pieBox.visible = representation === BuildingRepresentation.PIE;
         this.barBox.visible = representation === BuildingRepresentation.BAR;
diff --git a/js/matcher/view/LevelNode.js b/js/matcher/view/LevelNode.js
index 2aefd00..b6cc606 100644
--- a/js/matcher/view/LevelNode.js
+++ b/js/matcher/view/LevelNode.js
@@ -269,6 +269,7 @@ define( require => {
     };
 
     //show correct button depending on the previous actions
+    // REVIEW: Does this need an unlink?
     model.buttonStatusProperty.link( function updateButtonStatus( value ) {
       buttonOk.setVisible( value === 'ok' );
       buttonCheck.setVisible( value === 'check' );
@@ -293,19 +294,23 @@ define( require => {
       }
     } );
 
+    // REVIEW: Doc typo?
     //adjustion timer position if necessary
+    // REVIEW: Does this need an unlink?
     model.gameModel.isTimerProperty.link( function( isTimer ) {
       timeLabel.visible = isTimer;
       vBox.right = model.gameModel.width - margin;
     } );
 
     //update timer
+    // REVIEW: Does this need an unlink?
     model.timeProperty.link( function( newTime ) {
       timeLabel.text = StringUtils.format( timeNumberSecString, Util.toFixed( newTime, 0 ) );
       vBox.right = model.gameModel.width - margin;
     } );
 
     //update score
+    // REVIEW: Does this need an unlink?
     model.scoreProperty.link( function( newScore ) {
       scoreLabel.text = StringUtils.format( labelScoreString, newScore );
       vBox.right = model.gameModel.width - margin;
diff --git a/js/matcher/view/LevelsContainerNode.js b/js/matcher/view/LevelsContainerNode.js
index 7d08747..385bf48 100644
--- a/js/matcher/view/LevelsContainerNode.js
+++ b/js/matcher/view/LevelsContainerNode.js
@@ -108,6 +108,7 @@ define( require => {
 
     this.levelNodes = [];
 
+    // REVIEW: Does this need an unlink?
     model.currentLevelProperty.link( function( newLevel ) {
       if ( newLevel > 0 ) {
         //generate each node levelNode on demand, to make loading faster