yaptide / ui

Yet Another Particle Transport IDE - frontend
https://yaptide.github.io/web_dev/
GNU General Public License v3.0
9 stars 4 forks source link

1598 examples should be loaded from public instead packaged into app js bundle #1614

Closed dudiiiiiiii closed 1 month ago

grzanka commented 1 month ago

@dudiiiiiiii I think you need as well to rewrite the unit tests

grzanka commented 1 month ago

@dudiiiiiiii another question - is this PR ready for review ?

dudiiiiiiii commented 1 month ago

@dudiiiiiiii another question - is this PR ready for review ?

@grzanka Now the tests are passing cause all functionalities are implemented as they should but i still need to test if everything works well and clean it up from console logs and unnecessary comments

grzanka commented 1 month ago

@dudiiiiiiii early testing is important, even before code is fully refactored:

It seems that the examples are not working on gitpod, probably due to some URL issues. On gitpod UI is exposed at https://3000-yaptide-ui-1s7jt0zey66.ws-eu111.gitpod.io/, but frontend code tries to load examples from 127.0.0.1:3000/examples/ex4.json:1

image

I will check now on yap-dev

grzanka commented 1 month ago

Loading time and size of bundle is nice image

Still loading of examples fails, see errors below: image

The files are served properly: image

dudiiiiiiii commented 1 month ago

Fixed public folder url according to react documentation - for me gitpod works now

grzanka commented 1 month ago

@dudiiiiiiii do we need to keep examples in src/ThreeEditor/examples. Such location is far from obvious. What do you think of small refactor and moving them to src/examples ?

grzanka commented 1 month ago

Fixed public folder url according to react documentation - for me gitpod works now

Tests on yap-dev shows that it seems to work:

image

image

image

dudiiiiiiii commented 1 month ago

@grzanka We can move examples folder location rather freely so its easy refactor.

grzanka commented 1 month ago

@grzanka We can move examples folder location rather freely so its easy refactor.

Can you do it within this PR ?

grzanka commented 1 month ago

@dudiiiiiiii there are few minor comments on the PR - address them and I think we are almost ready to merge.

grzanka commented 1 month ago

@dudiiiiiiii what about this comment https://github.com/yaptide/ui/pull/1614/files#r1594054500 ?