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

Add support for dynamic locale #239

Closed
pixelzoom opened this issue Aug 30, 2022 · 8 comments
Closed

Add support for dynamic locale #239

pixelzoom opened this issue Aug 30, 2022 · 8 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Aug 30, 2022

... similar to what was done in phetsims/natural-selection#319.

This also includes ph-scale-basics.

@pixelzoom pixelzoom self-assigned this Aug 30, 2022
pixelzoom added a commit to phetsims/ph-scale-basics that referenced this issue Aug 30, 2022
@pixelzoom
Copy link
Contributor Author

Code specific to ph-scale-basics was completed in the above commit.

@pixelzoom
Copy link
Contributor Author

Next on my list, so raising priority.

@pixelzoom
Copy link
Contributor Author

Before doing this work, I published 1.6.0-dev.3 for brands=phet,phet-io as a baseline.

pixelzoom added a commit that referenced this issue Sep 1, 2022
pixelzoom added a commit that referenced this issue Sep 1, 2022
@pixelzoom
Copy link
Contributor Author

In the above commits, I did the low-hanging fruit -- the StringProperties that are passed directly to a Text/RichText constructor.

I can already tell that this sim is going to have significant layout problems.

pixelzoom added a commit that referenced this issue Sep 6, 2022
pixelzoom added a commit to phetsims/ph-scale-basics that referenced this issue Sep 7, 2022
pixelzoom added a commit that referenced this issue Sep 7, 2022
pixelzoom added a commit that referenced this issue Sep 7, 2022
pixelzoom added a commit that referenced this issue Sep 7, 2022
pixelzoom added a commit that referenced this issue Sep 7, 2022
pixelzoom added a commit that referenced this issue Sep 7, 2022
pixelzoom added a commit that referenced this issue Sep 7, 2022
pixelzoom added a commit that referenced this issue Sep 7, 2022
@pixelzoom
Copy link
Contributor Author

@arouinfar ready for review.

@arouinfar
Copy link
Contributor

@pixelzoom overall things look good in ph-scale. I didn't review the links to the StringProperties in the view components, since it sounds like that work is still outstanding. Let me know if you need any guidance before you begin.

I found a few issues with dynamic layout:

  1. pHStringProperty - the text does not remain centered and escapes the bounds of the pH Meter.
    image
  2. phScale.microScreen.view.beakerControlPanel.separator does not change in size when the panel expands to accommodate long strings. Not sure if this is something that can be handled within the scope of dynamic layout, but wanted to point it out in case this behavior is unexpected or can be corrected. (Feel free to veto if overly complicated.)
    image

In ph-scale-basics, I'm seeing every single ph-scale string. Since there isn't a way to re-add the additional screens in Basics, these strings seem completely irrelevant. For example, many of these StringProperties relate to later screens.
image

I wasn't sure how this was being handled in rosetta, so I compared to the Translation Utility. The strings specific to pH Scale (e.g. linearStringProperty or moleculeCountStringProperty) do not appear.
image

@pixelzoom
Copy link
Contributor Author

  1. and 2. above are certainly things that can be addressed. Thanks for identifying.

In ph-scale-basics, I'm seeing every single ph-scale string. ...

This is a general problem, not specific to ph-scale-basicis. A sim's PhET-iO API currently includes every string Property, from every dependency repo, regardless of whether those strings are using in the sim. Tracking in https://github.com/phetsims/phet-io/issues/1877.

@pixelzoom
Copy link
Contributor Author

I've created separate issues for each things that @arouinfar reported in #239 (comment). 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

2 participants