More REVIEW notes, see #173

phetsims · May 5, 2020 · 6e814b2 · 6e814b2
1 parent a5fe665
commit 6e814b2
Show file tree

Hide file tree

Showing 7 changed files with 8 additions and 0 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 );