Merging master=>main changes

dependencies.json

@@ -1,11 +1,11 @@
 {
-  "comment": "# keplers-laws 1.1.0-dev.2 Fri Aug 11 2023 14:52:46 GMT-0500 (Colombia Standard Time)",
+  "comment": "# keplers-laws 1.1.0-dev.3 Tue Aug 15 2023 12:12:21 GMT-0500 (Colombia Standard Time)",
   "assert": {
     "sha": "cc59d7f0574270924a886cb3d43a1b7507cd0553",
     "branch": "main"
   },
   "axon": {
-    "sha": "e5962f3e58a63096e2d52f19b368f91a79d6e984",
+    "sha": "192cb02fffb54fada63451c84593a374815972a6",
     "branch": "main"
   },
   "bamboo": {
@@ -29,7 +29,7 @@
     "branch": "main"
   },
   "keplers-laws": {
-    "sha": "111f7b06e024d788424314654f0cc2ad8115e844",
+    "sha": "7734830edebdd25e6177783a309e464898a9aeea",
     "branch": "main"
   },
   "kite": {
@@ -53,7 +53,7 @@
     "branch": "main"
   },
   "phet-io-sim-specific": {
-    "sha": "062f7ebd3c5c0937e4dc1883e0d2581dd9e1aa71",
+    "sha": "4cd449ab948d6f36d7a3142be3f83eab19c33deb",
     "branch": "main"
   },
   "phet-io-wrappers": {
@@ -77,7 +77,7 @@
     "branch": "main"
   },
   "scenery-phet": {
-    "sha": "ef286e8a8c435fe9051cb4f4aae00a7370b73c05",
+    "sha": "56031d4a215bba8ad78f3560af257823c250bd6b",
     "branch": "main"
   },
   "sherpa": {

js/KeplersLawsStrings.ts

@@ -47,6 +47,13 @@ type StringsType = {
     'aStringProperty': LocalizedStringProperty;
     'tStringProperty': LocalizedStringProperty;
   };
+  'units': {
+    'AUStringProperty': LocalizedStringProperty;
+    'AU2StringProperty': LocalizedStringProperty;
+    'yearsStringProperty': LocalizedStringProperty;
+    'kmsStringProperty': LocalizedStringProperty;
+    'degreesStringProperty': LocalizedStringProperty;
+  };
   'symbols': {
     'semiMajorAxisStringProperty': LocalizedStringProperty;
     'semiMajorAxisSquaredStringProperty': LocalizedStringProperty;

js/common/model/KeplersLawsModel.ts

@@ -49,11 +49,7 @@ class KeplersLawsModel extends SolarSystemCommonModel<EllipticalOrbitEngine> {
 
   // Stopwatch visibility
   public readonly stopwatchVisibleProperty = new BooleanProperty( false );
-  public readonly stopwatch = new Stopwatch( {
-    timePropertyOptions: {
-      units: 'years'
-    }
-  } );
+  public readonly stopwatch = new Stopwatch();
 
   // Booleans to keep track of which law is selected
   public readonly isFirstLawProperty = new BooleanProperty( false );
@@ -124,7 +120,6 @@ class KeplersLawsModel extends SolarSystemCommonModel<EllipticalOrbitEngine> {
     super( options );
 
     this.isSolarSystemProperty = new DerivedProperty( [ this.bodies[ 0 ].massProperty ], sunMass => sunMass === 200 );
-
     this.lawVisibilitiesMap.set( LawMode.FIRST_LAW, this.firstLawVisibilities );
     this.lawVisibilitiesMap.set( LawMode.SECOND_LAW, this.secondLawVisibilities );
     this.lawVisibilitiesMap.set( LawMode.THIRD_LAW, this.thirdLawVisibilities );

js/common/view/DistancesDisplayNode.ts

@@ -68,6 +68,7 @@ export default class DistancesDisplayNode extends VBox {
     public modelViewTransformProperty: TReadOnlyProperty<ModelViewTransform2>
   ) {
     super( {
+      isDisposable: false,
       spacing: 10,
       visibleProperty: DerivedProperty.and( [ model.stringsVisibleProperty, model.engine.allowedOrbitProperty ] )
     } );

js/common/view/EllipticalOrbitNode.ts

@@ -2,8 +2,6 @@
 /**
  * Visual Node for the Elliptical Orbit based on the Orbital Parameters
  *
- * TODO: The contents of this file should eventually be moved to individual files for each Law.
- *
  * @author Agustín Vallejo
  */
 
@@ -28,9 +26,9 @@ import Range from '../../../../dot/js/Range.js';
 import Utils from '../../../../dot/js/Utils.js';
 import KeplersLawsConstants from '../../KeplersLawsConstants.js';
 import PeriodTrackerNode from './PeriodTrackerNode.js';
-import SolarSystemCommonStrings from '../../../../solar-system-common/js/SolarSystemCommonStrings.js';
 import OrbitalSound from './OrbitalSound.js';
 import KeplersLawsColors from '../../KeplersLawsColors.js';
+import StringUtils from '../../../../phetcommon/js/util/StringUtils.js';
 
 
 export default class EllipticalOrbitNode extends Path {
@@ -258,15 +256,21 @@ export default class EllipticalOrbitNode extends Path {
         scale: 0.7,
         opacity: 0.8,
         numberFormatter: ( value: number ) => {
-          return Utils.toFixed( value, 2 ) + ' AU²';
+          return StringUtils.fillIn( KeplersLawsStrings.pattern.valueUnitsStringProperty, {
+            value: Utils.toFixed( value, 2 ),
+            units: KeplersLawsStrings.units.AU2StringProperty
+          } );
         }
       } ) );
       timeValueNumberDisplays.push( new NumberDisplay( timeValueProperty, timeValueRange, {
         scale: 0.7,
         opacity: 0.8,
         backgroundFill: '#aff',
         numberFormatter: ( value: number ) => {
-          return Utils.toFixed( value, 2 ) + ' ' + SolarSystemCommonStrings.units.yearsStringProperty.value;
+          return StringUtils.fillIn( KeplersLawsStrings.pattern.valueUnitsStringProperty, {
+            value: Utils.toFixed( value, 2 ),
+            units: KeplersLawsStrings.units.yearsStringProperty
+          } );
         }
       } ) );
     }

js/common/view/FirstLawGraph.ts

@@ -104,6 +104,7 @@ export default class FirstLawGraph extends AlignBox {
     } );
 
     super( new Node( {
+      isDisposable: false,
       children: [
         ...orbitAndValues,
         yAxis,

js/common/view/FirstLawPanels.ts

@@ -18,7 +18,6 @@ import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
 import Utils from '../../../../dot/js/Utils.js';
 import KeplersLawsStrings from '../../KeplersLawsStrings.js';
 import keplersLaws from '../../keplersLaws.js';
-import SolarSystemCommonStrings from '../../../../solar-system-common/js/SolarSystemCommonStrings.js';
 import KeplersLawsConstants from '../../KeplersLawsConstants.js';
 import Tandem from '../../../../tandem/js/Tandem.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
@@ -37,6 +36,7 @@ export default class FirstLawPanels extends VBox {
     } );
 
     super( {
+      isDisposable: false,
       margin: 5,
       children: [
         eccentricityPanel,
@@ -85,7 +85,7 @@ class ValuesPanel extends Panel {
   public constructor( model: KeplersLawsModel, providedOptions: PanelOptions ) {
 
     const conditionalAUStringProperty = new DerivedProperty(
-      [ SolarSystemCommonStrings.units.AUStringProperty, model.engine.allowedOrbitProperty ],
+      [ KeplersLawsStrings.units.AUStringProperty, model.engine.allowedOrbitProperty ],
       ( AUString, allowedOrbit ) => {
         return allowedOrbit ? AUString : '';
       } );
@@ -111,25 +111,25 @@ class ValuesPanel extends Panel {
 
     // Extra information: distance and velocity vector values
     const positionMagnitudeStringProperty = new PatternStringProperty( KeplersLawsStrings.pattern.valueUnitsStringProperty, {
-      units: 'AU',
+      units: KeplersLawsStrings.units.AUStringProperty,
       value: new DerivedProperty( [ model.bodies[ 1 ].positionProperty ], position => {
         return Utils.toFixed( position.magnitude / 100, 2 );
       } )
     }, { tandem: Tandem.OPT_OUT } );
     const velocityMagnitudeStringProperty = new PatternStringProperty( KeplersLawsStrings.pattern.valueUnitsStringProperty, {
-      units: 'km/s',
+      units: KeplersLawsStrings.units.kmsStringProperty,
       value: new DerivedProperty( [ model.bodies[ 1 ].velocityProperty ], velocity => {
         return Utils.toFixed( velocity.magnitude * SolarSystemCommonConstants.VELOCITY_MULTIPLIER, 2 );
       } )
     }, { tandem: Tandem.OPT_OUT } );
     const distanceAngleStringProperty = new PatternStringProperty( KeplersLawsStrings.pattern.valueUnitsStringProperty, {
-      units: '°',
+      units: KeplersLawsStrings.units.degreesStringProperty,
       value: new DerivedProperty( [ model.bodies[ 1 ].positionProperty ], position => {
         return Utils.toFixed( Utils.toDegrees( position.angle ), 2 );
       } )
     }, { tandem: Tandem.OPT_OUT } );
     const velocityAngleStringProperty = new PatternStringProperty( KeplersLawsStrings.pattern.valueUnitsStringProperty, {
-      units: '°',
+      units: KeplersLawsStrings.units.degreesStringProperty,
       value: new DerivedProperty( [ model.bodies[ 1 ].velocityProperty ], velocity => {
         return Utils.toFixed( Utils.toDegrees( velocity.angle ), 2 );
       } )

js/common/view/InfoDialog.ts

@@ -73,6 +73,7 @@ export default class InfoDialog extends Dialog {
         } )
       ]
     } ), {
+      isDisposable: false,
       titleAlign: 'center',
       title: new Text( 'Info', { font: new PhetFont( 32 ) } )
     } );

js/common/view/KeplersLawsControls.ts

@@ -92,6 +92,7 @@ class KeplersLawsControls extends VBox {
     };
 
     super( {
+      isDisposable: false,
       children: [
         // Target orbit and individual laws checkboxes
         createVBox( [

js/common/view/KeplersLawsOrbitalInformationBox.ts

@@ -113,9 +113,7 @@ class KeplersLawsOrbitalInformationBox extends VBox {
       createCheckbox(
         model.periapsisVisibleProperty,
         KeplersLawsStrings.periapsisStringProperty,
-        //REVIEW: visibFle looks like a typo introduced in https://github.com/phetsims/my-solar-system/commit/8ed18445210b1f2fbfdc88759cdb0894b8a3004a
-        //REVIEW: it should be fixed
-        'periapsisVisibFleCheckbox',
+        'periapsisVisibleCheckbox',
         new XNode( {
           fill: 'gold',
           stroke: SolarSystemCommonColors.foregroundProperty,

js/common/view/KeplersLawsPreferencesNode.ts

@@ -19,6 +19,7 @@ import PreferencesDialogConstants from '../../../../joist/js/preferences/Prefere
 export default class KeplersLawsPreferencesNode extends PreferencesControl {
   public constructor() {
     super( {
+      isDisposable: false,
       labelNode: new Text( 'More Orbital Data', PreferencesDialogConstants.CONTROL_LABEL_OPTIONS ),
       descriptionNode: new RichText( 'Display information about the planet\'s position and velocity vectors in the First Law Screen',
         PreferencesDialogConstants.CONTROL_DESCRIPTION_OPTIONS ),

js/common/view/KeplersLawsScreenIcon.ts

@@ -27,7 +27,10 @@ export default class KeplersLawsScreenIcon extends ScreenIcon {
   public constructor( contents: Node ) {
     super(
       contents,
-      { fill: SolarSystemCommonColors.backgroundProperty }
+      {
+        fill: SolarSystemCommonColors.backgroundProperty,
+        isDisposable: false
+      }
     );
   }
 

js/common/view/KeplersLawsScreenView.ts

@@ -36,6 +36,7 @@ import Success_mp3 from '../../../sounds/Success_mp3.js';
 import BodiesCollide_mp3 from '../../../sounds/BodiesCollide_mp3.js';
 import ObjectWillEscape_mp3 from '../../../sounds/ObjectWillEscape_mp3.js';
 import OrbitTypes from '../model/OrbitTypes.js';
+import KeplersLawsStrings from '../../KeplersLawsStrings.js';
 
 // constants
 const MARGIN = 10;
@@ -60,7 +61,8 @@ class KeplersLawsScreenView extends SolarSystemCommonScreenView {
   public constructor( model: KeplersLawsModel, providedOptions?: KeplersLawsScreenViewOptions ) {
     const options = optionize<KeplersLawsScreenViewOptions, SelfOptions, SolarSystemCommonScreenViewOptions>()( {
       playingAllowedProperty: model.engine.allowedOrbitProperty,
-      allowLawSelection: false
+      allowLawSelection: false,
+      isDisposable: false
     }, providedOptions );
 
     super( model, options );
@@ -87,7 +89,8 @@ class KeplersLawsScreenView extends SolarSystemCommonScreenView {
     const body = model.bodies[ 1 ];
     const sunNode = new BodyNode( model.bodies[ 0 ], this.modelViewTransformProperty, {
       draggable: false,
-      focusable: false
+      focusable: false,
+      pickable: false
     } );
     const bodyNode = new BodyNode( body, this.modelViewTransformProperty, {
       useCueingArrows: true,
@@ -286,7 +289,8 @@ class KeplersLawsScreenView extends SolarSystemCommonScreenView {
           numberFormatter: StopwatchNode.createRichTextNumberFormatter( {
             showAsMinutesAndSeconds: false,
             numberOfDecimalPlaces: 2,
-            units: 'years'
+            valueUnitsPattern: KeplersLawsStrings.pattern.valueUnitsStringProperty,
+            units: KeplersLawsStrings.units.yearsStringProperty
           } )
         }
       }

js/common/view/KeplersLawsTimeControlNode.ts

@@ -75,7 +75,9 @@ export default class KeplersLawsTimeControlNode extends TimeControlNode {
       speedRadioButtonGroupOptions: {
         labelOptions: KeplersLawsConstants.TEXT_OPTIONS,
         touchAreaXDilation: 10
-      }
+      },
+
+      isDisposable: false
     }, providedOptions );
 
     super( model.isPlayingProperty, options );

js/common/view/LawsButtons.ts

@@ -33,7 +33,9 @@ export default class LawsButtons extends RectangularRadioButtonGroup<LawMode> {
         }
       },
       touchAreaXDilation: 5,
-      touchAreaYDilation: 10
+      touchAreaYDilation: 10,
+
+      isDisposable: false
     }, providedOptions );
 
 

js/common/view/OrbitalWarningMessage.ts

@@ -13,10 +13,10 @@ import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransfo
 import Vector2 from '../../../../dot/js/Vector2.js';
 import OrbitTypes from '../model/OrbitTypes.js';
 import KeplersLawsStrings from '../../KeplersLawsStrings.js';
-import Multilink from '../../../../axon/js/Multilink.js';
 import keplersLaws from '../../keplersLaws.js';
 import SolarSystemCommonConstants from '../../../../solar-system-common/js/SolarSystemCommonConstants.js';
 import { combineOptions } from '../../../../phet-core/js/optionize.js';
+import StringUtils from '../../../../phetcommon/js/util/StringUtils.js';
 
 export default class OrbitalWarningMessage extends Node {
 
@@ -26,46 +26,31 @@ export default class OrbitalWarningMessage extends Node {
 
     const center = modelViewTransformProperty.value.modelToViewPosition( new Vector2( 0, -50 ) );
 
-    //REVIEW: Why a local variable for this?
-    //REVIEW: Please create a DerivedProperty<string> for this, and then pass that to the RichText constructor 1st param
-    //REVIEW: instead of the multilink that is used.
-    //REVIEW: no need for `message` variable
-    let message = '';
-    const warningText = new RichText( message, combineOptions<TextOptions>( {
-      maxWidth: 500
-    }, SolarSystemCommonConstants.TITLE_OPTIONS ) );
-
-
-    Multilink.multilink(
+    const messageProperty = new DerivedProperty(
       [
         orbitTypeProperty,
         KeplersLawsStrings.warning.warningStringProperty,
         KeplersLawsStrings.warning.crashOrbitStringProperty,
         KeplersLawsStrings.warning.escapeOrbitStringProperty
       ],
       ( orbitType, warningString, crashOrbitString, escapeOrbitString ) => {
-        //REVIEW: I believe i18n patterns should be used for this, instead of string concatenation with a colon
-        //REVIEW: why is the colon, space, and placeholder not available in the warning.warning string?
-        message = warningString + ': ';
-        switch( orbitType ) {
-          case OrbitTypes.CRASH_ORBIT:
-            message += crashOrbitString;
-            break;
-          case OrbitTypes.ESCAPE_ORBIT:
-            message += escapeOrbitString;
-            break;
-          default:
-            break;
-        }
-
-        warningText.setString( message );
-
-        //REVIEW: This would be a separate link to keep things centered
-        warningText.center = center;
+        return StringUtils.fillIn( warningString, {
+          message: orbitType === OrbitTypes.CRASH_ORBIT ? crashOrbitString :
+                    orbitType === OrbitTypes.ESCAPE_ORBIT ? escapeOrbitString : ''
+        } );
       }
     );
 
+    const warningText = new RichText( messageProperty, combineOptions<TextOptions>( {
+      maxWidth: 500
+    }, SolarSystemCommonConstants.TITLE_OPTIONS ) );
+
+    messageProperty.link( () => {
+      warningText.center = center;
+    } );
+
     super( {
+      isDisposable: false,
       visibleProperty: DerivedProperty.not( allowedOrbitProperty ),
       center: center,
       children: [ warningText ]