From 269d905cf21b1b9866c1c5896c86f17113c51a88 Mon Sep 17 00:00:00 2001 From: Michael Barlow Date: Wed, 18 Oct 2017 18:10:51 -0600 Subject: [PATCH] code review changes, documentation updates, see https://github.com/phetsims/coulombs-law/issues/33 --- README.md | 4 ++-- coulombs-law-strings_en.json | 3 +++ js/common/CoulombsLawColorProfile.js | 2 -- js/common/CoulombsLawCommonConstants.js | 1 + js/common/model/Charge.js | 4 +--- js/common/model/CoulombsLawCommonModel.js | 1 + js/common/view/ChargeControl.js | 1 + js/common/view/ChargeControlSliderThumb.js | 1 + js/common/view/ChargeNode.js | 3 ++- js/common/view/CoulombsLawCommonView.js | 10 ++-------- js/coulombs-law-config.js | 2 +- js/coulombs-law-main.js | 1 + js/macro/view/CoulombsLawMacroView.js | 3 ++- 13 files changed, 18 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 1ac10a4..4d9af0e 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ -Coulombs Law +Coulomb's Law ================ -"Coulombs Law" is an educational simulation in HTML5, by PhET Interactive Simulations +"Coulomb's Law" is an educational simulation in HTML5, by PhET Interactive Simulations at the University of Colorado Boulder. *This simulation is under development and has not been published.* diff --git a/coulombs-law-strings_en.json b/coulombs-law-strings_en.json index 7a448b8..f0e1dc8 100644 --- a/coulombs-law-strings_en.json +++ b/coulombs-law-strings_en.json @@ -35,6 +35,9 @@ "units.atomicLegendScale": { "value": "10 pm" }, + "units.macroLegendScale": { + "value": "1 cm" + }, "pmScale": { "value": "1 picometer (pm) = 1 x 10-12 m" }, diff --git a/js/common/CoulombsLawColorProfile.js b/js/common/CoulombsLawColorProfile.js index 80db62e..cd9f603 100644 --- a/js/common/CoulombsLawColorProfile.js +++ b/js/common/CoulombsLawColorProfile.js @@ -11,8 +11,6 @@ define(function(require) { var BLACK = new Color( 0, 0, 0 ); var WHITE = new Color( 255, 255, 255 ); var GREEN = new Color( 0, 255, 0 ); - // var RED = new Color( 255, 0, 0 ); - // var BLUE = new Color( 0, 0, 255 ); var CoulombsLawColorProfile = new ColorProfile( { background: { diff --git a/js/common/CoulombsLawCommonConstants.js b/js/common/CoulombsLawCommonConstants.js index 83f4f6c..b07782b 100644 --- a/js/common/CoulombsLawCommonConstants.js +++ b/js/common/CoulombsLawCommonConstants.js @@ -3,6 +3,7 @@ define( function ( require ) { 'use strict'; + // modules var coulombsLaw = require( 'COULOMBS_LAW/coulombsLaw' ); var Tandem = require( 'TANDEM/Tandem' ); diff --git a/js/common/model/Charge.js b/js/common/model/Charge.js index e93b783..eafe327 100644 --- a/js/common/model/Charge.js +++ b/js/common/model/Charge.js @@ -47,10 +47,8 @@ define( function( require ) { ISLCObject.call( this, initialCharge, initialPosition, valueRange, constantRadiusProperty, tandem, options ); // @public - object node color is will change with value - // radius changes will be moved into the Mass object - // color property will be changed and updated based on a boolean value (negative vs positive for Charge and Constant Radius for Mass) + // color property will be updated based on a boolean value (negative vs positive) // brightness will be set according to the Mass/Charge magnitude - this.baseColorProperty = new DerivedProperty( [ this.valueProperty ], function( value ) { diff --git a/js/common/model/CoulombsLawCommonModel.js b/js/common/model/CoulombsLawCommonModel.js index a4ed266..cf869f9 100644 --- a/js/common/model/CoulombsLawCommonModel.js +++ b/js/common/model/CoulombsLawCommonModel.js @@ -16,6 +16,7 @@ define( function( require ) { // phet-io modules var TBoolean = require( 'ifphetio!PHET_IO/types/TBoolean' ); + /** * @constructor */ diff --git a/js/common/view/ChargeControl.js b/js/common/view/ChargeControl.js index 8f75d1b..c183368 100644 --- a/js/common/view/ChargeControl.js +++ b/js/common/view/ChargeControl.js @@ -50,6 +50,7 @@ define( function( require ) { // varies between -10 and 10 this.chargeControlProperty = new Property( objectProperty.get() * scaleFactor ); + // no unlinking/disposing required as property is never destroyed this.chargeControlProperty.link( function( value ) { objectProperty.set( value / scaleFactor ); } ); diff --git a/js/common/view/ChargeControlSliderThumb.js b/js/common/view/ChargeControlSliderThumb.js index e27e7fc..cb20633 100644 --- a/js/common/view/ChargeControlSliderThumb.js +++ b/js/common/view/ChargeControlSliderThumb.js @@ -35,6 +35,7 @@ define( function( require ) { var self = this; // fills are axon Properties because they need to change with the objectProperty + // Since sliders are never disposed in the sim, there's no need to unlink the derived properties' functions var fillEnabledProperty = new DerivedProperty( [ objectProperty ], function( value ) { return self.getUpdatedFill( value ); } ); diff --git a/js/common/view/ChargeNode.js b/js/common/view/ChargeNode.js index 82ec6bf..835e7ce 100644 --- a/js/common/view/ChargeNode.js +++ b/js/common/view/ChargeNode.js @@ -69,6 +69,7 @@ define( function( require ) { ISLCObjectNode.call( this, model, chargeObjectModel, layoutBounds, modelViewTransform, pullForceRange, tandem.createTandem( 'chargeNode1' ), options ); + // scientific notation property is never removed/destroyed, no disposal required this.model.scientificNotationProperty.lazyLink( this.redrawForce.bind( this ) ); this.objectCircle.stroke = 'black'; @@ -91,7 +92,7 @@ define( function( require ) { }, redrawForce: function () { this.arrowNode.scientificNotationMode = this.model.scientificNotationProperty.get(); - ISLCObjectNode.prototype.redrawForce.call( this ); + ISLCObjectNode.prototype.redrawForce.call( this ); } } ); } ); diff --git a/js/common/view/CoulombsLawCommonView.js b/js/common/view/CoulombsLawCommonView.js index 82cbd19..813dd29 100644 --- a/js/common/view/CoulombsLawCommonView.js +++ b/js/common/view/CoulombsLawCommonView.js @@ -120,6 +120,7 @@ define( function( require ) { this.addChild( charge2Control ); // Reset All button + // buttons are never disposed in this sim var resetAllButton = new ResetAllButton( { listener: function() { coulombsLawModel.reset(); @@ -158,12 +159,5 @@ define( function( require ) { coulombsLaw.register( 'CoulombsLawCommonView', CoulombsLawCommonView ); - return inherit( ScreenView, CoulombsLawCommonView, { - - //TODO Called by the animation loop. Optional, so if your view has no animation, please delete this. - // @public - step: function( dt ) { - //TODO Handle view animation here. - } - } ); + return inherit( ScreenView, CoulombsLawCommonView ); } ); \ No newline at end of file diff --git a/js/coulombs-law-config.js b/js/coulombs-law-config.js index 19f60bd..6aea24b 100644 --- a/js/coulombs-law-config.js +++ b/js/coulombs-law-config.js @@ -20,7 +20,7 @@ require.config( { text: '../../sherpa/lib/text-2.0.12', // PhET plugins - audio: '../../chipper/js/requirejs-plugins/audio', + // audio: '../../chipper/js/requirejs-plugins/audio', image: '../../chipper/js/requirejs-plugins/image', mipmap: '../../chipper/js/requirejs-plugins/mipmap', string: '../../chipper/js/requirejs-plugins/string', diff --git a/js/coulombs-law-main.js b/js/coulombs-law-main.js index f89beeb..b5d2f42 100644 --- a/js/coulombs-law-main.js +++ b/js/coulombs-law-main.js @@ -37,6 +37,7 @@ define( function( require ) { optionsNode: new CoulombsLawGlobalOptionsNode( CoulombsLawCommonConstants.GLOBALS_TANDEM.createTandem( 'options' ) ) }; + // projectorModeProperty is never destroyed after initialization, disposal unnecessary CoulombsLawGlobals.projectorModeProperty.link( function( inProjectorMode ) { CoulombsLawColorProfile.profileNameProperty.set( inProjectorMode ? 'projector' : 'default' ); } ); diff --git a/js/macro/view/CoulombsLawMacroView.js b/js/macro/view/CoulombsLawMacroView.js index 704eeec..8bbcc4f 100644 --- a/js/macro/view/CoulombsLawMacroView.js +++ b/js/macro/view/CoulombsLawMacroView.js @@ -22,6 +22,7 @@ define( function( require ) { var charge2AbbreviatedString = require( 'string!COULOMBS_LAW/charge2Abbreviated' ); var charge2String = require( 'string!COULOMBS_LAW/charge2' ); var unitsMicrocoulombsString = require( 'string!COULOMBS_LAW/units.microcoulombs'); + var unitsMacroLegendScaleString = require( 'string!COULOMBS_LAW/units.macroLegendScale' ); // constants var CHARGE_NODE_Y_POSITION = 205; @@ -96,7 +97,7 @@ define( function( require ) { var legendNode = new ISLCLegendNode( legendNodeLineLength, // length of the line - '1 cm', // unit string + unitsMacroLegendScaleString, // unit string { fill: CoulombsLawColorProfile.legendNodeFillProperty, bottom: this.layoutBounds.maxY - 10,