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

Experimental autosave feature for collections #6198

Merged
merged 17 commits into from Jun 28, 2022

Conversation

wiebkevogel
Copy link
Contributor

@wiebkevogel wiebkevogel commented Jun 13, 2022

This PR includes an autosave implementation for collections.

Via the autosave configuration the feature can be activated in general, but to make it work for a collection there are two ways to integrate this part. With autosave: true the interval time from the autosave configuration would be used, which means every 5000ms it is checked if an entry has a not saved state and save it. With eg autosave: 8000 you can specify a time in which a change is to be checked.

@jasonvarga
Copy link
Member

Thanks for this!

Is the change to package-lock.json necessary? If not, can we remove that from the PR please?

This reverts commit d00af7a.
@wiebkevogel
Copy link
Contributor Author

The commit is reverted.
Now the tests failed because I started the branch from master and I need the npm changes from 3.3: #6161

Is it possible to bring the changes for this to the master branch?

@jasonvarga
Copy link
Member

I've brought master even with 3.3.

@jackmcdade
Copy link
Member

Can you remind me what the use case is for needing autosave? Trying to think of what it might be but coming up a little short.

Also, if you're using Revisions, I feel like it would make hundreds of working copies and kind of eliminate the usefulness of the features. 🤔

@jonassiewertsen
Copy link
Contributor

Can you remind me what the use case is for needing autosave? Trying to think of what it might be but coming up a little short.

Also, if you're using Revisions, I feel like it would make hundreds of working copies and kind of eliminate the usefulness of the features. 🤔

Personally, I do think that there are multiple pros.

In general: You would need to enable autosave per collection, so it won't kick in unexpected.

With autosave, you won't need to worry about saving your changes. That behaviour does work great for google docs.
If revisions are enabled, we do have 3 states (you might only expect 2, but there are three):

  • Revisions (one revision might be published)
  • Working copy
  • Browser state

A working copy might exist for a few days or weeks if making a change, preparing something for a later release etc. So having a working copy, which is a draft state, does make a lot of sense.
The Browser state does offer a lot of trouble though ... without autosave, you may have an important text that does only exist inside your browser. Closing that window (even with the dirty state), might be responsible for loosing all changes form multiple hours of work.

Image that part with autosave: You could max loose changes from a few seconds to minutes, but not hours etc and you don't need to worry about it. This does offer a great fallback.
Statamic does only create one Working copy per Entry. Saving multiple times would only update one single file, so you don't need to worry about hundreds of files: There is only one file as before.

Let's make autosave even more interesting and combine that with collaboration:
As you don't need to save, the Browser state (maybe a few hours old) will get synced through all clients working on that document. In any case of error, problem etc, you can't rely on reloading the article from your server, as that state might be very old and the only existing version from that article. That is really dangerous and can lead to a lot of problems. There is no safety net.

With autosave, you are keeping the Browser and saved state via Collaboration in a pretty close sync, which does help avoid problems with too many state, if using collaboration heavily (to for example write articles).

@jasonvarga
Copy link
Member

jasonvarga commented Jun 23, 2022

Sounds good 👍

The only real downside I can think of is that you'd be spamming EntrySaved/RevisionSaved events. But I don't know if that's really an issue.

config/autosave.php Outdated Show resolved Hide resolved
@wiebkevogel
Copy link
Contributor Author

Yes we have thought about that too, so far we haven't noticed it as a problem. But we should keep an eye on it, that's why we wanted to bring it as an experimental feature for now.

Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This is looking and working great. Just some small changes that I can't seem to commit myself.

Can you please also add autosave to the list on this line. (After assets)

This will allow artisan vendor:publish --tag=statamic to copy over the autosave.php file.

config/autosave.php Outdated Show resolved Hide resolved
src/Entries/Collection.php Outdated Show resolved Hide resolved
config/autosave.php Outdated Show resolved Hide resolved
config/autosave.php Outdated Show resolved Hide resolved
@wiebkevogel
Copy link
Contributor Author

Thanks! This is looking and working great. Just some small changes that I can't seem to commit myself.

Can you please also add autosave to the list on this line. (After assets)

This will allow artisan vendor:publish --tag=statamic to copy over the autosave.php file.

Great, thanks for the review! :)

Done: 13a0d17

@jasonvarga jasonvarga merged commit effa6d7 into statamic:master Jun 28, 2022
@jackmcdade jackmcdade added this to the 3.4 milestone Jan 4, 2023
@wiebkevogel wiebkevogel deleted the autosave branch January 15, 2024 13:28
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.

None yet

4 participants