-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Display Saving label while save in progress #1301
Conversation
I don't understand, that's exactly what we're doing. |
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.
Tested and approved :)
And thanks for the unit tests
Right, my question could have been phrased better. I mean to suggest maybe we shouldn't be doing this. |
@aduth We're doing so to avoid mixing the persisted edits with the edits added during the save. Diffing could be an alternative. |
Why do we need to avoid it? Isn't this why we include redux-optimist, so we can revert if we need to? |
@aduth but that's not about reverting. I'm having a hard time understanding what you mean exactly. We clear the edits before saving, that way new edits are added even if we didn't get the response yet and we're. So we're already optimistically clearing edits and moving them to the currentPost. |
Okay, you've reminded me of the fact we need to preserve edits made while the save is in-flight. That makes sense now. |
Closes #707
Related: #1203
This pull request seeks to resolve a regression introduced in #1203 where the "Saving" label is no longer shown in the header while a save is in progress. The issue is due to the fact that when a save begins, post edits are cleared, meaning that the post is considered un-saveable (no title, content, or excerpt). Due to the order of logic in
SavedState
, this resulted in nothing been shown, instead of the "Saving" label, despite a save being in progress.Implementation notes:
Should we really be clearing post edits when a save is initiated? My thinking is that we should optimistically persist them as being the current state of the post. cc @youknowriad
Testing instructions:
Verify that a Saving label is shown after clicking Save in the editor bar, while the request is in progress. Using Chrome Developer Tools network throttling to emulate a slower connection can help identify the change in label.