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

Improve autosaves #4156

Closed
wants to merge 73 commits into from
Closed

Conversation

adamsilverstein
Copy link
Member

@adamsilverstein adamsilverstein commented Dec 24, 2017

Description

New WP_REST_Autosaves_Controller handles post autosaving. WIP. Fixes #4112.

// todo:

  • delete the autosave when the post is updated/saved (a revision is created)
  • show the warning about an autosave existing on load when one does.

How Has This Been Tested?

Local testing, db monitoring.

Needs more testing.

Types of changes

  • New WP_REST_Autosaves_Controller
  • New REQUEST_POST_AUTOSAVE effect, autosavePost action
  • New wp.api.getPostTypeAutosaveModel helper

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

@adamsilverstein
Copy link
Member Author

@youknowriad worth noting and testing with a draft post as well, eg try your testing steps above but without publishing the post. also worth checking what is created in the database before/after the changes here. I can try to add some screenshots of that once the code feels final.

@adamsilverstein
Copy link
Member Author

Overall, this is working pretty well. This changes the meaning of Gutenberg "autosaving" but I think the WP users are already familiar with this behavior.

A agree - WordPress users will expect Gutenberg autosave to work as well as or better than the classic editor's autosave functionality.

@adamsilverstein
Copy link
Member Author

Update: Checking the db running the classic editor I realized that for draft posts the behavior should be slightly different:

  • when autosave fires the draft post itself is updated (no autosave is created for drafts)
  • when 'save draft' is clicked, draft is updated and a revision is created.

@adamsilverstein adamsilverstein changed the title Feature: autosaves for published posts Improve autosaves Dec 29, 2017
@adamsilverstein
Copy link
Member Author

Note: core currently core uses heartbeat for autosaves, as well as for post locking - see https://core.trac.wordpress.org/ticket/25272. I looked into using heartbeat here as well and while I think it is doable, I'm not sure how heartbeat should integrated into the app - possible via hooks.

We probably need to have a broader discussion of heartbeat and whether/how it will be integrated with Gutenberg. Currently heartbeat relies on $( document ).trigger events to communicate data to/from the server.

@adamsilverstein
Copy link
Member Author

Heartbeat for post locking in #4217 was straightforward, going to look at adding autosaves that way as well, probably simpler than this approach.

@adamsilverstein
Copy link
Member Author

Autosaving via heartbeat was more straightforward than I imagined - we can leverage most of the existing code. I started on this here:https://github.com/WordPress/gutenberg/compare/feature/heartbeat-autosaving?expand=1 - still need to bring over the notification and button status bits, but we won't need the new endpoints and state tracking this way (the change check runs on the heartbeat tick)

@adamsilverstein
Copy link
Member Author

Closing in favor of #4218 where I have autosave and post locking working via the heartbeat api. I'll work to bring over the remaining functionality from this branch (autosave notices, etc).

@aduth
Copy link
Member

aduth commented Jan 2, 2018

What's the status of the API endpoints introduced here? Are you expecting to pull these into #4218 as well?

@adamsilverstein
Copy link
Member Author

@aduth no, they are no longer needed. autosaves are transported via heartbeat and saved on the server side using core wp functionality.

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.

3 participants