Proposal: Alternate hook-based implementation of Android support

[thesunny]

Problem
User @clauderic believes that the approach to implementing Android support should be implemented as a hook and not as a separate `Editable`.

Solution
Will post clauderic's proposed solution in a separate comment below

Alternatives
The current in progress solution is with this PR #4200

Context
Related to closing this issue #3786

[thesunny]

This is a cut and paste of @clauderic's summary of the issues he has with the current approach which is currently in the PR.

As commented by @clauderic:

I've left a lot of disconnected pieces of feedback on this PR, so I'll try to summarize.

Level of abstraction

Conceptually, I disagree with the overall approach taken in this PR. It's the wrong level of abstraction in my view.

The <Editable> component manages matters related to making Slate compatible with React. Forking it in its entirety only to support Android isn't the right long term decision.

In the short term, perhaps that's an acceptable change, but I do worry that it will be a decision that has a lot of friction to revert / move away from later on.

Long term, the right abstraction to model this in my opinion is to abstract all logic related to user input reconciliation out of Editable and into a hook, something like useInputReconciliation.

Input reconciliation can further be broken down into different categories of user input that needs to be reconciled with Slate's internal value:

• Selection reconciliation
• Composition reconciliation
• Drag and drop
• Copy / cut / paste

All four of these types of user input can be further broken down into hooks (useSelectionReconciliation, etc.).

The text composition reconciliation layer can further be broken down into different reconciliation strategies that can be conditionally enabled depending on the browser capabilities:

• beforeinput-based reconciliation (useBeforeInput)
• MutationObserver-based reconciliation (useMutationObserver)

Mutation reconciliation

I believe that your concern stems from misunderstanding the overall strategy for resolving mutations. We do not convert the mutations to slate operations directly. For DOM structure changes (split/merge), mutations are applied immediately. For text changes (insert,remove), we keep track of the text difference and only apply the text difference when a dom structure changes or window selection changes.

I understand the current overall strategy for resolving mutations, but in my view the current strategy is flawed.

Mutations resulting from user input need to be reconciled as soon as they are flushed, not just when the selection changes or when a DOM structure changes.

Input reconciliation layers should be as closely aligned with one another as possible to avoid behavioural differences when authoring functionality on top of Slate.

Delayed.text.updates.mp4

Samsung Galaxy S20, Android 10

The value should be updated anytime a new character data mutation is detected to match the behaviour of the beforeinput reconciliation strategy. Doing so will avoid behavioural differences that could otherwise break consumer-built functionality on top of Slate.

In my experience this is possible to do, and doing so would greatly simplify the overall complexity of AndroidInputManager because you'd only need to process a single set of flushed mutations at a time as opposed to the current approach of delayed batch updates that requires you to check for text diff in selection changes and DOM structure changes.

Various bugs

Samsung Galaxy S20, Android 10

Text.deletion.of.expanded.selection.mp4

Impact.of.requestAnimationFrame.delay.before.reconciliation.mp4

Split.block.bug.mp4

Copy.paste.mp4

[thesunny]

Hi @clauderic

You've given a lot of thought and put a lot of work into your thoughts and I can see that you are passionate that this is the correct approach.

