fix(OrbitControls): expose ref #250
Conversation
|
|
✅ Deploy Preview for cientos-tresjs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -315,6 +315,8 @@ onUnmounted(() => { | |||
| controlsRef.value.dispose() | |||
| } | |||
| }) | |||
|
|
|||
| defineExpose({ value: controlsRef }) | |||
There was a problem hiding this comment.
Should we call expose defineExpose({ control: controlsRef })?
The value.value makes me uncomfortable
There was a problem hiding this comment.
I suggest we stick with value.value for the moment. Even if it's weird, it's consistent with the other Cientos components.
Problem
value.value is bad DX, but it's the established pattern in Cientos at the moment.
cientos/src/core/shapes/Box.vue
Line 34 in 106914e
cientos/src/core/abstractions/Levioso.vue
Line 24 in 106914e
Solution
@JaimeTorrealba opened an issue to discuss value.value a little while ago, but there wasn't any feedback.
I suggest we discuss there and then apply the decision to all components. Jaime is on vacation starting on the 14th and we'll want his input before moving forward.
In the meantime, I suggest we merge here to keep the components consistent.
There was a problem hiding this comment.
@andretchen0 you are totally right, let's merge this one as it is, and then let's discuss the value.value here #160 ✌️
Closes #249
Problem
In Cientos'
<OrbitControls />, the camera and update functions aren't exposed.Solution
Expose a ref to the component using Vue's
defineExpose. This is the approach taken by other Cientos' components, e.g., Levioso:cientos/src/core/abstractions/Levioso.vue
Line 23 in 106914e
With this PR, the camera is accessible via
[refName].value.value.object. Update() is available via[refName].value.value.update.