Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

appending a non-existent locale to standard wrapper does not load #341

Closed
Tracked by #899
arouinfar opened this issue Feb 8, 2023 · 10 comments
Closed
Tracked by #899

appending a non-existent locale to standard wrapper does not load #341

arouinfar opened this issue Feb 8, 2023 · 10 comments

Comments

@arouinfar
Copy link
Contributor

Related to phetsims/qa#899

The problem described in https://github.com/phetsims/phet-io/issues/1901 exists in the latest Friction RC.
image

This has already been corrected in master, but it's not obvious to me which commits need cherry-picking, so I recommend that @jessegreenberg consult with @samreid.

Additionally, I made a related change to phet-io-guide.md that will also need to be cherry-picked.
https://github.com/phetsims/phet-io-sim-specific/commit/b3b518e231cfa737aa5e51b6533647c2f3f7ca11

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Feb 8, 2023

Thanks @arouinfar. At first I thought this was #334, but that was about non-existent locales in studio. This is a different problem for the standard wrapper.

@jessegreenberg
Copy link
Contributor

I think that phetsims/ph-scale#274 lists what needs to be cherry-picked for this.

@jessegreenberg
Copy link
Contributor

phetsims/ph-scale#274 (comment) and the following comments confirm that those two commits are all that are needed.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Feb 9, 2023

I had a merge conflict with https://github.com/phetsims/phet-io-sim-specific/commit/b3b518e231cfa737aa5e51b6533647c2f3f7ca11. I met with @arouinfar and she helped me resolve conflicts the best way for friction-1.6.

@arouinfar said to test passing ?locale=xy with and without ?phetioDebug=true for the standard wrapper after these changes are cherry-picked.

jessegreenberg added a commit that referenced this issue Feb 9, 2023
@jessegreenberg
Copy link
Contributor

While testing locally I kept hitting an assertion running studio from compiled index about the namespace. @samreid identified that I needed phetsims/phet-core@ee0545a, Ill cherry-pick that into friction-1.6 SHAs as well.

jessegreenberg added a commit that referenced this issue Feb 9, 2023
@jessegreenberg
Copy link
Contributor

OK, I verified in 1.6 SHAs that ?locale=xy in the standard wrapper does not cause any error or assertion. That is true even when running with ?phetioDebug=true, which matches the behavior described in phetsims/ph-scale#274 (comment).

Ready to verify. @arouinfar if you wouldn't mind verifying this one in rc.3, that would probably be best.

@arouinfar
Copy link
Contributor Author

@jessegreenberg looks good in rc.3, but I'll leave it for QA to close during testing.

@pixelzoom
Copy link
Contributor

... 3, but I'll leave it for QA to close during testing.

Then I'll changed the label to fixed-awaiting-deploy.

@Nancy-Salpepi
Copy link

This is fixed in rc.3. When adding a nonexistent locale, such as xy, to the standard wrapper's url, it is ignored and the sim opens in English. With ?phetioDebug=true, there is no assertion error and the sim opens in English.
The client guide has also been updated:

If you specify a locale for which a translation does not exist, the locale query parameter is ignored and the simulation will retain the localeProperty used when creating the Standard PhET-iO Wrapper.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants