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

Wire-up the revert from history ability #8768

Merged
merged 4 commits into from
Dec 13, 2018
Merged

Conversation

marecar3
Copy link
Contributor

@marecar3 marecar3 commented Dec 11, 2018

Fixes part of wordpress-mobile/gutenberg-mobile#182

This change adds History option to the Gutenberg editor. The behavior is same as Aztec, when an older revision is selected among the old revision list the editor content is updated with that revision. After that we need to tap Update button if we want to publish the new content, we can still discard the content by tapping "Discard Local changes" or from Toolbar with Undo and Redo buttons.

It's tested against a corresponding gutenberg-mobile PR

Test :
Add wp.BUILD_GUTENBERG_FROM_SOURCE = true in gradle.properties

@marecar3 marecar3 added the Gutenberg Editing and display of Gutenberg blocks. label Dec 11, 2018
@marecar3 marecar3 self-assigned this Dec 11, 2018
@marecar3 marecar3 requested review from hypest and mzorz December 11, 2018 12:05
@hypest hypest changed the title Update gutenberg-mobile submodule to corresponding gutenberg-mobile b… Wire-up the revert from history ability Dec 11, 2018
@hypest
Copy link
Contributor

hypest commented Dec 11, 2018

Add wp.BUILD_GUTENBERG_FROM_SOURCE = true in gradle.properties

👋 @marecar3 , I wonder, why is building from source needed? Have you encountered some problem with the binary build?

@hypest
Copy link
Contributor

hypest commented Dec 11, 2018

or from Toolbar with Undo and Redo buttons

I tried using the Undo button from the toolbar but the post still ends up being marked as changed.
Steps:

  1. Open a GB post
  2. Load a historical version
  3. Wait for the "undo" snackbar to hide and tap on the "Undo" button of the toolbar. The post content returns to the previous one.
  4. Tap back to exit the editor. Notice the post being marked with "Local changes"

I think I'd expect the post to not be marked as such. WDYT?

@mzorz
Copy link
Contributor

mzorz commented Dec 11, 2018

I've tried the same as @hypest, apparently it tries to update the remote version (the Post uploader is triggered), when it should remain the same.

In my case I used this:

Steps to repro:

  1. start a new gutenberg post
  2. type a title
  3. start a paragraph, enter "first version" as text.
  4. tap back
  5. observe the post gets uploaded as a draft
  6. open it again and edit paragraph "first version" enter something else, for example I entered "second version"
  7. tap back to make the first entry in history
  8. open it again
  9. now add a new paragraph with text "hello"
  10. tap back, open it again and tap on History to see you have 2 entries there.
  11. load one of the older entries
  12. wait for the snackbar to hide
  13. tap on undo
  14. tap back - the post is uploaded (IIUC, it shouldn't).

NOTE: If you set airplane mode ON before step 14, the post is then marked with Local changes as described in the comment above.

@mzorz
Copy link
Contributor

mzorz commented Dec 11, 2018

We confirmed with @marecar3 that these situations (#8768 (comment) and #8768 (comment)) are happening with Aztec alone as well, so they are not related to this specific PR.

@hypest
Copy link
Contributor

hypest commented Dec 12, 2018

I'd suggest tackling what happens with the Undo/Redo buttons in a separate PR so we can merge this one that implements the revert from history.

@mzorz
Copy link
Contributor

mzorz commented Dec 12, 2018

I'd suggest tackling what happens with the Undo/Redo buttons in a separate PR so we can merge this one that implements the revert from history.

Agreed, that's being tackled by this other PR #8773. This one is LGTM from my side - will approve shortly.

@mzorz
Copy link
Contributor

mzorz commented Dec 12, 2018

will approve shortly.

@marecar3 please let us know when you're able to make the changes suggested by @hypest and this PR is ready for another round 👍

Copy link
Contributor

@hypest hypest left a comment

Choose a reason for hiding this comment

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

LGTM!

@hypest hypest added this to the 11.5 milestone Dec 12, 2018
@marecar3 marecar3 merged commit 23387af into develop Dec 13, 2018
@hypest
Copy link
Contributor

hypest commented Dec 13, 2018

👋 @marecar3 , it seems that this PR got merged without first updating the submodule hash to point to the gutenberg-mobile master after the dependent PR got merged. Can you open a new PR to point to master? Thanks!

@hypest hypest deleted the feature/gutenberg-history branch December 13, 2018 08:43
@marecar3
Copy link
Contributor Author

marecar3 commented Dec 13, 2018

Hey @hypest sorry for missing that one.
Here is the PR #8780

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Editing and display of Gutenberg blocks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants