Closed Bug 924398 Opened 11 years ago Closed 10 years ago

Implement Auto-Save on Projects

Categories

(Webmaker Graveyard :: Popcorn Maker, defect)

Product:
Webmaker Graveyard
Component:
Popcorn Maker
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: kat, Assigned: mjschranz)

References

Details

(Whiteboard: [crossteambugs])

Attachments

(1 file)

https://github.com/mozilla/popcorn.webmaker.org/pull/381
56 bytes, text/x-github-pull-request
thecount
: review+
Details | Review 
Hi guys. I've gotten the request from a few different Think Big leads around Europe that they'd really like to see an Auto-Save function on makes within both Thimble and Popcorn. 

The reason for needing this is that they work on the tools with very large school groups (30-60 kids often) and as a result of connectivity/many people being logged in at same time, work is often lost when browsers freeze. 

I know that sometimes work has been saved on reload, and am not sure how complex an Auto-Save function would be to implement, but thought I'd share it as it's one of our most-requested functions in Europe at moment.
We've actually got this fully implemented in Popcorn Maker, and have since 1.0.  However, it's not turned on because we've never been able to get the UX quite right.  It seems like a simple thing, but there are a million edge cases and annoyances.

I've love to re-enable it, though.  Do you think that any of them would be interested in helping us figure out how to make it work?  It probably requires a bunch of user testing.
Thanks for this Humph. Will get Ben (Think Big Schools) involved here.
Assignee: nobody → schranz.m
Yep, still lurks in the background of Popcorn Maker (you can actually see it technically still in some rare cases which is a bug).

We definitely need some assistance/ideas on the UI/UX side of it as that's why we didn't land it last year. Currently, you will get a popup like https://dl.dropboxusercontent.com/spa/qxpeeqhhsjhcqcf/ahhl7nav.png which wasn't a part we contested in general. It was more in the case the user didn't load it and they started making changes before saving something again. We wanted to leave some UI notification that backups existed until they explicitly saved something again.
Hey guys, hope you're well. Thanks for bringing this up Kat. I'm Ben and i'm the Think Big School lead in the UK for O2. We haven't actually been using Popcorn in our last sessions, we have been focusing on just using Thimble due to time constraints. Any news on if a auto-save feature will be installed on this yet? For the moment i'll be asking the young people to make a duplicate copy of the text they input into the page into notepad.
This is something we heard several times at #Mozfest, especially when a few folks lost their makes. Would love to get this feature upleveled :)
Whiteboard: [crossteambugs]
+1. still something folks are asking for, with sadness in their eyes ;)
Just going to mention one pitfall of autosave which is when it saves over work that you actually don't want saved. Say you're experimenting with an idea for an edit and its not working out the way you thought it would, so you decide to go back to the way it was when you first loaded the project...can't do that if it autosaves over your work.

We run this risk without a Save As or Undo option, which other autosave based programs have i.e. Google Docs.

There is a hacky Save As method which is to copy+paste the URL into a new tab and add /remix to it. 

Perhaps we can come up with a Save As that does this action, basically opens a new tab and forks a remix but instead of saying "remix of project title" it says by default "project title version 2" or something. Still would have UI challenges and the user might not get it.
Just bringing the above ^ up as a potential issue, but I do agree that it's probably better to have autosave implemented and not get crashed on then it is to potentially have to go back and redo work that you didn't want saved over.

We could also have a checkbox that defaults to autosave, but allows you to easily uncheck in case you're doing something experimental that you don't want to be autosaved over.
Autosave should save in a separate place, and not over your work. That's how it's implemented in things like LibreOffice. I can choose to restore the autosave, or not. And if I restore it and don't like it, I can just close the result and reopen the original file. Or I can purposefully save on top.

Given all these precedents, autosave which goes directly to the same file and overwrites your previous save without explicit user intervention violates the Principle of Least Surprise.

The UI given in the attached screenshot seems about right.

Gerv
Works well locally for me on popcorn. If it's disabled on prod, let's enable that, and if it has bugs/is not obvious, we should improve it. Let's just not invent something new ;)

The screenshot here: https://dl.dropboxusercontent.com/spa/qxpeeqhhsjhcqcf/ahhl7nav.png is what I see all the time when I lose work, I have never had a problem locally. Works very well.

We should also implement it in thimble.
If we don't just switch this on, we should do a stopgap patch which prevents Popcorn from erroneously telling the user that their work will be restored when it won't. That is really annoying :-)

But yes, switch it on!

