From 6e814b2eb45810dc02a8bbb6237390f8c57a460f Mon Sep 17 00:00:00 2001
From: Jonathan Olson <jonathan.olson@colorado.edu>
Date: Tue, 5 May 2020 15:34:21 -0600
Subject: [PATCH] More REVIEW notes, see
 https://github.com/phetsims/build-a-molecule/issues/173

---
 js/common/model/Molecule.js                       | 1 +
 js/common/model/MoleculeList.js                   | 1 +
 js/common/view/KitNode.js                         | 1 +
 js/common/view/KitPanel.js                        | 1 +
 js/common/view/MoleculeBondNode.js                | 1 +
 js/common/view/WarningDialog.js                   | 2 ++
 js/common/view/view3d/ShowMolecule3DButtonNode.js | 1 +
 7 files changed, 8 insertions(+)

diff --git a/js/common/model/Molecule.js b/js/common/model/Molecule.js
index f8abae02..c8f4ba82 100644
--- a/js/common/model/Molecule.js
+++ b/js/common/model/Molecule.js
@@ -13,6 +13,7 @@ import MoleculeStructure from './MoleculeStructure.js';
 
 class Molecule extends MoleculeStructure {
   /**
+   * REVIEW: Many call sites that don't specify these documented-required options. See MoleculeStrucutre, and mark as optional?
    * @param {number} numAtoms
    * @param {number} numBonds
    */
diff --git a/js/common/model/MoleculeList.js b/js/common/model/MoleculeList.js
index 45840516..04caa366 100644
--- a/js/common/model/MoleculeList.js
+++ b/js/common/model/MoleculeList.js
@@ -257,6 +257,7 @@ class MoleculeList {
 }
 
 // statics
+//REVIEW: visibility/type info on these?
 MoleculeList.masterInstance = null;
 MoleculeList.initialized = false;
 MoleculeList.initialList = new MoleculeList();
diff --git a/js/common/view/KitNode.js b/js/common/view/KitNode.js
index 176386ef..b1267117 100644
--- a/js/common/view/KitNode.js
+++ b/js/common/view/KitNode.js
@@ -50,6 +50,7 @@ class KitNode extends Node {
     // different layout.
     kit.buckets.forEach( bucket => {
       const bucketFront = new BucketFront( bucket, BAMConstants.MODEL_VIEW_TRANSFORM, {
+        //REVIEW: I can't see where this labelFont option is used? Can you find the API point?
         labelFont: new PhetFont( {
           weight: 'bold',
           size: 18
diff --git a/js/common/view/KitPanel.js b/js/common/view/KitPanel.js
index 3c1e7a59..8213252d 100644
--- a/js/common/view/KitPanel.js
+++ b/js/common/view/KitPanel.js
@@ -54,6 +54,7 @@ class KitPanel extends Node {
     this.kitCarousel = new Carousel( kitNodes, {
       fill: BAMConstants.KIT_BACKGROUND,
       stroke: BAMConstants.KIT_BORDER,
+      //REVIEW: I don't see xMargin in any doc for Carousel, what is the option doing here?
       xMargin: 10,
       itemsPerPage: 1,
       animationEnabled: false,
diff --git a/js/common/view/MoleculeBondNode.js b/js/common/view/MoleculeBondNode.js
index 14fad774..28596265 100644
--- a/js/common/view/MoleculeBondNode.js
+++ b/js/common/view/MoleculeBondNode.js
@@ -126,6 +126,7 @@ class MoleculeBondNode extends Node {
       visible: true
     } ) );
 
+    //REVIEW: ButtonListener is deprecated, can we use FireListener?
     cutTargetNode.addInputListener( new ButtonListener( {
       down() {
         cutTargetNode.cursor = closedCursor;
diff --git a/js/common/view/WarningDialog.js b/js/common/view/WarningDialog.js
index 57730be3..d6a39608 100644
--- a/js/common/view/WarningDialog.js
+++ b/js/common/view/WarningDialog.js
@@ -34,6 +34,8 @@ class WarningDialog extends Dialog {
       ],
       spacing: 15,
       align: 'center',
+
+      //REVIEW: What is this xMargin option for?
       xMargin: 15,
       cursor: 'pointer'
     } );
diff --git a/js/common/view/view3d/ShowMolecule3DButtonNode.js b/js/common/view/view3d/ShowMolecule3DButtonNode.js
index 433516b9..a6493728 100644
--- a/js/common/view/view3d/ShowMolecule3DButtonNode.js
+++ b/js/common/view/view3d/ShowMolecule3DButtonNode.js
@@ -40,6 +40,7 @@ class ShowMolecule3DButtonNode extends RectangularPushButton {
       soundPlayer: Playable.NO_SOUND
     }, options ) );
 
+    //REVIEW: ButtonListener is deprecated, can we use FireListener instead?
     this.addInputListener( new ButtonListener( {
       fire() {
         showDialogCallback( completeMolecule );