Skip to content

Commit

Permalink
Fix: Update selected note when making changes in the editor view
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dmsnell committed Feb 4, 2020
1 parent 4b915b1 commit c7a331b
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 2 deletions.
3 changes: 3 additions & 0 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 @@ -368,6 +369,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 c7a331b

Please sign in to comment.