Changes to integrate Gutenberg-Mobile into the WPiOS App

[etoledom]

This PR has all the minimum changes required for the simplest integration with the WPiOS app. This PR is intended to gather feedback rather than to merge it directly.

This is still early, but I decided to start a PR at this point before it becomes more complex and difficult to review. There's enough things implemented to get a general idea.

RN side changes:

There are a few changes in the "Gutenberg view" (aka block-manager):

• Added a Save button and a Cancel button:
  I was wondering if those buttons should go in the RN side or in the Native side. I decided to add them in RN as it increase the shared code between platforms, and the communication code is slightly simpler. We might decide later that the second option is better, but to start I'm giving this option a shot.

• Calls to parent app:
  Added method calls to the host app (savePost and close), these are resolved in iOS by the changes made to the example app, but they will error on Android when the new buttons are pressed until the communication are implemented there.

Native (iOS) side changes:

I added a new folder called Shared. It holds the files that are being shared with the WPiOS project. For now I added them to WPiOS via direct reference, but the ideal is to make a Gutenberg Bridge package, or maybe to bundle them together with RNAztec. We decided with @hypest that it's still too early to do this, so I'm sticking with referencing them for now.

The communication manager (PostManager for now) is global, since the bridge is initiated with the app launch, and they sirve as mediators more than managers (candidate for renaming). They shouldn't own business logic, instead it just pass messages to its delegate.

PostManager could easily become a generic mediator between RN and Native, taking care of all the communication we need.

GutenbergBridge is encapsulating all the RNBridge logic together with the dependency on React, and it holds a reference of the communication managers for easy access by the GutenbergController (spoiler for the WPiOS integration).

The Hack:
This is more relevant when reviewing the integration on WPiOS, but there are some // import React and //#import "WordPress-Swift.h". That is because I'm referencing the files directly into the WPiOS app, and the dependency management is different. So the WPiOS app won't compile unless we uncomment those import statements (not necessary to review this PR).

I'll be checking how to solve this issue next week and I appreciate any help I can get with it :]

---

I'm happy to accept ideas, name changes, concept changes, UI changes, etc...
Thank you!

BTW: Not sure why travis is failing, it errors trying to:
"eval git submodule update --init --recursive "

To test:

• Update all the submodules.
• Run on iOS as always.
• Check that it runs and behaves as normal.
• Check that it doesn't error when pressing the top buttons (save and close). They won't do anything.

[hypest]

I'm thinking, the changes in this PR show how we can send data from RN to the parent/hosting app, when the action is triggered via the RN layer (the save/close buttons introduced in this PR).

I would assume that we probably want to reuse the app bars already in place in the main WP apps and those already have their set of buttons like close/update. Tapping those means that the action will be triggered by the hosting layer and we will need to extract the html data off the RN layer. Have you found a good way to do that? AFAICT, we can't extract data synchronously but the current WP app implementations assume it to be synchronous, right?

My investigation so far (on Android) tells me we probably need to revise the main app implementation to support async extraction or just block the thread while waiting for the answer from the JS core. WDYT?

