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

Interactive highlight for Ice Age button needs to expand dynamically #349

Closed
Nancy-Salpepi opened this issue Aug 15, 2023 · 12 comments
Closed
Assignees
Labels
type:bug Something isn't working

Comments

@Nancy-Salpepi
Copy link

Test device
MacBook Air M1 chip

Operating System
13.5

Browser
Safari 16.6

Problem description
For phetsims/qa#970, the interactive highlight for the Ice Age button doesn't expand when I change locale.

Steps to reproduce

  1. Add ?stringTest=dynamic to end of url
  2. Turn on Interactive Highlights in the Preferences Menu
  3. On either the Waves or Photons screen, select the byDateRadioButton
  4. Press the right arrow key to expand the text
  5. Mouse over the iceAgeRadioButton
    --This is also seen using keyboard nav

Visuals
In Greek:
Screenshot 2023-08-15 at 8 12 20 AM

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Greenhouse Effect‬ URL: https://phet-dev.colorado.edu/html/greenhouse-effect/1.2.0-dev.2/phet/greenhouse-effect_all_phet.html Version: 1.2.0-dev.2 2023-08-10 04:15:01 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/115.0.0.0 Safari/537.36 Language: en-US Window: 1407x729 Pixel Ratio: 2/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: 31 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
@Nancy-Salpepi Nancy-Salpepi added the type:bug Something isn't working label Aug 15, 2023
@jbphet
Copy link
Contributor

jbphet commented Aug 17, 2023

This is bringing two problems with our code to light, both in the common code.

The first is that interactive highlights don't resize with dynamic layout changes. I'm going to assign this to @jessegreenberg and @jonathanolson for their take on this. It may be a known problem or something new, but they can let us know.

The second is that rectangular radio button groups are not getting resized correctly when the size of individual buttons change, which they should. I'll assign this to @marlitas and @jonathanolson to get their input on this portion.

@jbphet
Copy link
Contributor

jbphet commented Aug 17, 2023

For the record, here is an example of it happening in a more "real" case, which is when switching from English to Arabic.

image

@jbphet
Copy link
Contributor

jbphet commented Aug 17, 2023

@arouinfar - This is a bit of an edge case and will involve changes to common code to fix. I would consider it non-blocking for the release. What's your take?

@jessegreenberg
Copy link
Contributor

I opened the above issue for the highlights and pushed a potential fix which is ready for review.

@jessegreenberg jessegreenberg removed their assignment Aug 18, 2023
@arouinfar
Copy link
Contributor

This is a bit of an edge case and will involve changes to common code to fix. I would consider it non-blocking for the release. What's your take?

I wouldn't consider this blocking either, but looks like we won't have to make that call. Thanks @jessegreenberg!

@jbphet
Copy link
Contributor

jbphet commented Aug 21, 2023

The fix that @jessegreenberg committed is an improvement, but doesn't quite fully resolve the problem, see phetsims/sun#851 (comment). I've created another issue for working on the dynamic layout, see phetsims/sun#852.

@marlitas
Copy link
Contributor

marlitas commented Sep 6, 2023

I commented in phetsims/sun#852. I will continue to take a look in there. Just want to confirm that this is not blocking greenhouse.

Unassigning.

@marlitas marlitas removed their assignment Sep 6, 2023
@Nancy-Salpepi
Copy link
Author

Noting that this issue is still present in phetsims/qa#1033

@arouinfar
Copy link
Contributor

Thanks for reporting @Nancy-Salpepi. @jbphet this issue does not block Greenhouse interviews, but it would be nice to address before publishing the next version.

jonathanolson added a commit to phetsims/sun that referenced this issue Mar 13, 2024
@jonathanolson
Copy link
Contributor

I believe this should be working based on recent commits (including one above), can you verify?

@Nancy-Salpepi
Copy link
Author

This is working nicely on main JO!

@jonathanolson
Copy link
Contributor

Thanks, closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants