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

Blur previous element when interacting with Cesium canvas. #8662

Merged
merged 3 commits into from
Mar 4, 2020

Conversation

mramato
Copy link
Contributor

@mramato mramato commented Mar 4, 2020

Interacting with a canvas does not automatically blur the previously focused element. This leads to unexpected interaction if the last element was an input field. For example, clicking the mouse wheel could lead to the value in the field changing unexpectedly. The solution is to blur whatever has focus as soon as canvas interaction begins.

@emackey can you see any flaws/problems in this approach?

Interacting with a canvas does not automatically blur the previously
focused element. This leads to unexpected interaction if the last element
was an input field. For example, clicking the mouse wheel could lead to
the value in the field changing unexpectedly. The solution is to blur
whatever has focus as soon as canvas interaction begins.
@cesium-concierge
Copy link

Thanks for the pull request @mramato!

  • ✔️ Signed CLA found.
  • ❔ Unit tests were not updated.
    • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
  • Works (or fails gracefully) in IE11.

@emackey
Copy link
Contributor

emackey commented Mar 4, 2020

This seems like a good idea.

One concern I might have is that there might not be a way to get the canvas to receive keyboard input, like hotkeys, if any apps are using that. Clicking the canvas will cause it to blur itself.

Not sure the solution... maybe blur the active element and then attempt to focus the canvas? Or just double-check that it's not the canvas that's being blurred?

@mramato
Copy link
Contributor Author

mramato commented Mar 4, 2020

Not sure the solution... maybe blur the active element and then attempt to focus the canvas? Or just double-check that it's not the canvas that's being blurred?

Good thinking. However I'm having a hard time actually triggering what I agree would be a bug in the fix here.

I'm using this demo to test: http://localhost:8080/Apps/Sandcastle/index.html?src=HeadingPitchRoll.html

It listens to the document element, but with my code in this branch it still works. I assume this is because the event bubbles up to the document when no element has focus. This demo is also kind of sloppy by listening to document because if you active the geocoder and press up/down, it still affects the plane instead of the geocoder.

Working on that assumption, I then changed the Sandcastle to use viewer.scene.canvas.addEventListener('keydown', function(e) { so that it now only listens to the canvas. I then select the geocoder input field and nothing happens with up/down arrows, as to be expected. But, when I click and interact with the Cesium window and then use the arrows, they still work! As you pointed out, this doesn't seem to be possible because the canvas should no longer be the active element.

So it appears that nothing extra is actually needed here, but I don't fully understand what.

Thoughts?

@emackey
Copy link
Contributor

emackey commented Mar 4, 2020

Yeah that does seem to work. The demo's click handler is registered after the viewer is constructed (of course, because prior to that there's no canvas). So I guess the blur happens before the focus, and everything is OK.

I still feel like it would be good to check that the canvas isn't blurring itself. But I don't quite know how an app would manage to get into that state exactly.

@emackey
Copy link
Contributor

emackey commented Mar 4, 2020

So yeah, if the canvas doesn't focus itself but it has a tab index, then you can actually TAB your way into it if you try hard enough, and then a click will blur it. Kind of an extreme edge case, probably no one has ever done that before.

@emackey
Copy link
Contributor

emackey commented Mar 4, 2020

yep I went there

In this demo, click-to-focus is commented out. One must instead click on the Cesium tab title, where it says the branch name above the Cesium window. The branch name should get a blue outline when you do this. Then, press TAB, and now the canvas has focus. Now you can use the arrow keys to fly the plane. But if you click the canvas, it blurs itself, losing focus. Clearly a mainstream use case here.

@mramato
Copy link
Contributor Author

mramato commented Mar 4, 2020

Thanks for your diligence here @emackey, I pushed a fix to check if canvas is the element focused before blurring.

CHANGES.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants