Introduce GutenbergEditPostActivity

[mzorz]

This PR introduces a dedicated Activity for hosting the Gutenberg editor, as opposed to having all editors co-live within EditPostActivity, which already has lots of code to make a difference and tell which editor to launch in the first place, depending on user settings, Post contents, and more intricate conditions.

Context and goals

It is important to note where this particular effort comes from. This is not a real refactor of EditPostActivity which we've been discussing for long and postponing for when time is available to tackle such a non-trivial, and also not minor, effort.

This PR on the contrary, represents what I've found to be the less disruptive way to achieve these goals:

1. have the Activity hosting Gutenberg not be re-created on orientation changes (although yes on other configuration changes) to let React Native handle that flow (see Exclude EditPostActivity from handling orientation change for alpha #8800). This in particular would take care of fixing these two problems: Preserve scroll and caret position after rotating (Gutenberg Editor) #8739 and [Android] Undo/redo history is lost on rotate gutenberg-mobile#377
2. avoid introducing more bugs into the existing editor (particularly Aztec, Visual, and Legacy), that we won't have time to maintain, by isolating any further changes in the time those editors have to still live together.

For this, we're introducing a new Activity to host the Gutenberg editor only, but will keep the current EditPostActivity.
I've elevated the shared constants and only the most obvious common code into an abstract class, but I've no intention to maximize code reuse for this particular effort. This is a temporal situation that makes Aztec/Visual editors live without much of a problem while we introduce the new Gutenberg editor.
A real, thorough refactor would evidently demand that we not only remove code that is not used, but we'd also need to refactor and reuse code as much as possible. Such an effort would demand more valuable time and thought for a thing that we know we're going to remove soon as part of our known plans. Thus, containing new things in an isolated way and building up from there seems to come in very handy.

ToDo (general)

• for new intents, build a helper method that returns the Intent we need depending on the Editor we need to launch. (13d3d75 and 2ccd179)
• get all important constants up into an abstract EditPostBaseActivity class, and make EditPostActivity a derived class. (74765be)
• make a separate GutenbergEditPostActivity that also derives from EditPostBaseActivity, and add the configchanges attributes in AndroidManifest.xml (79eef32)
• check the different entry points everywhere in the app for which we need to decide to open which editor, possibly make that logic centralized ended up having it all in ActivityLauncher where Activity intents where created. (2ccd179 and 2f8ef0e)
• once we make sure all entry points work as expected, remove non-Gutenberg related code from GutenbergEditPostActivity (left only commented code 5e1c762)
• remove Gutenberg code from EditPostActivity (see ToDo lists below).

ToDo on GutenbergEditPostActivity

• remove mShowGutenbergEditor flag and usages as it's unneeded in this context (d008ed1)
• remove mShowAztec flag and usages as it won't belong here (5e1c762)
• remove Aztec references and AztecImageLoader and AztecVideoLoader (5e1c762)
• remove mShowNewEditor flag and usages as it won't belong here (85263bb)
• make "discardLocalChanges" functionality work the same on Gutenberg as it was on Aztec I'll tackle this on a separate PR once this one lands

Media - related ToDo (on separate PRs)

(5e1c762 commented out code that we'll need to reuse for Media soon)
Moved to issue wordpress-mobile/gutenberg-mobile#459

ToDo on EditPostActivity (cleanup)

• remove Gutenberg code from EditPostActivity (fd181f2)

To test:

CASE A: use gutenberg

1. go to app settings and enable gutenberg (compile the alpha version for this)
2. go to posts list and open a Gutenberg post written on the web
3. see it loads Gutenberg (you should be able to see the blocks interface when tapping somewhere in the body)

CASE B: use Aztec in compatibility mode

1. go to app settings and disable gutenberg (compile the alpha version for this)
2. go to posts list and tap on a Gutenberg post written on the web
3. you should see the compatibility warning dialog, tap OPEN
4. see Aztec is the editor opened, and the content is rendered (to the extent that Aztec can render Gutenberg posts ofc)

CASE C: use Gutenberg and rotate

1. follow case A
2. when the post is open, scroll down to the middle of it (make sure you have several blocks in your post to be able to tell the difference in scroll positioning).
3. tap on a pargraph block and see the keyboard appears, and the caret starts blinking where you tapped.
4. tap back to dismiss the keyboard
5. rotate the device
6. observe the place where you were standing is still there, and the caret is blinking

CASE D: use Gutenberg and rotate, observe undo/redo

1. follow case A
2. when the post is open, scroll down to the middle of it (make sure you have several blocks in your post to be able to tell the difference in scroll positioning).
3. write some new blocks, see the UNDO action becomes available
4. tap UNDO, observe the last action is undone
5. tap REDO, observe the text come back as it was
6. rotate the device
7. observe the UNDO action is still available
8. repeat steps 4 and 5 to confirm that the history was effectively available and actions can be undone/redone.

NOTE: other entry points, such as uploading an image to the Media section of your site and then tapping on "WRITE POST" on the success snackbar/notification to include the just-uploaded picture in your post is not yet supported. We'll support that once Media is all wired up together.

Update release notes:

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

[wpmobilebot]

	1 Warning
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 Danger

[hypest]

Hmm, having a Gutenberg-dedicated method (editPageForResultOnGutenberg) is generally fine but, only a few lines above (line 605) we're seeing a different pattern, where the method is just one and it it decides internally which editor to launch. Can we get away with using just one of the two patterns perhaps?

[mzorz]

I think it's fine for this case as otherwise we need to inject and use FluxC in ActivityLauncher just to be able to inflate the PageModel, read the content of the Post body and then only decide which Activity to launch. This, in the only case where this method is called, is already known at the time the call happens by the caller.

[hypest]

Feels to me that the check for Gutenberg editor is bleeding into this class (PagesFragment) but it'd better not. Can we leave it out, perhaps by having it inside ActivityLauncher.editPageForResult()?

[mzorz]

see comment above

[hypest]

Extensive work @mzorz , nice!

I haven't actually finished my review pass over the full changes yet, though I did leave a couple of comments already.

Thing is, I started having a feeling of unease when I realized that we'll find ourselves with effectively 2 "copies" of the EditPostActivity and most of its internals. We might not want to enter that situation. I guess I'd like others to have a pass at the PR too and we can then discuss a plan what to do next after this PR or to take another path forward.

@daniloercoli and @marecar3 , can you have a look at this PR and we can share thoughts afterwards? Thanks!

[mzorz]

Thanks for bringing valid concerns up @hypest as usual!

Thing is, I started having a feeling of unease when I realized that we'll find ourselves with effectively 2 "copies" of the EditPostActivity and most of its internals. We might not want to enter that situation.

Also, related to that (taken from the PR description):

Such an effort would demand more valuable time and thought for a thing that we know we're going to remove soon as part of our known plans.

We discussed this elsewhere already but bringing it here so others have the same info: this is an assumption I made: I assumed the timeframe for the editors coexisting would be rather short (just a few months) but it was brought up in our conversation that it may as well not be the case (i.e. stay with us for years). I agree we might give it a second thought if we're going to need to stick and maintain both classes for long, I just didn't see why we'd need to go through the work of refactoring only to kill it shortly after (I assumed Gutenberg to be the only editor in a few months from now).

In this PR I assume what's described in "goals" point 2 (that we won't dedicate much time to maintaining the other editors as much as Gutenberg) so it made sense to let the EditPostActivity live while advocating to Gutenberg, starting with this PR which basically is shamelessly a) copying b) then taking out what's not needed on the new class in subsequent PRs.

