From 19cd37ba4350e519d8917f31df34a7b2a76f98b4 Mon Sep 17 00:00:00 2001 From: "Jonathan (JB) Belcher" Date: Tue, 21 Jan 2020 14:26:01 -0500 Subject: [PATCH 1/5] Refactor selected note state from flux to redux This PR moves the selected note state from flux to redux. It also eliminates the need for selectedNoteId in appState. --- RELEASE-NOTES.txt | 4 +- babel.config.js | 1 + lib/app-layout/index.tsx | 9 +---- lib/app.tsx | 17 +++++---- lib/dialogs/share/index.tsx | 24 ++++++------ lib/flux/app-state.ts | 54 +-------------------------- lib/flux/test.ts | 66 --------------------------------- lib/note-editor/index.tsx | 5 ++- lib/note-info/index.tsx | 4 +- lib/note-list/index.tsx | 13 ++----- lib/note-toolbar/index.tsx | 7 +++- lib/revision-selector/index.tsx | 3 +- lib/state/index.ts | 1 - lib/state/ui/actions.ts | 5 +++ lib/state/ui/reducer.ts | 33 ++++++++++++++++- lib/types.ts | 2 +- package-lock.json | 16 -------- 17 files changed, 79 insertions(+), 185 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 43797090a..130043a14 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -14,10 +14,8 @@ ### Other Changes - Added types to state/ui/actions [#1849](https://github.com/Automattic/simplenote-electron/pull/1849) - -### Other Changes - - Updated Dependencies [#1848](https://github.com/Automattic/simplenote-electron/pull/1848) +- Refactored selected note state [1851](https://github.com/Automattic/simplenote-electron/pull/1851) ## [v1.14.0] diff --git a/babel.config.js b/babel.config.js index 1e0984cf1..a11f53d45 100644 --- a/babel.config.js +++ b/babel.config.js @@ -15,6 +15,7 @@ module.exports = function(api) { const plugins = [ '@babel/plugin-proposal-class-properties', '@babel/plugin-proposal-object-rest-spread', + '@babel/plugin-proposal-optional-chaining', '@babel/plugin-syntax-dynamic-import', ]; const env = { diff --git a/lib/app-layout/index.tsx b/lib/app-layout/index.tsx index 1327636bf..0982476b0 100644 --- a/lib/app-layout/index.tsx +++ b/lib/app-layout/index.tsx @@ -1,6 +1,5 @@ import React, { FunctionComponent, Suspense } from 'react'; import classNames from 'classnames'; -import { get } from 'lodash'; import NoteToolbarContainer from '../note-toolbar-container'; import NoteToolbar from '../note-toolbar'; @@ -40,7 +39,6 @@ export const AppLayout: FunctionComponent = ({ isNoteInfoOpen, isNoteOpen, isSmallScreen, - note, noteBucket, revisions, onNoteClosed, @@ -76,25 +74,20 @@ export const AppLayout: FunctionComponent = ({
} + toolbar={} />
diff --git a/lib/app.tsx b/lib/app.tsx index 8f11629b7..58b7f81a1 100644 --- a/lib/app.tsx +++ b/lib/app.tsx @@ -6,6 +6,7 @@ import 'focus-visible/dist/focus-visible.js'; import appState from './flux/app-state'; import { loadTags } from './state/domain/tags'; import reduxActions from './state/actions'; +import { setSelectedNote } from './state/ui/actions'; import selectors from './state/selectors'; import browserShell from './browser-shell'; import NoteInfo from './note-info'; @@ -85,6 +86,7 @@ function mapDispatchToProps(dispatch, { noteBucket }) { dispatch(actionCreators.setSearchFocus({ searchFocus: true })), setSimperiumConnectionStatus: connected => dispatch(toggleSimperiumConnectionStatus(connected)), + setSelectedNote: id => dispatch(setSelectedNote(id)), }; } @@ -296,12 +298,14 @@ export const App = connect( onNoteRemoved = () => this.onNotesIndex(); onNoteUpdate = (noteId, data, remoteUpdateInfo = {}) => { - if (remoteUpdateInfo.patch) { - this.props.actions.noteUpdatedRemotely({ - noteBucket: this.props.noteBucket, - noteId, - data, - remoteUpdateInfo, + const { + noteBucket, + setSelectedNote, + ui: { note }, + } = this.props; + if (remoteUpdateInfo.patch && note && noteId === note.id) { + noteBucket.get(noteId, (e, updatedNote) => { + setSelectedNote({ ...updatedNote, hasRemoteUpdate: true }); }); } }; @@ -439,7 +443,6 @@ export const App = connect( isNoteOpen={this.state.isNoteOpen} isNoteInfoOpen={state.showNoteInfo} isSmallScreen={isSmallScreen} - note={state.note} noteBucket={noteBucket} revisions={state.revisions} onNoteClosed={() => this.setState({ isNoteOpen: false })} diff --git a/lib/dialogs/share/index.tsx b/lib/dialogs/share/index.tsx index 603903bcd..18e8771d7 100644 --- a/lib/dialogs/share/index.tsx +++ b/lib/dialogs/share/index.tsx @@ -30,7 +30,7 @@ export class ShareDialog extends Component { onTogglePublished = event => { this.props.actions.publishNote({ noteBucket: this.props.noteBucket, - note: this.props.appState.note, + note: this.props.note, publish: event.currentTarget.checked, }); }; @@ -38,7 +38,7 @@ export class ShareDialog extends Component { getPublishURL = url => (isEmpty(url) ? undefined : `http://simp.ly/p/${url}`); onAddCollaborator = event => { - const { note } = this.props.appState; + const { note } = this.props; const tags = (note.data && note.data.tags) || []; const collaborator = this.collaboratorElement.value.trim(); @@ -57,7 +57,7 @@ export class ShareDialog extends Component { }; onRemoveCollaborator = collaborator => { - const { note } = this.props.appState; + const { note } = this.props; let tags = (note.data && note.data.tags) || []; tags = tags.filter(tag => tag !== collaborator); @@ -67,7 +67,7 @@ export class ShareDialog extends Component { }; collaborators = () => { - const { note } = this.props.appState; + const { note } = this.props; const tags = (note.data && note.data.tags) || []; const collaborators = tags.filter(isEmailTag); @@ -86,9 +86,7 @@ export class ShareDialog extends Component { }; render() { - const { dialog, requestClose } = this.props; - const { note } = this.props.appState; - + const { dialog, note, requestClose } = this.props; const data = (note && note.data) || {}; const isPublished = includes(data.systemTags, 'published'); const publishURL = this.getPublishURL(data.publishURL); @@ -207,9 +205,9 @@ export class ShareDialog extends Component { } } -export default connect( - state => ({ - settings: state.settings, - }), - { updateNoteTags } -)(ShareDialog); +const mapStateToProps = ({ settings, ui: { note } }) => ({ + settings, + note, +}); + +export default connect(mapStateToProps, { updateNoteTags })(ShareDialog); diff --git a/lib/flux/app-state.ts b/lib/flux/app-state.ts index 97653b19f..2a6225823 100644 --- a/lib/flux/app-state.ts +++ b/lib/flux/app-state.ts @@ -2,7 +2,6 @@ import { get, partition, some } from 'lodash'; import update from 'react-addons-update'; import Debug from 'debug'; import ActionMap from './action-map'; -import filterNotes from '../utils/filter-notes'; import analytics from '../analytics'; import { AppState, State } from '../state'; @@ -36,7 +35,6 @@ const toggleSystemTag = ( const initialState: AppState = { editorMode: 'edit', filter: '', - selectedNoteId: null, previousIndex: -1, notes: null, tags: [], @@ -100,8 +98,6 @@ export const actionMap = new ActionMap({ showTrash: { $set: false }, listTitle: { $set: 'All Notes' }, tag: { $set: null }, - note: { $set: null }, - selectedNoteId: { $set: null }, previousIndex: { $set: -1 }, }); }, @@ -113,8 +109,6 @@ export const actionMap = new ActionMap({ showTrash: { $set: true }, listTitle: { $set: 'Trash' }, tag: { $set: null }, - note: { $set: null }, - selectedNoteId: { $set: null }, previousIndex: { $set: -1 }, }); }, @@ -139,8 +133,6 @@ export const actionMap = new ActionMap({ showTrash: { $set: false }, listTitle: { $set: tag.data.name }, tag: { $set: tag }, - note: { $set: null }, - selectedNoteId: { $set: null }, previousIndex: { $set: -1 }, }); }, @@ -297,25 +289,8 @@ export const actionMap = new ActionMap({ note.data.systemTags.includes('pinned') ); const pinSortedNotes = [...pinned, ...notPinned]; - - let selectedNote = null; - - if (state.note) { - // Load the newest version of the selected note - selectedNote = notes.find(note => note.id === state.note.id); - } else { - // If no note is selected, select either the first note - // or the previous note in the list - const filteredNotes = filterNotes(state, pinSortedNotes); - if (filteredNotes.length > 0) { - selectedNote = filteredNotes[Math.max(state.previousIndex, 0)]; - } - } - return update(state, { notes: { $set: pinSortedNotes }, - note: { $set: selectedNote }, - selectedNoteId: { $set: get(selectedNote, 'id', null) }, }); }, @@ -395,11 +370,9 @@ export const actionMap = new ActionMap({ }, }, - selectNote(state: AppState, { note, hasRemoteUpdate }) { + selectNote(state: AppState) { return update(state, { editingTags: { $set: false }, - note: { $set: { ...note, hasRemoteUpdate } }, - selectedNoteId: { $set: note.id }, revision: { $set: null }, revisions: { $set: null }, }); @@ -407,35 +380,10 @@ export const actionMap = new ActionMap({ closeNote(state: AppState, { previousIndex = -1 }) { return update(state, { - note: { $set: null }, - selectedNoteId: { $set: null }, previousIndex: { $set: previousIndex }, }); }, - noteUpdatedRemotely: { - creator({ noteBucket, noteId, data, remoteUpdateInfo = {} }) { - return (dispatch, getState: () => State) => { - const state = getState().appState; - const { patch } = remoteUpdateInfo; - - debug('noteUpdatedRemotely: %O', data); - - if (state.selectedNoteId !== noteId || !patch) { - return; - } - - dispatch( - this.action('loadAndSelectNote', { - noteBucket, - noteId, - hasRemoteUpdate: true, - }) - ); - }; - }, - }, - /** * A note is being changed from somewhere else! If the same * note is also open and being edited, we need to make sure diff --git a/lib/flux/test.ts b/lib/flux/test.ts index 9203526a9..a595cd91a 100644 --- a/lib/flux/test.ts +++ b/lib/flux/test.ts @@ -42,69 +42,3 @@ describe('appState action creators', () => { }); }); }); - -describe('appState action reducers', () => { - describe('notesLoaded', () => { - const notesLoaded = appState.actionReducers['App.notesLoaded']; - - describe('when a note is currently selected', () => { - it('should load the newest version of the selected note', () => { - const oldState = { - notes: [{}], - note: { id: 'foo', data: { content: 'old', systemTags: [] } }, - }; - const newNote = { id: 'foo', data: { content: 'new', systemTags: [] } }; - const newNoteArray = [newNote]; - const newState = notesLoaded(oldState, { - notes: newNoteArray, - }); - expect(newState.notes).toEqual(newNoteArray); - expect(newState.note).toEqual(newNote); - }); - }); - - describe('when no note is currently selected', () => { - it('should load the first filtered note if there is no valid previousIndex', () => { - const oldState = { - notes: [], - note: null, - previousIndex: -1, - }; - const firstNote = { - id: 'foo', - data: { content: 'first', systemTags: [] }, - }; - const newNoteArray = [ - firstNote, - { id: 'bar', data: { content: 'boo', systemTags: [] } }, - ]; - const newState = notesLoaded(oldState, { - notes: newNoteArray, - }); - expect(newState.notes).toEqual(newNoteArray); - expect(newState.note).toEqual(firstNote); - }); - - it('should load the previousIndex note if there is a previousIndex', () => { - const oldState = { - notes: [], - note: null, - previousIndex: 1, - }; - const previousIndexNote = { - id: 'foo', - data: { content: 'previous', systemTags: [] }, - }; - const newNoteArray = [ - { id: 'bar', data: { content: 'boo', systemTags: [] } }, - previousIndexNote, - ]; - const newState = notesLoaded(oldState, { - notes: newNoteArray, - }); - expect(newState.notes).toEqual(newNoteArray); - expect(newState.note).toEqual(previousIndexNote); - }); - }); - }); -}); diff --git a/lib/note-editor/index.tsx b/lib/note-editor/index.tsx index a1579a578..0fd66f46d 100644 --- a/lib/note-editor/index.tsx +++ b/lib/note-editor/index.tsx @@ -1,6 +1,7 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; import { connect } from 'react-redux'; +import { get } from 'lodash'; import appState from '../flux/app-state'; import TagField from '../tag-field'; import { property } from 'lodash'; @@ -161,13 +162,15 @@ export class NoteEditor extends Component { } } -const mapStateToProps = ({ appState: state, settings }) => ({ +const mapStateToProps = ({ appState: state, settings, ui: { note } }) => ({ allTags: state.tags, filter: state.filter, fontSize: settings.fontSize, editorMode: state.editorMode, isEditorActive: !state.showNavigation, + note, revision: state.revision, + tags: get(note, 'data.tags', []), }); const { closeNote, setEditorMode } = appState.actionCreators; diff --git a/lib/note-info/index.tsx b/lib/note-info/index.tsx index 263eded50..5f39cf52e 100644 --- a/lib/note-info/index.tsx +++ b/lib/note-info/index.tsx @@ -188,9 +188,7 @@ function characterCount(content) { const { markdownNote, pinNote, toggleNoteInfo } = appState.actionCreators; -const mapStateToProps = ({ appState: state, ui: { filteredNotes } }) => { - const noteIndex = Math.max(state.previousIndex, 0); - const note = state.note ? state.note : filteredNotes[noteIndex]; +const mapStateToProps = ({ ui: { note } }) => { return { note, isMarkdown: note.data.systemTags.includes('markdown'), diff --git a/lib/note-list/index.tsx b/lib/note-list/index.tsx index 581e98ab8..a9cf35e4a 100644 --- a/lib/note-list/index.tsx +++ b/lib/note-list/index.tsx @@ -348,11 +348,6 @@ export class NoteList extends Component { this.recomputeHeights(); } - // Ensure that the note selected here is also selected in the editor - if (selectedNoteId !== prevProps.selectedNoteId) { - onSelectNote(selectedNoteId); - } - // Deselect the currently selected note if it doesn't match the search query if (filter !== prevProps.filter) { const selectedNotePassesFilter = notes.some( @@ -496,14 +491,12 @@ const { recordEvent } = tracks; const mapStateToProps = ({ appState: state, - ui: { filteredNotes }, + ui: { filteredNotes, note }, settings: { noteDisplay }, }) => { const tagResultsFound = getMatchingTags(state.tags, state.filter).length; - - const noteIndex = Math.max(state.previousIndex, 0); - const selectedNote = state.note ? state.note : filteredNotes[noteIndex]; - const selectedNoteId = get(selectedNote, 'id', state.selectedNoteId); + const selectedNote = note; + const selectedNoteId = selectedNote?.id; const selectedNoteIndex = filteredNotes.findIndex( ({ id }) => id === selectedNoteId ); diff --git a/lib/note-toolbar/index.tsx b/lib/note-toolbar/index.tsx index 125d65a69..824ba1fcf 100644 --- a/lib/note-toolbar/index.tsx +++ b/lib/note-toolbar/index.tsx @@ -1,4 +1,5 @@ import React, { Component } from 'react'; +import { connect } from 'react-redux'; import PropTypes from 'prop-types'; import { noop } from 'lodash'; @@ -171,4 +172,8 @@ export class NoteToolbar extends Component { }; } -export default NoteToolbar; +const mapStateToProps = ({ ui: { note } }) => ({ + note, +}); + +export default connect(mapStateToProps)(NoteToolbar); diff --git a/lib/revision-selector/index.tsx b/lib/revision-selector/index.tsx index 857324bbd..32b205a5a 100644 --- a/lib/revision-selector/index.tsx +++ b/lib/revision-selector/index.tsx @@ -167,8 +167,9 @@ export class RevisionSelector extends Component { } } -const mapStateToProps = ({ appState: state }) => ({ +const mapStateToProps = ({ appState: state, ui: { note } }) => ({ isViewingRevisions: state.isViewingRevisions, + note: note, }); const { setRevision, setIsViewingRevisions } = appState.actionCreators; diff --git a/lib/state/index.ts b/lib/state/index.ts index 4c5ed5b09..2d370a224 100644 --- a/lib/state/index.ts +++ b/lib/state/index.ts @@ -40,7 +40,6 @@ export type AppState = { previousIndex: number; revision: T.NoteEntity | null; searchFocus: boolean; - selectedNoteId: T.EntityId | null; shouldPrint: boolean; showNavigation: boolean; showNoteInfo: boolean; diff --git a/lib/state/ui/actions.ts b/lib/state/ui/actions.ts index 62a30ac26..0c8fdb62b 100644 --- a/lib/state/ui/actions.ts +++ b/lib/state/ui/actions.ts @@ -15,6 +15,11 @@ export const toggleSimperiumConnectionStatus: A.ActionCreator ({ + type: 'SET_SELECTED_NOTE', + note, +}); + export const toggleTagDrawer: A.ActionCreator = ( show: boolean ) => ({ diff --git a/lib/state/ui/reducer.ts b/lib/state/ui/reducer.ts index 816956e9f..ab34425ad 100644 --- a/lib/state/ui/reducer.ts +++ b/lib/state/ui/reducer.ts @@ -1,6 +1,5 @@ import { difference, union } from 'lodash'; import { combineReducers } from 'redux'; - import * as A from '../action-types'; import * as T from '../../types'; @@ -30,8 +29,40 @@ const visiblePanes: A.Reducer = ( return state; }; +const note = (state = null, action) => { + switch (action.type) { + case 'App.notesLoaded': { + return state + ? action.notes.find(({ id }) => id === state.id) || state + : action.notes[0] || null; + } + case 'App.noteUpdatedRemotely': + case 'App.selectNote': { + return { + ...action.note, + hasRemoteUpdate: action.hasRemoteUpdate, + }; + } + case 'App.closeNote': + case 'App.showAllNotes': + case 'App.selectTrash': + case 'App.selectTag': + return null; + case 'SET_SELECTED_NOTE': + return action.note; + case 'FILTER_NOTES': + // keep note if still in new filtered list otherwise try to choose first note in list + return state && action.notes.some(({ id }) => id === state.id) + ? state + : action.notes[0] || null; + default: + return state; + } +}; + export default combineReducers({ filteredNotes, + note, simperiumConnected, visiblePanes, }); diff --git a/lib/types.ts b/lib/types.ts index b6bc68ac8..eed453da1 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -25,7 +25,7 @@ export type Note = { tags: TagName[]; }; -export type NoteEntity = Entity; +export type NoteEntity = Entity & { hasRemoteUpdate?: boolean }; export type Tag = { index?: number; diff --git a/package-lock.json b/package-lock.json index 90bba59ca..7e0557947 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1856,14 +1856,6 @@ "requires": { "@babel/helper-plugin-utils": "^7.8.3", "@babel/plugin-syntax-optional-catch-binding": "^7.8.0" - }, - "dependencies": { - "@babel/helper-plugin-utils": { - "version": "7.8.3", - "resolved": "https://registry.npmjs.org/@babel/helper-plugin-utils/-/helper-plugin-utils-7.8.3.tgz", - "integrity": "sha512-j+fq49Xds2smCUNYmEHF9kGNkhbet6yVIBp4e6oeQpH1RUs/Ir06xUKzDjDkGcaaokPiTNs2JBWHjaE4csUkZQ==", - "dev": true - } } }, "@babel/plugin-proposal-optional-chaining": { @@ -1969,14 +1961,6 @@ "dev": true, "requires": { "@babel/helper-plugin-utils": "^7.8.3" - }, - "dependencies": { - "@babel/helper-plugin-utils": { - "version": "7.8.3", - "resolved": "https://registry.npmjs.org/@babel/helper-plugin-utils/-/helper-plugin-utils-7.8.3.tgz", - "integrity": "sha512-j+fq49Xds2smCUNYmEHF9kGNkhbet6yVIBp4e6oeQpH1RUs/Ir06xUKzDjDkGcaaokPiTNs2JBWHjaE4csUkZQ==", - "dev": true - } } }, "@babel/plugin-syntax-nullish-coalescing-operator": { From fd766588ee5f9554d847e88e8f448d6a1e29e1a7 Mon Sep 17 00:00:00 2001 From: "Jonathan (JB) Belcher" Date: Mon, 27 Jan 2020 14:24:19 -0500 Subject: [PATCH 2/5] Add typing --- lib/state/action-types.ts | 5 +++++ lib/state/index.ts | 1 - lib/state/ui/actions.ts | 4 +++- lib/state/ui/reducer.ts | 7 +++++-- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/state/action-types.ts b/lib/state/action-types.ts index 649a18109..c5eab5988 100644 --- a/lib/state/action-types.ts +++ b/lib/state/action-types.ts @@ -58,6 +58,10 @@ export type ToggleSimperiumConnectionStatus = Action< { simperiumConnected: boolean } >; export type ToggleTagDrawer = Action<'TAG_DRAWER_TOGGLE', { show: boolean }>; +export type SetSelectedNote = Action< + 'SET_SELECTED_NOTE', + { note: T.NoteEntity | null } +>; export type ActionType = | FilterNotes @@ -69,6 +73,7 @@ export type ActionType = | SetLineLength | SetMarkdownEnabled | SetNoteDisplay + | SetSelectedNote | SetSortReversed | SetSortTagsAlpha | SetSortType diff --git a/lib/state/index.ts b/lib/state/index.ts index 2d370a224..dbb942311 100644 --- a/lib/state/index.ts +++ b/lib/state/index.ts @@ -34,7 +34,6 @@ export type AppState = { isViewingRevisions: boolean; listTitle: T.TranslatableString; nextDialogKey: number; - note?: T.NoteEntity; notes: T.NoteEntity[] | null; preferences?: T.Preferences; previousIndex: number; diff --git a/lib/state/ui/actions.ts b/lib/state/ui/actions.ts index 0c8fdb62b..e17993e8c 100644 --- a/lib/state/ui/actions.ts +++ b/lib/state/ui/actions.ts @@ -15,7 +15,9 @@ export const toggleSimperiumConnectionStatus: A.ActionCreator ({ +export const setSelectedNote: A.ActionCreator = ( + note: T.NoteEntity | null +) => ({ type: 'SET_SELECTED_NOTE', note, }); diff --git a/lib/state/ui/reducer.ts b/lib/state/ui/reducer.ts index ab34425ad..2ec72ac99 100644 --- a/lib/state/ui/reducer.ts +++ b/lib/state/ui/reducer.ts @@ -1,5 +1,5 @@ import { difference, union } from 'lodash'; -import { combineReducers } from 'redux'; +import { AnyAction, combineReducers } from 'redux'; import * as A from '../action-types'; import * as T from '../../types'; @@ -29,7 +29,10 @@ const visiblePanes: A.Reducer = ( return state; }; -const note = (state = null, action) => { +const note: A.Reducer = ( + state = null, + action: AnyAction +) => { switch (action.type) { case 'App.notesLoaded': { return state From 6724dd322bddcca0416070412191fa71012c458e Mon Sep 17 00:00:00 2001 From: "Jonathan (JB) Belcher" Date: Mon, 27 Jan 2020 15:34:40 -0500 Subject: [PATCH 3/5] Remove uneeded code --- lib/state/ui/reducer.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/state/ui/reducer.ts b/lib/state/ui/reducer.ts index 2ec72ac99..e614e78b3 100644 --- a/lib/state/ui/reducer.ts +++ b/lib/state/ui/reducer.ts @@ -33,12 +33,8 @@ const note: A.Reducer = ( state = null, action: AnyAction ) => { + console.log(state, action); switch (action.type) { - case 'App.notesLoaded': { - return state - ? action.notes.find(({ id }) => id === state.id) || state - : action.notes[0] || null; - } case 'App.noteUpdatedRemotely': case 'App.selectNote': { return { From 54596f0f66441c4efbc5c173a5dd062dd4d956b1 Mon Sep 17 00:00:00 2001 From: "Jonathan (JB) Belcher" Date: Mon, 27 Jan 2020 16:16:35 -0500 Subject: [PATCH 4/5] Remove accidenntal console.log --- lib/state/ui/reducer.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/state/ui/reducer.ts b/lib/state/ui/reducer.ts index e614e78b3..481ae99f7 100644 --- a/lib/state/ui/reducer.ts +++ b/lib/state/ui/reducer.ts @@ -33,7 +33,6 @@ const note: A.Reducer = ( state = null, action: AnyAction ) => { - console.log(state, action); switch (action.type) { case 'App.noteUpdatedRemotely': case 'App.selectNote': { From d14a6e9141ecbfc237edf5ec2c2abd50a5c2ad05 Mon Sep 17 00:00:00 2001 From: Dennis Snell Date: Wed, 29 Jan 2020 11:49:11 -0700 Subject: [PATCH 5/5] Updates from review feedback - Add type annotations where we're introducing new state - Remove references to `noteUpdatedRemotely` - Remove ability to `selectNote(null)` - Remove unused `tags` prop --- lib/app.tsx | 31 +++++++++++++++++++++++-------- lib/dialogs/share/index.tsx | 18 +++++++++++++++--- lib/note-editor/index.tsx | 20 +++++++++++++++----- lib/note-info/index.tsx | 28 +++++++++++++++++----------- lib/note-list/index.tsx | 14 +++++++++++--- lib/note-toolbar/index.tsx | 14 +++++++++++--- lib/revision-selector/index.tsx | 29 +++++++++++++++++++---------- lib/state/action-types.ts | 7 ++----- lib/state/ui/actions.ts | 9 +++------ lib/state/ui/middleware.ts | 1 - lib/state/ui/reducer.ts | 16 ++++------------ 11 files changed, 120 insertions(+), 67 deletions(-) diff --git a/lib/app.tsx b/lib/app.tsx index 58b7f81a1..c19c61834 100644 --- a/lib/app.tsx +++ b/lib/app.tsx @@ -6,7 +6,6 @@ import 'focus-visible/dist/focus-visible.js'; import appState from './flux/app-state'; import { loadTags } from './state/domain/tags'; import reduxActions from './state/actions'; -import { setSelectedNote } from './state/ui/actions'; import selectors from './state/selectors'; import browserShell from './browser-shell'; import NoteInfo from './note-info'; @@ -35,15 +34,32 @@ import { toggleSimperiumConnectionStatus } from './state/ui/actions'; import * as settingsActions from './state/settings/actions'; +import actions from './state/actions'; +import * as S from './state'; +import * as T from './types'; + const ipc = getIpcRenderer(); +export type OwnProps = { + noteBucket: object; +}; + +export type DispatchProps = { + selectNote: (note: T.NoteEntity) => any; +}; + +export type Props = DispatchProps; + const mapStateToProps = state => ({ ...state, authIsPending: selectors.auth.authIsPending(state), isAuthorized: selectors.auth.isAuthorized(state), }); -function mapDispatchToProps(dispatch, { noteBucket }) { +const mapDispatchToProps: S.MapDispatch< + DispatchProps, + OwnProps +> = function mapDispatchToProps(dispatch, { noteBucket }) { const actionCreators = Object.assign({}, appState.actionCreators); const thenReloadNotes = action => a => { @@ -86,9 +102,9 @@ function mapDispatchToProps(dispatch, { noteBucket }) { dispatch(actionCreators.setSearchFocus({ searchFocus: true })), setSimperiumConnectionStatus: connected => dispatch(toggleSimperiumConnectionStatus(connected)), - setSelectedNote: id => dispatch(setSelectedNote(id)), + selectNote: note => dispatch(actions.ui.selectNote(note)), }; -} +}; const isElectron = (() => { // https://github.com/atom/electron/issues/2288 @@ -104,7 +120,7 @@ export const App = connect( mapStateToProps, mapDispatchToProps )( - class extends Component { + class extends Component { static displayName = 'App'; static propTypes = { @@ -122,7 +138,6 @@ export const App = connect( openTagList: PropTypes.func.isRequired, onSignOut: PropTypes.func.isRequired, settings: PropTypes.object.isRequired, - noteBucket: PropTypes.object.isRequired, preferencesBucket: PropTypes.object.isRequired, resetAuth: PropTypes.func.isRequired, setAuthorized: PropTypes.func.isRequired, @@ -300,12 +315,12 @@ export const App = connect( onNoteUpdate = (noteId, data, remoteUpdateInfo = {}) => { const { noteBucket, - setSelectedNote, + selectNote, ui: { note }, } = this.props; if (remoteUpdateInfo.patch && note && noteId === note.id) { noteBucket.get(noteId, (e, updatedNote) => { - setSelectedNote({ ...updatedNote, hasRemoteUpdate: true }); + selectNote({ ...updatedNote, hasRemoteUpdate: true }); }); } }; diff --git a/lib/dialogs/share/index.tsx b/lib/dialogs/share/index.tsx index 18e8771d7..5fa2b9d1c 100644 --- a/lib/dialogs/share/index.tsx +++ b/lib/dialogs/share/index.tsx @@ -13,16 +13,25 @@ import TabPanels from '../../components/tab-panels'; import PanelTitle from '../../components/panel-title'; import ToggleControl from '../../controls/toggle'; +import * as S from '../../state'; +import * as T from '../../types'; + const shareTabs = ['collaborate', 'publish']; -export class ShareDialog extends Component { +type StateProps = { + settings: S.State['settings']; + note: T.NoteEntity | null; +}; + +type Props = StateProps; + +export class ShareDialog extends Component { static propTypes = { actions: PropTypes.object.isRequired, dialog: PropTypes.object.isRequired, noteBucket: PropTypes.object.isRequired, appState: PropTypes.object.isRequired, requestClose: PropTypes.func.isRequired, - settings: PropTypes.object.isRequired, tagBucket: PropTypes.object.isRequired, updateNoteTags: PropTypes.func.isRequired, }; @@ -205,7 +214,10 @@ export class ShareDialog extends Component { } } -const mapStateToProps = ({ settings, ui: { note } }) => ({ +const mapStateToProps: S.MapState = ({ + settings, + ui: { note }, +}) => ({ settings, note, }); diff --git a/lib/note-editor/index.tsx b/lib/note-editor/index.tsx index 0fd66f46d..509e9de3c 100644 --- a/lib/note-editor/index.tsx +++ b/lib/note-editor/index.tsx @@ -1,13 +1,21 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; import { connect } from 'react-redux'; -import { get } from 'lodash'; import appState from '../flux/app-state'; import TagField from '../tag-field'; import { property } from 'lodash'; import NoteDetail from '../note-detail'; -export class NoteEditor extends Component { +import * as S from '../state'; +import * as T from '../types'; + +type StateProps = { + note: T.NoteEntity | null; +}; + +type Props = StateProps; + +export class NoteEditor extends Component { static displayName = 'NoteEditor'; static propTypes = { @@ -17,7 +25,6 @@ export class NoteEditor extends Component { isEditorActive: PropTypes.bool.isRequired, isSmallScreen: PropTypes.bool.isRequired, filter: PropTypes.string.isRequired, - note: PropTypes.object, noteBucket: PropTypes.object.isRequired, fontSize: PropTypes.number, onNoteClosed: PropTypes.func.isRequired, @@ -162,7 +169,11 @@ export class NoteEditor extends Component { } } -const mapStateToProps = ({ appState: state, settings, ui: { note } }) => ({ +const mapStateToProps: S.MapState = ({ + appState: state, + settings, + ui: { note }, +}) => ({ allTags: state.tags, filter: state.filter, fontSize: settings.fontSize, @@ -170,7 +181,6 @@ const mapStateToProps = ({ appState: state, settings, ui: { note } }) => ({ isEditorActive: !state.showNavigation, note, revision: state.revision, - tags: get(note, 'data.tags', []), }); const { closeNote, setEditorMode } = appState.actionCreators; diff --git a/lib/note-info/index.tsx b/lib/note-info/index.tsx index 5f39cf52e..22de994f0 100644 --- a/lib/note-info/index.tsx +++ b/lib/note-info/index.tsx @@ -10,11 +10,19 @@ import { connect } from 'react-redux'; import appState from '../flux/app-state'; import { setMarkdown } from '../state/settings/actions'; -export class NoteInfo extends Component { +import * as S from '../state'; +import * as T from '../types'; + +type StateProps = { + isMarkdown: boolean; + isPinned: boolean; + note: T.NoteEntity | null; +}; + +type Props = StateProps; + +export class NoteInfo extends Component { static propTypes = { - isMarkdown: PropTypes.bool.isRequired, - isPinned: PropTypes.bool.isRequired, - note: PropTypes.object, markdownEnabled: PropTypes.bool, onPinNote: PropTypes.func.isRequired, onMarkdownNote: PropTypes.func.isRequired, @@ -188,13 +196,11 @@ function characterCount(content) { const { markdownNote, pinNote, toggleNoteInfo } = appState.actionCreators; -const mapStateToProps = ({ ui: { note } }) => { - return { - note, - isMarkdown: note.data.systemTags.includes('markdown'), - isPinned: note.data.systemTags.includes('pinned'), - }; -}; +const mapStateToProps: S.MapState = ({ ui: { note } }) => ({ + note, + isMarkdown: !!note && note.data.systemTags.includes('markdown'), + isPinned: !!note && note.data.systemTags.includes('pinned'), +}); const mapDispatchToProps = (dispatch, { noteBucket }) => ({ onMarkdownNote: (note, markdown = true) => { diff --git a/lib/note-list/index.tsx b/lib/note-list/index.tsx index a9cf35e4a..63a213602 100644 --- a/lib/note-list/index.tsx +++ b/lib/note-list/index.tsx @@ -29,6 +29,15 @@ import { } from './decorators'; import TagSuggestions, { getMatchingTags } from '../tag-suggestions'; +import * as S from '../state'; +import * as T from '../types'; + +type StateProps = { + selectedNoteId?: T.EntityId; +}; + +type Props = StateProps; + AutoSizer.displayName = 'AutoSizer'; List.displayName = 'List'; @@ -295,7 +304,7 @@ const createCompositeNoteList = (notes, filter, tagResultsFound) => { ]; }; -export class NoteList extends Component { +export class NoteList extends Component { static displayName = 'NoteList'; list = createRef(); @@ -306,7 +315,6 @@ export class NoteList extends Component { tagResultsFound: PropTypes.number.isRequired, isSmallScreen: PropTypes.bool.isRequired, notes: PropTypes.array.isRequired, - selectedNoteId: PropTypes.any, onNoteOpened: PropTypes.func.isRequired, onSelectNote: PropTypes.func.isRequired, onPinNote: PropTypes.func.isRequired, @@ -489,7 +497,7 @@ const { } = appState.actionCreators; const { recordEvent } = tracks; -const mapStateToProps = ({ +const mapStateToProps: S.MapState = ({ appState: state, ui: { filteredNotes, note }, settings: { noteDisplay }, diff --git a/lib/note-toolbar/index.tsx b/lib/note-toolbar/index.tsx index 824ba1fcf..0c757bf29 100644 --- a/lib/note-toolbar/index.tsx +++ b/lib/note-toolbar/index.tsx @@ -13,11 +13,19 @@ import TrashIcon from '../icons/trash'; import ShareIcon from '../icons/share'; import SidebarIcon from '../icons/sidebar'; -export class NoteToolbar extends Component { +import * as S from '../state'; +import * as T from '../types'; + +type StateProps = { + note: T.NoteEntity | null; +}; + +type Props = StateProps; + +export class NoteToolbar extends Component { static displayName = 'NoteToolbar'; static propTypes = { - note: PropTypes.object, onRestoreNote: PropTypes.func, onTrashNote: PropTypes.func, onDeleteNoteForever: PropTypes.func, @@ -172,7 +180,7 @@ export class NoteToolbar extends Component { }; } -const mapStateToProps = ({ ui: { note } }) => ({ +const mapStateToProps: S.MapState = ({ ui: { note } }) => ({ note, }); diff --git a/lib/revision-selector/index.tsx b/lib/revision-selector/index.tsx index 32b205a5a..feed1ea03 100644 --- a/lib/revision-selector/index.tsx +++ b/lib/revision-selector/index.tsx @@ -8,15 +8,14 @@ import Slider from '../components/slider'; import appState from '../flux/app-state'; import { updateNoteTags } from '../state/domain/notes'; -import { NoteEntity } from '../types'; +import * as S from '../state'; +import * as T from '../types'; -const sortedRevisions = (revisions: NoteEntity[]) => +const sortedRevisions = (revisions: T.NoteEntity[]) => orderBy(revisions, 'version', 'asc'); -type Props = { - isViewingRevisions: boolean; - note: NoteEntity; - revisions: NoteEntity[]; +type OwnProps = { + revisions: T.NoteEntity[]; onUpdateContent: Function; setRevision: Function; resetIsViewingRevisions: Function; @@ -24,12 +23,19 @@ type Props = { updateNoteTags: Function; }; -type State = { - revisions: NoteEntity[]; +type StateProps = { + isViewingRevisions: boolean; + note: T.NoteEntity | null; +}; + +type ComponentState = { + revisions: T.NoteEntity[]; selection: number; }; -export class RevisionSelector extends Component { +type Props = OwnProps & StateProps; + +export class RevisionSelector extends Component { constructor(props: Props, ...args: unknown[]) { super(props, ...args); @@ -167,7 +173,10 @@ export class RevisionSelector extends Component { } } -const mapStateToProps = ({ appState: state, ui: { note } }) => ({ +const mapStateToProps: S.MapState = ({ + appState: state, + ui: { note }, +}) => ({ isViewingRevisions: state.isViewingRevisions, note: note, }); diff --git a/lib/state/action-types.ts b/lib/state/action-types.ts index c5eab5988..33c911f56 100644 --- a/lib/state/action-types.ts +++ b/lib/state/action-types.ts @@ -58,13 +58,11 @@ export type ToggleSimperiumConnectionStatus = Action< { simperiumConnected: boolean } >; export type ToggleTagDrawer = Action<'TAG_DRAWER_TOGGLE', { show: boolean }>; -export type SetSelectedNote = Action< - 'SET_SELECTED_NOTE', - { note: T.NoteEntity | null } ->; +export type SelectNote = Action<'SELECT_NOTE', { note: T.NoteEntity }>; export type ActionType = | FilterNotes + | SelectNote | SetAccountName | SetAuth | SetAutoHideMenuBar @@ -73,7 +71,6 @@ export type ActionType = | SetLineLength | SetMarkdownEnabled | SetNoteDisplay - | SetSelectedNote | SetSortReversed | SetSortTagsAlpha | SetSortType diff --git a/lib/state/ui/actions.ts b/lib/state/ui/actions.ts index e17993e8c..b7c3f43e8 100644 --- a/lib/state/ui/actions.ts +++ b/lib/state/ui/actions.ts @@ -15,12 +15,9 @@ export const toggleSimperiumConnectionStatus: A.ActionCreator = ( - note: T.NoteEntity | null -) => ({ - type: 'SET_SELECTED_NOTE', - note, -}); +export const selectNote: A.ActionCreator = ( + note: T.NoteEntity +) => ({ type: 'SELECT_NOTE', note }); export const toggleTagDrawer: A.ActionCreator = ( show: boolean diff --git a/lib/state/ui/middleware.ts b/lib/state/ui/middleware.ts index 4cb1506c1..12e9bf90b 100644 --- a/lib/state/ui/middleware.ts +++ b/lib/state/ui/middleware.ts @@ -30,7 +30,6 @@ export const middleware: Middleware<{}, S.State, Dispatch> = store => { // on updating the search field we should delay the update // so we don't waste our CPU time and lose responsiveness - case 'App.noteUpdatedRemotely': case 'App.search': clearTimeout(searchTimeout); if ('App.search' === action.type && !action.filter) { diff --git a/lib/state/ui/reducer.ts b/lib/state/ui/reducer.ts index 481ae99f7..2920daf60 100644 --- a/lib/state/ui/reducer.ts +++ b/lib/state/ui/reducer.ts @@ -29,24 +29,16 @@ const visiblePanes: A.Reducer = ( return state; }; -const note: A.Reducer = ( - state = null, - action: AnyAction -) => { +const note: A.Reducer = (state = null, action) => { switch (action.type) { - case 'App.noteUpdatedRemotely': - case 'App.selectNote': { - return { - ...action.note, - hasRemoteUpdate: action.hasRemoteUpdate, - }; - } + case 'App.selectNote': + return { ...action.note, hasRemoteUpdate: action.hasRemoteUpdate }; case 'App.closeNote': case 'App.showAllNotes': case 'App.selectTrash': case 'App.selectTag': return null; - case 'SET_SELECTED_NOTE': + case 'SELECT_NOTE': return action.note; case 'FILTER_NOTES': // keep note if still in new filtered list otherwise try to choose first note in list