-
Notifications
You must be signed in to change notification settings - Fork 20
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
fix(react-components): Improve Scene home camera button and bump to 0.55.2 #4695
fix(react-components): Improve Scene home camera button and bump to 0.55.2 #4695
Conversation
return position | ||
.clone() | ||
.add(new Vector3(0, 0, -positionToSceneCenterDistance).applyQuaternion(rotation)); | ||
return position.clone().add(new Vector3(0, 0, -50).applyQuaternion(rotation)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Models are not neccesarily loaded, will thus often have the wrong bounding box. Using a constant for distance to target instead
@@ -32,13 +32,42 @@ export const useSceneDefaultCamera = ( | |||
}; | |||
} | |||
|
|||
const cameraNotSet = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boolean values should be prefix with is
, should
or has
in my opinion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually, I would also prefer them to be positive. In this case, I would go for isCameraSet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to work!
Type of change
Description 📝
Improves Scene home camera position.
How has this been tested? 🔍
Have built the package locally and tested it in Fusion
Test instructions ℹ️
Build locally and use in Fusion. Verify that Scene home camera is correct
Checklist ☑️