codeLabs-summer2021 / 3D-Model-World

A real world environment to place, view, and share all your 3D models.
https://3d-model-world.netlify.app/
2 stars 0 forks source link

Local Storage #61

Closed CalebMcOlin closed 3 years ago

CalebMcOlin commented 3 years ago

Description

Added the ability to save and load models from the Local Storage. If there are any models in the Local Storage they are automatically loaded upon starting the app IF LOGGED IN. If not Logged in they will be loaded once logged in*

*This should work without extra code as when the user logs in, the site is refreshed causing the initial loading of models to be called

To Test

  1. Start the dev environment
  2. Log in using Sketchfab if needed
  3. Load at least two models in the map
  4. Verify the Local Storage is populated with the model's information
  5. Make sure when you delete the model it is also deleted from Local Storage
  6. Refresh the page
  7. Verify the models are loaded from Local Storage
  8. *Currently there is no way to check if the models are loaded upon log in. (might need to push to live for that check)
OmarShehata commented 3 years ago

One issue I found while testing here is that nothing seems to check for the Sketchfab token in the URL, which is how the app gets the token from Sketchfab after login.

You can test this by copying the URL from the live app into localhost. So if you open the following URL (and as long as this token is still valid) it should detect that I am logged in, but it doesn't seem to:

http://localhost:8080/#access_token=Hvsc3RG26siONxk0Ckte0CnJMmgB4J&expires_in=2592000&token_type=Bearer&scope=read+write&state=123456789

I think this actually broke from this commit https://github.com/codeLabs-summer2021/3D-Model-World/pull/61/commits/4818773818fbb98ebf1c57907b8d1493da825783. The function sketchfabIntegration.checkToken(); is what checks the token in the URL.

There should be no issue with there being two instances of the app (live & localhost)

OmarShehata commented 3 years ago

I just noticed that we're using jquery in this project. Do you know when that was added? If we're only using it for selecting elements, we can replace that with:

document.querySelector('#elementId')

I think there are equivalents natively in JavaScript now for stuff like $('#overlay').css('display', 'none') as well. Might be worth taking a crack at since a lot of modern codebases don't use Jquery. Might be good to do as a separate PR so as not to complicate this one.

OmarShehata commented 3 years ago

Ok other than the checkToken issue this works really well! I think the next big thing is gonna be all the UX things that make this a lot more usable like: