[WIP] Attempt on fixing known Android issues #2031

fabiomcosta · 2019-03-08T17:48:26Z

Summary

This PR attempts to fix long standing issues on Android, mostly related to how it handles composition events.
Currently it's impossible to edit anything on a draft-js Editor field. Even basic operations like "[word][space]" are not possible.
The intent of this PR is not to fix every single Android bug, but to have an editing experience on Android without introducing bugs.

TODO, known issues:

Removing empty line breaks with doesn’t remove blocks.

TODO, others:

Unit tests

Known issues, that might not be fixed on this PR:

Pressing ENTER during a composition doesn’t commit it. Ex: type "hel[enter]", and notice that nothing happens when you click enter, but the expected behavior is to commit that composition and add a line break.
[minor] Arrow keys won't work during composition

Test Plan

On Android 9 Pie (API 28)

Type "d[space]"
Renders the letter "d" followed by a space.

Type "da[space]"
Renders the expected highlighted autocomplete option, "day" in this case.

Type "day[space][backspace][backspace][backspace][backspace]"
Properly removes the text without re-adding the previous text.

[doesn't work yet] Type "d[space][enter][backspace][enter]"
Shouldn't cause an error and should render "d[space][enter]".

Acknowledgments

Based on ideas from:
#1672
#1774
ianstormtaylor/slate#2565

Useful links:

draft-js 0.10.5 sandbox: https://codesandbox.io/s/ko4zmx633v
plain contenteditable sandbox: https://codesandbox.io/s/q77yqk1nww

…tempt_android_fix

fabiomcosta · 2019-03-08T17:58:19Z

gulpfile.js

  'dev',
  gulp.series(function() {
-    gulp.watch(paths.src, ['modules', 'dist']);
+    gulp.watch(paths.src, gulp.parallel('modules', 'dist'));


These changes are introduced separately by #2030

claudiopro · 2019-03-10T22:15:00Z

Hi @fabiomcosta, thanks for taking the time to submit improvements for the Android editing experience. I'll set some time aside to review them in the coming days as I'm not familiar with the quirks of this platform. In the meantime, I'll look for someone internal to Facebook who can give you a code review faster.

fabiomcosta · 2019-03-10T22:53:37Z

Hi Claudio, I appreciate you taking the time to answering! This is not yet ready for a full review and merge, I’m still working on it and I’ll make sure to ping you and other interested people when it’s ready.

On Sun, Mar 10, 2019 at 7:15 PM Claudio Procida ***@***.***> wrote: Hi @fabiomcosta <https://github.com/fabiomcosta>, thanks for taking the time to submit improvements for the Android editing experience. I'll set some time aside to review them in the coming days as I'm not familiar with the quirks of this platform. In the meantime, I'll look for someone internal to Facebook who can give you a code review faster. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2031 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABvcrHfYw44-NbMnwBZF9gH0A9CEDW7ks5vVYPugaJpZM4bl01i> .

-- -- Fabio Miranda Costa *twitter:* @fabioMiranda *github:* fabiomcosta

claudiopro · 2019-03-10T22:56:30Z

Sounds good @fabiomcosta, thank you so much for this!

fabiomcosta · 2019-03-11T12:19:37Z

@claudiopro this is a quick one though: #2030

claudiopro · 2019-03-11T14:50:04Z

Sure, see my comments on #2030 😄

…tempt_android_fix

…will never work with the data that comes from the event and will be working on a new approach next

fabiomcosta · 2019-03-14T18:02:51Z

I noticed this approach won't work when doing multiple compositions, which can happen on Android.
I'm working on a new experiment that uses Prosemirror's DOMObserver: https://github.com/ProseMirror/prosemirror-view/blob/409f267d6f4dc534637acc708fcac582274da6ce/src/domobserver.js

Prosemirror, for the ones that don't know, is a very solid project that works flawlessly on Android and other devices.

I might create a new PR for that, we'll see.

fabiomcosta · 2019-03-15T14:50:20Z

@claudiopro I'm closing this in favor of #2035, which is handling composition events in a much more consistent way.
I'm pretty pumped!

Summary: This PR is a new attempt to address #1895 On #2031 I was trying to make compositions work using the data provided by that event, and even though that works well for most operational system, that doesn't work well for Android, where you can make composition updates in multiple places on the same composition transaction. This PR is an improvement over that PR, in that it uses a DOM mutation observer during the `compositionStart` -> `compositionEnd` interval to detect the changes the user made, and then re-apply those to the ContentState on `compositionEnd`. This approach is the one used by [Prosemirror](http://prosemirror.net/) (see https://github.com/ProseMirror/prosemirror-view/blob/master/src/domobserver.js), which is the only Rich Text Editor I've tried that works well on Android. Like previously mentioned, it allows multiple mutations on multiple places in the same composition transaction, which was impossible with the previous approach, and would cause DOM<->state inconsistencies in multiple use cases. The intent of this PR is not to fix every single Android bug, but to have a consistent editing experience on Android without introducing bugs (ideally). **TODO, known issues:** - [x] Removing empty line breaks with <backspace> doesn’t remove blocks. - [x] Mutations on the same block (different leaf nodes) are not being properly applied. - [x] Selection is not properly updated during composition events - [ ] Keep `inlineStyleOverride` working with a consistent behavior **TODO, others:** - [x] Test on Android Pie v9 API 28 - [x] Test on Android Oreo v8.1 API 27 - [x] Test on Android Oreo v8.0 API 26 - [x] Test on iPhone 12.1 (with Japanese Kana keyboard) - [x] Test composition events on Chrome Desktop v73 - [x] Test composition on IE11 (I didn't know how to test composition events though) - [x] Unit tests There are 3 ways to try out this PR. Codesandbox: https://3ymzzlj9n5.codesandbox.io/ (uses `draft-js-android-fix-beta.3`) Use the published [draft-js-android-fix](https://www.npmjs.com/package/draft-js-android-fix) package: `yarn install draft-js-android-fix` Note that this package might not be up-to-date, it's hard for me to guarantee I'll always remember to update the package, but I'll do my best. The other way is guaranteed to be up-to-date, but has a longer setup: * run `git clone https://github.com/facebook/draft-js.git` * run `git remote add fabiomcosta https://github.com/fabiomcosta/draft-js.git` * run `git fetch fabiomcosta` * run `git checkout -b attempt_android_fix_with_dom_diff fabiomcosta/attempt_android_fix_with_dom_diff` * run `yarn install` (or use `npm`) * run `open examples/draft-0-10-0/rich/rich.html`, or any other example you'd like to test Pull Request resolved: #2035 Reviewed By: kedromelon Differential Revision: D14568528 Pulled By: claudiopro fbshipit-source-id: 16861de52eca41cd98f884b0aecf034213fc1bd0

Summary: This PR is a new attempt to address facebookarchive#1895 On facebookarchive#2031 I was trying to make compositions work using the data provided by that event, and even though that works well for most operational system, that doesn't work well for Android, where you can make composition updates in multiple places on the same composition transaction. This PR is an improvement over that PR, in that it uses a DOM mutation observer during the `compositionStart` -> `compositionEnd` interval to detect the changes the user made, and then re-apply those to the ContentState on `compositionEnd`. This approach is the one used by [Prosemirror](http://prosemirror.net/) (see https://github.com/ProseMirror/prosemirror-view/blob/master/src/domobserver.js), which is the only Rich Text Editor I've tried that works well on Android. Like previously mentioned, it allows multiple mutations on multiple places in the same composition transaction, which was impossible with the previous approach, and would cause DOM<->state inconsistencies in multiple use cases. The intent of this PR is not to fix every single Android bug, but to have a consistent editing experience on Android without introducing bugs (ideally). **TODO, known issues:** - [x] Removing empty line breaks with <backspace> doesn’t remove blocks. - [x] Mutations on the same block (different leaf nodes) are not being properly applied. - [x] Selection is not properly updated during composition events - [ ] Keep `inlineStyleOverride` working with a consistent behavior **TODO, others:** - [x] Test on Android Pie v9 API 28 - [x] Test on Android Oreo v8.1 API 27 - [x] Test on Android Oreo v8.0 API 26 - [x] Test on iPhone 12.1 (with Japanese Kana keyboard) - [x] Test composition events on Chrome Desktop v73 - [x] Test composition on IE11 (I didn't know how to test composition events though) - [x] Unit tests There are 3 ways to try out this PR. Codesandbox: https://3ymzzlj9n5.codesandbox.io/ (uses `draft-js-android-fix-beta.3`) Use the published [draft-js-android-fix](https://www.npmjs.com/package/draft-js-android-fix) package: `yarn install draft-js-android-fix` Note that this package might not be up-to-date, it's hard for me to guarantee I'll always remember to update the package, but I'll do my best. The other way is guaranteed to be up-to-date, but has a longer setup: * run `git clone https://github.com/facebook/draft-js.git` * run `git remote add fabiomcosta https://github.com/fabiomcosta/draft-js.git` * run `git fetch fabiomcosta` * run `git checkout -b attempt_android_fix_with_dom_diff fabiomcosta/attempt_android_fix_with_dom_diff` * run `yarn install` (or use `npm`) * run `open examples/draft-0-10-0/rich/rich.html`, or any other example you'd like to test Pull Request resolved: facebookarchive#2035 Reviewed By: kedromelon Differential Revision: D14568528 Pulled By: claudiopro fbshipit-source-id: 16861de52eca41cd98f884b0aecf034213fc1bd0

fabiomcosta added 3 commits March 2, 2019 19:28

Much saner experience, but still with known issues that Im taking notes.

83730c9

Merge branch 'master' of https://github.com/facebook/draft-js into at…

939f935

…tempt_android_fix

Improving code a bit

c831fcf

facebook-github-bot added the CLA Signed label Mar 8, 2019

fabiomcosta changed the title ~~Attempt on fixing known Android issues~~ [WIP] Attempt on fixing known Android issues Mar 8, 2019

fabiomcosta commented Mar 8, 2019

View reviewed changes

fabiomcosta added 5 commits March 8, 2019 14:59

minor cleanup

1d55ab4

fixing flow issues

7322ff4

revert header changes

7e67808

keeping draft_handlebeforeinput_composed_text experiment

314e3db

Removing code added by accident

65120e8

fabiomcosta added 2 commits March 12, 2019 10:45

Merge branch 'master' of https://github.com/facebook/draft-js into at…

8c3e6ab

…tempt_android_fix

fixing more issues realted to composition, but I finaly noticed this …

2f7ac3f

…will never work with the data that comes from the event and will be working on a new approach next

fabiomcosta mentioned this pull request Mar 14, 2019

Fixing major Android editing issues #2035

Closed

11 tasks

fabiomcosta closed this Mar 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[WIP] Attempt on fixing known Android issues #2031

[WIP] Attempt on fixing known Android issues #2031

Uh oh!

fabiomcosta commented Mar 8, 2019 •

edited

Loading

Uh oh!

fabiomcosta Mar 8, 2019

Uh oh!

claudiopro commented Mar 10, 2019

Uh oh!

fabiomcosta commented Mar 10, 2019 via email

Uh oh!

claudiopro commented Mar 10, 2019

Uh oh!

fabiomcosta commented Mar 11, 2019

Uh oh!

claudiopro commented Mar 11, 2019

Uh oh!

fabiomcosta commented Mar 14, 2019

Uh oh!

fabiomcosta commented Mar 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[WIP] Attempt on fixing known Android issues #2031

[WIP] Attempt on fixing known Android issues #2031

Uh oh!

Conversation

fabiomcosta commented Mar 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

On Android 9 Pie (API 28)

Uh oh!

fabiomcosta Mar 8, 2019

Choose a reason for hiding this comment

Uh oh!

claudiopro commented Mar 10, 2019

Uh oh!

fabiomcosta commented Mar 10, 2019 via email

Uh oh!

claudiopro commented Mar 10, 2019

Uh oh!

fabiomcosta commented Mar 11, 2019

Uh oh!

claudiopro commented Mar 11, 2019

Uh oh!

fabiomcosta commented Mar 14, 2019

Uh oh!

fabiomcosta commented Mar 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fabiomcosta commented Mar 8, 2019 •

edited

Loading