phetsims / circuit-construction-kit-common

"Circuit Construction Kit: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
10 stars 10 forks source link

Meters in Sensor Toolbox are misaligned when Labels is unchecked #966

Closed Nancy-Salpepi closed 1 year ago

Nancy-Salpepi commented 1 year ago

Test device Dell

Operating System Win10

Browser FF/Chrome

Problem description For https://github.com/phetsims/qa/issues/900, the meters are misaligned when labels are hidden.

Steps to reproduce

  1. On either screen, uncheck the Labels checkbox.

Visuals

Screenshot 2023-02-15 at 6 23 54 PM

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Circuit Construction Kit: DC‬ URL: https://phet-dev.colorado.edu/html/circuit-construction-kit-dc/1.3.0-dev.20/phet/circuit-construction-kit-dc_all_phet.html Version: 1.3.0-dev.20 2023-02-15 16:13:54 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/110.0.0.0 Safari/537.36 Language: en-US Window: 1374x712 Pixel Ratio: 1.7999999523162842/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: {}

samreid commented 1 year ago

Good discovery! Fixed in the commit. I also observed you cannot hide the sensor tool icon labels using Studio. @arouinfar or @matthew-blackman is that by design? Also, let's leave this issue open for verification to make sure nothing else was disrupted.

@Nancy-Salpepi would you like to test on phettest?

arouinfar commented 1 year ago

I also observed you cannot hide the sensor tool icon labels using Studio. @arouinfar or @matthew-blackman is that by design?

Somewhat. In https://github.com/phetsims/scenery/issues/1447 we decided not to instrument visibleProperty for text/rich text by default. Instead, we would opt-in as we see fit. Generally, we would opt-in to a visibleProperty if an empty string is insufficient (such as a string being used multiple places throughout the sim). In this particular case an empty string is sufficient to hide the labels.

image
arouinfar commented 1 year ago

@samreid should this issue be moved to cck-common so we can tag it for the milestone?

samreid commented 1 year ago

Thanks, I moved it to circuit-construction-kit-common and added it to the milestone.

Nancy-Salpepi commented 1 year ago

This looks good in master.

samreid commented 1 year ago

Thanks, it seems this issue has been fixed and verified. Closing.