How should pan and zoom work on Factor screen?

[Nancy-Salpepi]

Test device MacBook Air M1 chip

Operating System 14.3.1

Browser Safari 17.3.1

Problem description For https://github.com/phetsims/qa/issues/1046, on the Factor Screen, when I am zoomed in on the play area the screen doesn't pan if I move the hand using the mouse. If I use arrow keys, the hand moves and the screen pans.

Steps to reproduce

1. In any scene on the Factor Screen, zoom in and try to move the hand to a box that isn't on the screen--screen doesn't pan
2. Use the arrow keys to pan --they move the hand as well as causing the screen to pan

Visuals This video starts with the mouse and then moves to arrow keys:

https://github.com/phetsims/arithmetic/assets/87318828/9da3c6d5-7066-4aea-821c-f4708a5b0e54

[Luisav1]

Hm considering the arrow keys work in the zoomed in version, I would expect the screen to pan with just the mouse moving the hand too. I see this is also an issue in the published version.

[Nancy-Salpepi]

I see this is also an issue in the published version.

The published version doesn't have pan and zoom enabled @Luisav1.

In version 1.1.0, I am wondering about 2 things for the Factor screen:

1. Should the screen pan while trying to move the hand with the mouse?
2. It seemed like keyboard navigation is enabled for the hand when zoomed in. Was that intentional?

[Luisav1]

Ah oops I wasn't aware, thanks for the clarification @Nancy-Salpepi!

It seems that keyboard navigation for the hand wasn't intentional since, in the code, an area is highlighted whenever there's a mouse over on a certain area. So even without bringing the mouse to the play area, if you use arrows to move across the screen, it is "hovering" over the area thus highlighting it anyway.

Although this isn't intentional, since that is there, I think it's ideal to pan the screen when trying to move the hand with the mouse. This was done above.

@amanda-phet can you please review that this behaviour is looking good design-wise? Thanks.

[amanda-phet]

It seems to be panning now, so I think the fix is working. @Nancy-Salpepi should also review to be sure it's what you expect.

[Nancy-Salpepi]

Panning works as expected when not zoomed in all the way. When I am zoomed in all the way, if I drag the hand to the edge of the screen (ex. left edge) with my mouse the screen doesn't pan. If I then start to move the hand back to the right, the screen will pan left a bit.

https://github.com/phetsims/arithmetic/assets/87318828/d8d10c1d-1aaa-495f-a424-5b1fc19ab1e6

[Luisav1]

This is fixed now but I'm not sure how.

https://github.com/phetsims/arithmetic/assets/52978048/767934c3-f1a1-4612-ac9f-60e434405c47

I added an invisible rectangle around the cellPointer so it has the bounds of the activeCell instead of the more smaller bounds of the image cellPointer hand. This seems to have fixed the issue but I am confused why since the panToNode is still panning to the cellPointer, not the rectangle with bigger bounds. The cellPointer and invisibleRectangle aren't linked to each other either. @marlitas could you review why this invisible rectangle is necessary? Is there another alternative? Thanks!

[marlitas]

@Luisav1 I am also not sure why this is fixing the pan when the listener is hooked in the cellPointer instead of the invisibleRectangle. At first I thought it was maybe due to the fact that when we were creating the invisibleRectangle we were passing in the cellPointer instead of a copy of the cellPointer's bounds.

const invisibleRectangle = new Rectangle( { rectBounds: this.cellPointer } );

However, changing that didn't bring back the buggy behavior either. I don't know if this needs a lot of in depth investigation. I'm assuming that there is something about the node structure and the ways the bounds are interacting that may be causing this to work, but I still would recommend a couple of changes just to make things more explicit.

• When creating the invisibleRectangle make sure to set the rectBounds to this.cellPointer.bounds.copy().
• Rename invisibleRectangle to something like panToNodeRectangle.
• Pass in panToNodeRectangle to animatedPanZoomSingleton.listener.panToNode() to be explicit about what bounds we are actually tracking.
• I would also add documentation similar to what you had in your comment above invisible rectangle around the cellPointer so it has the bounds of the activeCell instead of the more smaller bounds of the image cellPointer hand.

With those changes I think this will be good to move forward and wrap up.

[Luisav1]

Thanks @marlitas! I'm glad more investigation isn't necessary. I applied all the recommendations above. Closing.

[Nancy-Salpepi]

Reopening. In https://github.com/phetsims/qa/issues/1057, panning is very choppy now with the arrow keys in dev.5. It was smooth in dev.4.

https://github.com/phetsims/arithmetic/assets/87318828/95705d0f-cce0-4def-b45d-1294f93d976e

Here is what it looks like in dev.4:

https://github.com/phetsims/arithmetic/assets/87318828/bacac68c-8225-49b9-ba26-f76182a9a543

[Luisav1]

@marlitas found the cause for the the choppiness was due to both the arrow keys being used to pan and the panToNode listener attempting to keep the current cell in the visible frame. @jessegreenberg suggested removing the panToNode feature and altering the behaviour of the listeners to allow users to click and drag without submitting an answer. The commit above adds a pointerListener along with an interrupt and cancel event, effectively preventing the up emitters from firing during dragging.

Now both the choppiness is removed and squares can be selected (from #217). @amanda-phet and @Nancy-Salpepi since the panning behaviour when zoomed in changed slightly with this new approach, could you review to ensure it's still the expected behaviour?

[Nancy-Salpepi]

Comparing to dev.4: Now I can click on the screen and drag the mouse to have the screen move without selecting an answer. However the original issue that I reported is still the same. The screen doesn't automatically pan with the mouse when I hover over the hand and try and move it off screen.

[Luisav1]

In #x-raisin stand-up today, @Nancy-Salpepi, @marlitas, and I talked that even though there is no more panning behaviour, that's alright as long as that's the expected behaviour since pan and zoom is more typically for static nodes.

[Nancy-Salpepi]

I believe this can be closed now.