Skip to content

Commit

Permalink
code review changes, documentation updates, see #33
Browse files Browse the repository at this point in the history
  • Loading branch information
mbarlow12 committed Oct 19, 2017
1 parent 5f5ce09 commit 269d905
Show file tree
Hide file tree
Showing 13 changed files with 18 additions and 18 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Coulombs Law
Coulomb's Law
================

"Coulombs Law" is an educational simulation in HTML5, by <a href="https://phet.colorado.edu/" target="_blank">PhET Interactive Simulations</a>
"Coulomb's Law" is an educational simulation in HTML5, by <a href="https://phet.colorado.edu/" target="_blank">PhET Interactive Simulations</a>
at the University of Colorado Boulder.

*This simulation is under development and has not been published.*
Expand Down
3 changes: 3 additions & 0 deletions coulombs-law-strings_en.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
"units.atomicLegendScale": {
"value": "10 pm"
},
"units.macroLegendScale": {
"value": "1 cm"
},
"pmScale": {
"value": "1 picometer (pm) = 1 x 10<sup>-12</sup> m"
},
Expand Down
2 changes: 0 additions & 2 deletions js/common/CoulombsLawColorProfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
1 change: 1 addition & 0 deletions js/common/CoulombsLawCommonConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
define( function ( require ) {
'use strict';

// modules
var coulombsLaw = require( 'COULOMBS_LAW/coulombsLaw' );
var Tandem = require( 'TANDEM/Tandem' );

Expand Down
4 changes: 1 addition & 3 deletions js/common/model/Charge.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Expand Down
1 change: 1 addition & 0 deletions js/common/model/CoulombsLawCommonModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ define( function( require ) {

// phet-io modules
var TBoolean = require( 'ifphetio!PHET_IO/types/TBoolean' );

/**
* @constructor
*/
Expand Down
1 change: 1 addition & 0 deletions js/common/view/ChargeControl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
} );
Expand Down
1 change: 1 addition & 0 deletions js/common/view/ChargeControlSliderThumb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
} );
Expand Down
3 changes: 2 additions & 1 deletion js/common/view/ChargeNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 );
}
} );
} );
10 changes: 2 additions & 8 deletions js/common/view/CoulombsLawCommonView.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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 );
} );
2 changes: 1 addition & 1 deletion js/coulombs-law-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions js/coulombs-law-main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
} );
Expand Down
3 changes: 2 additions & 1 deletion js/macro/view/CoulombsLawMacroView.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 269d905

Please sign in to comment.