Skip to content

Commit

Permalink
Fix: Update selected note when making changes in the editor view (#1880)
Browse files Browse the repository at this point in the history
* Fix: Update selected note when making changes in the editor view

Resolves #1879

It appears that we removed a flow in #1851 which updated app state in response
to updates from the note editor[1]. While this didn't impact the operation of
the editor it did present itself as a bug when previewing markdown-rendered
notes. The editor is stateful on its own and so continued to function
independently of the state of the selected note's content. The preview,
however, grabs the contents of the selected note whenever it's activated and
then renders those into a DOM node. This means that it showed the old contents
of the note unless you navigated to another note and back, which swapped out
the selected note with the new updated contents.

In this patch, which currently is probably wrong in some way, we're updating
the selected note when calling `onChangeContent` so that we don't get out of
sync with the data sources. Hopefully at the end of our state cleanup and
stabilization project this will disappear but for now this fix catches the
gap that left split data sources.

Probably there were other bugs we didn't discover as a result of this.

Mainly we need to test and audit for traces of cycles/loops in the update
so that we don't start spinning or overwriting data we just entered. In my
brief testing the easy cases worked as I expected them to simply and singly.

[1]: https://github.com/Automattic/simplenote-electron/pull/1851/files#diff-3c87750a7cc32e0127447ffc78f5c0d5L300

* Handle tag updates too by always re-selecting note on updates from the server

When tags were being updated in the pap they also came through this function
but they weren't coming through `onUpdateContent`, meaning that our fix
overlooked tags and probably also system tags and the like.

In this patch we're no longer selectively updating the note if it has a remote
patch (indicating that it came from another device). Instead, we're always
updating and selectively setting `hasRemoteUpdate` based on that knowledge.
This ensures that we maintain consistency with local updates.
  • Loading branch information
dmsnell authored Feb 6, 2020
1 parent 4b915b1 commit 9c14685
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 6 deletions.
21 changes: 17 additions & 4 deletions lib/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ const mapDispatchToProps: S.MapDispatch<

openTagList: () => dispatch(actionCreators.toggleNavigation()),
resetAuth: () => dispatch(reduxActions.auth.reset()),
selectNote: (note: T.NoteEntity) => dispatch(actions.ui.selectNote(note)),
setAuthorized: () => dispatch(reduxActions.auth.setAuthorized()),
setSearchFocus: () =>
dispatch(actionCreators.setSearchFocus({ searchFocus: true })),
Expand Down Expand Up @@ -316,15 +317,25 @@ export const App = connect(

onNoteRemoved = () => this.onNotesIndex();

onNoteUpdate = (noteId, data, remoteUpdateInfo = {}) => {
onNoteUpdate = (
noteId: T.EntityId,
data,
remoteUpdateInfo: { patch?: object } = {}
) => {
const {
noteBucket,
selectNote,
ui: { note },
} = this.props;
if (remoteUpdateInfo.patch && note && noteId === note.id) {
noteBucket.get(noteId, (e, updatedNote) => {
selectNote({ ...updatedNote, hasRemoteUpdate: true });
if (note && noteId === note.id) {
noteBucket.get(noteId, (e: unknown, storedNote: T.NoteEntity) => {
if (e) {
return;
}
const updatedNote = remoteUpdateInfo.patch
? { ...storedNote, hasRemoteUpdate: true }
: storedNote;
selectNote(updatedNote);
});
}
};
Expand Down Expand Up @@ -368,6 +379,8 @@ export const App = connect(
},
};

this.props.selectNote(updatedNote);

const { noteBucket } = this.props;
noteBucket.update(note.id, updatedNote.data, {}, { sync });
if (sync) {
Expand Down
3 changes: 2 additions & 1 deletion lib/note-detail/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,10 @@ export class NoteDetail extends Component {
}
}

const mapStateToProps = ({ appState: state, settings }) => ({
const mapStateToProps = ({ appState: state, ui, settings }) => ({
dialogs: state.dialogs,
filter: state.filter,
note: state.revision || ui.note,
shouldPrint: state.shouldPrint,
showNoteInfo: state.showNoteInfo,
spellCheckEnabled: settings.spellCheckEnabled,
Expand Down
1 change: 0 additions & 1 deletion lib/note-editor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ export class NoteEditor extends Component<Props> {
storeFocusEditor={this.storeFocusEditor}
storeHasFocus={this.storeEditorHasFocus}
filter={this.props.filter}
note={revision}
noteBucket={noteBucket}
previewingMarkdown={
this.markdownEnabled() && editorMode === 'markdown'
Expand Down

0 comments on commit 9c14685

Please sign in to comment.