-
Notifications
You must be signed in to change notification settings - Fork 40
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
Migration to React three fiber #775
Conversation
Signed-off-by: angatupyry <fierrofenix@gmail.com>
…x component noise Signed-off-by: angatupyry <fierrofenix@gmail.com>
…x component noise Signed-off-by: angatupyry <fierrofenix@gmail.com>
…x component noise Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Codecov Report
@@ Coverage Diff @@
## main #775 +/- ##
==========================================
- Coverage 53.92% 51.21% -2.72%
==========================================
Files 263 281 +18
Lines 6639 7055 +416
Branches 882 948 +66
==========================================
+ Hits 3580 3613 +33
- Misses 2919 3299 +380
- Partials 140 143 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 9 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
* Disable walls Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Use useLoader with url only, catch CORS errors potentially due to timeout issues when images are large Signed-off-by: Aaron Chong <aaronchongth@gmail.com> --------- Signed-off-by: Aaron Chong <aaronchongth@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
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.
Thanks for helping with this huge migration! I think we're in pretty good shape now. There might be small issues here and there that pops up, but we can fix them later on.
I'll move this to a reviewable so CI can be run on it. Once we address all the check failures and CI passes, I think we are good.
export const ReactThreeFiberImageMaker = ({ level, imageUrl }: ImageThreeProps): JSX.Element => { | ||
let texture: Texture | undefined = undefined; | ||
try { | ||
texture = useLoader(TextureLoader, imageUrl); |
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 is a tricky check failure (sorry I introduced this), however I'm unsure how we can be sure the imageUrl
is accessible in the first place, these were some of the things I found, https://www.freecodecamp.org/news/how-to-validate-urls-in-javascript/.
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.
It is currently working properly, could you try it?
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
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.
Just a comment left regarding linting, otherwise LGTM
packages/react-components/lib/map/three-fiber/shape-three-rendering.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: angatupyry <fierrofenix@gmail.com>
Signed-off-by: angatupyry <fierrofenix@gmail.com>
* Set max heap size Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Stop producing sourcemap for CI Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * max space to ci Signed-off-by: angatupyry <fierrofenix@gmail.com> * Try build without sourcemap for CI, alongside heap size increment Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Increasing heapsize on bootstrap step Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Use experimental support for ECMAScript modules Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Use react-components as a module instead Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Remove use of meshes for robot icons Signed-off-by: Aaron Chong <aaronchongth@gmail.com> * Revert module call Signed-off-by: Aaron Chong <aaronchongth@gmail.com> --------- Signed-off-by: Aaron Chong <aaronchongth@gmail.com> Signed-off-by: angatupyry <fierrofenix@gmail.com> Co-authored-by: angatupyry <fierrofenix@gmail.com>
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.
Thank you! Looks good, just a few tweaks and naming suggestions left
Signed-off-by: angatupyry <fierrofenix@gmail.com>
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.
I think we're in a good state! LGMT, thanks for this huge effort!
What's new
Migration to React three fiber
Pending:
Screencast.from.09-19-2023.07.41.42.PM.webm
Self-checks
Discussion