zepumph
commented
3 weeks ago This patch is much more put together, and I think we are close to a commit point. This is the change set meant for main, and not to be MR'd to all hydrogen sims.
```diff
Subject: [PATCH] better debug-info support for strictAxonDependencies, https://github.com/phetsims/aqua/issues/212
---
Index: chipper/js/initialize-globals.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/initialize-globals.js b/chipper/js/initialize-globals.js
--- a/chipper/js/initialize-globals.js (revision 739e543ddef46bcda67e058ed0e0e2a422142fbd)
+++ b/chipper/js/initialize-globals.js (date 1718060867163)
@@ -34,22 +34,28 @@
assert && assert( window.QueryStringMachine, 'QueryStringMachine is used, and should be loaded before this code runs' );
+ // Create the attachment point for all PhET globals
+ window.phet = window.phet ?? {};
+ window.phet.chipper = window.phet.chipper ?? {};
+
// packageObject may not always be available if initialize-globals used without chipper-initialization.js
- const packageObject = _.hasIn( window, 'phet.chipper.packageObject' ) ? phet.chipper.packageObject : {};
- const packagePhet = packageObject.phet || {};
+ const packageObject = phet.chipper.packageObject ?? {};
+ const packagePhet = packageObject.phet ?? {};
// Not all runtimes will have this flag, so be graceful
- const allowLocaleSwitching = _.hasIn( window, 'phet.chipper.allowLocaleSwitching' ) ? phet.chipper.allowLocaleSwitching : true;
+ const allowLocaleSwitching = phet.chipper.allowLocaleSwitching ?? true;
// duck type defaults so that not all package.json files need to have a phet.simFeatures section.
- const packageSimFeatures = packagePhet.simFeatures || {};
+ const packageSimFeatures = packagePhet.simFeatures ?? {};
// The color profile used by default, if no colorProfiles are specified in package.json.
// NOTE: Duplicated in SceneryConstants.js since scenery does not include initialize-globals.js
const DEFAULT_COLOR_PROFILE = 'default';
+ const FALLBACK_LOCALE = 'en';
+
// The possible color profiles for the current simulation.
- const colorProfiles = packageSimFeatures.colorProfiles || [ DEFAULT_COLOR_PROFILE ];
+ const colorProfiles = packageSimFeatures.colorProfiles ?? [ DEFAULT_COLOR_PROFILE ];
// Private Doc: Note: the following jsdoc is for the public facing PhET-iO API. In addition, all query parameters in the schema
// that are a "memberOf" the "PhetQueryParameters" namespace are used in the jsdoc that is public (client facing)
@@ -323,7 +329,7 @@
*/
locale: {
type: 'string',
- defaultValue: 'en'
+ defaultValue: window.phet.chipper.locale ?? FALLBACK_LOCALE
// Do NOT add the `public` key here. We want invalid values to fall back to en.
},
@@ -956,12 +962,8 @@
}
};
- // Initialize query parameters, see docs above
- ( function() {
-
- // Create the attachment point for all PhET globals
- window.phet = window.phet || {};
- window.phet.chipper = window.phet.chipper || {};
+ // Initialize query parameters in a new scope, see docs above
+ {
// Read query parameters
window.phet.chipper.queryParameters = QueryStringMachine.getAll( QUERY_PARAMETERS_SCHEMA );
@@ -1028,70 +1030,119 @@
stringTest;
};
- // We will need to check for locale validity (once we have localeData loaded, if running unbuilt), and potentially
- // either fall back to `en`, or remap from 3-character locales to our locale keys.
- phet.chipper.checkAndRemapLocale = () => {
- // We need both to proceed. Provided as a global, so we can call it from load-unbuilt-strings
- // (IF initialize-globals loads first)
- if ( !phet.chipper.localeData || !phet.chipper.locale ) {
- return;
- }
+ phet.chipper.remapLocale = ( locale, assertInsteadOfWarn = false ) => {
+ assert && assert( locale );
+ assert && assert( phet.chipper.localeData );
- let locale = phet.chipper.locale;
+ const inputValueLocale = locale;
- if ( locale ) {
- if ( locale.length < 5 ) {
- locale = locale.toLowerCase();
- }
- else {
- locale = locale.replace( /-/, '_' );
+ if ( locale.length < 5 ) {
+ locale = locale.toLowerCase();
+ }
+ else {
+ locale = locale.replace( /-/, '_' );
- const parts = locale.split( '_' );
- if ( parts.length === 2 ) {
- locale = parts[ 0 ].toLowerCase() + '_' + parts[ 1 ].toUpperCase();
- }
- }
+ const parts = locale.split( '_' );
+ if ( parts.length === 2 ) {
+ locale = parts[ 0 ].toLowerCase() + '_' + parts[ 1 ].toUpperCase();
+ }
+ }
- if ( locale.length === 3 ) {
- for ( const candidateLocale of Object.keys( phet.chipper.localeData ) ) {
- if ( phet.chipper.localeData[ candidateLocale ].locale3 === locale ) {
- locale = candidateLocale;
- break;
+ if ( locale.length === 3 ) {
+ for ( const candidateLocale of Object.keys( phet.chipper.localeData ) ) {
+ if ( phet.chipper.localeData[ candidateLocale ].locale3 === locale ) {
+ locale = candidateLocale;
+ break;
+ }
+ }
+ }
+
+ // Permissive patterns for locale query parameter patterns.
+ // We don't want to show a query parameter warning if it matches these patterns, EVEN if it is not a valid locale
+ // in localeData, see https://github.com/phetsims/qa/issues/1085#issuecomment-2111105235.
+ const pairRegex = /^[a-zA-Z]{2}$/;
+ const tripleRegex = /^[a-zA-Z]{3}$/;
+ const doublePairRegex = /^[a-zA-Z]{2}[_-][a-zA-Z]{2}$/;
+
+ // Sanity checks for verifying localeData (so hopefully we don't commit bad data to localeData).
+ if ( assert ) {
+ for ( const locale of Object.keys( phet.chipper.localeData ) ) {
+ // Check the locale itself
+ assert( pairRegex.test( locale ) || doublePairRegex.test( locale ), `Invalid locale format: ${locale}` );
+
+ // Check locale3 (if it exists)
+ if ( phet.chipper.localeData[ locale ].locale3 ) {
+ assert( tripleRegex.test( phet.chipper.localeData[ locale ].locale3 ), `Invalid locale3 format: ${phet.chipper.localeData[ locale ].locale3}` );
+ }
+
+ // Check fallbackLocales (if it exists)
+ if ( phet.chipper.localeData[ locale ].fallbackLocales ) {
+ for ( const fallbackLocale of phet.chipper.localeData[ locale ].fallbackLocales ) {
+ assert( phet.chipper.localeData[ fallbackLocale ] );
}
}
}
}
if ( !phet.chipper.localeData[ locale ] ) {
- const badLocale = phet.chipper.queryParameters.locale;
+ const badLocale = inputValueLocale;
- // Be permissive with case for the query parameter warning, see https://github.com/phetsims/qa/issues/1085#issuecomment-2111105235
- const isPair = /^[a-zA-Z]{2}$/.test( badLocale );
- const isTriple = /^[a-zA-Z]{3}$/.test( badLocale );
- const isPair_PAIR = /^[a-zA-Z]{2}[_-][a-zA-Z]{2}$/.test( badLocale );
+ if ( !pairRegex.test( badLocale ) && !tripleRegex.test( badLocale ) && !doublePairRegex.test( badLocale ) ) {
+ if ( assertInsteadOfWarn ) {
+ assert && assert( false, 'invalid locale:', inputValueLocale );
+ }
+ else {
+ QueryStringMachine.addWarning( 'locale', inputValueLocale, `Invalid locale format received: ${badLocale}. ?locale query parameter accepts the following formats: "xx" for ISO-639-1, "xx_XX" for ISO-639-1 and a 2-letter country code, "xxx" for ISO-639-2` );
+ }
+ }
- if ( !isPair && !isTriple && !isPair_PAIR ) {
- QueryStringMachine.addWarning( 'locale', phet.chipper.queryParameters.locale, `Invalid locale format received: ${badLocale}. ?locale query parameter accepts the following formats: "xx" for ISO-639-1, "xx_XX" for ISO-639-1 and a 2-letter country code, "xxx" for ISO-639-2` );
- }
+ locale = FALLBACK_LOCALE;
+ }
- locale = 'en';
+ return locale;
+ };
+
+ // Get the "most" valid locale, see https://github.com/phetsims/phet-io/issues/1882
+ // As part of https://github.com/phetsims/joist/issues/963, this as changed. We check a specific fallback order based
+ // on the locale. In general, it will usually try a prefix for xx_XX style locales, e.g. 'ar_SA' would try 'ar_SA', 'ar', 'en'
+ // NOTE: If the locale doesn't actually have any strings: THAT IS OK! Our string system will use the appropriate
+ // fallback strings.
+ phet.chipper.getValidRuntimeLocale = locale => {
+ const possibleLocales = [
+ locale,
+ ...( phet.chipper.localeData[ locale ]?.fallbackLocales ?? [] ),
+ FALLBACK_LOCALE
+ ];
+
+ return possibleLocales.find( possibleLocale => !!phet.chipper.strings[ possibleLocale ] );
+ };
+
+ // We will need to check for locale validity (once we have localeData loaded, if running unbuilt), and potentially
+ // either fall back to `en`, or remap from 3-character locales to our locale keys. This overwrites phet.chipper.locale.
+ // Used when setting locale through JOIST/localeProperty also.
+ phet.chipper.checkAndRemapLocale = ( locale = phet.chipper.locale, assertInsteadOfWarn = false ) => {
+ // We need both to proceed. Provided as a global, so we can call it from load-unbuilt-strings
+ // (IF initialize-globals loads first)
+ if ( !phet.chipper.localeData || !phet.chipper.strings || !locale ) {
+ return locale;
}
- phet.chipper.locale = locale;
+ const remappedLocale = phet.chipper.remapLocale( locale, assertInsteadOfWarn );
+ const finalLocale = phet.chipper.getValidRuntimeLocale( remappedLocale );
+
+ // Export this for analytics, see gogole-analytics.js
+ // (Yotta and GA will want the non-fallback locale for now, for consistency)
+ phet.chipper.remappedLocale = remappedLocale;
+ phet.chipper.locale = finalLocale; // NOTE: this will change with every setting of JOIST/localeProperty
+ return finalLocale;
};
- // If locale was provided as a query parameter, then change the locale used by Google Analytics.
- if ( QueryStringMachine.containsKey( 'locale' ) ) {
- phet.chipper.locale = phet.chipper.queryParameters.locale;
+ // Query parameter default will pick up the phet.chipper.locale default from the built sim, if it exists.
+ phet.chipper.locale = phet.chipper.queryParameters.locale;
- // NOTE: If we are loading in unbuilt mode, this may execute BEFORE we have loaded localeData. We have a similar
- // remapping in load-unbuilt-strings when this happens.
- phet.chipper.checkAndRemapLocale();
- }
- else if ( !window.phet.chipper.locale ) {
- // Fill in a default
- window.phet.chipper.locale = 'en';
- }
+ // NOTE: If we are loading in unbuilt mode, this may execute BEFORE we have loaded localeData. We have a similar
+ // remapping in load-unbuilt-strings when this happens.
+ phet.chipper.checkAndRemapLocale();
const stringOverrides = JSON.parse( phet.chipper.queryParameters.strings || '{}' );
@@ -1115,7 +1166,7 @@
const fallbackLocales = [
phet.chipper.locale,
...( phet.chipper.localeData[ phet.chipper.locale ]?.fallbackLocales || [] ),
- ( phet.chipper.locale !== 'en' ? [ 'en' ] : [] )
+ ( phet.chipper.locale !== FALLBACK_LOCALE ? [ FALLBACK_LOCALE ] : [] )
];
let stringMap = null;
@@ -1129,7 +1180,7 @@
return phet.chipper.mapString( stringMap[ key ] );
};
- }() );
+ }
/**
* Utility function to pause synchronously for the given number of milliseconds.
Index: number-suite-common/js/common/view/LanguageAndVoiceControl.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/number-suite-common/js/common/view/LanguageAndVoiceControl.ts b/number-suite-common/js/common/view/LanguageAndVoiceControl.ts
--- a/number-suite-common/js/common/view/LanguageAndVoiceControl.ts (revision 2c829f16f30d1d4869786781bc5bde00159d39d0)
+++ b/number-suite-common/js/common/view/LanguageAndVoiceControl.ts (date 1718045176740)
@@ -11,8 +11,7 @@
import numberSuiteCommon from '../../numberSuiteCommon.js';
import { Color, HBox, HBoxOptions, ManualConstraint, Node, RichText, RichTextOptions, Text, TextOptions, VBox } from '../../../../scenery/js/imports.js';
import optionize, { combineOptions, EmptySelfOptions } from '../../../../phet-core/js/optionize.js';
-import { Locale } from '../../../../joist/js/i18n/localeProperty.js';
-import Property from '../../../../axon/js/Property.js';
+import { Locale, LocaleProperty } from '../../../../joist/js/i18n/localeProperty.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
import Carousel, { CarouselItem, CarouselOptions } from '../../../../sun/js/Carousel.js';
import TProperty from '../../../../axon/js/TProperty.js';
@@ -45,7 +44,7 @@
export default class LanguageAndVoiceControl extends HBox {
- public constructor( localeProperty: Property,
+ public constructor( localeProperty: LocaleProperty,
voiceProperty: TProperty,
utteranceQueue: NumberSuiteCommonUtteranceQueue,
providedOptions?: LanguageAndVoiceControlOptions ) {
@@ -60,7 +59,7 @@
}, providedOptions );
// Carousel for choosing a language.
- const languageCarouselItems: LanguageCarouselItem[] = localeProperty.validValues!.map(
+ const languageCarouselItems: LanguageCarouselItem[] = localeProperty.availableRuntimeLocales.map(
locale => {
return {
locale: locale,
Index: chipper/js/LocalizedStringProperty.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/LocalizedStringProperty.ts b/chipper/js/LocalizedStringProperty.ts
--- a/chipper/js/LocalizedStringProperty.ts (revision 739e543ddef46bcda67e058ed0e0e2a422142fbd)
+++ b/chipper/js/LocalizedStringProperty.ts (date 1718060867146)
@@ -31,7 +31,7 @@
super( localeProperty, {
derive: ( locale: Locale ) => localizedString.getLocaleSpecificProperty( locale ),
- bidirectional: true,
+ bidirectional: true, // TODO: Why is this allowed to change the localeProperty, https://github.com/phetsims/joist/issues/970
phetioValueType: StringIO,
phetioState: false,
tandem: tandem,
Index: chipper/js/load-unbuilt-strings.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/chipper/js/load-unbuilt-strings.js b/chipper/js/load-unbuilt-strings.js
--- a/chipper/js/load-unbuilt-strings.js (revision 739e543ddef46bcda67e058ed0e0e2a422142fbd)
+++ b/chipper/js/load-unbuilt-strings.js (date 1718060867151)
@@ -177,6 +177,10 @@
// The callback to execute when all string files are processed.
const finishProcessing = () => {
+ // Because load-unbuilt-strings' "loading" of the locale data and strings might not have happened BEFORE initialize-globals
+ // runs (and sets phet.chipper.locale), we'll attempt to handle the case where it hasn't been set yet. You'll see the same call over in initialize-globals
+ phet.chipper.checkAndRemapLocale && phet.chipper.checkAndRemapLocale();
+
// Progress with loading modules
window.phet.chipper.loadModules();
};
@@ -207,10 +211,6 @@
requestJSONFile( '../babel/localeData.json', json => {
phet.chipper.localeData = json;
- // Because load-unbuilt-strings' "loading" of the locale data might not have happened BEFORE initialize-globals
- // runs (and sets phet.chipper.locale), we'll attempt to handle the case where it hasn't been set yet.
- phet.chipper.checkAndRemapLocale && phet.chipper.checkAndRemapLocale();
-
rtlLocales = Object.keys( phet.chipper.localeData ).filter( locale => {
return phet.chipper.localeData[ locale ].direction === 'rtl';
} );
Index: phetcommon/js/analytics/google-analytics.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/phetcommon/js/analytics/google-analytics.js b/phetcommon/js/analytics/google-analytics.js
--- a/phetcommon/js/analytics/google-analytics.js (revision 3d8102ba5a66bbe637793cb79ca1a0b52d7f497f)
+++ b/phetcommon/js/analytics/google-analytics.js (date 1718045176744)
@@ -15,13 +15,15 @@
return;
}
+ const locale = phet.chipper.remappedLocale;
+
assert && assert( window.phet && phet.chipper, 'We will require multiple things from the chipper preload namespace' );
assert && assert( !!phet.chipper.brand, 'A brand is required, since some messages depend on the brand' );
assert && assert( !!phet.chipper.queryParameters, 'We will need query parameters to be parsed for multiple purposes' );
assert && assert( !!phet.chipper.buildTimestamp, 'buildTimestamp is required for GA messages' );
assert && assert( !!phet.chipper.project, 'project is required for GA messages' );
assert && assert( !!phet.chipper.version, 'version is required for GA messages' );
- assert && assert( !!phet.chipper.locale, 'locale is required for GA messages' );
+ assert && assert( !!locale, 'locale is required for GA messages' );
const ua = navigator.userAgent;
const hasIESecurityRestrictions = !!( ua.match( /MSIE/ ) || ua.match( /Trident\// ) || ua.match( /Edge\// ) );
@@ -73,7 +75,7 @@
'project='}${encodeURIComponent( phet.chipper.project )}&` +
`brand=${encodeURIComponent( phet.chipper.brand )}&` +
`version=${encodeURIComponent( phet.chipper.version )}&` +
- `locale=${encodeURIComponent( phet.chipper.locale )}&` +
+ `locale=${encodeURIComponent( locale )}&` +
`buildTimestamp=${encodeURIComponent( phet.chipper.buildTimestamp )}&` +
`domain=${encodeURIComponent( document.domain )}&` +
`href=${encodeURIComponent( window.location.href )}&` +
@@ -154,7 +156,7 @@
simBrand: phet.chipper.brand,
simName: phet.chipper.project,
simVersion: phet.chipper.version,
- simLocale: phet.chipper.locale,
+ simLocale: locale,
simBuildTimestamp: phet.chipper.buildTimestamp,
simLoadType: loadType,
documentReferrer: document.referrer
Index: joist/js/preferences/LocalePanel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/preferences/LocalePanel.ts b/joist/js/preferences/LocalePanel.ts
--- a/joist/js/preferences/LocalePanel.ts (revision ff8657b06963957a9cd45db83fcb83690268c19e)
+++ b/joist/js/preferences/LocalePanel.ts (date 1718045176725)
@@ -13,16 +13,15 @@
import joist from '../joist.js';
import Panel from '../../../sun/js/Panel.js';
import { GridBox } from '../../../scenery/js/imports.js';
-import Property from '../../../axon/js/Property.js';
import LanguageSelectionNode from './LanguageSelectionNode.js';
-import { Locale } from '../i18n/localeProperty.js';
import JoistStrings from '../JoistStrings.js';
import StringUtils from '../../../phetcommon/js/util/StringUtils.js';
+import { LocaleProperty } from '../i18n/localeProperty.js';
class LocalePanel extends Panel {
- public constructor( localeProperty: Property ) {
+ public constructor( localeProperty: LocaleProperty ) {
- const locales = localeProperty.validValues!;
+ const locales = localeProperty.availableRuntimeLocales;
// Sort these properly by their localized name (without using _.sortBy, since string comparison does not provide
// a good sorting experience). See https://github.com/phetsims/joist/issues/965
Index: joist/js/i18n/localeProperty.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/i18n/localeProperty.ts b/joist/js/i18n/localeProperty.ts
--- a/joist/js/i18n/localeProperty.ts (revision ff8657b06963957a9cd45db83fcb83690268c19e)
+++ b/joist/js/i18n/localeProperty.ts (date 1718060867168)
@@ -13,53 +13,45 @@
import Tandem from '../../../tandem/js/Tandem.js';
import StringIO from '../../../tandem/js/types/StringIO.js';
import joist from '../joist.js';
-
-const FALLBACK_LOCALE = 'en';
+import { ReadOnlyPropertyState } from '../../../axon/js/ReadOnlyProperty.js';
export type Locale = keyof typeof localeInfoModule;
+assert && assert( phet.chipper.locale );
+assert && assert( phet.chipper.localeData );
+assert && assert( phet.chipper.strings );
+
// All available locales for the runtime
-export const availableRuntimeLocales = _.sortBy( Object.keys( phet.chipper.strings ), locale => {
+const availableRuntimeLocales = _.sortBy( Object.keys( phet.chipper.strings ), locale => {
return StringUtils.localeToLocalizedName( locale ).toLowerCase();
} ) as Locale[];
-// Start only with a valid locale, see https://github.com/phetsims/phet-io/issues/1882
-const isLocaleValid = ( locale?: Locale ): boolean => {
- return !!( locale && availableRuntimeLocales.includes( locale ) );
-};
-
-// Get the "most" valid locale, see https://github.com/phetsims/phet-io/issues/1882
-// As part of https://github.com/phetsims/joist/issues/963, this as changed. We check a specific fallback order based
-// on the locale. In general, it will usually try a prefix for xx_XX style locales, e.g. 'ar_SA' would try 'ar_SA', 'ar', 'en'
-// NOTE: If the locale doesn't actually have any strings: THAT IS OK! Our string system will use the appropriate
-// fallback strings.
-const validInitialLocale = [
- phet.chipper.locale,
- ...( phet.chipper.localeData[ phet.chipper.locale ]?.fallbackLocales ?? [] ),
- FALLBACK_LOCALE
-].find( isLocaleValid );
+export class LocaleProperty extends Property {
+ public readonly availableRuntimeLocales: Locale[] = availableRuntimeLocales;
-// Just in case we had an invalid locale, remap phet.chipper.locale to the "corrected" value
-phet.chipper.locale = validInitialLocale;
-
-class LocaleProperty extends Property {
protected override unguardedSet( value: Locale ): void {
- if ( availableRuntimeLocales.includes( value ) ) {
- super.unguardedSet( value );
- }
- else {
- assert && assert( false, 'Unsupported locale: ' + value );
+ // NOTE: updates phet.chipper.locale as a side-effect
+ super.unguardedSet( phet.chipper.checkAndRemapLocale( value, true ) );
+ }
+
+ protected override toStateObject(): ReadOnlyPropertyState {
+ const parentObject = super.toStateObject();
- // Do not try to set if the value was invalid
- }
+ // Provide via validValues without forcing validation assertions if a different value is set.
+ parentObject.validValues = this.availableRuntimeLocales as StateType[];
+ return parentObject;
+ }
+
+ protected override applyState( stateObject: ReadOnlyPropertyState ): void {
+ stateObject.validValues = null;
+ super.applyState( stateObject );
}
}
-const localeProperty = new LocaleProperty( validInitialLocale, {
+const localeProperty = new LocaleProperty( phet.chipper.locale, {
tandem: Tandem.GENERAL_MODEL.createTandem( 'localeProperty' ),
phetioFeatured: true,
phetioValueType: StringIO,
- validValues: availableRuntimeLocales,
phetioDocumentation: 'Specifies language currently displayed in the simulation'
} );
Index: number-suite-common/js/common/model/NumberSuiteCommonPreferences.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/number-suite-common/js/common/model/NumberSuiteCommonPreferences.ts b/number-suite-common/js/common/model/NumberSuiteCommonPreferences.ts
--- a/number-suite-common/js/common/model/NumberSuiteCommonPreferences.ts (revision 2c829f16f30d1d4869786781bc5bde00159d39d0)
+++ b/number-suite-common/js/common/model/NumberSuiteCommonPreferences.ts (date 1718060867154)
@@ -14,7 +14,7 @@
import Property from '../../../../axon/js/Property.js';
import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
-import localeProperty, { availableRuntimeLocales, Locale } from '../../../../joist/js/i18n/localeProperty.js';
+import localeProperty, { Locale, LocaleProperty } from '../../../../joist/js/i18n/localeProperty.js';
//TODO https://github.com/phetsims/number-suite-common/issues/18 type string map, perhaps getStringModule.TStringModule?
//TODO https://github.com/phetsims/number-suite-common/issues/18 replace any
@@ -29,7 +29,7 @@
public readonly showSecondLocaleProperty: Property;
// the second locale
- public readonly secondLocaleProperty: Property;
+ public readonly secondLocaleProperty: LocaleProperty;
// whether the Ones are included on the 'Lab' Screen
public readonly showLabOnesProperty: Property;
@@ -60,9 +60,7 @@
this.showSecondLocaleProperty = new BooleanProperty( !!NumberSuiteCommonQueryParameters.secondLocale );
// if a secondLocale was provided via a query parameter, use that, otherwise default to the primaryLocale
- this.secondLocaleProperty = new Property( NumberSuiteCommonQueryParameters.secondLocale as Locale || localeProperty.value, {
- validValues: availableRuntimeLocales
- } );
+ this.secondLocaleProperty = new LocaleProperty( NumberSuiteCommonQueryParameters.secondLocale as Locale || localeProperty.value ); // TODO: review this please, it was using validValues, https://github.com/phetsims/joist/issues/970
this.showLabOnesProperty = new BooleanProperty( NumberSuiteCommonQueryParameters.showLabOnes );
Index: joist/js/preferences/PreferencesModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/preferences/PreferencesModel.ts b/joist/js/preferences/PreferencesModel.ts
--- a/joist/js/preferences/PreferencesModel.ts (revision ff8657b06963957a9cd45db83fcb83690268c19e)
+++ b/joist/js/preferences/PreferencesModel.ts (date 1718045176731)
@@ -19,7 +19,7 @@
import optionize, { EmptySelfOptions } from '../../../phet-core/js/optionize.js';
import SpeechSynthesisAnnouncer from '../../../utterance-queue/js/SpeechSynthesisAnnouncer.js';
import Tandem from '../../../tandem/js/Tandem.js';
-import localeProperty, { Locale } from '../i18n/localeProperty.js';
+import localeProperty, { LocaleProperty } from '../i18n/localeProperty.js';
import merge from '../../../phet-core/js/merge.js';
import TReadOnlyProperty from '../../../axon/js/TReadOnlyProperty.js';
import IOType from '../../../tandem/js/types/IOType.js';
@@ -170,7 +170,7 @@
} & Required;
export type LocalizationModel = BaseModelType & {
- localeProperty: Property;
+ localeProperty: LocaleProperty;
} & Required;
type FeatureModel = SimulationModel | AudioModel | VisualModel | InputModel | LocalizationModel;
@@ -228,7 +228,7 @@
}, providedOptions.inputOptions ),
localizationOptions: optionize()( {
tandemName: 'localizationModel',
- supportsDynamicLocale: !!localeProperty.validValues && localeProperty.validValues.length > 1 && phet.chipper.queryParameters.supportsDynamicLocale,
+ supportsDynamicLocale: !!localeProperty.availableRuntimeLocales && localeProperty.availableRuntimeLocales.length > 1 && phet.chipper.queryParameters.supportsDynamicLocale,
customPreferences: [],
includeLocalePanel: true
}, providedOptions.localizationOptions )
@@ -256,7 +256,7 @@
// Running with english locale OR an environment where locale switching is supported and
// english is one of the available languages.
phet.chipper.locale.startsWith( 'en' ) ||
- ( phet.chipper.queryParameters.supportsDynamicLocale && _.some( localeProperty.validValues, value => value.startsWith( 'en' ) ) )
+ ( phet.chipper.queryParameters.supportsDynamicLocale && _.some( localeProperty.availableRuntimeLocales, value => value.startsWith( 'en' ) ) )
);
// Audio can be disabled explicitly via query parameter
Index: number-suite-common/js/common/view/SecondLanguageControl.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/number-suite-common/js/common/view/SecondLanguageControl.ts b/number-suite-common/js/common/view/SecondLanguageControl.ts
--- a/number-suite-common/js/common/view/SecondLanguageControl.ts (revision 2c829f16f30d1d4869786781bc5bde00159d39d0)
+++ b/number-suite-common/js/common/view/SecondLanguageControl.ts (date 1718052644033)
@@ -17,7 +17,7 @@
import ToggleSwitch from '../../../../sun/js/ToggleSwitch.js';
import PreferencesDialogConstants from '../../../../joist/js/preferences/PreferencesDialogConstants.js';
import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
-import { availableRuntimeLocales } from '../../../../joist/js/i18n/localeProperty.js';
+import localeProperty from '../../../../joist/js/i18n/localeProperty.js';
import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
import NumberSuiteCommonConstants from '../NumberSuiteCommonConstants.js';
import LanguageAndVoiceControl from './LanguageAndVoiceControl.js';
@@ -56,7 +56,7 @@
labelNode: labelText,
descriptionNode: descriptionText,
controlNode: toggleSwitch,
- enabled: ( availableRuntimeLocales.length > 1 ), // disabled if we do not have multiple locales available
+ enabled: ( localeProperty.availableRuntimeLocales.length > 1 ), // disabled if we do not have multiple locales available
ySpacing: NumberSuiteCommonConstants.PREFERENCES_DESCRIPTION_Y_SPACING
} );
Index: joist/js/preferences/VoicingPanelSection.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/preferences/VoicingPanelSection.ts b/joist/js/preferences/VoicingPanelSection.ts
--- a/joist/js/preferences/VoicingPanelSection.ts (revision ff8657b06963957a9cd45db83fcb83690268c19e)
+++ b/joist/js/preferences/VoicingPanelSection.ts (date 1718045176736)
@@ -111,7 +111,7 @@
// Voicing feature only works when running in English. If running in a version where you can change locale,
// indicate through the title that the feature will only work in English.
- const titleStringProperty = ( localeProperty.validValues && localeProperty.validValues.length > 1 ) ?
+ const titleStringProperty = ( localeProperty.availableRuntimeLocales && localeProperty.availableRuntimeLocales.length > 1 ) ?
voicingEnglishOnlyLabelStringProperty : voicingLabelStringProperty;
// the checkbox is the title for the section and totally enables/disables the feature
Index: number-suite-common/js/common/NumberSuiteCommonQueryParameters.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/number-suite-common/js/common/NumberSuiteCommonQueryParameters.ts b/number-suite-common/js/common/NumberSuiteCommonQueryParameters.ts
--- a/number-suite-common/js/common/NumberSuiteCommonQueryParameters.ts (revision 2c829f16f30d1d4869786781bc5bde00159d39d0)
+++ b/number-suite-common/js/common/NumberSuiteCommonQueryParameters.ts (date 1718060867157)
@@ -19,6 +19,7 @@
// specifies a second locale to make available on the 'Ten', 'Twenty', and 'Compare' screens. Values are specified
// with a locale code, e.g. "en" or "zh_CN".
+ // TODO: Use checkAndRemapLocales to support this one too, https://github.com/phetsims/joist/issues/970
secondLocale: {
public: true,
type: 'string',
```
This is a nice change because it doesn't involve any PhET-iO API changes to accomplish.
For MR:
- The PhET-iO changes may likely instead want to create a LocalePropertyIO the hacks the availableRuntimeLocales into the validValues in toStateObject.
- For the PhET branded changes, we will still want to be able to call checkAndRemapLocale from localeProperty, but I'm not sure how much of this patch needs to come along with that.
Next steps:
- [x] Work with @jonathanolson to commit this patch to main.
- [x] Work with @jonathanolson to create an MR patch for Hydrogen sims.
- [ ] add
supportsDynamicLocaleflag on localeProperty that just checks onavailableLocales length> 1 - [ ] aboutDialog links should use current localeProperty, instead of hard coded to the set locale when the dialog was created.
- [ ] sort the validValues in toStateObject for an alphabetical validValues in studio and the API file (maybe?)
- [ ] secondLocale in number suite should use this new logic.
- [ ] Yotta does not correctly track the locale for standard wrapper use. This is because the locale is not provided through a startup query parameter, and phet-io only changes the localeProperty, much later in the process than when GA and yotta process. We can likely fix this by providing the locale to the standard wrapper, knowing that it will flip flop later, but allowing the startup state to be correct for preloads.
- [ ] I noticed two entries in the QSM warning for the same query parameter, is this just because of unbuilt race condition? http://localhost:8080/center-and-variability/center-and-variability_en.html?brand=phet&ea&debugger&locale=es_Pfjdskal;fjkdsla;fj
brent-phet
From https://github.com/phetsims/joist/issues/963 and https://github.com/phetsims/chipper/issues/1441, lots of changes occurred to the locale system (like support for localeData in main/babel, locale3, hyphens and case-insensitivity), but we didn't get things working for Standard PhET-iO Wrappers to pass through the locale query parameter. This was reported in https://github.com/phetsims/phet-io/issues/1881#issuecomment-2142674672.
From this @jonathanolson @samreid and I have a new plan for how to support this. We want localeProperty to support setting values that are identical to any value you can provide via the query parameter. This means using the same logic as is in initialize-globals'
checkAndRemapLocale().Beginning work can be found here: https://github.com/phetsims/phet-io/issues/1881#issuecomment-2155281836 and below.