I would encourage you to continue work on your PR (Let's call it the Hook PR and the other one the Editable PR) and when it becomes a better solution to the Editable PR, fair consideration will be given to replace the Editable PR. Similarly, I helped built out a TypeScript proposal with some known weaknesses, which several others got working and which I helped to complete. Ian is now working on one that uses generics that will probably replace it. Great! Maybe our TypeScript work helped get some ideas fleshed out. Maybe it just inspired Ian to built his solution. Either way, things are getting better.

One of the benefits of Editable is that it's really easy to remove the Editable PR and replace it. It's pretty much all in one file which imports what it needs. This means replacing it with a hook based version can probably done by simply removing EditableAndroid. Please, keep on working on your PR. I hope it's better and solves more issues and it's easier to maintain.

So why aren't "we" going to fix the Editable PR? There are three really good reasons that aren't about which approach is better:

1. We don't have the resources. We raised some money, but for clarity, there is no profit in this. In fact, before we even began, the work into Android was losing money for my company. We spent more than what we raised on the last version which was unfortunately made obsolete during the 0.50+ switch and this new version we're doing with Wayne. There is no profit in this and it uses a lot of resources from a small company. We are not Shopify.
2. If we were to restart the process, it would mean a long time. It would probably reduce some bugs/issues, but it would introduce different ones. It sounds like you have a lot of experience with using a Hook based approach so it feels like you would be the ideal candidate to complete work on this.
3. Something is better than nothing. I think many just want something they can start using even if it has some bugs and it's not perfect. If it's not good enough, the community, including yourself, should feel free to improve it. Wayne and I are no different from the tens of thousands of people that use Slate. I was personally waiting for Android support for years, and because nobody could get it working, I invested a lot of time into adding some support for it. There were many many attempts prior to mine.

Android support is hard and IMO it's incredibly draining work. You fix one bug, and it breaks another device. You have to handle compositions, gesture writing, IME, and really mundane things like hitting enter at different places in a line or word create different code paths, stuff that is easily handled in the default handler with one approach.

I'm actually quite surprised at how there isn't a lot of alternative code paths that were required to set up the Editable PR which is a good sign to the hooks method. That said, I don't think that was obvious at the start. In 0.47x, I had huge problems where the existing code would interfere with the Android code. You are getting to work off of all the work that others including myself have already done. That's great! And you should. But I would ask that you be respectful of the time, work and the problems that we already solved that you get to build on top of it. For example, there were literally years of attempts at getting Android working without mutations and there was lots of push back that using mutations was the right approach. Because of that, I spent months, full-time, figuring out that it was impossible to do without mutations by investigating alternative solutions. The last mutation based approach was relatively quick to build but it was a result of the months of work exploring the alternatives.

I'm not referring to your summary above, which I think is written fairly; however, I'd like to ask you to be mindful of your tone at times. We are all trying to contribute to this code base and sometimes your tone is condescending and dismissive. We are in this together trying to solve a problem and I think we want to encourage engagement in the community.

[clauderic]

Hey @thesunny, I don't mean to come off as dismissive or condescending, I'm sorry if that's how some of my messages were perceived. I've said this before and will state it again, I'm truly appreciative of all the work you and @wleroux have put into this, and the community should be too.

I'm no stranger to how gruesome and frustrating working on Android reconciliation is, and the work you have both done is heroic and should not be understated. It is because of all those efforts that we're able to sit here and discuss the merits of one approach over the other.

I mentioned this from the offset, I'm only hoping to spark discussions on the technical merits of one approach over the other in terms of the level of abstraction. I don't think we necessarily need to ship the perfect abstraction from the get-go, but we should pause and make sure we don't introduce anything that makes the project harder to maintain or is a decision that's hard to revert.

Level of abstraction aside, I think there is some work to be done with regards to the actual mutation reconciliation, I've summarized some of the issues I've found so far. I definitely agree with you that something is better than nothing though.

If there's buy-in to validate different approaches, I'm happy to put more time aside to invest into the PR I had started a while back to clean it up so we can compare.

[thesunny]

Totally happy to see a different attempt at Android and have it replace what we've built if it turns out to be superior.

I hope that the approach works and it is easier to maintain.

I think one thing that would be valuable is to reintroduce the test cases I made from the 0.47x version. It's kind of a summary of all the edge cases that I encountered. It's so easy to forget things like gesture writing, or auto-correct, or spell check correct, or IME, or splitting at the beginning/middle/end of word/line, or holding down the backspace key, or selecting exactly a word and delete, etc. etc.

This would make it easier to check coverage. The problem though is that these test cases have to be executed manually (at the moment). It would be great if we could somehow run this through a testing tool but I expect that won't be trivial to run across multiple devices.

[clauderic]

@thesunny I've been investigating authoring automated Browserstack tests for this purpose

[thesunny]

That's awesome! That will be a huge win once it's working.

I'm not sure if BrowserStack uses virtual devices or real devices but a note that I've found that the virtual devices behave similar, but not quite the same as the real devices. A virtual test is better than no test, but if we have the option, real devices would be the best.

[clauderic]

BrowserStack uses real devices.

They recently launched a beta for authoring automated Cypress tests running in Browserstack devices, but unfortunately the beta is currently limited to desktop browsers, hoping to see that expanded to mobile devices in the future.

You can write automated selenium tests against mobile devices in BrowserStack which is what I have been looking into, but I'm not sure if the sendKeys() API of selenium running in those browsers would really trigger real events that modify the contenteditable element or if they are just mocked events, haven't spent enough time digging into this yet.

[clauderic]

Opened a PR here that implements the described approach, but does not introduce any changes related to mutation observer. That should happen in a separate PR.

Feedback is welcome when you can make time to review the PR #4224

The PR lays all the groundwork needed to have swappable composition reconciliation layers.