Gerv
The problem was never the UI linked in that screenshot. The disagreeing was always what to do if they didn't explicitly accept it the first time. Do we immediately destroy that backup? Do we keep it until they save something again? If the latter, what kind of UI do we keep around to notify the user that it still exists?
We can start with the simple implementation - if you reject the auto-restore, it's gone for ever. If people still experience data loss, we can work out why, and consider either saving it for longer, or changing the UI to avoid whatever confusion is arising.

Gerv
Hi guys - just checking in on status of this. We're still getting asked for this feature a lot by different distribution partners who are worried kids will lose their work in the shuffle of workshops.
Follow bugs will need to be filed for not only determining the UI mentioned for Popcorn Maker but also with Thimble and other areas of Webmaker that will desire this.

For now, this is going to be about just re-enabling the basics.
Status: NEW → ASSIGNED
Component: webmaker.org → Popcorn Maker
Man, triggering the auto save is a pain :P I see it locally all the time, so, what is it that turns it on? Obviously not code. It must be a config value of some sort?

Do we have tickets for the other parts, like implement this in thimble?

Also, anyone that is familiar with the current popcorn auto save, file new tickets (not this one) for improvements.
It is 100% a config variable being turned on to have things actually backup again. There are some UI bugs that need to be fixed however.

I'll file the other bugs.
Can we be clear on these UI bugs/list the new bugs here? I'm curious because I think it works good enough locally, and didn't even know it was disabled on prod until now, and now that I know this, I'm struggling to see a real need to have it disabled.
I can strip out the butter ready event changes into another bug that can land before this, but that code is needed to ensure we actually wait for the project's data to be loaded before we trigger our ready event.
Attachment #8345350 - Flags: review?(scott)
Yeah, I am stuggling to see why we need to wait for project data to be loaded before we trigger the ready event.

Also, it looks like you just squashed both ready events into one? At one point, they were different for a reason. So, I just want to make sure that those differences are no longer real, or that is what you're attempting here.
Comment on attachment 8345350 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/381

This looks fairly sane.

I had one question in the pull request, just need calrification as to what was wrong, and why it needed to be changed. R+ with that.
Attachment #8345350 - Flags: review?(scott) → review-
Attachment #8345350 - Flags: review- → review?(scott)
Comment on attachment 8345350 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/381

A crash or the below dialog is what I think should trigger the backup.

There was a problem saving your project, so it was backed up to your browser's storage i.e., you can close or reload this page and it will be recovered). Please try again.

That dialog is from a failure to save.

Basically, if we go into an error state, I want the backup to come up.

One way I see this as being broken is if locally I change my time, It'll fail to publish, and display the error above, however I don't get the dialog. Oddly enough, my project DID save. So that's odd.

If I crash using Butter.app.simulateError(); it should also load the backup.

Can we do this and sort out where it is failing?
Attachment #8345350 - Flags: review?(scott) → review-
Comment on attachment 8345350 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/381

Updated. Those two cases should also trigger backups now.
Attachment #8345350 - Flags: review- → review?(scott)
Comment on attachment 8345350 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/381

I don't want to use the concept of a polling backup. It is aggressive and unpredictable.

It saves a backup even if they got the unload dialog and said yes they wanted to leave. So in essense we ask them twice. That is both aggresive and unexpected.

Even if I save a project, and then make no changes, it saves a backup, that is too aggresive.

It might be a number of seconds out of date, which makes it unpredictable.

It only does the backup if it has been saved once. This is unexpected. How does the user know they have to save once before they are safe from lost data. Also, what if there is a bug that makes it so they cannot save at all, one of the best reasons to have backups in the first place, but they cannot get to that point.

I want to backup in cases where they left the page without getting an "are you sure you want to leave" unload dialog.

Some cases this can happen are if the app crashes, or if a save fails.

If a save failed, obviously, saving is now impossible, so we should backup.

If the app crashed, it may be impossible to save, so again, we should create a backup.

I also think a lesser case, which I don't know how we can handle, is if the broswer process is killed.

Any others?

