phetsims / qa

Quality Assurance Task Tracking
MIT License
12 stars 8 forks source link

Locale MR 2 - PhET-iO Hydrogen Standard Wrapper changes #1098

Closed zepumph closed 2 months ago

zepumph commented 2 months ago

Hello QA! We have another MR related to recent locale changes. This time it is only for PhET-iO Hydrogen simulations. The goal is to support the new form of ?locale in the PhET-iO Standard Wrapper. The previous changeset did not do this correctly.

Some issue linking: https://github.com/phetsims/joist/issues/970 https://github.com/phetsims/phet-io/issues/1881 https://github.com/phetsims/chipper/issues/1441 https://github.com/phetsims/qa/issues/1089 central main locale feature issue: https://github.com/phetsims/joist/issues/963 https://github.com/phetsims/joist/issues/965

previous QA testing: original dev tests: https://github.com/phetsims/qa/issues/1085 phet brand MR: https://github.com/phetsims/qa/issues/1089

@jonathanolson @kathy-phet @brent-phet @JacquiHayes @arouinfar.

The feature is described here: https://github.com/phetsims/phet-io/issues/1881#issuecomment-1287618206

For testing, here is an outline of the things to test for each sim.

Thanks again and let me know if you have any questions.

Links:

acid-base-solutions 1.3 (https://github.com/phetsims/joist/issues/970)

beers-law-lab 1.7 (https://github.com/phetsims/joist/issues/970)

calculus-grapher 1.0 (https://github.com/phetsims/joist/issues/970)

center-and-variability 1.1 (https://github.com/phetsims/joist/issues/970)

circuit-construction-kit-dc 1.3 (https://github.com/phetsims/joist/issues/970)

circuit-construction-kit-dc-virtual-lab 1.3 (https://github.com/phetsims/joist/issues/970)

concentration 1.7 (https://github.com/phetsims/joist/issues/970)

density 1.1 (https://github.com/phetsims/joist/issues/970)

friction 1.6 (https://github.com/phetsims/joist/issues/970)

geometric-optics 1.3 (https://github.com/phetsims/joist/issues/970)

geometric-optics-basics 1.3 (https://github.com/phetsims/joist/issues/970)

graphing-quadratics 1.3 (https://github.com/phetsims/joist/issues/970)

gravity-and-orbits 1.6 (https://github.com/phetsims/joist/issues/970)

greenhouse-effect 1.2 (https://github.com/phetsims/joist/issues/970)

greenhouse-effect 1.3 (https://github.com/phetsims/joist/issues/970)

keplers-laws 1.2 (https://github.com/phetsims/joist/issues/970)

molecule-polarity 1.3 (https://github.com/phetsims/joist/issues/970)

molecule-shapes 1.6 (https://github.com/phetsims/joist/issues/970)

molecule-shapes-basics 1.6 (https://github.com/phetsims/joist/issues/970)

my-solar-system 1.3 (https://github.com/phetsims/joist/issues/970)

natural-selection 1.5 (https://github.com/phetsims/joist/issues/970)

ph-scale 1.6 (https://github.com/phetsims/joist/issues/970)

ph-scale-basics 1.6 (https://github.com/phetsims/joist/issues/970)

projectile-data-lab 1.0 (https://github.com/phetsims/joist/issues/970)

projectile-sampling-distributions 1.0 (https://github.com/phetsims/joist/issues/970)

Nancy-Salpepi commented 2 months ago

@zepumph in https://github.com/phetsims/qa/issues/1089 if I added ?locale= without anything after the equals sign, the sim loaded in English. Now the sim doesn't go past the splash screen and this appears in the console:

Screenshot 2024-06-20 at 11 58 30 AM
Nancy-Salpepi commented 2 months ago

@zepumph if I add a locale not in the desired format (e.g. ?locale=deut) to https://phet-dev.colorado.edu/html/acid-base-solutions/1.3.7-rc.1/phet-io/wrappers/studio I don't see the Invalid Query Parameter dialog. Instead I get errors. I have to also add phetioDebug=false to the url to see the dialog.

images Screenshot 2024-06-20 at 12 44 21 PM Screenshot 2024-06-20 at 12 44 53 PM Screenshot 2024-06-20 at 12 45 10 PM

But if I add ?locale=deut to https://phet-dev.colorado.edu/html/beers-law-lab/1.7.12-rc.1/phet-io/wrappers/studio I do get the Invalid Query Parameter dialog.

image Screenshot 2024-06-20 at 12 49 07 PM

Which is correct?

zepumph commented 2 months ago

These are both excellent reports.

Nancy-Salpepi commented 2 months ago

@zepumph EDIT: When a regional locale is not translated, opening the localization menu in the preferences will show no selected language. (as @KatieWoe stated in the comment below it seems to happen when the regional locale is not translated but the fallback locale is). Likewise, in studio the localeProperty will indicate that de-li is the current language, but there will be no radio button for it. This is different behavior than in https://github.com/phetsims/qa/issues/1089 which will indicate that German is the selected language.

Screenshot 2024-06-20 at 4 02 01 PM
KatieWoe commented 2 months ago

https://github.com/phetsims/qa/issues/1098#issuecomment-2181489185. This also seems to be the case if you use a locale that isn't translated, but its fallback is. Such as es_ES

Nancy-Salpepi commented 2 months ago

For https://github.com/phetsims/qa/issues/1098#issuecomment-2181489185 is does still seem to work correctly in the standard wrapper When adding ?locale =de-li to the end of the url, German will be selected in the localization tab.

image Screenshot 2024-06-20 at 5 06 57 PM
KatieWoe commented 2 months ago

When testing the locale parameter on a downloaded sim from the studio wrapper, it does not give an error for invalid entries, such as locale=x

UPDTE FROM MK: This is expected though unfortunate for PhET-iO Standard Wrappers. Item 6 in https://github.com/phetsims/phet-io/issues/1881#issuecomment-1287618206 explains this. Assert with phetioDebug=true, and en graceful fallback without any dialog or assertion.

Nancy-Salpepi commented 2 months ago

This no longer seems to be working:

String fallback should work, where if a translation does NOT have a translation for a string, but a fallback DOES, the first fallback with a translation will be used for that string. (e.g. in center-and-variability, es_PE itself does not yet have translations for the region-and-culture section

image Screenshot 2024-06-20 at 8 20 43 PM

UPDATE FROM MK: Looks like this is because of a translation that cleared the es string, not from the MR: https://github.com/phetsims/babel/commit/ffb1a5296f0693b709bfb98dd73082ffd237e6d5. We will look into it.

Nancy-Salpepi commented 2 months ago

Further investigation of https://github.com/phetsims/qa/issues/1098#issuecomment-2181768905 shows that with ?locale=es the region and culture section isn't translated either.

I looked back at https://github.com/phetsims/qa/issues/1085 and we tested CAV 1.2.0-dev.5, but here were are testing 1.1.8-rc.1.

KatieWoe commented 2 months ago

On hold until say so from @zepumph

zepumph commented 2 months ago

New RCs have been created. We will pick things back up in https://github.com/phetsims/qa/issues/1103. Thanks QA.