phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
MIT License
8 stars 6 forks source link

Locale codes are not displayed in sims #905

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago

LanguageSelectionNode is the UI component that is used to select a language in the Preferences dialog. Here's what it looks like for the Localization tab:

screenshot_2247

Here's what it looks like for Number Play's "Second Language" preference:

screenshot_2246

While we've been developing Number Play (@chrisklus @marlitas), it's been frustrating to test features that are locale related. We have to keep looking up these Language strings in localeInfoModule.ts in order to find out what values to use for the locale and locales query parameter.

So...

(1) For development/testing, modify LanguageSelectionNode to include the locale code when a private query parameter is present.

(2) Consider whether this will be a problem for teachers and PhET-iO instructional designers. How will they determine what values to use for locale and locales? Should locale codes always be shown in the Localization preferences?

pixelzoom commented 1 year ago

In the above commit, I made this change to LanguageSelectionNode, so that running with ?dev will include locale codes:

+    // Include the locale code when running with ?dev.
+    const string = phet.chipper.queryParameters.dev ?
+                   `${localeInfoModule[ locale ].localizedName} (${locale})` :
+                   localeInfoModule[ locale ].localizedName;
+
-    const text = new Text( localeInfoModule[ locale ].localizedName, {
+    const text = new Text( string, {

Localization preferences looks like this:

screenshot_2245

And Number Play's "Second Language" preference looks like this

screenshot_2244

Assigning to @jessegreenberg to review the changes to LanguageSelectionNode.

Assigning to @arouinfar to answer question (2) above:

(2) Consider whether this will be a problem for teachers and PhET-iO instructional designers. How will they determine what values to use for locale and locales? Should locale codes always be shown in the Localization preferences?

If the answer to (2) is "yes", the we should modify LanguageSelectionNode accordingly. If the answer to (2) is "no", then we might consider using a different query parameter than ?dev for this.

jessegreenberg commented 1 year ago

Code change in https://github.com/phetsims/joist/commit/83f081c5d5d15e405c7021e6928df154ddc26b2e looks good to me.

arouinfar commented 1 year ago

(2) Consider whether this will be a problem for teachers and PhET-iO instructional designers. How will they determine what values to use for locale and locales? Should locale codes always be shown in the Localization preferences?

Is locales intended to be public? We tell PhET-iO clients about locale, but not locales. In the PhET-iO Guide, we recommend inspecting localeProperty to determine the appropriate locale code to use with the query parameter.

As far as I know, we haven't shared either of these query parameters with teachers, since the _all file isn't the version we deliver on the sim pages. Students can select their desired language directly from the Localization tab, and I don't think the locale code is meaningful to most people. If teachers want to share a specific locale with a student, they're likely accessing it from the Translations table on a sim page.

pixelzoom commented 1 year ago

OK, so it sounds like there's no general need/desire to show locale codes in the UI, and this issue can be closed.

But this concerns me:

Is locales intended to be public? We tell PhET-iO clients about locale, but not locales. In the PhET-iO Guide, we recommend inspecting localeProperty to determine the appropriate locale code to use with the query parameter.

locale and locales are both private in initialize-globales.js. If you're telling anyone outside of the PhET team about a query parameter, it needs to be set to public: true. Please open an issue in phet-core if that is the case for locale.

arouinfar commented 1 year ago

There's a note in initialize-globals about locale:

// Private Doc: DON'T add the public key here, because we want it to graceful bad values in production (falling back to en)

I think this means that we are intentionally not marking it as public: true even if it's being shared externally.

pixelzoom commented 1 year ago

That comment was added by @zepumph for https://github.com/phetsims/phet-io/issues/1882. So I guess we should him -- what did you mean by this barely readable comment? (And while you're there, please wordsmith it a bit.)

pixelzoom commented 1 year ago

Whilie I'm looking at this... I moved the comment to a more effective location - where the public option would be added, see below. I never noticed it when it was tacked on above the query-parameter description. I also attempted to wordsmith, but I'm not positive that I deciphered this correctly. @zepumph please refine if I did not get it right. Then feel free to close this issue.

    /**
     * Select the language of the sim to the specific locale. Default to "en".
     * @memberOf PhetQueryParameters
     * @type {string}
     */
    locale: {
      type: 'string',
      defaultValue: 'en'
      // Do NOT add the `public` key here. We want invalid values to fall back to en.
    },
zepumph commented 1 year ago

Amazing thanks. Closing