search

phetsims / mean-share-and-balance

"Mean: Share and Balance" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
2 stars 1 forks source link

Should fireOnHold be set to True for the Number Spinners? #320

Closed Nancy-Salpepi closed 2 weeks ago

Nancy-Salpepi commented 1 month ago

Test device MacBook Air M1 chip and Dell

Operating System macOS14.5 and Win11

Browser Safari and Chrome

Problem description For https://github.com/phetsims/qa/issues/1105, I currently have to continually press the increment/decrement buttons with the mouse to change the Number of Cups/People/Balls, which is different behavior than in the prototype and with the keyboard.

@amanda-phet do you like the current mouse behavior or should it be changed?

marlitas commented 1 month ago

I think this might have been a multi-touch issue... @jbphet can confirm

amanda-phet commented 1 month ago

I just chatted with @jbphet about this and it does sound like it was maybe a multi-touch issue, and I vaguely remember now saying it's fine if you can't press and hold because it's only 7 clicks max. However, he will look into it tomorrow and if it's not too hard to fix we'll allow fireOnHold!

jbphet commented 4 weeks ago

@amanda-phet and I discussed this in person and decided that making the "Kick" button work for press-and-hold should also be included in the scope of this issue.

jbphet commented 4 weeks ago

Okay, the spinners for number of cups/plates/balls now works with the press-and-hold behavior. I removed the interruptSubtreeInput calls that were preventing this from working and added some more targeted ones. I tested the basic, mouse-only cases as well as some multi-touch cases and fixed up any problems I found. I think this is now ready for code review, and we should definitely leave it open and have QA test it as an "issue to verify" in the next round of testing.

marlitas commented 4 weeks ago

Sending this back to @jbphet since we discovered that the kick button is still getting interrupted during standup.

jbphet commented 4 weeks ago

Yes indeed, I messed this up at the last moment. I added some code that was intended to interrupt input on the "Kick" button if the user tried to interact with it and the "Number of Balls" spinner at the same time. My code was flawed though, and I won't get into the details as to why, but I've now backed it out.

The code that was added for the other direction, meaning where interaction with the "Kick" button interrupts input to the "Number of Balls" spinner, seems to be working fine. I've tried testing a number of multi-touch scenarios where I tried to interact with both controls at the same time and wasn't able to create any problematic behavior in the sim, so I'm now thinking that this is sufficient to allow fire-and-hold to work on both of these controls without introducing multi-touch vulnerabilities.

@marlitas - please have another look and see if the behavior now looks good and if the code changes seem reasonable.

marlitas commented 4 weeks ago

I looked at the code and this seems to be handling situations effectively. I also tested a few scenarios on my phone and it's looking good. This is ready to close on my end.

Over to @Nancy-Salpepi or @KatieWoe to confirm the behavior is looking good on main. Feel free to close!

KatieWoe commented 3 weeks ago

Things look ok on main to me.

Nancy-Salpepi commented 2 weeks ago

Looks good in rc.1! Closing.