-
Notifications
You must be signed in to change notification settings - Fork 142
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
feat: save and restore workspaces, add menu bar #79
Conversation
✅ Deploy Preview for webcrack ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
A few questions/comments/thoughts.
See also my feedback in this comment:
|
||
function restoreSavedModels() { | ||
batch(() => { | ||
models().forEach((model) => model.dispose()); |
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.
Does this clear any currently open tabs? If so, I wonder if that might be unexpected for users sometimes?
Realistically (at least with the current 5sec countdown on the restore dialog) it's probably rare (but not impossible) that you would get in a state of having open work that gets lost via this.
I wonder if making it so it only closes if it's the default/empty 'new tab' might be safer?
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.
Does this clear any currently open tabs
Yes, they can't be kept because each model needs to have an unique URI (path)
A possibility would be saving them before or showing a confirm dialog
Closes #78