Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drag Listeners aren't interrupted #156

Closed
Tracked by #1074 ...
Nancy-Salpepi opened this issue Apr 15, 2024 · 11 comments
Closed
Tracked by #1074 ...

Drag Listeners aren't interrupted #156

Nancy-Salpepi opened this issue Apr 15, 2024 · 11 comments

Comments

@Nancy-Salpepi
Copy link

Test device
iPad 9th generation

Operating System
iPadOS 17.4.1

Browser
Safari

Problem description
For phetsims/qa#1066 (and phetsims/qa#1067) on the first 3 screens when I am holding a sphere with one finger and press Reset All or use the number picker with another, the drag listener isn't interrupted. I see the same thing with the Point Tool and Reset All button.

Steps to reproduce
Here is an example:

  1. On the Slope Screen, move the purple sphere and hold it with one finger
  2. Press Reset All with another finger
  3. While still holding the sphere, change a value for it with the number picker
  4. Grab a Point tool and place it somewhere
  5. While your finger is still on the Point tool, press Reset All with another finger

Visuals

dragListeners.mov
@pixelzoom
Copy link
Contributor

This is an older sim, and was missing the call to interruptSubtreeInput that we've been adding to resetAllButton. I've added them in the above commit. Sure would be nice if we didn't need to add this manually to every sim, but I don't see an easy way to make that happen.

@Nancy-Salpepi please review in master. If it looks OK, assign back to me for cherry picking.

@Nancy-Salpepi
Copy link
Author

@ pixelzoom

  • When pressing Reset All, in addition to that sound I also hear the release sound for the sphere.
  • It would also be good to interrupt the drag listener for the sphere when using the number picker.

@pixelzoom
Copy link
Contributor

pixelzoom commented Apr 16, 2024

When pressing Reset All, in addition to that sound I also hear the release sound for the sphere.

The grab/release sounds are handled by RichDragListener, and the sim does not have any control over muting its sounds. So I suspect that other sims using RichDragListener (area-model suite, FEL, KL, MSAB, wave-interfernce) also have this problem. RichDragListener should be checking before playing the release sound, to verify that the drag did not end due to an interruption. I've asked @AgustinVallejo and @zepumph to address this for RichDragListener/RichKeyboardDragListener in phetsims/scenery-phet#849.

It would also be good to interrupt the drag listener for the sphere when using the number picker.

And visa versa, I suspect. In this case, it's complicated to be that selective about what to interrupt -- the manipulators and NumberPickers are in entirely different areas of the code. I don't think this warrants the additional complexity, but I'll investigate.

@pixelzoom
Copy link
Contributor

It would also be good to interrupt the drag listener for the sphere when using the number picker.

And visa versa, I suspect. In this case, it's complicated to be that selective about what to interrupt -- the manipulators and NumberPickers are in entirely different areas of the code. I don't think this warrants the additional complexity, but I'll investigate.

Much too complicated, and for negligible benefit. The worst that will happen if a manipulator and NumberPicker are used at the same time is that they will appear to "fight" since they are both changing the same Property. So this request will be "won't fix".

@pixelzoom
Copy link
Contributor

When pressing Reset All, in addition to that sound I also hear the release sound for the sphere.

In phetsims/scenery-phet#849 (comment), @zepumph indicated that this is fixed in main. I cannot confirm because I don't have a multitouch device with me. @Nancy-Salpepi can you please confirm? Then assign back to me for next steps.

@Nancy-Salpepi
Copy link
Author

This is fixed in main.

One more thing to think about:
For Game Levels: interrupting the drag listener when pressing the Start Over button. As of now, if I am holding the sphere with one finger and press the Start Over button with another, I exit the level but I am still holding onto an invisible sphere. Once I lift my finger, I hear the release sound.

@pixelzoom
Copy link
Contributor

pixelzoom commented Apr 18, 2024

One more thing to think about: ...

Good point. Unfortunately, each sim needs to specify what happens when the 'Start Over' button is pressed. So while I'd bet that other sims have this issue, there's no way to address it for all sims. Each sim needs to specify startOverButtonOptions as I've done for GL/GSI in 487bb55.

@Nancy-Salpepi please verify in main, assign back to me to cherry pick.

pixelzoom added a commit to phetsims/graphing-slope-intercept that referenced this issue Apr 18, 2024
@pixelzoom
Copy link
Contributor

I went ahead and did all of the cherry picks, including the scenery and scenery-phet commits identified in phetsims/scenery-phet#849 (comment).

@Nancy-Salpepi
Copy link
Author

Looks good in main!

@Nancy-Salpepi Nancy-Salpepi removed their assignment Apr 19, 2024
@pixelzoom
Copy link
Contributor

pixelzoom commented Apr 19, 2024

Thanks @Nancy-Salpepi!

Ready for review in GL 1.4.0-rc2 and GSI 1.2.0-rc2. Please close this issue if it tests OK.

Summary of what to test:

  1. Pressing the 'Reset All' or 'Start Over' button should cancel interactions
  2. When pressing 'Reset All' or 'Start Over' button while dragging a manipulator, you should hear no "release" sound from the manipulator.
  3. Same as 2, but while dragging a manipulator with the keyboard, then pressing 'Reset All' or 'Start Over' button with the mouse.

@Nancy-Salpepi
Copy link
Author

All looks and sounds good in GL 1.4.0-rc.2 and GSI 1.2.0-rc.2.
Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants