phetsims / number-line-integers

"Number Line: Integers" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 4 forks source link

Temperatures can go below the nav bar #55

Closed KatieWoe closed 4 years ago

KatieWoe commented 4 years ago

Test device Dell Operating System Win 10 Browser Chrome Problem description For https://github.com/phetsims/QA/issues/457. It is possible to put a thermometer in such a place that it's reading on the number line is hidden partially, or even mostly, by the nav bar. It can be seen fully if you adjust the window size to be more tall than wide, but most default sizes (excluding tablet and phone screens in portrait mode) will be wider than tall and show the issue. Steps to reproduce

  1. Go to the first screen and the temperature scene
  2. Make sure temp is in F
  3. Set month to July
  4. Put a thermometer in location of A bellow

Visuals Number Line_ Integers screenshot (2) Number Line_ Integers screenshot (1) Number Line_ Integers screenshot

Troubleshooting information:

!!!!! DO NOT EDIT !!!!! Name: ‪Number Line: Integers‬ URL: https://phet-dev.colorado.edu/html/number-line-integers/1.0.0-dev.9/phet/number-line-integers_en_phet.html Version: 1.0.0-dev.9 2019-11-13 22:18:32 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.97 Safari/537.36 Language: en-US Window: 1536x722 Pixel Ratio: 2.5/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 commented 4 years ago

Honestly, the fact that the bottom of the grey box holding the number line gets cut off looks a bit odd to me.

amanda-phet commented 4 years ago

That is indeed a bug. Un-assigning since I don't think this requires a design decision.

KatieWoe commented 4 years ago

On iPadOS 13 it looks better, but changing browsers (firefox vs chrome vs edge) didn't seem to make a difference.

jbphet commented 4 years ago

Yep, that's a bug. I've repositioned the panel that contains the number lines and it looks much better. @amanda-phet - I thought I'd run this by you to make sure you're okay with how it looks at the lowest temperature (I'm okay with it). Screenshot below, please close if you approve.

image

amanda-phet commented 4 years ago

Looks good to me. Thanks!

@KatieWoe can you close if the bug is gone?

KatieWoe commented 4 years ago

Looks good on master in chrome and firefox

KatieWoe commented 4 years ago

For https://github.com/phetsims/QA/issues/465, I did find one more, very mild, instance of this. minorlyoffline

jbphet commented 4 years ago

I just discussed this with @amanda-phet and she is fine with the number going off the bottom, but would like to solicit input from @kathy-phet and @ariel-phet.

It's an easy thing to change, it just scrunches the space over which the numbers can move.

jbphet commented 4 years ago

The conclusion is that I should go through, identify the highest and lowest temperature values, and scale the number line to match. No one is concerned that the values won't be round numbers, such as 83. This may even serve to make the scene a bit more engaging, since users may be motivated to see if they can find the highest and lowest values.

jbphet commented 4 years ago

I've gone through the data set in order to identify the highest and lowest temperatures. For each of these, the time and location are for the first occurrence of that particular temperature in the data set, it may recur at other times and locations. The highest temperature is 108 degrees F at lat 26 long 2 (In Ghar Algeria) in July, here is a screenshot:

image

The lowest is -83 degrees F at lat -74 long 116 in June, here is a screenshot:

image

jbphet commented 4 years ago

Here is what the same space-time locations look like after the revisions:

image

image

jbphet commented 4 years ago

In case I need it again, here is the debug code that I added to TemperatureSceneModel.getTemperatureAtLocation to identify the high and low temperature values and their locations:

```js if ( phet.jbTest ){ // find the min a max temperature values let minTemp = Number.POSITIVE_INFINITY; let minLon, minLat, minMonth, maxLon, maxLat, maxMonth; let maxTemp = Number.NEGATIVE_INFINITY; for ( let month = 1; month <= 12; month++ ){ for ( let lat = -90; lat <= 90; lat += 2 ){ for ( let long = 0; long < 360; long += 2 ){ const tempAtThisLocation = temperatureDataSet.getNearSurfaceTemperature( month, lat, long ); if ( tempAtThisLocation < minTemp ){ minTemp = tempAtThisLocation; minMonth = month; minLon = long; minLat = lat; } if ( tempAtThisLocation > maxTemp ){ maxTemp = tempAtThisLocation; maxMonth = month; maxLon = long; maxLat = lat; } } } } console.log( '-----------------' ); console.log( 'maxTemp = ' + maxTemp ); console.log( 'maxMonth = ' + maxMonth ); console.log( 'maxLat = ' + maxLat ); console.log( 'maxLon = ' + maxLon ); console.log( '-----------------' ); console.log( 'minTemp = ' + minTemp ); console.log( 'minMonth = ' + minMonth ); console.log( 'minLat = ' + minLat ); console.log( 'minLon = ' + minLon ); phet.jbTest = false; } ```
jbphet commented 4 years ago

This is adjusted on both the 1.0 and master branches, I will have it verified on the next RC.

amanda-phet commented 4 years ago

Looks good to me. Thanks for making this change!

KatieWoe commented 4 years ago

Looks good in rc.2