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

On "both hands" keyboard nav tab and grab hands using mouse, the arrow key buttons do not disappear #350

Closed
Tracked by #617
brooklynlash opened this issue Feb 15, 2021 · 7 comments
Assignees
Labels
type:bug Something isn't working

Comments

@brooklynlash
Copy link

Test device
Lenovo ThinkPad

Operating System
Windows 10

Browser
Chrome

Problem description
This is for phetsims/qa#609
When using the keyboard navigation and tabbing over to the "both hands" option, the squares that demonstrate the buttons to click to move the hands pop up, but if you grab a hand with the mouse, those squares don't go away.

Visuals
ratioandpropissue2

Troubleshooting information:
!!!!! DO NOT EDIT !!!!!
Name: ‪Ratio and Proportion‬
URL: https://phet-dev.colorado.edu/html/ratio-and-proportion/1.0.0-rc.2/phet/ratio-and-proportion_all_phet.html
Version: 1.0.0-rc.2 2021-02-13 05:25:44 UTC
Features missing: applicationcache, applicationcache, touch
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/88.0.4324.150 Safari/537.36
Language: en-US
Window: 1707x818
Pixel Ratio: 2.25/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: 30 uniform: 4095
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 32767x32767
OES_texture_float: true
Dependencies JSON: {}

@zepumph
Copy link
Member

zepumph commented Feb 15, 2021

@jessegreenberg, this is another byproduct of having focus remain (but invisible) when you mouse-click on something that has focus. To work around this currently buggy behavior I added the blur logic to the down event. Can you please review?

I also want to note that I just glanced through all focus and blur listeners in the project, and didn't see any other obvious issues from this focus-changing behavior.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Feb 19, 2021

I think this is reasonable for the release. Would another way to do this be to link the visibility of interaction cues on focus to Display's focusHighlightsVisibleProperty? I am wondering if that is the pattern we need to adopt or abstract generally.

@zepumph
Copy link
Member

zepumph commented Feb 19, 2021

I'm trying to figure out which is less confusing as a patter. I'm unsure if down is a general enough solution to counter-act the keyboard focus. That said, it may be nicer than requiring a display in order to handle this. Either way feels a bit clumsey. I also am uncertain in general about how we now have focusHighlightsVisible to check in our focus events. part of me wants to try to use focusHighlightsVisible as little as possible.

@zepumph
Copy link
Member

zepumph commented Feb 22, 2021

I feel ok about 9cfb1cc for now. I'll move on with this, and we can discuss generality at a later time.

@zepumph
Copy link
Member

zepumph commented Feb 22, 2021

Cherry-picked

@phet-steele
Copy link

I am somewhat confused! It looks like there are three "states" for these interaction cues:

  • (1) The individual movement Up/Down arrow keys
  • (2) The simultaneous movement W/S arrow keys
  • (3) The simultaneous movement Up/Down arrow keys

What I notice is that you clear the cue for (1) by pressing Up/Down while having focus on just one hand, which is great! This keeps the cues for (2) and (3) when you tab to simulatneous focus, which is also great! Then you have to press W/S to get rid of (2), while still keeping (3) visible, then you have to press Up/Down to get rid of (3). That's all great!

However, my confusion stems from the idea that clearing (1) first does not clear (2) or (3), BUT clearing (2) and (3) first does clear (1)?

The sensical steps:

  1. Enter a screen
  2. Tab until you are controlling just one hand
  3. Press Up/Down to clear (1)
  4. Tab to go to simultaneous hand control, and notice the cues for (2) and (3) are still there. That makes sense to me.

The confusing steps:

  1. Enter a screen
  2. Tab until you are controlling both hands
  3. Press W/S to clear (2)
  4. Press Up/Down to clear (3)
  5. Shift-Tab to go back to single hand control, but notice the cues for (1) are gone? Why?

@phet-steele
Copy link

Alright I am going to move #350 (comment) to its own issue, but otherwise I am seeing that the bug reported here is fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants