Multitouch behavior is different than in Published

[Nancy-Salpepi]

Test device iPad 9th generation

Operating System 16.3.1

Browser safari

Problem description For https://github.com/phetsims/qa/issues/905, on the iPad, the behavior is different between the current rc and published when I am rotating a molecule by moving my finger around the screen and then go to press something else with another finger.

• In published-- The rotating action isn't interrupted and the section action, such as checking a checkbox, occurs.
• In current--The screen zooms in and pans OR the rotating action is interrupted but the second action doesn't occur. This issue doesn't happen when I rotate the molecule by grabbing and moving an atom around.

Steps to reproduce

1. Move your finger around the screen to rotate the molecule
2. With a different finger, check the "Show Bond Angles" checkbox

Visuals This is the behavior in published:

https://user-images.githubusercontent.com/87318828/220941728-4a7c763a-f65a-4398-9fb0-45127a9bc95e.mov

This is the behavior in the current version:

https://user-images.githubusercontent.com/87318828/220941964-3a1236ba-b9bc-44d9-aa64-369650514b1a.mov

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Molecule Shapes‬ URL: https://phet-dev.colorado.edu/html/molecule-shapes/1.6.0-rc.1/phet/molecule-shapes_all_phet.html Version: 1.6.0-rc.1 2023-02-21 18:00:32 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/110.0.0.0 Safari/537.36 Language: en-US Window: 1511x775 Pixel Ratio: 1.7999999523162842/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: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}

[jonathanolson]

I'm going to be looking into this, I'm reproducing it on a Chromebook. Curiously, if the initial finger is NOT moving, the checkbox toggle works. @jessegreenberg is this type of behavior expected?

[jessegreenberg]

The seems strange, Ill take a look. Also, it seems to be less about keeping the finger still, but having the finger start over the molecule. If my finger starts right on top of the molecule even while moving this doesn't happen.

[jessegreenberg]

It could be related to https://github.com/phetsims/molecule-shapes/issues/213 and https://github.com/phetsims/molecule-shapes/commit/84eeb073e1e8762f7310fd340d18fe3f7af36b4a.

Specifically

        // If a drag starts on a pair group, input should only be for dragging. Indicate to other listeners that
        // behavior is reserved (specifically the pan/zoom listener that should not interrupt for pan).
        if ( dragMode === 'pairExistingSpherical' ) {
          pointer.reserveForDrag();
        }

Should reserveForDrag be used for other drag modes?

[jessegreenberg]

This patch seems to fix it, but I am not sure if it causes other problems:

Subject: [PATCH] Combine fireOnKeyDown and fireOnKeyUp options, see https://github.com/phetsims/scenery/issues/1520
---
Index: js/common/view/MoleculeShapesScreenView.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/MoleculeShapesScreenView.js b/js/common/view/MoleculeShapesScreenView.js
--- a/js/common/view/MoleculeShapesScreenView.js    (revision c6224b3142ed391e1a6d46bff88d0a3783c7e2a0)
+++ b/js/common/view/MoleculeShapesScreenView.js    (date 1677168252062)
@@ -229,11 +229,9 @@

         const lastGlobalPoint = pointer.point.copy();

-        // If a drag starts on a pair group, input should only be for dragging. Indicate to other listeners that
+        // input on the background should only be for dragging. Indicate to other listeners that
         // behavior is reserved (specifically the pan/zoom listener that should not interrupt for pan).
-        if ( dragMode === 'pairExistingSpherical' ) {
-          pointer.reserveForDrag();
-        }
+        pointer.reserveForDrag();

         const onEndDrag = function( event, trail ) {
           if ( dragMode === 'pairExistingSpherical' ) {

[jonathanolson]

I believe the above commit fixes this. @Nancy-Salpepi can you test out master? (I've patched the release branches)

This changes how pan-zoom multi-touch works a bit.

• Previously, any press that was tagged with a drag intent prevented any presses from starting to participate in pan/zoom. Additionally, if the currently-moving press had moved past the movement threshold, it moved ALL presses to participate. This meant that the finger rotating the molecule had moved far enough, and didn't need the finger trying to press the checkbox to move AT ALL.
• Now: we just look for presses that have no drag intent AND have moved past their own threshold. We either need 2 of those, OR to be zoomed in already (1 could get added to pan). I believe this significantly improves the behavior in these cases, as now it's still POSSIBLE to start a pan/zoom with the second finger over the checkbox, but you have to move THAT finger past the movement threshold.

@jessegreenberg would you be able to review https://github.com/phetsims/scenery/commit/7417b985d1daf1a812e7ec1eacdbb68907c66fdc?

[Nancy-Salpepi]

This is working well now 🙂. I can rotate the molecule and check boxes/add bonds without an issue. I see what you are saying about still being able to zoom in, but I have to move the second finger in a way I wouldn't normally do after selecting an item. I probably never would have gotten it to happen if you didn't mention it.

[zepumph]

@jessegreenberg, can you give this a look. We are otherwise ready for the next RC.

[jessegreenberg]

This change looks really good, thank you! The documentation is very helpful and I am very happy you found a way without adding a new Intent.

I do see one issue though - since only the background presses are checked listeners get interrupted for zoom for multitouch drag cases. For example, go to BASE and make both balloons visible. Drag both at the same time and the DragListeners will be interrupted for zoom. Ill look for a fix later today but will also hand back to @jonathanolson in case he sees the solution quickly.

[jessegreenberg]

@jonathanolson and I met to review https://github.com/phetsims/scenery/commit/7417b985d1daf1a812e7ec1eacdbb68907c66fdc together and addressed that bug in the above commit. This issue is no longer blocking, back to @jonathanolson to cherry pick this commit and next steps.

[jonathanolson]

Patched in @jessegreenberg's bugfix above to the release branches, I'll proceed with RCs. Thanks!

[jonathanolson]

Please close after verifying!

[Nancy-Salpepi]

This looks good in rc.2 for full/basics versions. Closing.