Hope that gives a bit more context 👍

Now, that said and looking forward to taking the conversation to the next step: I still think for Gutenberg (well, RN in particular) we need to have an Activity that doesn't handle orientation changes (i.e. preventing the system from killing it in that specific case of rotation as a common use case), given it's handled in RN already. If we wanted to reuse the existing EditPostActivity and make a modification like that (on the Activity's configChanges attr) then we have the problem of needing to make sure we didn't break anything on current Editors, which seems riskier (again point 2 in the Goals section of this PR described above), and has been brought up before. That might still make sense to be done if we're going to keep Aztec as a standalone editor for years to come, but simply didn't make sense under the assumption of a short timeframe.

[daniloercoli]

Now, that said and looking forward to taking the conversation to the next step: I still think for Gutenberg (well, RN in particular) we need to have an Activity that doesn't handle orientation changes (i.e. preventing the system from killing it in that specific case of rotation as a common use case), given it's handled in RN already. If we wanted to reuse the existing EditPostActivity and make a modification like that (on the Activity's configChanges attr) then we have the problem of needing to make sure we didn't break anything on current Editors, which seems riskier (again point 2 in the Goals section of this PR described above), and has been brought up before. That might still make sense to be done if we're going to keep Aztec as a standalone editor for years to come, but simply didn't make sense under the assumption of a short timeframe.

I had already took a look at potential solutions to the history lost issue, just before Mario implemented the fix by adding a new activity for GB only. In my short investigation there were 3 solutions on the table:

1. Try to set/get the history from GB and store them together with the content in the editor on Android native side. I pinged @Tug on this, to check if there was a way to read the history "state", but it wasn't clear to us if there were any possibility to read/store/set it.

2. Try to change the configChanges configuration on the EditPostActivity at run-time, when a GB post is detected. Unfortunately doesn't seem a viable solution.

3. The solution implemented in this PR - a new EditPostActivity for GB only.

Having 2 activities is not ideal at all, but at the moment don't have better solutions to the problem. We also need to consider that EditPostActivity may need some customizations for GB only, making it even more complex and hard to fix in case in case of errors in one of the 2 editor.

[hypest]

I'd add a 4th option @daniloercoli:

4. A new, dedicated Activity for GB started from scratch and adding/implementing the features in it as we go. Goal is to have a modern architecture in it and avoid the legacy implementations done in the existing EditPostActivity. (Option 3 and this PR is based on the existing EditPostActivity)

[hypest]

I had a look again today and I'm wondering what y'all think about this idea (a 5th option?):

Introduce a new Activity that completely inherits from the original EditPostActivity but is set to avoid recreation on rotate. We'll keep the different launch origins so the new activity launches when Gutenberg.

The assumption this requires is that the code in EditPostActivity that runs when Gutenberg, should be compat with the fact that the activity will not get recreated. Ideally and hopefully, this might be easier to scope and handle in steps.

All in all, nothing would change for the Aztec codepath, but the GB codepath will have the orientation "hack".

Thoughts?

[mzorz]

Introduce a new Activity that completely inherits from the original EditPostActivity but is set to avoid recreation on rotate. We'll keep the different launch origins so the new activity launches when Gutenberg.

I like this idea better than writing all from scratch.

[hypest]

I cannot shake the feeling that retaining the Activity (new or not) on rotate will be problematic. I mean, it might even limit our options regarding layouts on landscape or on tablets.

From https://developer.android.com/guide/topics/resources/runtime-changes#HandlingTheChange:

If your application doesn't need to update resources during a specific configuration change and you have a performance limitation that requires you to avoid the activity restart, then you can declare that your activity handles the configuration change itself which prevents the system from restarting your activity.

In our case, I think we'll want to have the option to update resources on rotate.

So instead, I went after a different route: retain the RN view only. I opened a PR with that option so we can assess it better.

[hypest]

By the way, and on a different matter, the work with the RN detach/attach in has enabled a nice trick: we can preload the RN view if we want and make the editor appear instantly. Check out the branch https://github.com/wordpress-mobile/WordPress-Android/compare/gb/experiment-preload-gb?expand=1. In the branch, I have hardcoded the bootstrap of GB during the main app activity just to test it out. Try going into the editor and back to the post list, and into the editor again to see the speed.

[mzorz]

it might even limit our options regarding layouts on landscape or on tablets.

Not exactly sure which limitations you would see there ☝️ , but we're definitely going down the other path for now as suggested here, after issues observed are addressed.

Closing this one.

[hypest]

it might even limit our options regarding layouts on landscape or on tablets.

Not exactly sure which limitations you would see there

What I had in mind there was the ability of defining different resources for different screen configurations and rotations. Think: have bigger content margins when in landscape for example. Bypassing the framework managed rotation would make it harder to have such features.

[mzorz]

Think: have bigger content margins when in landscape for example. Bypassing the framework managed rotation would make it harder to have such features.

That for sure needs to be taken care of, but I honestly don't see a hassle in it given we're talking about the Gutenberg-holding Activity only and we handle all that in RN.