forked from facebookarchive/draft-js
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fixing major Android editing issues (facebookarchive#2035)
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
- Loading branch information
1 parent
6d030ba
commit 01e05bf
Showing
10 changed files
with
635 additions
and
77 deletions.
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| /** | ||
| * Copyright (c) Facebook, Inc. and its affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| * | ||
| * @format | ||
| * @flow strict-local | ||
| * @emails oncall+draft_js | ||
| */ | ||
|
|
||
| 'use strict'; | ||
|
|
||
| const UserAgent = require('UserAgent'); | ||
|
|
||
| const findAncestorOffsetKey = require('findAncestorOffsetKey'); | ||
| const Immutable = require('immutable'); | ||
| const invariant = require('invariant'); | ||
| const nullthrows = require('nullthrows'); | ||
|
|
||
| const {Map} = Immutable; | ||
|
|
||
| type MutationRecordT = | ||
| | MutationRecord | ||
| | {|type: 'characterData', target: Node, removedNodes?: void|}; | ||
|
|
||
| // Heavily based on Prosemirror's DOMObserver https://github.com/ProseMirror/prosemirror-view/blob/master/src/domobserver.js | ||
|
|
||
| const DOM_OBSERVER_OPTIONS = { | ||
| subtree: true, | ||
| characterData: true, | ||
| childList: true, | ||
| characterDataOldValue: false, | ||
| attributes: false, | ||
| }; | ||
| // IE11 has very broken mutation observers, so we also listen to DOMCharacterDataModified | ||
| const USE_CHAR_DATA = UserAgent.isBrowser('IE <= 11'); | ||
|
|
||
| class DOMObserver { | ||
| observer: ?MutationObserver; | ||
| container: HTMLElement; | ||
| mutations: Map<string, string>; | ||
| onCharData: ?({target: EventTarget, type: string}) => void; | ||
|
|
||
| constructor(container: HTMLElement) { | ||
| this.container = container; | ||
| this.mutations = Map(); | ||
| if (window.MutationObserver && !USE_CHAR_DATA) { | ||
| this.observer = new window.MutationObserver(mutations => | ||
| this.registerMutations(mutations), | ||
| ); | ||
| } else { | ||
| this.onCharData = e => { | ||
| invariant( | ||
| e.target instanceof Node, | ||
| 'Expected target to be an instance of Node', | ||
| ); | ||
| this.registerMutation({ | ||
| type: 'characterData', | ||
| target: e.target, | ||
| }); | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| start(): void { | ||
| if (this.observer) { | ||
| this.observer.observe(this.container, DOM_OBSERVER_OPTIONS); | ||
| } else { | ||
| /* $FlowFixMe(>=0.68.0 site=www,mobile) This event type is not defined | ||
| * by Flow's standard library */ | ||
| this.container.addEventListener( | ||
| 'DOMCharacterDataModified', | ||
| this.onCharData, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| stopAndFlushMutations(): Map<string, string> { | ||
| const {observer} = this; | ||
| if (observer) { | ||
| this.registerMutations(observer.takeRecords()); | ||
| observer.disconnect(); | ||
| } else { | ||
| /* $FlowFixMe(>=0.68.0 site=www,mobile) This event type is not defined | ||
| * by Flow's standard library */ | ||
| this.container.removeEventListener( | ||
| 'DOMCharacterDataModified', | ||
| this.onCharData, | ||
| ); | ||
| } | ||
| const mutations = this.mutations; | ||
| this.mutations = Map(); | ||
| return mutations; | ||
| } | ||
|
|
||
| registerMutations(mutations: Array<MutationRecord>): void { | ||
| for (let i = 0; i < mutations.length; i++) { | ||
| this.registerMutation(mutations[i]); | ||
| } | ||
| } | ||
|
|
||
| getMutationTextContent(mutation: MutationRecordT): ?string { | ||
| const {type, target, removedNodes} = mutation; | ||
| if (type === 'characterData') { | ||
| // When `textContent` is '', there is a race condition that makes | ||
| // getting the offsetKey from the target not possible. | ||
| // These events are also followed by a `childList`, which is the one | ||
| // we are able to retrieve the offsetKey and apply the '' text. | ||
| if (target.textContent !== '') { | ||
| return target.textContent; | ||
| } | ||
| } else if (type === 'childList') { | ||
| // `characterData` events won't happen or are ignored when | ||
| // removing the last character of a leaf node, what happens | ||
| // instead is a `childList` event with a `removedNodes` array. | ||
| // For this case the textContent should be '' and | ||
| // `DraftModifier.replaceText` will make sure the content is | ||
| // updated properly. | ||
| if (removedNodes && removedNodes.length) { | ||
| return ''; | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| registerMutation(mutation: MutationRecordT): void { | ||
| const textContent = this.getMutationTextContent(mutation); | ||
| if (textContent != null) { | ||
| const offsetKey = nullthrows(findAncestorOffsetKey(mutation.target)); | ||
| this.mutations = this.mutations.set(offsetKey, textContent); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| module.exports = DOMObserver; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.