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

move xxxInternal extendable #189

Merged
merged 1 commit into from
Jun 4, 2021
Merged

Conversation

yomotsu
Copy link
Owner

@yomotsu yomotsu commented Jun 4, 2021

this PR could solve the issue: #187

Currently rotateInternal() and so are scoped and cannot modify by users. this change allows users to modify them.

based on @farfromrefug's PR https://github.com/yomotsu/camera-controls/pulls

use case:

class MyCameraControls extends CameraControls {
	constructor() {
		super();
	}

	private _rotateInternal( deltaX: number, deltaY: number ): void => {
		// what ever
	}
}

@yomotsu
Copy link
Owner Author

yomotsu commented Jun 4, 2021

@farfromrefug
Sorry, I made another PR that I mentioned...
I know you have spent your time on the PR but I thought this is the best way to show my thought...
Hope this change would be some hints for your next PR, which is to make event handlers to be extendable.

Could you take a look at the change and does this solve the part of your problem?

@farfromrefug
Copy link
Contributor

@yomotsu thanks and really dont worry, just glad we are getting it.
Yes it solves my issue about handling zoom while dragging!
i ll try and create the other PR. Thanks

@yomotsu
Copy link
Owner Author

yomotsu commented Jun 4, 2021

Thanks for understanding and taking a look.
I'll merge this.

@yomotsu yomotsu merged commit 42b76b7 into dev Jun 4, 2021
@yomotsu yomotsu deleted the feature/make-xxx-internal-extendable branch June 4, 2021 23:14
@PlopTheReal
Copy link

PlopTheReal commented Feb 18, 2022

Hey,
Thanks for that feature. I'm trying to animate the dolly by I'm getting the following within the constructor:
TypeError: Cannot read properties of undefined (reading 'up')
Any idea on such error?
Thanks

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