phetsims / sound-waves

"Sound Waves" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 1 forks source link

Is it possible to increase font size? #29

Closed Nancy-Salpepi closed 1 year ago

Nancy-Salpepi commented 1 year ago

Test device MacBook Air M1 chip

Operating System 13.5

Browser Safari 16.6

Problem description For https://github.com/phetsims/qa/issues/972, the font size is unusually small in this sim. I know this is meant to be a quick release, but I wanted to mention it.

Here is a comparison with Gravity and Orbits:

Screenshot 2023-08-21 at 3 30 26 PM
Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Sound Waves‬ URL: https://phet-dev.colorado.edu/html/sound-waves/1.0.0-dev.5/phet/sound-waves_all_phet.html? Version: 1.0.0-dev.5 2023-08-21 17:16:24 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.6 Safari/605.1.15 Language: en-US Window: 1099x624 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 commented 1 year ago

Also, the location of the frequency readout makes it look like it is the value of the middle tick mark. The java version had the value off-center with more space between it and the scale.

Screenshot 2023-08-22 at 8 23 29 AM
matthew-blackman commented 1 year ago

Great suggestions @Nancy-Salpepi - will implement and tag you when ready for review

Nancy-Salpepi commented 1 year ago

Font looks much better! I can actually read it! haha

matthew-blackman commented 1 year ago

Reopening to confirm during next dev test.

KatieWoe commented 1 year ago

Looks much better to me. I did notice that the "ATM" on the pressure gauge on the Air Pressure screen was a bit small. Is that worth a look? atm

matthew-blackman commented 1 year ago

I expanded the options of GaugeNode to allow a custom font size for the label, and also increased the font of the numerical labels on the pressure gauge. @KatieWoe can you spot-check on main?

@samreid might be good to get a review here as well since GaugeNode is a scenery component. Let me know if you have any questions!

samreid commented 1 year ago

Thanks, looks good and I have a few recommendations for next steps:

  1. Let's name the option "label" instead of "title" to match with existing terminology.
  2. Let's convert to the nested options pattern. So it would become labelTextOptions: TextOptions. We'll still need to combineOptions to get the tandem. Let's leave the existing logic for maxWidth. See NumberControl for an example of the nested options pattern: https://github.com/phetsims/scenery-phet/blob/234fc50e13fee7c04ee9e79bc62f4539475e6a24/js/NumberControl.ts#L164
KatieWoe commented 1 year ago

Looks ok to me on main

matthew-blackman commented 1 year ago

Thanks for the feedback @samreid! I gave this a try in the above commits.

matthew-blackman commented 1 year ago

The text in the GaugeNode is ready for a spot check on main.

KatieWoe commented 1 year ago

Looks good on main

Nancy-Salpepi commented 1 year ago

Font size looks good to me in rc.2. Closing.