Skip to content

Switch between editors while editing the post #9247

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Feb 18, 2019
Merged

Switch between editors while editing the post #9247

merged 13 commits into from
Feb 18, 2019

Conversation

hypest
Copy link
Contributor

@hypest hypest commented Feb 14, 2019

Fixes #9037

This PR adds the ability to switch between Aztec and Gutenberg while editing the post/page.

Implementation details:

  • The EditPostActivity is let to finish and then we restart it with some proper flags
  • Switching this way means that the post will at least get saved locally and then reloaded (in the "other" editor). It also means that Tracks that are triggered in the normal end/start of the editing Activity will get triggered here too. Hopefully the noise added will not be too harmful for the Tracks collected.
  • The appbar 3-dots menu now has a "Switch to ... editor" option. Depending on the current editor and the content, the appropriate menu option is shown. See Gutenberg - add switch to come back/from Aztec #9037 (comment) for the conditions.

To test A:

  1. Open a GB post
  2. Open the 3-dots menu and verify that the "Switch to Classic Editor" is there. Tap it.
  3. Verify that the post is now opened in Aztec (look for the "H" button in the format bar)
  4. Open the 3-dots menu and verify that the "Switch to Block Editor" is now there. Tap it.
  5. Verify that the post is now opened in Gutenberg again (look for the undo/redo buttons in the format bar)
  6. Make some change to the post (title or content)
  7. Switch to Classic via the 3-dot menu
  8. Verify that the changes made it to the Aztec
  9. Make some more changes
  10. Change to Gutenberg and verify that the new changes are there

To test B:

  1. Enable "Use Block Editor for new posts" in the App Settings
  2. Create a new post. Verify it's Gutenberg.
  3. Switch to Aztec via the 3-dot menu and verify that Aztec loads
  4. Switch to Gutenberg via the 3-dot menu and verify that Gutenberg loads

To test C:

  1. Disable "Use Block Editor for new posts" in the App Settings
  2. Create a new post. Verify it's Aztec.
  3. Switch to Gutenberg via the 3-dot menu and verify that Gutenberg loads
  4. Switch to Aztec via the 3-dot menu and verify that Aztec loads

To test D:

Same as A but for pages.

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

hypest added 12 commits February 13, 2019 15:58
@hypest
Menu options to swich between the editors
c608d68
@hypest
Able to switch editors
5dc403f 
When new post, only working OK when starting it from the main screen.
@hypest
Able to switch editors starting off post list
0939116
@hypest
Local id can be non-zero but invalid. Pass a flag too.
9ab783a 
When restarting the editor Activity, the temptorary local post record has
 been deleted so, the local id passed around is invalid. Let's pass the
"inNewPost" flag as well.
@hypest
No need for editor warning for pages
09cbdf2
@hypest
Switch Block to Classic editor for existing pages
5ae5416
@hypest
Able to switch editor in new page
5435ddd
@hypest
Merge branch 'develop' into gb/switch-between-editors-on-the-fly
a4560cf
@hypest
Able to pass edited new post to other editor
5ca1d7a 
While at it, simpler implementation.
@hypest
Avoid pushing the post online when switching editors
fa57115
@hypest
Hide switch to editor button depending on editor and content
16c42dc
@hypest
Offer in-editor switch to Gutenberg if empty post
132b8ee
@hypest hypest requested a review from daniloercoli February 14, 2019 15:29
@hypest hypest self-assigned this Feb 14, 2019
@hypest hypest added this to the 11.9 milestone Feb 14, 2019
@daniloercoli
Copy link
Contributor

daniloercoli commented Feb 15, 2019 •
edited
Loading

I think I've found another testing scenario that does't switch the editor properly.

To test B_1:

Edited: Turn on Don't keep activities

  • Enable "Use Block Editor for new posts" in the App Settings
  • On the My Site Tab tap on the New post button at the center of the main toolbar
  • Create a new post. Verify it's Gutenberg.
  • Try to switch to Aztec via the 3-dot menu
    Boom!

This is because site is null here: https://github.com/wordpress-mobile/WordPress-Android/pull/9247/files#diff-cb472478231b297b0399fbbe2bde9f32R615

I tried by selecting another site, and repeated the steps above, but site is always null there. https://github.com/wordpress-mobile/WordPress-Android/pull/9247/files#diff-1cf6428dcfe0d429dfdcf7398ef4c056R735 is returning null.

I guess this is because the Don't keep activities flag does terminate WPMainActivity.

@hypest
Get site model from the bundle
5db8e73 
When "Don't keep Activities" is ON, the instance var (mSelectedSite) is
not avail when onActivityResult is called so, better to get it from the
intent extras.
@hypest
Copy link
Contributor Author

hypest commented Feb 18, 2019

I think I've found another testing scenario that does't switch the editor properly.

To test B_1:

Edited: Turn on Don't keep activities

Nice catch @daniloercoli ! Fixed with 5db8e73.

daniloercoli
daniloercoli approved these changes Feb 18, 2019
View reviewed changes
Copy link
Contributor

@daniloercoli daniloercoli left a comment

Choose a reason for hiding this comment

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

Run another round of testing and works good.

LGTM!

@hypest hypest merged commit c559734 into develop Feb 18, 2019
@hypest hypest deleted the gb/switch-between-editors-on-the-fly branch February 18, 2019 11:15
This was referenced Feb 18, 2019
Closed
Closed
@hypest hypest mentioned this pull request Feb 19, 2019
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@daniloercoli daniloercoli daniloercoli approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@hypest hypest

Labels

None yet

Projects

None yet

Milestone

11.9 ❄️

Development

Successfully merging this pull request may close these issues.

2 participants

@hypest @daniloercoli