Some of the issues I have are not fundemental to it being in an interval, and could be fixed and still have an interval, but that doesn't seem worth it if you can instead just grab it on an error. This means we'll end up with the exact version they had, and it means we don't always have some loop running.
Attachment #8345350 - Flags: review?(scott) → review-
(In reply to Scott [:thecount] Downe from comment #25)
> Comment on attachment 8345350 [details] [review]
> https://github.com/mozilla/popcorn.webmaker.org/pull/381
> 

> It saves a backup even if they got the unload dialog and said yes they
> wanted to leave. So in essense we ask them twice. That is both aggresive and
> unexpected.

It only does this if the project is in a dirty state and they have saved once. I don't think this is too aggressive.

> Even if I save a project, and then make no changes, it saves a backup, that
> is too aggresive.

Nope. https://github.com/mjschranz/popcorn.webmaker.org/blob/92ef287acb8203bb12ca8158ffbd3629fdb5bb99/public/src/core/project.js#L379-L381

> It might be a number of seconds out of date, which makes it unpredictable.
 
> It only does the backup if it has been saved once. This is unexpected. How
> does the user know they have to save once before they are safe from lost
> data. Also, what if there is a bug that makes it so they cannot save at all,
> one of the best reasons to have backups in the first place, but they cannot
> get to that point.

If the user doesn't save the project they don't necessarily know they want the project. We could consider adding functionality to backup the project on unload if they didn't save it. It wouldn't be hard but now we are creeping into the territory of reasons why this didn't land 15 months ago.

> I want to backup in cases where they left the page without getting an "are
> you sure you want to leave" unload dialog.

What are these cases? We cover crashes and failed saves.
 
> Some cases this can happen are if the app crashes, or if a save fails.

Does.

> If a save failed, obviously, saving is now impossible, so we should backup.

Does.

> If the app crashed, it may be impossible to save, so again, we should create
> a backup.

Does.

> I also think a lesser case, which I don't know how we can handle, is if the
> broswer process is killed.

Can't.
Attachment #8345350 - Flags: review- → review?(scott)
I think we should remove the idea of saved once, and always backup. What is the goal of saved once to create a backup? Someone could make a huge project without saving, and then try to save and crash.

Shouldn't create a backup if I closed the page and said I want to leave. To do this, there is no way to turn the backup off if they click "yes, let me leave" so instead let's reverse that and turn it on if crashed or error on save.

Also, strs to getting a backup in a saved state.

1. Load a fresh project.
2. Name it.
3. Save it.
4. Refresh.

Expected: it is saved, so it should not try to load a backup.
Actual: It asks if I want to load a backup.

This goes away if I save again. Not sure what's up there.

I'm OK with it not happening on a killed process.

My main goal here is to work well with the unload dialog.
Comment on attachment 8345350 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/381

Opps, that above comment should have been an r-
Attachment #8345350 - Flags: review?(scott) → review-
Comment on attachment 8345350 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/381

It's still asking if I want to load a backup even if I leave by choice.

STR

1. set backup interval to 1 ms to ensure ideal cases.
2. open popcorn maker.
3. add a track event.
4. refresh.
5. clicl "leave page"

Actual: I clicked leave page, thus discarding my changes. The app frustratingly keeps them, though.
Expected: I should see an empty page and no jaring popup.

This is still my #1 complaint.
Attachment #8345350 - Flags: review?(scott) → review-
Comment on attachment 8345350 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/381

I'm going to suggest that we file some sort of follow up to determine if we want backups between the page load and the initial save of a new project. I'm under the impression it's something people want but I also believe it's something that you and I should not be deciding right now without more information.
Attachment #8345350 - Flags: review- → review?(scott)
Please CC me on such a follow-up; my use case is that students, despite being told, don't press the Save button for ages and then either before they do or even when they do, they get a crash - and it's all gone. This happened to one student under exam conditions...

Gerv
Using the patch, I just saved one project over another because it didn't clear a backup when I refreshed.

STRS:

1. Make two different projects.
2. Open one of them, with the other not open.
3. Make a change to it.
4. Don't save.
5. Open the other project.

Expected: It should open the project no nonsense.
Actual: It is telling me I have a project backup, but the backup is for my other project, and if I load it and save, I lost my previous project, if I don't reload it, I lost the backup. What I would have to do is load it, but don't save it, and then know what I have done, and load a fresh page to hold the backup, and save it there.

If we set the backup to be off by default, that is, never load it, and on a crash or fail to save, turn off the unload dialog and set the last known good backup to display on refresh.
Comment on attachment 8345350 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/381

So I know how I want this, given the current system.

I'm willing to do it, and I am pretty sure I know what I need to do to get there.

I would need everything in this ticket to land anyway, or do it on top of this ticket.

So yeah, I'm thinking if you want to land this and I'll do a follow up, I can r+ it.

However your most recent addition of turnOnDialog has a comment on it now. R+ with that.
Attachment #8345350 - Flags: review?(scott) → review-
Comment on attachment 8345350 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/381

Alright, still some things I'm not sure about, but it's not buggy and better than nothing by a large margin.

I'm going to say r+, let's try it, get feedback, move forward.
Attachment #8345350 - Flags: review?(scott) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: