phetsims / graphing-quadratics

"Graphing Quadratics" is an educational simulation in HTML5, by PhET Interactive Simulations.
MIT License
1 stars 4 forks source link

Point tool stays old color when line is changed beneath it #196

Closed KatieWoe closed 1 year ago

KatieWoe commented 1 year ago

Test device Samsung Operating System Win 11 Browser Chrome Problem description For https://github.com/phetsims/qa/issues/959 The point tool no longer changes color when a line moves out from under it. If it is orange when on a line, and the line is changed, the point tool no longer goes white, it stays orange. This worked properly in published Steps to reproduce

  1. Put the point tool on the line
  2. Change the line

Visuals buggq

Troubleshooting information:

!!!!! DO NOT EDIT !!!!! Name: ‪Graphing Quadratics‬ URL: https://phet-dev.colorado.edu/html/graphing-quadratics/1.3.0-dev.5/phet/graphing-quadratics_all_phet.html Version: 1.3.0-dev.5 2023-06-27 15:52:45 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/114.0.0.0 Safari/537.36 Language: en-US Window: 1536x714 Pixel Ratio: 1.25/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: 4096 Texture: size: 8192 imageUnits: 32 (vertex: 32, combined: 64) Max viewport: 8192x8192 OES_texture_float: true Dependencies JSON: {}
KatieWoe commented 1 year ago

It then stays the incorrect color even if you drag it around. If the line passes underneath it this happens too.

KatieWoe commented 1 year ago

When dragging it around, it seems to "stick" to an invisible line

pixelzoom commented 1 year ago

Reproduced in master -- both the lack of color change, and how it somehow "sticks" to where the line used to be.

I suspect that this was broken by https://github.com/phetsims/graphing-quadratics/issues/167 -- wherein we changed the look of the point tool, and made changes in both grapghing-lines and graphing-quadaratics. Note that this problem does not occur in graphing-lines, because it has a different implementation of point tool (for lines vs quadratics).

pixelzoom commented 1 year ago

The problem was introduced in PointTool.ts by https://github.com/phetsims/graphing-quadratics/commit/2dfd1cbb on 2/18/23. This was the first phase of TypeScript conversion.

pixelzoom commented 1 year ago

Fixed in the above commit. (Below commit, actually :)

When I converted to TypeScript, this change was made because TypeScript prefers includes:

-         quadratics.indexOf( onQuadratic ) === -1 ||
+         quadratics.includes( quadraticNear ) ||

The problem is that I got the logic inverted. I wanted "does not include", so it should have been:

+         !quadratics.includes( quadraticNear ) ||
pixelzoom commented 1 year ago

Reopening. Please verify in 1.3.0-rc.1, just in case.

Nancy-Salpepi commented 1 year ago

This looks good in rc.1. Closing