Refactor Android Plugin

[thesunny]

Do you want to request a feature or report a bug?

Refactor the Android plugin in order to improve readability and make the code easier to understand.

What's the current behavior?

The Android plugin merges all Android versions into one plugin and uses if/else and switch/case statements to execute the correct code for the correct version. It also uses if/else statements to split execution based on native/non-native onBeforeInput handlers.

It is difficult to follow the code and easy to break code in another version of Android without realizing it. For example, some code in Android 8 may not be branched and fixing this may break code in Android 9. Furthermore, we might edit code in the wrong branch inadvertently.

What's the expected behavior?

First, split the code into plugins for each version making it impossible for changes in one version of Android to break changes in another.

Second, internally refactor the code so that onBeforeInput calls out to onBeforeInputNative and onTextInput in the Android plugin.

[ianstormtaylor]

I’m not sure about this. How do other editors do it? I’d sooner think that we either want a single plugin for everything. Or that we want to split plugins by area instead of by platform.

[Nantris]

@ianstormtaylor can you clarify what you mean by, "by area"?

Even if we aim to split up the code in a fashion besides by platform, might it be a good idea to keep Android support somewhat self-contained until it's refactored, more mature, and understood by more maintainers/contributors?

[ianstormtaylor]

"By concern" might have been clearer. But things like clipboard usage, pointer events, scrolling behaviors, composition events, etc. I'm not saying it should be though, as they might be too interrelated.

My current thought is that I'd like for Android to be unseparated over time. Otherwise it will continue to be something that is not understood by 90% of maintainers, and it will languish or just be buggy.

[Nantris]

"By concern" is crystal clear and I totally agree. I think in the medium-long term Android should be parceled out throughout the project to where each piece of the code resides with other like pieces.

That said, when I set out on some complicated code though, I find myself often breaking this good long term convention in favor of a trying to circumscribe the work so I can view all the moving parts at once for faster iterations. I've been using the Android work only a little so far, mostly due to some outstanding issues. I think we're still in the "need for faster iterations" phase here.

Is there some way we can separate code by concerns, but also avoid a potential maintainability nightmare regarding different Android versions as outlined by @thesunny?

I'm kind of thinking out loud, you two are the brains here; would it be possible to chunk the logic for only older Android SDKs into plugins that modify core behavior, and then have Slate core handle Android logic for the latest SDK onward? This seems like it might help with maintainability, but I'm not sure of:

1. How the code is split up and if this is achievable.
2. If this pattern actually helps us keep the code base clean and intelligible, or if it just sounds good and it actually makes things worse

[thesunny]

@slapbox Thanks for asking for the clarification and @ianstormtaylor for the response.

I'd be happy to have this merged in with the other code at some point in the future. At this point, however, I think it's premature.

Right now even I (the author!) have to read my own notes and source code to understand what I did. And the way it's written is spaghetti and hard to understand because the concerns are all over the place in one file. The goal for splitting this is to make it easier to understand what the code does on re-reading and of course for new readings as well. This was the immediate impression I got when coming back to the code after a hiatus.

In the future, however, I can see that we might extract some common modules that could be used in a more general manner and integrated hopefully. I don't think I have the right building blocks yet though and IMO that needs to come first before trying to integrate.

[Nantris]

the way it's written is spaghetti and hard to understand because the concerns are all over the place in one file. The goal for splitting this is to make it easier to understand what the code does on re-reading and of course for new readings as well. This was the immediate impression I got when coming back to the code after a hiatus.

God, I know this feeling way too well. When working on something tough and you just finally get it to work, and the main thing keeping you fully grasping what's going on is the fact you're completely immersed in it every day. Then you take 3 days off and poof. Takes a few days to get back into it, or in the worst case... you get the sad case of my PR 2347 to fix quirks in history management. I'd link, but don't want to dirty up the cross-linking.

[thesunny]

As an update, I've separated Android plugins by major version number and in combination with an ActionManager object that I added this has made understanding the code easier and the iterations faster. It is less common for a fix in one area to break things elsewhere.

Some insights:

• There are some methods that overlap between Android versions; however, we must be careful because sometimes they are similar but not the same and the difference matters. I'm on the fence about how and when to share methods between Android versions.
• There is almost no overlap between Android's special handling and all other browsers. These issues are mostly unique to Android.
• Special classes like ActionManager and DomSnapshot are not useful for other browsers
• Reconciler might be useful in other browsers

[ianstormtaylor]

Hey, there are lots of open Android-related issues that are out of date with the latest 0.50 release because the codebase was completely rearchitected. We'll need to figure out how to have proper support for Android going forward based on beforeinput events somehow. I'm tracking this in #3112, so I'm going to close this in favor of that one.

If it cannot be implemented simply in core with beforeinput we'll likely need to have a separate plugin for android support be created, because it's too much of a departure from all of the other simpler logic in core and very hard for me to test or respond to issues. Using a separate plugin should be possible now as the newest version simplifies a lot of the internals.

Thank you for understanding.