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

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

Closed
Tracked by #985
samreid opened this issue Aug 30, 2023 · 21 comments
Closed
Tracked by #985

Comments

@samreid
Copy link
Member

samreid commented Aug 30, 2023

From #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 samreid self-assigned this Aug 30, 2023
@samreid samreid changed the title When the hand appears, move the keyboard + focus indications to that hand item. When showing the hand: if there is a focused soccer ball, show the hand on that one. Aug 30, 2023
samreid added a commit that referenced this issue Aug 30, 2023
…elected or tabbed to via the keyboard, that takes precedence for indication, see #521
@samreid
Copy link
Member Author

samreid commented Aug 30, 2023

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

@catherinecarter
Copy link
Contributor

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
Copy link
Contributor

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:

  • If a user tabs into the soccer area prior to exhausting maxKicks, use the same logic as the hand/arrow indicator
    • go to the most recent kick
    • avoid the median
  • If a user tabs into the soccer area using the above logic, moves focus back to the kick buttons, and then exhausts all kicks, move focus to the hand/arrow

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

@marlitas
Copy link
Contributor

marlitas commented Sep 6, 2023

^^ 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
Copy link
Contributor

marlitas commented Sep 7, 2023

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
Copy link
Contributor

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

  • If a user never uses focus, the hand/arrow will appear on the most recently kicked ball except avoiding the median (current logic, I believe)
    • If the first time the user puts focus on the ball group is after maxKicks has been reached (i.e. the hand is already there), the focus matches where the hand is
  • If the user focuses on the ball group at any time prior to exhausting maxKicks, focus is placed on the most recently-kicked ball up to the point of focusing on the ball group, and then saves its state if a user tabs out of the ball area and back in
    • upon exhausting all kicks, the hand matches the location of the most-recent-focused position
    • if the last-focused position happens to be the median, ensure the arrow cue to move a ball is in front of the median arrow, as I think it already is (if a ball hasn't been moved yet and the arrow is still visible)

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

@catherinecarter
Copy link
Contributor

@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 added a commit to phetsims/soccer-common that referenced this issue Sep 14, 2023
@marlitas
Copy link
Contributor

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
Copy link
Contributor

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 :)

@catherinecarter catherinecarter removed their assignment Sep 15, 2023
@matthew-blackman
Copy link
Contributor

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

@matthew-blackman matthew-blackman added the dev:help-wanted Extra attention is needed label Sep 18, 2023
@matthew-blackman matthew-blackman removed the dev:help-wanted Extra attention is needed label Sep 19, 2023
@matthew-blackman
Copy link
Contributor

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
Copy link
Contributor

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

@marlitas
Copy link
Contributor

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

@KatieWoe
Copy link

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.

Center.and.Variability.-.Google.Chrome.2023-09-29.12-52-40.mp4

@kathy-phet
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

marlitas commented Oct 5, 2023

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

@marlitas marlitas closed this as completed Oct 5, 2023
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

6 participants