phetsims / center-and-variability

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

When showing the hand: if there is a focused soccer ball, show the hand on that one. #521

Closed samreid closed 11 months ago

samreid commented 1 year ago

From https://github.com/phetsims/center-and-variability/issues/518

When the hand appears, move the keyboard + focus indications to that hand item. We chose to move the focus to the hand rather than moving the hand to the focus because: it's important the hand appear on the most recently kicked ball (unless median or too tall stack).

In discussion, we realized that if the user has used the keyboard modality before, they are likely to use it again, so we would rather not move that focus. So in that case, the hand will be shown where the focused ball is.

So the implementation will be: When showing the hand: if there is a focused soccer ball, show the hand on that one. But if the user never selected a soccer ball via keyboard, just follow the pre-existing logic.

samreid commented 1 year ago

I implemented that as prescribed. Would be good to test.

catherinecarter commented 1 year ago

I tested a couple scenarios, and found one where the focus happened to be on the median.

I put focus on the soccerArea prior to exhausting all kicks. The focus went to the ball at position 4. Then, I kicked all of them until maxKicks was reached, and position 4 happened to be the median. There's no way to have predicted this: image

So... I will think on this some more and see if we can find a solution.

catherinecarter commented 1 year ago

I kicked 5 (maxKicks=15) and pressed tab to put focus on a ball. I also had the Median checked. The focus and the median were in the same location, so it seems the first time a ball is focused, it needs to have the same logic as the current hand/arrow to not appear on the same ball as the Median:

image

In terms of the focus moving the the hand/arrow cue when all kicks have been exhausted, I think we'll have to have the focus follow the hand/arrow. I realize this might be confusing for some users if the focus has already been on another ball, but if they kick all the soccer balls and a hand appears, their focus has already shifted to the hand.

The case where the focus sits on the median prior to all kicks being kicked seems more of a priority to avoid. So let's make these decisions:

I know it's not ideal, but it seems to be a good solution for most cases.

marlitas commented 1 year ago

^^ This sounds really confusing from a user standpoint... let's discuss together, because I'm also confused about implementation here. I feel like there should be a more straightforward solution to this as well.

marlitas commented 1 year ago

I want to also mention that there is no way to ensure that a cueing arrow will not occlude the median arrow at any point. A user can move focus to the median stack and then grab that ball and the next cueing arrow will obfuscate the median arrow. There is no way to stop a user from doing that. See the picture below for that scenario:

image
catherinecarter commented 1 year ago

That's a good point, @marlitas. I talked with @kathy-phet about it, too, and here's what we decided:

Hopefully that makes sense, writing it out is surprisingly challenging.

catherinecarter commented 1 year ago

@marlitas and I talked about the situation for when the focus happens to be on a ball that is also the median stack. In this situation, as noted above, the median down arrow and the 'move-me' cue occlude each other. @marlitas suggested making the arrow move up so the 'move-me' arrow is not occluded.

This situation will happen in a very few cases, and once the ball has been moved, the arrow will never appear again.

This is a good solution and will be a nice way to be able to see both arrows (median and cue).

marlitas commented 1 year ago

Okay this is ready for design and code review... but I have a feeling that we'll need a in depth check-in from QA on this. I kept hitting weird scenarios and I think I got all of them, but it is very likely that I still missed something.

Over to @catherinecarter and @matthew-blackman for review.

Let's keep it open for QA to test during RC as well.

catherinecarter commented 1 year ago

I set the maxKicks to be 5, kicked 2 with my mouse, tabbed in, and...

image

I can't seem to reproduce it, though.

marlitas commented 1 year ago

I set the maxKicks to be 5, kicked 2 with my mouse, tabbed in, and...

I can't reproduce this either... I'll keep an eye on it, but it's hard to solve a problem that I can't iterate on.

marlitas commented 1 year ago

While working on #534 I noticed that there was vestigial code from #518 that is no longer needed since we removed the cueing arrow for selecting a different ball. I went ahead and removed that code. This also cleaned up listeners that were being added to stackChangedEmitter.

catherinecarter commented 1 year ago

Looks really good. The median arrow moving out of the way is a nice solution for when the median and alt-input arrows are in the same place. Ready from my end :)

matthew-blackman commented 1 year ago

Given the complexity, this looks like it may be worth a synchronous review. Marking help-wanted.

matthew-blackman commented 12 months ago

Commits and behavior looked great. Reviewed code changes and made one small refactor to make avoidAccordionBox reusable for the drag indicator and avoid some code duplication. Over to you to review the changes @marlitas. Feel free to close when ready.

marlitas commented 12 months ago

Loved those changes. Thanks @matthew-blackman! Closing.

marlitas commented 12 months ago

I forgot I wanted to keep this open for QA. Reopening

KatieWoe commented 11 months ago

I found a potential oddity with this in dev.16. If the mouse is hovering over the ball the keyboard nav is focusing on, the hand and drag arrows will appear even though keyboard nav is activated/in use.

https://github.com/phetsims/center-and-variability/assets/41024075/ca08c359-3728-4d34-ab50-f054a8879ee5

kathy-phet commented 11 months ago

To me this behavior seems OK, your mouse is active at the time of the cuing showing up. I don't think I would change the behavior here.

marlitas commented 11 months ago

This was something that I believe we went back and forth with as well. I agree with @kathy-phet, the mouse is active so that cueing hand should reappear, but we don't want the keyboard highlight to disappear because a user should be able to press a keyboard shortcut and still interact with that balls as long as no click event happens.

Sending over to @catherinecarter to confirm, but I believe this is okay.

catherinecarter commented 11 months ago

Agree, @marlitas. I wouldn't know how to detect the mouse hover since it could be very active. This is one of those things that I noticed as well, that the cueing hand/arrow appears when the focus is on a ball, making a lot of things happening at once on a single ball.

I agree it's distracting, but from what I understand, I believe the behavior is doing what it's supposed to do. Perhaps I'm incorrect, but I thought the behavior was that if a user has focus on a ball and then mouses over it, focus remains on the ball but the sim thinks the mouse has taken over, so the hand appears.

Long story short, yes, I think this is ok and behaving as expected.

marlitas commented 11 months ago

@catherinecarter sounds good. I'll go ahead and close this issue then.