Skip to content
This repository was archived by the owner on Feb 6, 2023. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
83730c9
Much saner experience, but still with known issues that Im taking notes.
fabiomcosta Mar 2, 2019
939f935
Merge branch 'master' of https://github.com/facebook/draft-js into at…
fabiomcosta Mar 8, 2019
c831fcf
Improving code a bit
fabiomcosta Mar 8, 2019
1d55ab4
minor cleanup
fabiomcosta Mar 8, 2019
7322ff4
fixing flow issues
fabiomcosta Mar 8, 2019
7e67808
revert header changes
fabiomcosta Mar 8, 2019
314e3db
keeping draft_handlebeforeinput_composed_text experiment
fabiomcosta Mar 8, 2019
65120e8
Removing code added by accident
fabiomcosta Mar 8, 2019
8c3e6ab
Merge branch 'master' of https://github.com/facebook/draft-js into at…
fabiomcosta Mar 12, 2019
2f7ac3f
fixing more issues realted to composition, but I finaly noticed this …
fabiomcosta Mar 13, 2019
1800a0e
Detecting DOM changes using a mutation observer on composition events
fabiomcosta Mar 14, 2019
49ddeb5
removing console.logs and removing unecessary compositionUpdate handler
fabiomcosta Mar 15, 2019
b10bcc7
improving comments around the new onInput changes
fabiomcosta Mar 15, 2019
2be6cfb
fixing flow and lint errors and warnings
fabiomcosta Mar 16, 2019
2fbe986
v0.10.6-beta.0
fabiomcosta Mar 16, 2019
fbfb983
adding unit tests for getContentEditableContainer
fabiomcosta Mar 16, 2019
4b4c028
const editorState
fabiomcosta Mar 16, 2019
9ff84ce
better info on keeping the selection state.
fabiomcosta Mar 16, 2019
586ce9e
Fixes an issue that happens when the target textContent is an empty s…
fabiomcosta Mar 17, 2019
a560b8b
v0.10.6-beta.1
fabiomcosta Mar 17, 2019
9e0ac50
Making sure multiple mutations are properly applied to the editorStat…
fabiomcosta Mar 18, 2019
d3a3f98
v0.10.6-beta.2
fabiomcosta Mar 18, 2019
96d810d
reverting package name changes
fabiomcosta Mar 18, 2019
2bec631
adding unit tests for the editOnInput changes
fabiomcosta Mar 19, 2019
e4fdd89
cleanup unit tests and add "editor.update" check
fabiomcosta Mar 19, 2019
c11bce3
Adding unit tests for DraftEditorCompositionHandler. It is still miss…
fabiomcosta Mar 19, 2019
121997f
removing lines of code I was using just for testing and some unused r…
fabiomcosta Mar 19, 2019
2c95809
This change became unecessary after the new way of handling compositi…
fabiomcosta Mar 20, 2019
997edf7
Fixes style application issue on selection
fabiomcosta Mar 22, 2019
0c4bf1b
Merge branch 'master' of https://github.com/facebook/draft-js into at…
fabiomcosta Mar 22, 2019
6219a3f
Revert incorrect changes to the example file
fabiomcosta Mar 22, 2019
eef794e
Merge branch 'master' of https://github.com/facebook/draft-js into at…
fabiomcosta May 1, 2019
5a015a7
Improving invariant message
fabiomcosta May 1, 2019
5c70301
Merge branch 'master' of https://github.com/facebook/draft-js into at…
fabiomcosta May 7, 2019
2927e4f
updates based on CR
fabiomcosta May 7, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion meta/bundle-size-stats/Draft.js.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion meta/bundle-size-stats/Draft.min.js.json

Large diffs are not rendered by default.

