[RNMobile] update mobile to not use ListEdit

[mchowning]

related gutenberg PR

Description

This update is in response to changes made on the web side in #15113
that resulted in a bundling error on mobile due to a missing module. Because the web side now uses a PanelBody component that mobile does not have in part of the list implementation that is shared with mobile, we needed to separate that out on the mobile side.

Sharing Logic Between Web and Mobile

I went with the straightforward and safe approach of just creating a separate list/edit.native.js file that is a copy of list/edit.js but without the components that mobile does not use/have. This seemed like the best approach because it is both quick and safe since it doesn't change any code that the web relies on. It also seems to be the approach I've seen used elsewhere. With that said, I don't really like this as a general approach because not only do now have most of list/edit.js duplicated in two files, but list/edit.js and listedit.native.js are now going to almost inevitably drift apart, which will likely lead to further problems like the one that led to this PR. Long story short, I'm trying to think about what might be a better long-term approach, so please don't hesitate to suggest another approach that might be better.

The only real substantive change in list/edit.native.js from list/edit.js is the removal of the "Start Value" and "Reverse" controls that are used on the web. For example, I left in the keyboard shortcut components from web even though we're not using them on mobile in order to (1) keep the native implementation as similar to web as possible; and (2) because I know we want to add keyboard shortcuts on mobile eventually.

I pushed a new commit moving away from ^that approach. Instead, I am now separating the logic by extracting the web-only part of the component to a separate additional-settings.js file/component. Then for mobile I just made a component function in additional-settings.native.js that always returns null. I prefer this because it avoids the duplication in the separate edit.native.js file and also makes it clear where mobile and web are differing.

A similar approach would be to just give mobile an empty component for all of the components used on web but not on mobile (fedbdc8 shows what this PR would look like with that approach). The disadvantage of this imo is that it makes it less obvious to determine after-the-fact where the behavior of mobile and web differs as compared to having the single additional-settings component like this PR has. The advantage of this approach, however, is that it requires no changes to the web-side's js files.

How has this been tested?

Mobile

Without the changes in this PR, attempting to bundle gutenberg's master branch for mobile should result in a bundling error. With the changes in this PR there is no error and no problems or changes in behavior with respect to:

• Loading web-created post with a list block on mobile
• Viewing mobile-created post with a list block on web
• Adjusting indentation in a list block on mobile
• Adjusting bulllet style (ul/ol) on mobile

Web

Tested that this PR causes no changes on the web side. In particular, verified that the changes added in #15113 work as they did before with ordered lists:

• Modifying the "Start Value" setting
• Modifying the "Reverse List" setting

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.

[mchowning]

👋 @ellatrix . You may want to give this PR a look since it relates to some recent changes in #15113.

[etoledom]

I do prefer this approach too.
On the second proposal, having empty components implementation might be problematic when we do implement those component for other purposes.

This solves the building error and seems to work fine on Mobile 👍

I wonder what would happen when we load a list block using these new props on mobile.
Tried running gutenberg web locally from latest master but I didn't see the new options on Ordered List.

[ellatrix]

Just a suggestion: perhaps call it OrderedListSettings and move the ordered check?

[mchowning]

I like that much better. Thanks for the suggestion!

[ellatrix]

This is fine with me if it solves the error. :)

[mchowning]

👋 @etoledom Thanks for the review!

I wonder what would happen when we load a list block using these new props on mobile.

I updated initial-html.js to include this block (copied from html mode on a local server running the web editor from this branch), and I didn't observe any problems on mobile:

<!-- wp:list {"ordered":true,"start":10,"reversed":true} -->
<ol reversed start="10"><li>First</li><li>Second<ol><li>Third Indented</li></ol></li><li>Fourth</li></ol>
<!-- /wp:list -->

Tried running gutenberg web locally from latest master but I didn't see the new options on Ordered List.

Well that is concerning! I just double-checked this and everything seems to be working fine for me when I run a local dev server. I checked that I was actually observing the changes from my branch on my local dev server by observing that running the server off of my branch shows the new options if no changes are made, but if I made changes that broke the new options then those options did not appear for me (in other words, my breaking the feature during development was most-definitely just a part of my carefully-considered and oh-so-thorough testing strategy, it definitely wasn't that I just messed something up 😉).

If you're still not seeing the additional options on web, let's sync up and figure out what we're doing differently.

[etoledom]

Thank you for the update, great job 🎉

I managed to run locally gutenberg-web successfully now, and I noticed something that I thought might happen:

I updated initial-html.js to include this block (copied from html mode on a local server running the web editor from this branch), and I didn't observe any problems on mobile

So, gutenberg-mobile loads the block properly, except that it doesn't take into account the new parameters. This is expected since we haven't implemented that functionality on Aztec.

The problem is that if we modify the list block in any way, the new settings will be lost and will override what was done on web.

This is probably difficult to fix and will have us implementing that functionality, so I think the best would be to merge this and solve the build error, and to create a ticket with this issue to keep it in mind.

Thank you again @mchowning for tacking this!

[mchowning]

The problem is that if we modify the list block in any way, the new settings will be lost and will override what was done on web.

This is probably difficult to fix and will have us implementing that functionality, so I think the best would be to merge this and solve the build error, and to create a ticket with this issue to keep it in mind.

Good catch @etoledom ! I'll make the ticket.

UPDATE: Created gutenberg-mobile#1304