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

Post Editor: adds autosave parameter to outbound queries #19261

Merged
merged 1 commit into from
Nov 16, 2017

Conversation

roccotripaldi
Copy link
Member

@roccotripaldi roccotripaldi commented Oct 29, 2017

When the post editor decides to save automatically, add an autosave parameter to
the API query.

This allows the server to differentiate between autosaves, and manual saves, and cleans out unnecessary noise the Activity Log.

@roccotripaldi roccotripaldi added [Feature] Post/Page Editor The editor for editing posts and pages. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Type] Enhancement labels Oct 29, 2017
@matticbot
Copy link
Contributor

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

There's a lint error on adding to the var.

Noting that I raised UX implications of not distinguishing between autosaves and intentional saves (seamless interactions for users).

That said, this works as advertised. With lint issue, might consider only adding to the query if the autosave option is explicitly true. Don't know that we need to be passing autosave=false for every save if this is inferred as the default from the REST API end.

@roccotripaldi
Copy link
Member Author

Thanks for taking a look!

With lint issue, might consider only adding to the query if the autosave option is explicitly true.

Good call!

Noting that I raised UX implications of not distinguishing between autosaves and intentional saves (seamless interactions for users).

With Activity Log, we simply plan on not showing autosave activities in their logs. This behavior is already present when someone creates a post in wp-admin. Can you elaborate on your concerns a bit? Do you forsee any negative side-effects?

@aduth
Copy link
Contributor

aduth commented Nov 1, 2017

Can you elaborate on your concerns a bit?

It's not as much a technical concern as much as it is: There seems to be some general UX desire toward users not ever needing to explicitly press a Save button, and if this were the case there would be no activity log entries whatsoever for drafts.

@roccotripaldi roccotripaldi force-pushed the update/post-editor-autosave branch 3 times, most recently from 753bd74 to c7e1f1c Compare November 15, 2017 19:01
When the post editor decides to save automatically, add an autosave parameter to
the API query.

This allows the server to differentiate between autosaves, and manual saves.
@roccotripaldi roccotripaldi force-pushed the update/post-editor-autosave branch from c7e1f1c to a3a0b8d Compare November 15, 2017 19:09
@roccotripaldi
Copy link
Member Author

@aduth - i fixed the lint errors, and addressed your feedback.
any chance i can get a 👍 ?

@roccotripaldi roccotripaldi merged commit 258a95d into master Nov 16, 2017
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 16, 2017
@apeatling apeatling deleted the update/post-editor-autosave branch February 18, 2018 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Post/Page Editor The editor for editing posts and pages. [Type] Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants