search

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

Panning is buggy #128

Closed marlitas closed 9 months ago

marlitas commented 9 months ago

Originally found in Number Line Operations: https://github.com/phetsims/number-line-operations/issues/108

Will need it's own implementation specific fix.

jessegreenberg commented 9 months ago

Im trying to understand how the PointControllerNode works and the code related to pointController.proposePosition.

It seems like the number line and the character are connected somehow because at particular zoom levels the scenery tries to make both the character AND the number line tick in view. image

jessegreenberg commented 9 months ago

OK, so it seems like the PointControllerNode does have the character, point, and line all in one Node. The DragListener is added all of it. I think this is a good case for targetNode of DragListener, so that DragListener (and the AnimatedPanZoomListener) can get transforms and bounds correctly for the logical "dragged Node".

This fixes both NL:I and NL:D. I went ahead and committed it. @marlitas can you please review, and anything left to verify before cherry-picking?

marlitas commented 9 months ago

Thanks for the quick investigation and fix on that @jessegreenberg. This looks great and I think is ready for cherry-pick. I'll go ahead and mark as such.

Nancy-Salpepi commented 9 months ago

It definitely pans now--sometimes it pans more than I am expecting. When zoomed in a lot, it can be hard to place objects--especially the thermometers, where I want them because the vertical panning that happens. I have to use very small mouse clicks to get it where I want it to be. If this behavior is OK, feel free to close.

marlitas commented 9 months ago

I zoomed in all the way on my machine and did not find the panning to be debilitating. There are times when it was a bit annoying to place a thermometer arrow, especially when moving large distances, but I thought overall it behaved as I would expect... @amanda-phet can you test on your end and add your thoughts as well?

RC link: https://phet-dev.colorado.edu/html/number-line-integers/1.2.0-rc.4/phet/number-line-integers_all_phet.html

amanda-phet commented 9 months ago

For the thermometer specifically, it would be a dramatic improvement to only focus on the arrow position and not the entire thermometer. If it isn't too involved, I'd like to try that. Otherwise I think it's ok to publish as-is, and be mindful of this kind of node in future sims.

marlitas commented 9 months ago

@Nancy-Salpepi, @amanda-phet, and I met to discuss an approach that would change the targetNode in panning to be just the triangle at the bottom of the thermometer. However, while this did correct the original problem and made it easier to maneuver the thermometer when a user is grabbing the bottom portion of a thermometer, it almost eliminated panning when a user is grabbing the top most part of the thermometer.

Here is a video to demonstrate:

https://github.com/phetsims/number-line-integers/assets/42476338/91c78abe-c18c-492e-bdd8-ff379910e136

It was decided that this was a poorer user experience overall, so we are leaving it as is.