Simplify secondLocale query parameter validation

[zepumph]

While working on #15, I found manual warnings for incorrect secondLocale when I believe we can just use the isValidValue feature when grabbing the query parameter itself.

number-suite-common/js/common/model/NumberSuiteCommonPreferences.ts

Lines 34 to 42 in 1d67efd

	const isSecondLocaleProvided = QueryStringMachine.containsKey( 'secondLocale' );
	const isSecondLocaleValid = !!phet.chipper.strings[ NumberSuiteCommonQueryParameters.secondLocale! ] &&
	Object.keys( phet.chipper.strings ).length > 1;
	if ( isSecondLocaleProvided && !isSecondLocaleValid ) {
	QueryStringMachine.addWarning( 'secondLocale', NumberSuiteCommonQueryParameters.secondLocale,
	`Second locale doesn't exist: ${NumberSuiteCommonQueryParameters.secondLocale}` );
	NumberSuiteCommonQueryParameters.secondLocale = phet.chipper.locale;
	}

Can instead become:

Subject: [PATCH] linking groups work in progress
---
Index: js/common/NumberSuiteCommonQueryParameters.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/NumberSuiteCommonQueryParameters.ts b/js/common/NumberSuiteCommonQueryParameters.ts
--- a/js/common/NumberSuiteCommonQueryParameters.ts	(revision 6a5e09e95d376d58b4be534ddb7ae1f653a23df0)
+++ b/js/common/NumberSuiteCommonQueryParameters.ts	(date 1673560444190)
@@ -6,6 +6,7 @@
  * @author Chris Klusendorf (PhET Interactive Simulations)
  */
 
+import localeInfoModule from '../../../chipper/js/data/localeInfoModule.js';
 import numberSuiteCommon from '../numberSuiteCommon.js';
 
 const NumberSuiteCommonQueryParameters = QueryStringMachine.getAll( {
@@ -22,6 +23,7 @@
   secondLocale: {
     public: true,
     type: 'string',
+    isValidValue: locale => !!locale && localeInfoModule.hasOwnProperty( locale ),
     defaultValue: phet.chipper.locale
   },
 

[zepumph]

I have to dart, but here was a working progress

Subject: [PATCH] use withServer, https://github.com/phetsims/chipper/issues/1371
---
Index: js/common/NumberSuiteCommonQueryParameters.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/NumberSuiteCommonQueryParameters.ts b/js/common/NumberSuiteCommonQueryParameters.ts
--- a/js/common/NumberSuiteCommonQueryParameters.ts	(revision 106d41dfbf3fa70c10acea0ba36d694cb9dbe1a6)
+++ b/js/common/NumberSuiteCommonQueryParameters.ts	(date 1673647433119)
@@ -22,7 +22,9 @@
   secondLocale: {
     public: true,
     type: 'string',
-    defaultValue: phet.chipper.locale
+    isValidValue: locale => !!locale && !!phet.chipper.strings[ locale ] &&
+                            Object.keys( phet.chipper.strings ).length > 1,
+    defaultValue: phet.chipper.locale // This default skates around localeProperty, and also why do we need a default for secondLocale
   },
 
   // whether the paper ones are visible on the 'Lab' screen
Index: js/common/model/NumberSuiteCommonPreferences.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/NumberSuiteCommonPreferences.ts b/js/common/model/NumberSuiteCommonPreferences.ts
--- a/js/common/model/NumberSuiteCommonPreferences.ts	(revision 106d41dfbf3fa70c10acea0ba36d694cb9dbe1a6)
+++ b/js/common/model/NumberSuiteCommonPreferences.ts	(date 1673647353870)
@@ -13,7 +13,7 @@
 import Property from '../../../../axon/js/Property.js';
 import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
-import { Locale } from '../../../../joist/js/i18n/localeProperty.js';
+import localeProperty, { Locale } from '../../../../joist/js/i18n/localeProperty.js';
 
 // TODO: type string map, perhaps getStringModule.TStringModule? https://github.com/phetsims/number-suite-common/issues/18
 //TODO https://github.com/phetsims/number-suite-common/issues/18 replace any
@@ -35,17 +35,8 @@
   public constructor() {
     this.readAloudProperty = new BooleanProperty( NumberSuiteCommonQueryParameters.readAloud );
 
-    const isSecondLocaleProvided = QueryStringMachine.containsKey( 'secondLocale' );
-    const isSecondLocaleValid = !!phet.chipper.strings[ NumberSuiteCommonQueryParameters.secondLocale! ] &&
-                                Object.keys( phet.chipper.strings ).length > 1;
-
-    if ( isSecondLocaleProvided && !isSecondLocaleValid ) {
-      QueryStringMachine.addWarning( 'secondLocale', NumberSuiteCommonQueryParameters.secondLocale,
-        `Second locale doesn't exist: ${NumberSuiteCommonQueryParameters.secondLocale}` );
-      NumberSuiteCommonQueryParameters.secondLocale = phet.chipper.locale;
-    }
-
-    this.showSecondLocaleProperty = new BooleanProperty( isSecondLocaleProvided && isSecondLocaleValid );
+    this.showSecondLocaleProperty = new BooleanProperty( QueryStringMachine.containsKey( 'secondLocale' ) &&
+                                                         NumberSuiteCommonQueryParameters.secondLocale !== localeProperty.value );
 
     this.secondLocaleProperty = new Property<Locale>( NumberSuiteCommonQueryParameters.secondLocale! as Locale );
 

[zepumph]

The the great help from @chrisklus, we got to a commit point. Lots of cleanup in the number suite common preferences, and how we initialize second-locale-related Properties and values. We also took this opportunity to factor out as many usages of phet.chipper.strings and phet.chipper.locale, since the globals seem sketchy compared to using the same local variables and/or factored out usages from localeProperty. Anything else here @chrisklus?

[chrisklus]

Thanks @zepumph! I tested with unbuilt, built en, and built all, and all seems to be working well. I really like these changes! I think we are all good to close here.