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

Automatic status change triggers undo level #2931

Closed
ellatrix opened this issue Oct 9, 2017 · 4 comments
Closed

Automatic status change triggers undo level #2931

ellatrix opened this issue Oct 9, 2017 · 4 comments
Labels
[Type] Bug An existing feature does not function as intended
Milestone

Comments

@ellatrix
Copy link
Member

ellatrix commented Oct 9, 2017

Issue Overview

When the status changes from auto-draft to draft by auto-save, an undo level is created. This seems unexpected as it is an automatic background change, and not something that is caused by a manual action and should be undoable.

Steps to Reproduce (for bugs)

  1. Log undo state changes with the action.
  2. Open the demo content and change some things. Wait for auto-save to trigger.
  3. Observe:
{"type":"EDIT_POST","edits":{"status":"draft"}}
@ellatrix ellatrix added the [Type] Bug An existing feature does not function as intended label Oct 9, 2017
@aduth
Copy link
Member

aduth commented Oct 9, 2017

This is challenging, because background changes should be expected, but this is in conflict with the approach to the undoable reducer where such changes would be captured as a distinct point in history which can be recovered.

Two options I could imagine are:

  • Actions can opt-out of being counted for undo history, so in the case of background changes such as this one, we can include additional metadata with the action to flag it as such
  • Actions must opt-in to being counted for undo history, so instead of undoable reducer reflecting any changes to state over time, it only reflects those changes which are flagged to be counted

@ellatrix
Copy link
Member Author

ellatrix commented Oct 9, 2017

That's something I explored in #2930. Your second option might be even better as it's explicit.

@karmatosed karmatosed added this to the Merge Proposal milestone Jan 25, 2018
@designsimply
Copy link
Member

To test this, I tried creating a new post and adding content then waiting for autosave and clicking the undo button. In my test, undo worked to remove the last text change. I suspect if this were still an issue that undo in this context would have to be clicked twice: once for the autosave undo and once for the prior text change. Does this make sense and do you agree this issue is resolved?

@ellatrix
Copy link
Member Author

Thanks @designsimply, I can so longer see an extra history record being added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

4 participants