From 92c48d9f1d2fe836c4a819de0cea762554d5df76 Mon Sep 17 00:00:00 2001 From: Yifeng Wang Date: Sat, 28 Oct 2017 16:39:18 -0500 Subject: [PATCH] Improve desktop IME stability (#1316) * add missing composing consts * limit usage of `onbeforeinput` * fix ime error on new line * fix empty block test * fix leaky case * add comment for magic char * fix condition logic * revert magic char --- packages/slate-react/src/components/content.js | 13 ++++++++++--- packages/slate-react/src/constants/environment.js | 1 + packages/slate-react/src/constants/hotkeys.js | 4 +++- packages/slate-react/src/plugins/before.js | 11 +++++++++-- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/packages/slate-react/src/components/content.js b/packages/slate-react/src/components/content.js index 73bfaa446..3e6c501d5 100644 --- a/packages/slate-react/src/components/content.js +++ b/packages/slate-react/src/components/content.js @@ -10,7 +10,12 @@ import Node from './node' import findDOMRange from '../utils/find-dom-range' import findRange from '../utils/find-range' import scrollToSelection from '../utils/scroll-to-selection' -import { IS_FIREFOX, SUPPORTED_EVENTS } from '../constants/environment' +import { + IS_FIREFOX, + IS_IOS, + IS_ANDROID, + SUPPORTED_EVENTS +} from '../constants/environment' /** * Debug. @@ -87,7 +92,8 @@ class Content extends React.Component { */ componentDidMount = () => { - if (SUPPORTED_EVENTS.beforeinput) { + // Restrict scoped of `beforeinput` to mobile. + if ((IS_IOS || IS_ANDROID) && SUPPORTED_EVENTS.beforeinput) { this.element.addEventListener('beforeinput', this.onNativeBeforeInput) } @@ -103,7 +109,8 @@ class Content extends React.Component { */ componentWillUnmount() { - if (SUPPORTED_EVENTS.beforeinput) { + // Restrict scoped of `beforeinput` to mobile. + if ((IS_IOS || IS_ANDROID) && SUPPORTED_EVENTS.beforeinput) { this.element.removeEventListener('beforeinput', this.onNativeBeforeInput) } } diff --git a/packages/slate-react/src/constants/environment.js b/packages/slate-react/src/constants/environment.js index acaba2d35..c967e305c 100644 --- a/packages/slate-react/src/constants/environment.js +++ b/packages/slate-react/src/constants/environment.js @@ -92,6 +92,7 @@ export const IS_FIREFOX = BROWSER === 'firefox' export const IS_SAFARI = BROWSER === 'safari' export const IS_IE = BROWSER === 'ie' +export const IS_ANDROID = OS === 'android' export const IS_IOS = OS === 'ios' export const IS_MAC = OS === 'macos' export const IS_WINDOWS = OS === 'windows' diff --git a/packages/slate-react/src/constants/hotkeys.js b/packages/slate-react/src/constants/hotkeys.js index 1d4349530..6ac3ede99 100644 --- a/packages/slate-react/src/constants/hotkeys.js +++ b/packages/slate-react/src/constants/hotkeys.js @@ -83,7 +83,9 @@ const COMPOSING = e => ( e.key == 'ArrowDown' || e.key == 'ArrowLeft' || e.key == 'ArrowRight' || - e.key == 'ArrowUp' + e.key == 'ArrowUp' || + e.key == 'Backspace' || + e.key == 'Enter' ) /** diff --git a/packages/slate-react/src/plugins/before.js b/packages/slate-react/src/plugins/before.js index adf08c789..6266758f9 100644 --- a/packages/slate-react/src/plugins/before.js +++ b/packages/slate-react/src/plugins/before.js @@ -4,7 +4,12 @@ import getWindow from 'get-window' import { findDOMNode } from 'react-dom' import HOTKEYS from '../constants/hotkeys' -import { IS_FIREFOX, SUPPORTED_EVENTS } from '../constants/environment' +import { + IS_FIREFOX, + IS_IOS, + IS_ANDROID, + SUPPORTED_EVENTS +} from '../constants/environment' import findNode from '../utils/find-node' /** @@ -44,7 +49,9 @@ function BeforePlugin() { // since it provides more useful information about the range being affected // and also preserves compatibility with iOS autocorrect, which would be // broken if we called `preventDefault()` on React's synthetic event here. - if (SUPPORTED_EVENTS.beforeinput) return true + // Since native `onbeforeinput` mainly benefits autocorrect and spellcheck + // for mobile, on desktop it brings IME issue, limit its scope for now. + if ((IS_IOS || IS_ANDROID) && SUPPORTED_EVENTS.beforeinput) return true debug('onBeforeInput', { event }) }