phetsims / circuit-construction-kit-common

"Circuit Construction Kit: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
10 stars 10 forks source link

Make it possible to switch locales at runtime #960

Closed samreid closed 1 year ago

samreid commented 2 years ago

May be easier once https://github.com/phetsims/mean-share-and-balance/issues/99 is complete.

samreid commented 2 years ago

Also, on Thursday we discussed a fillInDynamic sort of method in StringUtils, which would help move this forward. I think @jonathanolson is assigned, but I couldn't see a paper trail.

samreid commented 2 years ago

I made a first pass at this and it's working well. Will need more attention once we have fillInDynamic and correct suffixes StringProperty. Also, do I need to addLinkedElement on this? I used TReadOnlyProperty<string> everywhere, but not sure if that's a match for studio autoselect.

samreid commented 2 years ago

Studio autoselect seems like it's working great. All that's left here is waiting on 3 features:

On hold until there is progress on those features.

jonathanolson commented 2 years ago

I believe all of those items are done (*StringProperty, PatternStringProperty, and the .value/.link inline)

samreid commented 2 years ago

The remaining work for this issue is to convert fillIn to PatternStringProperty. I started on that for the circuit element readouts (when values is checked).

```diff Index: js/view/ValueNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/view/ValueNode.ts b/js/view/ValueNode.ts --- a/js/view/ValueNode.ts (revision 0233cf6cfa0edd8695fc9683441bb327ab172ee9) +++ b/js/view/ValueNode.ts (date 1662151035025) @@ -31,6 +31,9 @@ import CircuitElement from '../model/CircuitElement.js'; import CircuitElementViewType from '../model/CircuitElementViewType.js'; import Multilink from '../../../axon/js/Multilink.js'; +import PatternStringProperty from '../../../phetcommon/js/util/PatternStringProperty.js'; +import DerivedProperty, { DerivedPropertyOptions } from '../../../axon/js/DerivedProperty.js'; +import TReadOnlyProperty from '../../../axon/js/TReadOnlyProperty.js'; const capacitanceFaradsSymbolStringProperty = circuitConstructionKitCommonStrings.capacitanceFaradsSymbolStringProperty; const fuseValueStringProperty = circuitConstructionKitCommonStrings.fuseValueStringProperty; @@ -64,6 +67,12 @@ font: FONT }, providedOptions ) ); +class ToFixedProperty extends DerivedProperty { + public constructor( dependency: TReadOnlyProperty, numberOfDecimalPlaces: number, options?: DerivedPropertyOptions ) { + super( [ dependency ], ( value: number ) => Utils.toFixed( value, numberOfDecimalPlaces ), options ); + } +} + const infinitySpan = ''; export default class ValueNode extends Panel { @@ -83,14 +92,14 @@ let update: null | ( () => void ) = null; if ( circuitElement instanceof VoltageSource ) { - const voltageText = createText( tandem.createTandem( 'voltageText' ) ); - const voltageListener = ( voltage: number ) => { - - voltageText.text = StringUtils.fillIn( voltageUnitsStringProperty, { - voltage: Utils.toFixed( voltage, circuitElement.numberOfDecimalPlaces ) - } ); - update && update(); - }; + const voltageText = new Text( new PatternStringProperty( voltageUnitsStringProperty, { + voltage: new ToFixedProperty( circuitElement.voltageProperty, circuitElement.numberOfDecimalPlaces ) + } ), { + tandem: tandem.createTandem( 'voltageText' ), + font: FONT + } ); + + const voltageListener = ( voltage: number ) => update && update(); circuitElement.voltageProperty.link( voltageListener ); // Battery readouts shows voltage and internal resistance if it is nonzero ```

However, I found that these are not even PhET-iO instrumented. So it seems a question for the designers: which cases of fillIn should be phet-io instrumented and/or studio autoselectable?

UPDATE: I'm also asking since it is nontrivial to add these features, so I want to make sure it's desired.

matthew-blackman commented 1 year ago

@samreid and I tested this with locales=* and runtime changes to locale are updating the sim strings nicely, so this issue can be closed. There are a few strings that are not being updated, but we will create a new issue for this.