117 changes: 117 additions & 0 deletions src/component/handlers/composition/DOMObserver.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/**
* 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 Immutable = require('immutable');

const findAncestorOffsetKey = require('findAncestorOffsetKey');
const invariant = require('invariant');
const nullthrows = require('nullthrows');

const {OrderedMap} = Immutable;

type MutationRecordT = MutationRecord | {|type: 'characterData', target: Node|};

// 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,
characterDataOldValue: true,
childList: true,
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please initialize this property or make it nullable changing the type to ?MutationObserver.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, good call.

container: HTMLElement;
mutations: OrderedMap<string, string>;
onCharData: ({target: EventTarget, type: string}) => void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be initialized, or make it nullable.


constructor(container: HTMLElement) {
this.container = container;
this.mutations = new OrderedMap();
if (window.MutationObserver && !USE_CHAR_DATA) {
this.observer = new window.MutationObserver(mutations =>
this.registerMutations(mutations),
);
} else {
this.onCharData = e => {
invariant(e.target instanceof Node);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should there be a second argument for invariant here? perhaps something like "expected target to be an instance of Node"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

this.registerMutation({
type: 'characterData',
target: e.target,
});
};
}
}

start() {
if (this.observer) {
this.observer.observe(this.container, DOM_OBSERVER_OPTIONS);
} else {
this.container.addEventListener(
'DOMCharacterDataModified',
this.onCharData,
);
}
}

stopAndFlushMutations() {
if (this.observer) {
this.registerMutations(this.observer.takeRecords());
this.observer.disconnect();
} else {
this.container.removeEventListener(
'DOMCharacterDataModified',
this.onCharData,
);
}
const mutations = this.mutations;
this.mutations = new OrderedMap();
return mutations;
}

registerMutations(mutations: Array<MutationRecord>) {
for (let i = 0; i < mutations.length; i++) {
this.registerMutation(mutations[i]);
}
}

getMutationTextContent({target, type, removedNodes}: MutationRecord) {
if (type === 'characterData') {
return target.textContent;
}
// `characterData` events won't happen 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 (type === 'childList' && removedNodes.length) {
return '';
}
return null;
}

registerMutation(mutation: MutationRecordT) {
const textContent = this.getMutationTextContent(mutation);
if (textContent != null) {
const offsetKey = nullthrows(findAncestorOffsetKey(mutation.target));
this.mutations = this.mutations.set(offsetKey, textContent);
}
}
}

module.exports = DOMObserver;
162 changes: 100 additions & 62 deletions src/component/handlers/composition/DraftEditorCompositionHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ import type DraftEditor from 'DraftEditor.react';
const DraftModifier = require('DraftModifier');
const EditorState = require('EditorState');
const Keys = require('Keys');
const DraftOffsetKey = require('DraftOffsetKey');
const DOMObserver = require('DOMObserver');

const nullthrows = require('nullthrows');
const getEntityKeyForSelection = require('getEntityKeyForSelection');
const gkx = require('gkx');
const isEventHandled = require('isEventHandled');
const isSelectionAtLeafStart = require('isSelectionAtLeafStart');
const getDraftEditorSelection = require('getDraftEditorSelection');
const getContentEditableContainer = require('getContentEditableContainer');

/**
* Millisecond delay to allow `compositionstart` to fire again upon
Expand All @@ -42,19 +44,23 @@ const RESOLVE_DELAY = 20;
*/
let resolved = false;
let stillComposing = false;
let textInputData = '';
let domObserver = null;

