diff --git a/js/KeplersLawsStrings.ts b/js/KeplersLawsStrings.ts index 50b9cd3..e259669 100644 --- a/js/KeplersLawsStrings.ts +++ b/js/KeplersLawsStrings.ts @@ -69,7 +69,7 @@ type StringsType = { 'valueUnitsStringProperty': LocalizedStringProperty; }; 'warning': { - 'warningStringProperty': LocalizedStringProperty; + 'warningPatternStringProperty': LocalizedStringProperty; 'crashOrbitStringProperty': LocalizedStringProperty; 'escapeOrbitStringProperty': LocalizedStringProperty; }; diff --git a/js/common/view/KeplersLawsOrbitalInformationBox.ts b/js/common/view/KeplersLawsOrbitalInformationBox.ts index b783d25..19a6761 100644 --- a/js/common/view/KeplersLawsOrbitalInformationBox.ts +++ b/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, diff --git a/js/common/view/OrbitalWarningMessage.ts b/js/common/view/OrbitalWarningMessage.ts index 0566565..a4172c6 100644 --- a/js/common/view/OrbitalWarningMessage.ts +++ b/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,45 +26,29 @@ 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 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( { - maxWidth: 500 - }, SolarSystemCommonConstants.TITLE_OPTIONS ) ); - - - Multilink.multilink( + const messageProperty = new DerivedProperty( [ orbitTypeProperty, - KeplersLawsStrings.warning.warningStringProperty, + KeplersLawsStrings.warning.warningPatternStringProperty, 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( { + maxWidth: 500 + }, SolarSystemCommonConstants.TITLE_OPTIONS ) ); + + messageProperty.link( () => { + warningText.center = center; + } ); + super( { isDisposable: false, visibleProperty: DerivedProperty.not( allowedOrbitProperty ), diff --git a/keplers-laws-strings_en.json b/keplers-laws-strings_en.json index cbe8641..9273729 100644 --- a/keplers-laws-strings_en.json +++ b/keplers-laws-strings_en.json @@ -140,8 +140,8 @@ "pattern.valueUnits": { "value": "{{value}} {{units}}" }, - "warning.warning": { - "value": "Warning" + "warning.warningPattern": { + "value": "Warning: {{message}}" }, "warning.crashOrbit": { "value": "The body will crash into the sun"