(EDIT: there's probably already a pattern in place to wait for an async result in the case of the older, hybrid editor, pre-Aztec. We might just try that.)

[hypest]

A few thoughts:

1. Yay! for having this first connection/integration in place! The very first important step to establish a communication channel between the layers!
2. The "Shared" folder inside the ios/ dir is a good idea but we probably need to put it even more "up" and have a dedicated native module project just for that, one that will include the Android counterpart as well. I'm thinking, let's create a library with https://facebook.github.io/react-native/docs/native-modules-setup. We can name it react-native-gutenberg-bridge and place it at the root of the mobile repo (no submodule or anything, just add it there).
3. We can introduce a JS module that that the import NativeModules from 'reac-native and export a friendlier name for the bridge module (https://github.com/frostney/react-native-create-library does that automatically actually).
4. I think the JS based save/close buttons are not the solution we'd like to go down with. We better rely on the respective buttons already in the parent app. It's probably OK to leave them in as an example for now but, let's try to remove them.

Points 2 and 3 above can actually be performed in another PR, no blockers for this one. I think I will open a PR myself for those. Point 4 though is kinda a blocker for me. For ease of development, let's keep them in but make them use the methods from the bridge to simulate the button clicks from the parent app. Does that make sense?

[etoledom]

Update:

• Save bridge action is back. 🎉
• Added a nav bar to the example project for easy tests and faster iterations of the bridge.

To test:

• yarn clean:install
• yarn start:reset
• Open the ios/gutenberg.xcproj project in Xcode.
• Run the app.
• Press the save button on the nav bar.
• Check that the HTML code is printed out in Xcode terminal.

---

Note:

• iOS 12 simulators have an issue noted here.
• I recommend running the app on a iOS 11 sim.

cc @pinarol

[etoledom]

@hypest

There is a new block-serializer.js helper that had sense in the previous implementation.
I still like that it clean a bit he html-text-input.js component, but it's not really necessary. I can remove it if you'd prefer. 👍

[hypest]

There is a new block-serializer.js helper

It seems that we have no more use of the serializeBlock() helper method anyway though since, we don't have a block named aztec anymore. That used to be an early test before we have RichText based on Aztec.

In that sense, and since serializeBlocksToHtml() is only used inside the html-text-input.js atm, having a separate module to have the helper is perhaps a bit too much. No strong feelings about any solution here though. Ideally, I'd favor whichever is closer to how GB-web does the serialization, but I don't which is that.

[diegoreymendez]

Just 1 comment for now.

Also a question: is there an integration PR I can use to test this in WPiOS?

[diegoreymendez]

Going the route of providing a VC in Gutenberg is a double edged sword, and one that I'd really try to avoid, if possible.

I realize I may not see the full picture so my initial question is: was this choice because of advantages? requirements?

To explain a bit more why I think this is a bigger deal that can be apparent at first... this is all coming from my experience working in Aztec:

• This couples the UI surrounding the editor, to gutenberg-mobile. If the UI needs to be customized we'll need to add extra code to support any such customization (that'd be otherwise for free). Think of the app needing to show different colors... change the whole layout... move buttons around. Or even worse, us wanting to use this library in two different Apps.
• This potentially couples other things too, such as service calls, business logic (that may already exist WPiOS). We may also need to add native dependencies to gutenberg-mobile (things like WordPressShared... the media provider... services... UI components that exist in WPiOS... etc).
• It would reduce our ability to reuse Gutenberg elsewhere. I don't have a clear picture of where Gutenberg may end up showing up, but the advantage in working the editor as a view component is that you can place it anywhere.

Overall, while it may seem simpler, it may end up adding a ton of extra work to access things that we'd get for free if we just provided a view component.

[etoledom]

Thanks for your comments @diegoreymendez !

I believe there's a misunderstanding. Probably I haven't explained this implementation properly.
This GutenbergViewController only exists in the Example app and is meant to be a testing platform for the native modules (the communication between RN and Native).

The plan (as I see it so far) would be to provide just the Gutenberg.h interface to the gutenberg-mobile users (i.e: WPiOS). Gutenberg.h currently lives inside react-native-gutenberg-bridge but ideally we would provide it as the public header in a Pod framework. It provide access to the Gutenberg RootView (plain UIView), and it doesn't have any React / ReactNative public dependencies (all the React dependencies are managed internally). It also has methods to send messages to the RN side and a delegate to receive messages from RN.

[etoledom]

Maybe we could rename it GutenbergExampleController, or plane ExampleController to be more explicit about it.

[diegoreymendez]

I think my confusion stems from the fact there's no mention of Example in the file path or name... it's not a big deal to be honest. If this is an Example App VC we should be good.

[etoledom]

Hey @diegoreymendez ,

There is a branch that I'm using to test the integration of this branch on WPiOS:
try/gutenberg.

I recommend you to clone it on a clean folder.

The steps to make it run are (so far):

• Clone the project.
• git submodule update --init (clone this project as submodule of WPiOS)
• cd Gutenberg -> This project (and not gutenberg-web)
• All the normal steps to make gutenberg-mobile work. (yarn install... submodules... etc...)
• Check that the Example app run properly (yarn ios)
• Go back to the WPiOS root folder and pod install.
• Build and run the project.
• It will (or should) be an error here:

// USE "WordPress-Swift.h" to run on WPiOS, or "RNTAztecView-Swift.h" to run Example App.
#import "RNTAztecView-Swift.h"
//#import "WordPress-Swift.h"

Following the comment instructions should fix the error and should run the app.

The whole dependency management is quite messy for now, but that is the part I just "have been making it work". If you have ideas, everything is welcome!

This little import Hack is the next thing I'm planing to fix, as soon as we finish with the Aztec stuff. 😅

---

