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

Set a tabindex on the canvas #7484

Merged
merged 2 commits into from Nov 12, 2018

Conversation

Projects
None yet
2 participants
@kripken
Copy link
Owner

kripken commented Nov 9, 2018

Without it, it cannot be focused, so the user cannot click on it and have the canvas receive events. For example,

  emscripten_set_keypress_callback("#canvas", 0, 1, key_callback);

(note #canvas) will just not receive any events.

Set a value of -1, which just means 'focusable' and does not include it in the tab ordering, which is the same as before.

I'm surprised we didn't notice this before, so perhaps I'm missing something here? The attached testcase is fixed by this PR though, and it seems like something that should work out of the box?

set a tabindex on the canvas: without it, it cannot be focused, so th…
…e user cannot click on it and have the canvas receive events. set a value of -1, which just means 'focusable' and does not include it in the tab ordering, which is the same as before

@kripken kripken requested a review from juj Nov 9, 2018

@juj

This comment has been minimized.

Copy link
Collaborator

juj commented Nov 12, 2018

A popular practice is to register key events on window or document, so that one is not required to click (or call canvas.focus();) to manage focus, e.g. https://codeincomplete.com/posts/javascript-game-foundations-player-input/ or https://developer.mozilla.org/en-US/docs/Games/Techniques/Control_mechanisms/Desktop_with_mouse_and_keyboard .

Tabindex affects how one tab cycles across fillable form/text input elements, so canvas will then become one of those elements when a tabindex is given to it.

Looks good to me to add tabindex to default shell, but perhaps the proper fix is to document in emscripten_set_key*_callback() functions that the target element needs to be focusable (if it is not window or document or an input element, for more see https://stackoverflow.com/a/1600194).

@juj

juj approved these changes Nov 12, 2018

@kripken

This comment has been minimized.

Copy link
Owner

kripken commented Nov 12, 2018

Thanks @juj, I'll update the docs too.

@kripken kripken merged commit 1fc1c90 into incoming Nov 12, 2018

23 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build-docs Your tests passed on CircleCI!
Details
ci/circleci: flake8 Your tests passed on CircleCI!
Details
ci/circleci: test-ab Your tests passed on CircleCI!
Details
ci/circleci: test-binaryen0 Your tests passed on CircleCI!
Details
ci/circleci: test-binaryen1 Your tests passed on CircleCI!
Details
ci/circleci: test-binaryen2 Your tests passed on CircleCI!
Details
ci/circleci: test-binaryen3 Your tests passed on CircleCI!
Details
ci/circleci: test-browser-chrome Your tests passed on CircleCI!
Details
ci/circleci: test-browser-firefox Your tests passed on CircleCI!
Details
ci/circleci: test-c Your tests passed on CircleCI!
Details
ci/circleci: test-d Your tests passed on CircleCI!
Details
ci/circleci: test-e Your tests passed on CircleCI!
Details
ci/circleci: test-f Your tests passed on CircleCI!
Details
ci/circleci: test-ghi Your tests passed on CircleCI!
Details
ci/circleci: test-jklmno Your tests passed on CircleCI!
Details
ci/circleci: test-other Your tests passed on CircleCI!
Details
ci/circleci: test-p Your tests passed on CircleCI!
Details
ci/circleci: test-qrst Your tests passed on CircleCI!
Details
ci/circleci: test-sanity Your tests passed on CircleCI!
Details
ci/circleci: test-uvwxyz Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@kripken kripken deleted the canvas_focus branch Nov 12, 2018

Beuc added a commit to Beuc/emscripten that referenced this pull request Nov 17, 2018

set a tabindex on the default html canvas (kripken#7484)
Without it, it cannot be focused, so the user cannot click on it and have the canvas receive events. For example,

  emscripten_set_keypress_callback("#canvas", 0, 1, key_callback);

(note #canvas) will just not receive any events.

Set a value of -1, which just means 'focusable' and does not include it in the tab ordering, which is the same as before.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment