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

Overlapping long strings #246

Closed
Tracked by #885 ...
KatieWoe opened this issue Jan 6, 2023 · 9 comments
Closed
Tracked by #885 ...

Overlapping long strings #246

KatieWoe opened this issue Jan 6, 2023 · 9 comments
Assignees

Comments

@KatieWoe
Copy link
Contributor

KatieWoe commented Jan 6, 2023

Test device
Samsung
Operating System
Win 11
Browser
Chrome
Problem description
For phetsims/qa#871
When strings are long they may overlap with other strings or buttons. On the second screen, the two checkboxes on the bottom of the screen will overlap with the pause/play button slightly. On the third screen all the strings on the bottom of the screen overlap or go off screen.

Steps to reproduce

  1. Use the parameter ?stringTest=long
  2. Observe the second and third screen

Visuals
bigoverlap
longoverlap

Troubleshooting information:

!!!!! DO NOT EDIT !!!!!
Name: 12345678901234567890123456789012345678901234567890
URL: https://phet-dev.colorado.edu/html/greenhouse-effect/1.1.0-rc.1/phet/greenhouse-effect_all_phet.html?stringTest=long
Version: 1.1.0-rc.1 2022-12-23 00:15:29 UTC
Features missing: applicationcache, applicationcache, touch
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/108.0.0.0 Safari/537.36
Language: en-US
Window: 1536x714
Pixel Ratio: 1.25/1
WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium)
GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium)
Vendor: WebKit (WebKit WebGL)
Vertex: attribs: 16 varying: 30 uniform: 4096
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 32767x32767
OES_texture_float: true
Dependencies JSON: {}

@KatieWoe KatieWoe added type:bug Something isn't working type:i18n labels Jan 6, 2023
@marlitas
Copy link
Contributor

marlitas commented Jan 6, 2023

There is also overlap with the Thermometer checkbox with stringTest=double
Screenshot 2023-01-06 at 10 39 49 AM

@jbphet
Copy link
Contributor

jbphet commented Jan 10, 2023

My bad - I forgot to run the string tests and adjust before this publication. I've adjusted them now.

@Nancy-Salpepi
Copy link

Nancy-Salpepi commented Jan 22, 2023

I still see the overlapping reporting by @KatieWoe with stringTest=long in rc.2, but in master things look good.

@arouinfar
Copy link
Contributor

@jbphet sounds like you may have missed 95fa513 when cherry-picking.

jbphet added a commit that referenced this issue Jan 25, 2023
@jbphet
Copy link
Contributor

jbphet commented Jan 25, 2023

Good thing we check these issues. As @arouinfar noted above, I missed moving the fix from the master branch to the 1.1 release branch. I've done so now.

@Nancy-Salpepi
Copy link

In rc.3, on the Photons screen the text at the bottom of the concentration panel is cut off slightly on the right side with stingTest=long.

Screenshot 2023-01-30 at 7 57 57 AM

@arouinfar
Copy link
Contributor

@jbphet looks like this is the only issue that remains. I do not think this is a serious enough issue to send Greenhouse back through QA. If we can verify the change ourselves, great. If not, let's wait until full publication.

This is a case where stringTest=long isn't particularly accurate. The only portion of these strings that is actually translatable is the unit, which is rarely translated. I translated "parts per million" into ~40 different locales and most are shorter or similar to English in terms of character count. This was the worst-case scenario:

image

If a translator runs into an issue with the length of the strings, they can instead use the unit's abbreviation.

@jbphet
Copy link
Contributor

jbphet commented Feb 1, 2023

This last observation about the concentration readout strings has been fixed on master and propagated to the 1.1 branch. Here is a screenshot from RC.4:

image

@arouinfar
Copy link
Contributor

@jbphet I reviewed in rc.4 and the strings all look appropriately bounded.

@jbphet jbphet closed this as completed Feb 1, 2023
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