In the WPiOS project there's a new GutenbergController class, that will probably turn out to be quite similar to AztecPostViewController.

Same than Aztec, Gutenberg is just a UIView. This will make much of the code existing in AztecPostViewController a candidate to be reused on GutenbergController. There is a bit of that "extract and reuse code" work done for the navigation bar buttons.

I'm waiting forward for your observations and thoughts on this approach.

[diegoreymendez]

@etoledom - In the WPiOS integration branch, when doing yarn clean:install in the Gutenberg submodule, I'm getting an eternal cleaning loop.

$ yarn clean; yarn
$ yarn clean:aztec; yarn cache clean; yarn clean:haste; yarn clean:jest; yarn clean:metro; yarn clean:react; yarn clean:watchman; yarn clean:node;
$ cd react-native-aztec && yarn clean && cd example && yarn clean
$ yarn clean:aztec; yarn cache clean; yarn clean:haste; yarn clean:jest; yarn clean:metro; yarn clean:react; yarn clean:watchman; yarn clean:node;
$ cd react-native-aztec && yarn clean && cd example && yarn clean
$ yarn clean:aztec; yarn cache clean; yarn clean:haste; yarn clean:jest; yarn clean:metro; yarn clean:react; yarn clean:watchman; yarn clean:node;
$ cd react-native-aztec && yarn clean && cd example && yarn clean
$ yarn clean:aztec; yarn cache clean; yarn clean:haste; yarn clean:jest; yarn clean:metro; yarn clean:react; yarn clean:watchman; yarn clean:node;
$ cd react-native-aztec && yarn clean && cd example && yarn clean
$ yarn clean:aztec; yarn cache clean; yarn clean:haste; yarn clean:jest; yarn clean:metro; yarn clean:react; yarn clean:watchman; yarn clean:node;

[etoledom]

Does it happens on this branch? (Without being a submodule of WPiOS).
I testes in both cases and for me it works as it should: 🤔

$ yarn clean; yarn
$ yarn clean:aztec; yarn cache clean; yarn clean:haste; yarn clean:jest; yarn clean:metro; yarn clean:react; yarn clean:watchman; yarn clean:node;
$ cd react-native-aztec && yarn clean && cd example && yarn clean
$ yarn clean-watchman; yarn clean-node; yarn clean-react; yarn clean-metro; yarn clean-jest;
$ command -v watchman >/dev/null 2>&1 && watchman watch-del-all;
{
    "version": "4.9.0",
    "roots": []
}
$ rm -rf node_modules/;
$ rm -rf $TMPDIR/react-*; rm -rf $TMPDIR/react-native-packager-cache-*;
$ rm -rf $TMPDIR/metro-cache-*; rm -rf $TMPDIR/metro-bundler-cache-*;
$ rm -rf $TMPDIR/jest_*;
...

[koke]

Sorry, I'm picky with names 😁, but why not just Gutenberg?

[etoledom]

This is just the Bridge module to communicate between parent and the RN app.
Probably we want to name Gutenberg to the whole Gutenberg module (our RN App) 😄

We will see it more clearly when we work on the integration with the WPiOS app in detail.

[hypest]

Not sure if relevant but, I'd avoid using the "Gutenberg" generic term just yet. We need to be able to say "bridge" and understand which component we're talking about and ideally how to search for it. Just my 2c.

[koke]

Got it, I think I need to catch up a bit on how the details of integrating work 😁

In that case, GutenbergBridge seems reasonable. RN and ReactNative is saying the same thing twice, and IMO both are redundant since we only have a “bridge” because Gutenberg is RN. This isn’t super important, but the odd naming caught my attention

[pinarol]

thank you for your effort! I just entered some small comments, nothing critical. some of them are just questions.

[etoledom]

@hypest -

we don't have a block named aztec anymore.

That's even better! I rolled back the block-serializer changes. We can clean html-text-input up on a separated PR. 👍

@pinarol - Thanks for your observations.
I removed the singleton pattern and refactored Gutenberg.m a bit following your suggestions.

For now we are obligated to only use one instance of Gutenberg in the parent app at a given time. But as we talked with @hypest some time ago, this is fine for now and we can improve it when the need arrives.

This is also deallocating RCTBridge when the user close the editor, and that might be a good thing for memory usage, but we have to create a new one when the user opens a new editor, and this might be too slow. We can test and improve performance after Alfa (or Beta).

[pinarol]

Looks good! Thanks!

[diegoreymendez]

Works as described.