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

Unintended zoom/keyboard behavior #287

Closed
KatieWoe opened this issue Dec 14, 2020 · 4 comments
Closed

Unintended zoom/keyboard behavior #287

KatieWoe opened this issue Dec 14, 2020 · 4 comments
Assignees
Labels
meeting:a11y-team type:bug Something isn't working

Comments

@KatieWoe
Copy link
Contributor

Test device
Dell
Operating System
Win 10
Browser
Chrome
Problem description
For phetsims/qa#582
It seems to be possible to manipulate both the hands in the sim and the magnification at the same time, which can lead to unintended behavior that can cause some confusion. If keyboard nav focus is on both hands and you are zoomed in, you can use ctrl + 0 to zoom out to normal, but in doing so you will press 0, which moves both hands to the bottom. Since ctrl was pressed, presumably only the zooming out behavior was intended.

Visuals
ctrl0behavior

Troubleshooting information:

!!!!! DO NOT EDIT !!!!!
Name: ‪Ratio and Proportion‬
URL: https://phet-dev.colorado.edu/html/ratio-and-proportion/1.0.0-dev.77/phet/ratio-and-proportion_all_phet.html
Version: 1.0.0-dev.77 2020-12-08 00:32:10 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/87.0.4280.88 Safari/537.36
Language: en-US
Window: 1280x658
Pixel Ratio: 1.5/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: 4096
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 32767x32767
OES_texture_float: true
Dependencies JSON: {}

@KatieWoe KatieWoe added type:bug Something isn't working type:a11y-bug labels Dec 14, 2020
@KatieWoe KatieWoe changed the title Unintended zoom behavior Unintended zoom/keyboard behavior Dec 14, 2020
@jessegreenberg
Copy link
Contributor

I think the best way to fix this would be to not have the sim specific behavior on "0" key if the "ctrl" key is down. However, let me know if that isn't possible @zepumph.

I considered adding a way "reserve" the Pointer for this case but it started to get complicated. Presumably we want to continue to allow zoom, we just want to prevent behavior on "0". So we would need an Intent on Pointer that would reserve behavior for a specific key. In my opinion that shouldn't be Pointer's responsibility which is why I am recommending a sim change.

@jessegreenberg jessegreenberg removed their assignment Dec 14, 2020
@zepumph
Copy link
Member

zepumph commented Dec 15, 2020

@jessegreenberg does this feel like a slippery slope, in which a Sim specific issues needs to know about all global functionality in the sim and manually avoid duplicating it? If the future might there be a Sim with general listeners for zoom, playback, and perhaps other things? Is it solely a design consideration to know about these all and avoid overloading keys?

I think the answer is probably to handle this in a Sim specific way, and I'm fine with that here.

If we can forget about the overlap in this case, we can certainly forget again.

Do you have any thoughts about at least marking certain keys as "reserved" so we can fail loudly if we overload key commands?

@jessegreenberg
Copy link
Contributor

I am not sure. This does seem like a problem, and if we cannot catch in code then we need a well documented list of the taken hotkeys. Currently, the overlapping key is in an arbitrary listener somewhere in the sim code, and I can't think of a way to detect this. Not sure how it would work, but perhaps one day we could have "hot key registration" class that keeps track of this for us.

@zepumph
Copy link
Member

zepumph commented Dec 22, 2020

@jessegreenberg and I decided that the manual approach seemed acceptable for now, and we added doc in the code review checklist to look for these overlaps in the future.

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

No branches or pull requests

3 participants