OrbitControls: get keydown events from scope.domElement

[greggman]

Binding to window has issue that keys events are received
even when not meant for OrbitControls. It also means arrow
keys can be blocked since when an arrow key is pressed
the OrbitControls call event.preventDefault so that the
page does not scroll.

Binding to scope.domElement is either the document if no
element is passed in (in which case the same issue as mentioned
above exists) but if you pass in an element like the canvas
itself then you can receive keyboard events on other elements
providing a workaround when needed to the issue mentioned
above.

fixes #16597

[Mugen87]

Can you please rebuild the module file via node utils/modularize.js? In this way, it's possible to verify the change with rawgit.

If everything is fine, I vote to merge this PR.

[Mugen87]

I have tested this change on my local computer. While the fiddle from #16597 is fixed, the behavior of the official example (https://threejs.org/examples/#misc_controls_orbit) changes.

Right now, when you open it you can directly use the key controls. With this PR, you have to click on the canvas first so it has the focus.

[EliasHasle]

Can https://www.w3schools.com/jsref/met_html_focus.asp be used for that example? Perhaps with an optional constructor param autofocus to the controls.

[Mugen87]

Perhaps with an optional constructor param autofocus to the controls.

I don't think it's worth to modify the ctor just for this use case. Instead, I would accept this change in behavior since adding event listeners to domElement seems the less obtrusive approach to me. We just have to be aware of this side effect (I bet there will be a user that is going to complain about this change^^).

However, the PR needs an update: It's not necessary anymore to test for document. So this is sufficient:

if ( scope.domElement.tabIndex === - 1 ) scope.domElement.tabIndex = 0;

[Mugen87]

Closing. Feel free to reopen the PR when my suggested changes are implemented.

[WestLangley]

Feel free to reopen the PR when my suggested changes are implemented.

Why was this PR closed?

[mrdoob]

I think it's because we no longer support document passed as domElement.

[EliasHasle]

No, because this PR relates to this line, where a global listener is defined, irrespective of the domElement argument:

three.js/examples/js/controls/OrbitControls.js

Line 1139 in 8fcc7b8

window.addEventListener( 'keydown', onKeyDown, false );

[Mugen87]

I think it's because we no longer support document passed as domElement.

Correct. Line 940 needs a fix that I've posted here: #16676 (comment). I've closed the PR since @greggman did not react on the feedback.

@EliasHasle Would you like to perform this change?

[EliasHasle]

Hm, I still think this is @greggman's "territory", so to speak. You gave him "only" a week to answer, whereupon you closed the PR. I think the PR is still just as relevant. And a one week delay for something not life-critical is not that much. (Compare to the delays in handling PRs. Some are years old.)

[EliasHasle]

(So much for "territory"...)

[Mugen87]

I've also requested a rebuild of the module files here (with no reaction): #16676 (comment)

So I think it's okay if somebody else works on the issue.