-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Improve autosaves #4156
Changes from all commits
08cdb15
9d6b95c
1e9572c
b82fa9d
cbd0c62
9374837
218299c
bb7b896
3ee6738
2e3ec2e
86b70c2
db02aea
e82ffc1
9e5ccf8
968241d
52110b7
9997325
f389030
4b31a3c
36e3183
f4f0d47
e484958
4d81b62
15e0573
6bf285c
39fb050
a5b26db
17c26ca
fd6464c
51ac91e
ad4e8b3
9d01248
9dad2ff
471b507
8928813
76e7973
bf03be9
5ba2bf1
ccd6ac6
2e887a2
22cfcfd
96a348b
bf1ab9d
5bf6c7f
a0b5858
4d84074
14147c6
557d653
a988bb8
8d75bb7
389237f
dd2d9b4
3ee7326
8e8e9c0
91a07c0
b2b89ec
ecbf499
19dabd9
7965579
62230fd
2dd8774
765ae5d
06be44d
4bdfad9
c1ef42c
c05796c
95a245b
b7ae85a
8d40fd6
3e559ca
7ace629
12d7225
4d03072
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,8 +30,10 @@ import { | |
replaceBlocks, | ||
createSuccessNotice, | ||
createErrorNotice, | ||
createWarningNotice, | ||
removeNotice, | ||
savePost, | ||
toggleAutosave, | ||
editPost, | ||
requestMetaBoxUpdates, | ||
updateReusableBlock, | ||
|
@@ -44,7 +46,8 @@ import { | |
getDirtyMetaBoxes, | ||
getEditedPostContent, | ||
getPostEdits, | ||
isCurrentPostPublished, | ||
getEditedPostTitle, | ||
getEditedPostExcerpt, | ||
isEditedPostDirty, | ||
isEditedPostNew, | ||
isEditedPostSaveable, | ||
|
@@ -57,6 +60,7 @@ import { | |
* Module Constants | ||
*/ | ||
const SAVE_POST_NOTICE_ID = 'SAVE_POST_NOTICE_ID'; | ||
const AUTOSAVE_POST_NOTICE_ID = 'AUTOSAVE_POST_NOTICE_ID'; | ||
const TRASH_POST_NOTICE_ID = 'TRASH_POST_NOTICE_ID'; | ||
const SAVE_REUSABLE_BLOCK_NOTICE_ID = 'SAVE_REUSABLE_BLOCK_NOTICE_ID'; | ||
|
||
|
@@ -71,26 +75,65 @@ export default { | |
content: getEditedPostContent( state ), | ||
id: post.id, | ||
}; | ||
const isAutosave = action.options && action.options.autosave; | ||
let Model, newModel; | ||
|
||
dispatch( { | ||
type: 'UPDATE_POST', | ||
edits: toSend, | ||
optimist: { type: BEGIN, id: POST_UPDATE_TRANSACTION_ID }, | ||
} ); | ||
dispatch( removeNotice( SAVE_POST_NOTICE_ID ) ); | ||
const Model = wp.api.getPostTypeModel( getCurrentPostType( state ) ); | ||
new Model( toSend ).save().done( ( newPost ) => { | ||
dispatch( { | ||
type: 'RESET_POST', | ||
post: newPost, | ||
} ); | ||
if ( isAutosave ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know I told you to merge the two effects, but I'm starting to regret 🙃 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fine to separate them again; as you can see in the code, autosaving is a pretty different action than saving the post - for both drafts and published posts. To summarize the differences:
Also:
|
||
toSend.parent = post.id; | ||
delete toSend.id; | ||
Model = wp.api.getPostTypeAutosaveModel( getCurrentPostType( state ) ); | ||
newModel = new Model( toSend ); | ||
} else { | ||
dispatch( { | ||
type: 'REQUEST_POST_UPDATE_SUCCESS', | ||
previousPost: post, | ||
post: newPost, | ||
optimist: { type: COMMIT, id: POST_UPDATE_TRANSACTION_ID }, | ||
type: 'UPDATE_POST', | ||
edits: toSend, | ||
optimist: { type: BEGIN, id: POST_UPDATE_TRANSACTION_ID }, | ||
} ); | ||
dispatch( removeNotice( SAVE_POST_NOTICE_ID ) ); | ||
dispatch( removeNotice( AUTOSAVE_POST_NOTICE_ID ) ); | ||
Model = wp.api.getPostTypeModel( getCurrentPostType( state ) ); | ||
newModel = new Model( toSend ); | ||
} | ||
|
||
newModel.save().done( ( newPost ) => { | ||
if ( isAutosave ) { | ||
const autosave = { | ||
id: newPost.id, | ||
title: getEditedPostTitle( state ), | ||
excerpt: getEditedPostExcerpt( state ), | ||
content: getEditedPostContent( state ), | ||
}; | ||
dispatch( { | ||
type: 'RESET_AUTOSAVE', | ||
post: autosave, | ||
} ); | ||
dispatch( toggleAutosave( false ) ); | ||
|
||
dispatch( { | ||
type: 'REQUEST_POST_UPDATE_SUCCESS', | ||
previousPost: post, | ||
post: post, | ||
isAutosave: true, | ||
} ); | ||
} else { | ||
// dispatch post autosaved false | ||
// delete the autosave | ||
dispatch( { | ||
type: 'RESET_POST', | ||
post: newPost, | ||
} ); | ||
dispatch( { | ||
type: 'REQUEST_POST_UPDATE_SUCCESS', | ||
previousPost: post, | ||
post: newPost, | ||
optimist: { type: COMMIT, id: POST_UPDATE_TRANSACTION_ID }, | ||
} ); | ||
} | ||
} ).fail( ( err ) => { | ||
if ( isAutosave ) { | ||
dispatch( toggleAutosave( false ) ); | ||
} | ||
|
||
dispatch( { | ||
type: 'REQUEST_POST_UPDATE_FAILURE', | ||
error: get( err, 'responseJSON', { | ||
|
@@ -99,12 +142,28 @@ export default { | |
} ), | ||
post, | ||
edits, | ||
optimist: { type: REVERT, id: POST_UPDATE_TRANSACTION_ID }, | ||
optimist: isAutosave ? false : { type: REVERT, id: POST_UPDATE_TRANSACTION_ID }, | ||
} ); | ||
} ); | ||
}, | ||
REQUEST_AUTOSAVE_EXISTS( action, store ) { | ||
const { autosave } = action; | ||
const { dispatch } = store; | ||
if ( autosave ) { | ||
dispatch( createWarningNotice( | ||
<p> | ||
<span>{ __( 'There is an autosave of this post that is more recent than the version below.' ) }</span> | ||
{ ' ' } | ||
{ <a href={ autosave.edit_link }>{ __( 'View the autosave' ) }</a> } | ||
</p>, | ||
{ | ||
id: AUTOSAVE_POST_NOTICE_ID, | ||
} | ||
) ); | ||
} | ||
}, | ||
REQUEST_POST_UPDATE_SUCCESS( action, store ) { | ||
const { previousPost, post } = action; | ||
const { previousPost, post, isAutosave } = action; | ||
const { dispatch, getState } = store; | ||
|
||
const publishStatus = [ 'publish', 'private', 'future' ]; | ||
|
@@ -128,7 +187,7 @@ export default { | |
private: __( 'Post published privately!' ), | ||
future: __( 'Post scheduled!' ), | ||
}[ post.status ]; | ||
} else { | ||
} else if ( ! isAutosave ) { | ||
// Generic fallback notice | ||
noticeMessage = __( 'Post updated!' ); | ||
} | ||
|
@@ -263,25 +322,19 @@ export default { | |
return; | ||
} | ||
|
||
if ( ! isEditedPostNew( state ) && ! isEditedPostDirty( state ) ) { | ||
// Change status from auto-draft to draft, saving the post. | ||
if ( isEditedPostNew( state ) ) { | ||
dispatch( editPost( { status: 'draft' } ) ); | ||
dispatch( savePost() ); | ||
return; | ||
} | ||
|
||
if ( isCurrentPostPublished( state ) ) { | ||
// TODO: Publish autosave. | ||
// - Autosaves are created as revisions for published posts, but | ||
// the necessary REST API behavior does not yet exist | ||
// - May need to check for whether the status of the edited post | ||
// has changed from the saved copy (i.e. published -> pending) | ||
if ( ! isEditedPostDirty( state ) ) { | ||
return; | ||
} | ||
|
||
// Change status from auto-draft to draft | ||
if ( isEditedPostNew( state ) ) { | ||
dispatch( editPost( { status: 'draft' } ) ); | ||
} | ||
|
||
dispatch( savePost() ); | ||
dispatch( toggleAutosave( true ) ); | ||
dispatch( savePost( { autosave: true } ) ); | ||
}, | ||
SETUP_EDITOR( action ) { | ||
const { post, settings } = action; | ||
|
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.
should we do
if ( isPublished ) { return null; }
instead?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.
ok, unless we still want to use this component to show the autosave progress while in action? right now it says 'Autosaving' then 'Saved'. if we do remove this entirely, would disabling/re-enabling the update button be enough to indicate autosaving?
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.
Calling designers :P @jasmussen @karmatosed
As I said, maybe we'd want to separate "autosaving" from "saving drafts" but I'll let our awesome designers find a solution for this. And I don't consider this a blocking for this PR.