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

Set a tabindex on the canvas #7484

Merged
merged 2 commits into from Nov 12, 2018
Merged

Set a tabindex on the canvas #7484

merged 2 commits into from Nov 12, 2018

Conversation

@kripken
Copy link
Member

@kripken 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?

…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
Copy link
Collaborator

@juj 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
Copy link
Member Author

@kripken 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
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
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.
haberbyte added a commit to haberbyte/emscripten that referenced this pull request Jan 31, 2021
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
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants