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

OrbitControl: Fixed rotation toggle in KeyDown handler #29834

Merged
merged 4 commits into from
Nov 12, 2024

Conversation

i3ene
Copy link

@i3ene i3ene commented Nov 7, 2024

Implemented the toggle via enableRotate also on _handleKeyDown. The previous behaviour seemed unexpected, allowing rotation via keys, even when rotating should be disabled if enableRotate was set to false.

Description

The previous implementation of _handleKeyDown did not take the enableRotate state into account. This seemed unexpected when enableRotate was set to false, but rotation was still possible with key events.
This commit guards the rotation of the keydown event additionally with the already available enableRotate variable and makes it more intuitive in its behavior again.

Implemented the toggle via `enableRotate` also on `_handleKeyDown`. The previous behaviour seemed unexpected, allowing rotation via keys, even when rotating should be disabled if `enableRotate` was set to `false`.
@i3ene i3ene changed the title Fixed rotation toggle in KeyDown handler OrbitControl: Fixed rotation toggle in KeyDown handler Nov 7, 2024
Refactor `enablePan` in `_handleKeyDown()`.
Correctly set `needsUpdate` in `_handleKeyDown()`.
@Mugen87 Mugen87 added this to the r171 milestone Nov 12, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 12, 2024

I've slightly refactored the PR to handle enablePan in _handleKeyDown() more correctly.

Revert `needsUpdate` change.
@Mugen87 Mugen87 merged commit 6f5d901 into mrdoob:master Nov 12, 2024
8 of 9 checks passed
Mugen87 added a commit that referenced this pull request Nov 12, 2024
Mugen87 added a commit that referenced this pull request Nov 12, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 12, 2024

Unfortunately, the PR targeted master so I had to revert the PR. Added the changes to dev via e08c3e7.

@i3ene Please always target dev when filing PRs.

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.

2 participants