Skip to content
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

Auto save #957

Merged
merged 12 commits into from
Dec 20, 2024
Merged

Auto save #957

merged 12 commits into from
Dec 20, 2024

Conversation

rahulkgupta
Copy link
Collaborator

@rahulkgupta rahulkgupta commented Dec 9, 2024

What this does

  • autosaves an imported scene from streetmix
  • A modal for creating new scenes
  • new scenes are autosaved
  • Thumbnails are part of autosave
  • explicit thumbnail save has been removed

What's needed

  • code cleanup
  • make sure the css of the new tabs look right
  • there might be a scenes modal issue when opening after a new scene was saved
  • link to open scene

Future Work

  • Open up the geomodal after creating a scene
  • block new scene creation upon payment
  • add more new scene options

@kfarr
Copy link
Collaborator

kfarr commented Dec 12, 2024

a couple reactions after playing around for a bit:

  • removing .json is not backwards compatible for links, ie visiting a link with .json does not load.
  • it is not clear to the user that the scene has been saved in 3dstreet except for the change in URL path. The save icon turning purple is too fast and also doesn't make it clear that a save action was completed. Would it make sense to bring back a more explicit success notification for first save?
  • i see the issue with the thumbnail preview now, it is not that the thumbnail is blank (black) instead it is that it is accurately getting a screenshot but the streetmix street hasn't loaded yet.
  • i see 2 odd visual issues that i haven't seen before: for some reason i see a strange vertically oriented street for a second while the street is loading before it places correctly [1]; one of the striping elements is incorrect after loading [2]; i don't see these on the main branch. i haven't dug into the possible causes; one crazy idea is that it's saving the scene json "too soon" before these things have been resolved properly
  • i tried for a few minutes to resolve toolbar conflict manually to update from main but that was too hard for me

[1]
image

[2]
image

@rahulkgupta
Copy link
Collaborator Author

rahulkgupta commented Dec 12, 2024

yeah i think you're right there's some issue with saving too soon

@rahulkgupta
Copy link
Collaborator Author

i think we could add a route handler for the json, and im not too worried about users knowing if things are saved when first imported because i think the assumption is that the scene is saved when imported.

@kfarr
Copy link
Collaborator

kfarr commented Dec 19, 2024

here are a few more minor things, I'll try working on the ui stuff:

  • change "open scene" button icon from streetmix to > open icon
  • change "create new scene" button icon from streetmix to > new scene (pencil) icon
  • clicking "cancel" on streetmix url doesn't work (loads a blank scene), intended behavior would be return to new modal
  • add preview jpg for the 2 new options of blank and streetmix import
  • minor issue, maybe don't need to resolve it but fyi -- what happens when i create a new file, make edits, and then delete that file that is still open from open menu? then if i make more changes after the auto save there is a red permission denied error.

@kfarr
Copy link
Collaborator

kfarr commented Dec 20, 2024

check out attached, lmk if ok to merge like this

image

DONE- change "open scene" icon from streetmix to > open scene
DONE- change "create new scene" icon from streetmix to > new scene pencil
DONE- add preview jpg for the 2 new options of blank and streetmix import
WON'T FIX- minor issue, maybe don't need to resolve it but fyi -- what happens when i create a new file, make edits, and then delete that file that is still open from open menu? then if i make more changes after the auto save there is a red permission denied error.
WON'T FIX- clicking "cancel" on streetmix url doesn't work (loads a blank scene), intended behavior would be return to new modal

@kfarr kfarr merged commit 2926a00 into main Dec 20, 2024
1 check passed
@vincentfretin vincentfretin deleted the auto-save branch January 16, 2025 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants