Skip to content

Commit

Permalink
Fix: Defer re-decorating note when changing search (#2073)
Browse files Browse the repository at this point in the history
* Fix: Defer re-decorating note when changing search

See #1941
See #1982

When we rebuilt the search to return instant results and removed the debounce
on the search filter we exposed an issue with note rendering performance that
ironically made the new instant search slower than the old one in certain
circumstances, namely when a note in the search results takes a very long time
to render.

The leading reason for the performance issue is that `draft-js` was applying
a new decorator to its note on every change to the search field. With the
search field updating instantly that left no time for the decorators to redraw
(and they were very inefficient to make it worse).

In this patch we're delaying the re-decoration until the search field settles.
This doesn't eliminate the problem but it should bring it roughly on par with
the behavior from before the search updates. Further we have eliminated the
`MultiDecorator` dependency since that functionality is provided by `draft-js`
itself. Since there's no need to create a composite decorator when the search
query doesn't contain any text terms we can futher skip it and only decorate
with the checkbox decorator.

These changes should make searching tolerant to slow notes and should
additionally cut the time it takes to decorate notes approximately in half.

* Don't remove multidecorator - leave that for later

* Also don't update decorators when switching notes
  • Loading branch information
dmsnell authored May 12, 2020
1 parent c19ce2e commit 3308f20
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 23 deletions.
64 changes: 43 additions & 21 deletions lib/note-content-editor.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { ContentState, Editor, EditorState, Modifier } from 'draft-js';
import MultiDecorator from 'draft-js-multidecorators';
import { compact, get, has, invoke, noop } from 'lodash';
import { ContentState, Editor, EditorState, Modifier } from 'draft-js';
import { debounce, get, has, invoke, noop } from 'lodash';

import {
getCurrentBlock,
Expand Down Expand Up @@ -79,19 +79,21 @@ class NoteContentEditor extends Component<Props> {
};

generateDecorators = (searchQuery: string) => {
return new MultiDecorator(
compact([
filterHasText(searchQuery) &&
matchingTextDecorator(searchPattern(searchQuery)),
const queryHasTerms = filterHasText(searchQuery);

if (queryHasTerms) {
return new MultiDecorator([
matchingTextDecorator(searchPattern(searchQuery)),
checkboxDecorator(this.replaceRangeWithText),
])
);
]);
}

return checkboxDecorator(this.replaceRangeWithText);
};

createNewEditorState = (text: string, searchQuery: string) => {
const newEditorState = EditorState.createWithContent(
ContentState.createFromText(text, TEXT_DELIMITER),
this.generateDecorators(searchQuery)
ContentState.createFromText(text, TEXT_DELIMITER)
);

// Focus the editor for a new, empty note when not searching
Expand Down Expand Up @@ -196,23 +198,27 @@ class NoteContentEditor extends Component<Props> {
{
editorState: this.createNewEditorState(content.text, searchQuery),
},
() =>
__TEST__ &&
window.testEvents.push([
'editorNewNote',
plainTextContent(this.state.editorState),
])
() => {
this.queueDecoratorUpdate();

if (content.text.length < 10000) {
this.queueDecoratorUpdate.flush();
}

if (__TEST__) {
window.testEvents.push([
'editorNewNote',
plainTextContent(this.state.editorState),
]);
}
}
);
return;
}

// If searchQuery changes, re-set decorators
if (searchQuery !== prevProps.searchQuery) {
this.setState({
editorState: EditorState.set(editorState, {
decorator: this.generateDecorators(searchQuery),
}),
});
this.queueDecoratorUpdate();
}

// If a remote change comes in, push it to the existing editor state.
Expand All @@ -221,6 +227,22 @@ class NoteContentEditor extends Component<Props> {
}
}

queueDecoratorUpdate = debounce(() => {
const { searchQuery } = this.props;
const { editorState } = this.state;

if (this.props.noteId === null) {
// oops, we unselected a note - don't recompute
return;
}

this.setState({
editorState: EditorState.set(editorState, {
decorator: this.generateDecorators(searchQuery),
}),
});
}, 500);

saveEditorRef = (ref) => {
this.editor = ref;
};
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"email": "support@simplenote.com"
},
"productName": "Simplenote",
"version": "1.16.0-2066",
"version": "1.16.0-2073",
"main": "desktop/index.js",
"license": "GPL-2.0",
"homepage": "https://simplenote.com",
Expand Down

0 comments on commit 3308f20

Please sign in to comment.