From 7eaceaf80647ff2ceef7ffedf57028e3c9862b29 Mon Sep 17 00:00:00 2001 From: denz1994 Date: Mon, 11 May 2020 17:02:57 -0400 Subject: [PATCH] Handling more REVIEW statements. https://github.com/phetsims/masses-and-springs/issues/359 --- build-a-molecule_en.html | 4 +--- js/common/view/KitNode.js | 9 +-------- js/common/view/KitPanel.js | 2 -- js/common/view/MoleculeBondNode.js | 31 ++++++++++++++---------------- js/common/view/WarningDialog.js | 3 --- js/multiple/MultipleScreen.js | 10 +++------- js/playground/PlaygroundScreen.js | 10 +++------- js/single/SingleScreen.js | 10 +++------- 8 files changed, 25 insertions(+), 54 deletions(-) diff --git a/build-a-molecule_en.html b/build-a-molecule_en.html index 31b94fc6..c6ede957 100644 --- a/build-a-molecule_en.html +++ b/build-a-molecule_en.html @@ -119,9 +119,7 @@ if ( brand === 'phet-io' ) { preloads = preloads.concat( [ '../phet-io/js/phet-io-initialize-globals.js', - '../build-a-molecule/js/phet-io/build-a-molecule-baseline.js', - '../build-a-molecule/js/phet-io/build-a-molecule-overrides.js', - '../build-a-molecule/js/phet-io/build-a-molecule-types.js' + '../build-a-molecule/js/phet-io/build-a-molecule-overrides.js' ] ); } diff --git a/js/common/view/KitNode.js b/js/common/view/KitNode.js index b1267117..aa21a78b 100644 --- a/js/common/view/KitNode.js +++ b/js/common/view/KitNode.js @@ -10,7 +10,6 @@ import Shape from '../../../../kite/js/Shape.js'; import BucketFront from '../../../../scenery-phet/js/bucket/BucketFront.js'; import BucketHole from '../../../../scenery-phet/js/bucket/BucketHole.js'; -import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; import DragListener from '../../../../scenery/js/listeners/DragListener.js'; import Node from '../../../../scenery/js/nodes/Node.js'; import buildAMolecule from '../../buildAMolecule.js'; @@ -49,13 +48,7 @@ class KitNode extends Node { // Create a bucket based on a the kit's model bucket. This includes a front and back for the bucket contained in // 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 - } ) - } ); + const bucketFront = new BucketFront( bucket, BAMConstants.MODEL_VIEW_TRANSFORM ); const bucketHole = new BucketHole( bucket, BAMConstants.MODEL_VIEW_TRANSFORM ); // NOTE: we will use the Bucket's hole with an expanded touch area to trigger the "grab by touching the bucket" behavior bucketHole.touchArea = bucketHole.mouseArea = new Shape() diff --git a/js/common/view/KitPanel.js b/js/common/view/KitPanel.js index 8213252d..51a2d34a 100644 --- a/js/common/view/KitPanel.js +++ b/js/common/view/KitPanel.js @@ -54,8 +54,6 @@ 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, buttonSoundPlayer: Playable.NO_SOUND diff --git a/js/common/view/MoleculeBondNode.js b/js/common/view/MoleculeBondNode.js index 0f8f3207..d6cde4c0 100644 --- a/js/common/view/MoleculeBondNode.js +++ b/js/common/view/MoleculeBondNode.js @@ -11,22 +11,19 @@ import platform from '../../../../phet-core/js/platform.js'; import Circle from '../../../../scenery/js/nodes/Circle.js'; import Line from '../../../../scenery/js/nodes/Line.js'; import Node from '../../../../scenery/js/nodes/Node.js'; -import closedUpCursorImage from '../../../images/scissors-closed-up_cur.js'; -import closedUpImage from '../../../images/scissors-closed-up_png.js'; -import closedCursorImage from '../../../images/scissors-closed_cur.js'; -import closedImage from '../../../images/scissors-closed_png.js'; -import upCursorImage from '../../../images/scissors-up_cur.js'; -import upImage from '../../../images/scissors-up_png.js'; -import cursorImage from '../../../images/scissors_cur.js'; +import scissorsClosedUpCursorImage from '../../../images/scissors-closed-up_cur.js'; +import scissorsClosedUpImage from '../../../images/scissors-closed-up_png.js'; +import scissorsClosedCursorImage from '../../../images/scissors-closed_cur.js'; +import scissorsClosedImage from '../../../images/scissors-closed_png.js'; +import scissorsUpCursorImage from '../../../images/scissors-up_cur.js'; +import scissorsUpImage from '../../../images/scissors-up_png.js'; +import scissorsCursorImage from '../../../images/scissors_cur.js'; import scissorsImage from '../../../images/scissors_png.js'; import buildAMolecule from '../../buildAMolecule.js'; import BAMConstants from '../BAMConstants.js'; import Direction from '../model/Direction.js'; import FireListener from '../../../../scenery/js/listeners/FireListener.js'; -//REVIEW: Imports don't match up with the image files, I would expect to see: -//REVIEW: scissorsClosedUpImage, scissorsClosedUpImage, scissorsClosedImage, scissorsClosedImage, scissorsUpImage, scissorsUpImage, scissorsImage - /* Notes on .cur file generation, all from the images directory, with "sudo apt-get install icoutils" for icotool: icotool -c -o scissors.ico scissors.png icotool -c -o scissors-closed.ico scissors-closed.png @@ -41,13 +38,13 @@ import FireListener from '../../../../scenery/js/listeners/FireListener.js'; const images = { 'scissors.png': scissorsImage, - 'scissors-closed.png': closedImage, - 'scissors-up.png': upImage, - 'scissors-closed-up.png': closedUpImage, - 'scissors.cur': cursorImage, - 'scissors-closed.cur': closedCursorImage, - 'scissors-up.cur': upCursorImage, - 'scissors-closed-up.cur': closedUpCursorImage + 'scissors-closed.png': scissorsClosedImage, + 'scissors-up.png': scissorsUpImage, + 'scissors-closed-up.png': scissorsClosedUpImage, + 'scissors.cur': scissorsCursorImage, + 'scissors-closed.cur': scissorsClosedCursorImage, + 'scissors-up.cur': scissorsUpCursorImage, + 'scissors-closed-up.cur': scissorsClosedUpCursorImage }; const bondRadius = 6; // "Radius" of the bond target that will break the bond diff --git a/js/common/view/WarningDialog.js b/js/common/view/WarningDialog.js index d6a39608..05c04774 100644 --- a/js/common/view/WarningDialog.js +++ b/js/common/view/WarningDialog.js @@ -34,9 +34,6 @@ class WarningDialog extends Dialog { ], spacing: 15, align: 'center', - - //REVIEW: What is this xMargin option for? - xMargin: 15, cursor: 'pointer' } ); warningNode.mouseArea = warningNode.touchArea = warningNode.localBounds; diff --git a/js/multiple/MultipleScreen.js b/js/multiple/MultipleScreen.js index 661cb7b0..7827891e 100644 --- a/js/multiple/MultipleScreen.js +++ b/js/multiple/MultipleScreen.js @@ -23,13 +23,9 @@ class MultipleScreen extends BAMScreen { backgroundColorProperty: new Property( BAMConstants.PLAY_AREA_BACKGROUND_COLOR ), homeScreenIcon: BAMIconFactory.createMultipleScreen() }; - super( //REVIEW: Prefer non-block arrow functions here, e.g. () => new MultipleModel(),... - () => { - return new MultipleModel(); - }, - model => { - return new MoleculeCollectingScreenView( model, false ); - }, + super( + () => new MultipleModel(), + model => new MoleculeCollectingScreenView( model, false ), options ); } } diff --git a/js/playground/PlaygroundScreen.js b/js/playground/PlaygroundScreen.js index af2f05ac..0ae055cb 100644 --- a/js/playground/PlaygroundScreen.js +++ b/js/playground/PlaygroundScreen.js @@ -23,13 +23,9 @@ class PlaygroundScreen extends BAMScreen { backgroundColorProperty: new Property( BAMConstants.PLAY_AREA_BACKGROUND_COLOR ), homeScreenIcon: BAMIconFactory.createPlaygroundScreen() }; - super( //REVIEW: Prefer non-block arrow functions here, e.g. () => new PlaygroundModel(),... - () => { - return new PlaygroundModel(); - }, - model => { - return new BAMScreenView( model ); - }, + super( + () => new PlaygroundModel(), + model => new BAMScreenView( model ), options ); } diff --git a/js/single/SingleScreen.js b/js/single/SingleScreen.js index 3460d936..1217f16b 100644 --- a/js/single/SingleScreen.js +++ b/js/single/SingleScreen.js @@ -23,13 +23,9 @@ class SingleScreen extends BAMScreen { backgroundColorProperty: new Property( BAMConstants.PLAY_AREA_BACKGROUND_COLOR ), homeScreenIcon: BAMIconFactory.createSingleScreen() }; - super( //REVIEW: Prefer non-block arrow functions here, e.g. () => new SingleModel(),... - () => { - return new SingleModel(); - }, - model => { - return new MoleculeCollectingScreenView( model, true ); - }, + super( + () => new SingleModel(), + model => new MoleculeCollectingScreenView( model, true ), options ); }