Closed brad closed 3 years ago
This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.
🔍 Inspect: https://vercel.com/zalo/cascade-studio/mejkc3jb3
✅ Preview: https://cascade-studio-git-fork-brad-better-saving.zalo.vercel.app
Wow; this looks great! Thank you for adding .gifs! I’ll try to review this in the next few days.
The functionality/statefulness around the “MainProject” is a bit squirrelly (I was trying to emulate a bit of paper.js’s Sketch IDE auto-saving functionality, but probably didn’t hit the mark completely); hopefully I can push towards internal consistency there a bit more...
Thank you!
Thoughts-in-progress (I'll be reviewing more and probably submitting changes over the week):
The saving and loading flow seems to work well; a big improvement over what was there before!
Hrmm usually loading a project file refreshes the page, but since this isn't, it's making trouble with the typescript editor thinking variables are getting redeclared in the global scope.
This also (incompletely) removes the local storage-saving aspect, which I'd be a little sad to see go, but I understand why it's confusing.
Wow; this looks great! Thank you for adding .gifs! I’ll try to review this in the next few days.
👍 No problem! It's the least I can do to contribute a little back to your awesome application
Hrmm usually loading a project file refreshes the page, but since this isn't, it's making trouble with the typescript editor thinking variables are getting redeclared in the global scope.
Shoot, I didn't notice that. I changed things to avoid refreshing the page in order to retain the file handle. I couldn't come up with any way to keep the file handle through a refresh, I don't think it can be serialized to localstorage for instance.
No worries there; when I have time, I’ll come back through and figure out how to reset Monaco’s internal model.
It’s saying goodbye to the MainProject/automatic local storage-side of things that I’m going to need a bit more time to come to terms with...
Sorry this took me so long to get to; I've added a bit of state cleanup (the initialize function didn't properly kill all the old state of the window on its second call before). This should also fix the intellisense throwing a fit every time a new project is loaded.
I'm still going to need a bit of time to think through the consequences of getting rid of local storage and the possible redundancy/conflict of URL sharing...
@zalo Thanks for the updates. I'll test it out over the holidays and maybe I'll be lucky and have a brilliant insight for how to resolve the conflict
I think I'm going to merge this now and face the consequences later...
Sounds good! Sorry, I forgot to report back but I did not have any insights 🤷
Sorry, I'm late to this. I'm a bit concerned that we've just nuked some cross-browser compatibility with this change.
Compatibility for Window.showOpenFilePicker
is pretty low.
I mean chrome (and by extension edge) have the lion's share of browser users, but making exports incompatible with Firefox plus the default browser on mac and all phones seem like a bit of a shame. Maybe we could put a fall back in place for non-chrome browsers?
Thoughts @zalo, @brad?
Another thought. Instead of adding our own fallback, surely this problem of cross-browser file picking has been solved before, might there be an npm package that does this for us? (I haven't checked to see if there's anything good).
Sorry @zalo that was a rookie mistake not to check support on that feature. Looks like there are a couple ponyfills that could smooth out support: https://github.com/GoogleChromeLabs/browser-nativefs https://github.com/jimmywarting/native-file-system-adapter/
But as I understand it, it's not super easy to incorporate new dependencies at this point?
Polyfills nice @brad,
I've actually got a PR up to convert the project to modules with webpack in #39, I'm not sure how likely that is to be merged but that should make adding those polyfills much easier.
FWIW, this isn't supported in Brave either, even though it's Chromium based.
That stinks that this feature is taking so long to be taken up by other browsers...
I generally just use Chrome, but if there's a PR to add the old file support back, I'll take a look.
@zalo Here's what I've come up with for #30. The changes:
*
if the code has changed since the last saveNOTE: One caveat on this. There is a little weirdness when using this in a browser tab and reloading the page with
?code
in the URL since we lose the file handle but retain the code. I'm not sure how to fix this. I prefer to use the app in an installed fashion so this didn't bother me.Saving the project
Saving other file types
Loading then saving