-
Notifications
You must be signed in to change notification settings - Fork 960
Fix reference errors in THREE.Math and THREEx #523
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
Conversation
Hi @Hyeonoo-Park I will review and will check the code in the next days when i have a bit of time. |
Hi @Hyeonoo-Park I tested your changes, and as @DougReeder pointed out in this PR #525 the aframe version used in the examples need to be upgraded to 1.3.0 can you make these changes for all the examples (location, markers and nft) ? |
aframe/src/new-location-based/arjs-device-orientation-controls.js
Outdated
Show resolved
Hide resolved
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.
LGTM
All the aframe examples will need to upgrade the Aframe version but i can do this. If nothing against @nickw1 i will merge this. POST-EDIT note: see this example is using aframe 1.0.4
|
aframe/src/new-location-based/arjs-device-orientation-controls.js
Outdated
Show resolved
Hide resolved
continue on #532 |
Bug fix!
What kind of change does this PR introduce?
bugfix
Can it be referenced to an Issue? If so what is the issue # ?
Fixes #515
How can we test it?
See https://github.com/Hyeonoo-Park/react-ar-js-test sample project
Summary
Does this PR introduce a breaking change?
???
Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser
Samsung Galaxy Note10+s(SM-N976N)
Chrome (110.0.5481.153)
Other information