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

Inconsistent outline around temperature values in combobox #256

Closed
Tracked by #885
Nancy-Salpepi opened this issue Jan 7, 2023 · 7 comments
Closed
Tracked by #885

Inconsistent outline around temperature values in combobox #256

Nancy-Salpepi opened this issue Jan 7, 2023 · 7 comments
Labels

Comments

@Nancy-Salpepi
Copy link

Test device
MacBook Air (m1 chip)

Operating System
macOS 13.1

Browser
safari and chrome

Problem description
For phetsims/qa#871:
With interactive highlights off, when I move the mouse over the temperature units options in the combobox, I see a gray outline. The size around Kelvin is different from the others.

Screenshot 2023-01-07 at 8 19 23 AM

Screenshot 2023-01-07 at 8 20 23 AM

Also I wasn't sure if that was the intended design or if it should be highlighted like this:

Screenshot 2023-01-07 at 8 21 38 AM

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Greenhouse Effect‬ URL: https://phet-dev.colorado.edu/html/greenhouse-effect/1.1.0-rc.1/phet/greenhouse-effect_all_phet.html 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 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.2 Safari/605.1.15 Language: en-US Window: 1216x617 Pixel Ratio: 2/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 (1.0) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 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 Jan 7, 2023
@Nancy-Salpepi
Copy link
Author

The same inconsistency is seen when clicking on the arrow to expand the combobox.

Screenshot 2023-01-07 at 8 43 37 AM

Screenshot 2023-01-07 at 8 44 07 AM

@arouinfar
Copy link
Contributor

Nice find @Nancy-Salpepi.

@jbphet is a nice pixel polish, but I don't think it's necessary for the prototype.

@jbphet
Copy link
Contributor

jbphet commented Jan 13, 2023

I have added a fix on both master and the 1.1 branch.

@jbphet jbphet removed their assignment Jan 13, 2023
@zepumph zepumph self-assigned this Jan 13, 2023
@zepumph
Copy link
Member

zepumph commented Jan 13, 2023

We have a good working solution for the branch, but after discussion with @jbphet, it would be better to use the nodes, once created, and set dynamically the min width of the number displays. I'll work on doing that for master, since I blocked that commit originally over in phetsims/sun#797.

@zepumph
Copy link
Member

zepumph commented Jan 13, 2023

Ok. I think this is what @jbphet decided on. I like it much more! Good work team. Thanks all.

over to @jbphet

@zepumph zepumph assigned jbphet and unassigned zepumph Jan 13, 2023
@jbphet jbphet assigned jbphet and unassigned jbphet Jan 13, 2023
jbphet added a commit that referenced this issue Jan 13, 2023
@jbphet
Copy link
Contributor

jbphet commented Jan 13, 2023

Looks good to me. I enhanced the docs a bit, and tested out the behavior, and I think we're good to go.

I'm not going to propagate this to the 1.1 branch, since it would be a fair amount of work to update the dependences, and there would be no change to the behavior. I'll have QA verify the behavior in the next RC.

@Nancy-Salpepi
Copy link
Author

The gray outline is now the same for all temperature units.
Closing.

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

No branches or pull requests

4 participants