const DraftEditorCompositionHandler = {
onBeforeInput: function(editor: DraftEditor, e: SyntheticInputEvent<>): void {
textInputData = (textInputData || '') + e.data;
},
function startDOMObserver(editor: DraftEditor) {
if (!domObserver) {
domObserver = new DOMObserver(getContentEditableContainer(editor));
domObserver.start();
}
}

var DraftEditorCompositionHandler = {
/**
* A `compositionstart` event has fired while we're still in composition
* mode. Continue the current composition session to prevent a re-render.
*/
onCompositionStart: function(editor: DraftEditor): void {
stillComposing = true;
startDOMObserver(editor);
},

/**
Expand All @@ -71,7 +77,10 @@ const DraftEditorCompositionHandler = {
* twice could break the DOM, we only use the first event. Example: Arabic
* Google Input Tools on Windows 8.1 fires `compositionend` three times.
*/
onCompositionEnd: function(editor: DraftEditor, e: SyntheticEvent<>): void {
onCompositionEnd: function(
editor: DraftEditor,
e: SyntheticCompositionEvent<>,
): void {
resolved = false;
stillComposing = false;
setTimeout(() => {
Expand Down Expand Up @@ -136,69 +145,98 @@ const DraftEditorCompositionHandler = {
return;
}

const mutations = nullthrows(domObserver).stopAndFlushMutations();
domObserver = null;
resolved = true;
const composedChars = textInputData;
textInputData = '';

const editorState = EditorState.set(editor._latestEditorState, {
let editorState = EditorState.set(editor._latestEditorState, {
inCompositionMode: false,
});

const currentStyle = editorState.getCurrentInlineStyle();
const entityKey = getEntityKeyForSelection(
editorState.getCurrentContent(),
editorState.getSelection(),
);

const mustReset =
!composedChars ||
isSelectionAtLeafStart(editorState) ||
currentStyle.size > 0 ||
entityKey !== null;

if (mustReset) {
editor.restoreEditorDOM();
}

editor.exitCurrentMode();

if (composedChars) {
if (
gkx('draft_handlebeforeinput_composed_text') &&
editor.props.handleBeforeInput &&
isEventHandled(
editor.props.handleBeforeInput(
composedChars,
editorState,
event.timeStamp,
),
)
) {
return;
}
// If characters have been composed, re-rendering with the update
// is sufficient to reset the editor.
const contentState = DraftModifier.replaceText(
editorState.getCurrentContent(),
editorState.getSelection(),
composedChars,
currentStyle,
entityKey,
);
editor.update(
EditorState.push(editorState, contentState, 'insert-characters'),
);
if (!mutations.size) {
return;
}

if (mustReset) {
editor.update(
EditorState.set(editorState, {
nativelyRenderedContent: null,
forceSelection: true,
}),
);
}
// Save current selection before restoring the DOM, so we can, so
// we can re-apply on compositionEnd.
const documentSelection = getDraftEditorSelection(
editorState,
getContentEditableContainer(editor),
);
const initialSelectionState = documentSelection.selectionState;

editor.restoreEditorDOM();

// TODO, check if Facebook still needs this flag or if it could be removed.
// Since there can be multiple mutations providing a `composedChars` doesn't
// apply well on this new model.
Copy link
Contributor Author

@fabiomcosta fabiomcosta Mar 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claudiopro would you know if we can remove this code or do we need to keep it somehow working?
If we keep it working it will be buggy, because we are now applying multiple mutations (which leads to multiple composedChars) on compositionEnd, which is not compatible with the arguments that we provide to handleBeforeInput here.

If draft_handlebeforeinput_composed_text is still being used at FB, I'd suggest to provide the composedChars from the first mutation, completely ignoring the other mutations, which should be fine for the common case. My main suggestion is still to stop depending on this code/gkflag.

// if (
// gkx('draft_handlebeforeinput_composed_text') &&
// editor.props.handleBeforeInput &&
// isEventHandled(
// editor.props.handleBeforeInput(
// composedChars,
// editorState,
// event.timeStamp,
// ),
// )
// ) {
// return;
// }

const contentState = mutations.reduce(
(contentState, composedChars, offsetKey) => {
const {blockKey, decoratorKey, leafKey} = DraftOffsetKey.decode(
offsetKey,
);

const {start, end} = editorState
.getBlockTree(blockKey)
.getIn([decoratorKey, 'leaves', leafKey]);

const replacementRange = editorState.getSelection().merge({
anchorKey: blockKey,
focusKey: blockKey,
anchorOffset: start,
focusOffset: end,
isBackward: false,
});

const entityKey = getEntityKeyForSelection(
contentState,
replacementRange,
);
const currentStyle = contentState
.getBlockForKey(blockKey)
.getInlineStyleAt(start);

// If characters have been composed, re-rendering with the update
// is sufficient to reset the editor.
return DraftModifier.replaceText(
contentState,
replacementRange,
composedChars,
currentStyle,
entityKey,
);
},
editorState.getCurrentContent(),
);

const contentWithAdjustedSelection = contentState.merge({
selectionBefore: contentState.getSelectionAfter(),
selectionAfter: initialSelectionState,
});

editor.update(
EditorState.push(
editorState,
contentWithAdjustedSelection,
'insert-characters',
),
);
},
};

Expand Down
23 changes: 22 additions & 1 deletion src/component/handlers/edit/editOnInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,20 @@ const UserAgent = require('UserAgent');
const findAncestorOffsetKey = require('findAncestorOffsetKey');
const gkx = require('gkx');
const nullthrows = require('nullthrows');
const keyCommandPlainBackspace = require('keyCommandPlainBackspace');

const isGecko = UserAgent.isEngine('Gecko');

const DOUBLE_NEWLINE = '\n\n';

function onInputType(inputType: string, editorState: EditorState): EditorState {
switch (inputType) {
case 'deleteContentBackward':
return keyCommandPlainBackspace(editorState);
}
return editorState;
}

/**
* This function is intended to handle spellcheck and autocorrect changes,
* which occur in the DOM natively without any opportunity to observe or
Expand All @@ -38,7 +47,7 @@ const DOUBLE_NEWLINE = '\n\n';
* when an `input` change leads to a DOM/model mismatch, the change should be
* due to a spellcheck change, and we can incorporate it into our model.
*/
function editOnInput(editor: DraftEditor): void {
function editOnInput(editor: DraftEditor, e: SyntheticInputEvent<>): void {
if (editor._pendingStateFromBeforeInput !== undefined) {
editor.update(editor._pendingStateFromBeforeInput);
editor._pendingStateFromBeforeInput = undefined;
Expand Down Expand Up @@ -111,6 +120,18 @@ function editOnInput(editor: DraftEditor): void {
// standard onkeydown/pressed events and only fired editOnInput
// so domText is already changed by the browser and ends up being equal
// to modelText unexpectedly

/* $FlowFixMe inputType is only defined on a draft of a standard.
* https://w3c.github.io/input-events/#dom-inputevent-inputtype */
const {inputType} = e.nativeEvent;
// TODO add check for "input events level 2" https://w3c.github.io/input-events/#dom-inputevent-inputtype
if (inputType) {
let newEditorState = onInputType(inputType, editorState);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is never reassigned, please declare it as const.

if (newEditorState !== editorState) {
editor.restoreEditorDOM();
return void editor.update(newEditorState);
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is necessary because keyDown on draft-js edit mode on Android doesn't come with the correct keyCode, so, for example, when users press backspace we are not actually executing keyCommandPlainBackspace, which leads to an incorrect content state.

The input event comes with this inputType which lets us execute the correct action.
I still need to make sure older versions of Android also support this property (will do soon).

return;
}

Expand Down
Loading