Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

State Refactor: Move note info into Redux #1896

Merged
merged 1 commit into from
Feb 14, 2020
Merged

State Refactor: Move note info into Redux #1896

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions lib/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ export const App = connect(
settings,
tagBucket,
isSmallScreen,
ui: { showNoteInfo },
} = this.props;
const isMacApp = isElectronMac();

Expand All @@ -464,7 +465,7 @@ export const App = connect(
});

const mainClasses = classNames('simplenote-app', {
'note-info-open': state.showNoteInfo,
'note-info-open': showNoteInfo,
'navigation-open': state.showNavigation,
'is-electron': isElectron(),
'is-macos': isMacApp,
Expand All @@ -482,7 +483,7 @@ export const App = connect(
isFocusMode={settings.focusModeEnabled}
isNavigationOpen={state.showNavigation}
isNoteOpen={this.state.isNoteOpen}
isNoteInfoOpen={state.showNoteInfo}
isNoteInfoOpen={showNoteInfo}
isSmallScreen={isSmallScreen}
noteBucket={noteBucket}
revisions={state.revisions}
Expand All @@ -491,7 +492,7 @@ export const App = connect(
onUpdateContent={this.onUpdateContent}
syncNote={this.syncNote}
/>
{state.showNoteInfo && <NoteInfo noteBucket={noteBucket} />}
{showNoteInfo && <NoteInfo noteBucket={noteBucket} />}
</div>
) : (
<Auth
Expand Down
16 changes: 0 additions & 16 deletions lib/flux/app-state.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ const initialState: AppState = {
revision: null,
showTrash: false,
showNavigation: false,
showNoteInfo: false,
isViewingRevisions: false,
editingTags: false,
dialogs: [],
Expand Down Expand Up @@ -70,7 +69,6 @@ export const actionMap = new ActionMap({

return update(state, {
showNavigation: { $set: true },
showNoteInfo: { $set: false },
});
},

Expand Down Expand Up @@ -490,20 +488,6 @@ export const actionMap = new ActionMap({
});
},

toggleNoteInfo(state: AppState) {
if (state.showNoteInfo) {
return update(state, {
showNoteInfo: { $set: false },
});
}

return update(state, {
showNoteInfo: { $set: true },
showNavigation: { $set: false },
editingTags: { $set: false },
});
},

tagsLoaded(
state: AppState,
{ tags, sortTagsAlpha }: { tags: T.TagEntity[]; sortTagsAlpha: boolean }
Expand Down
19 changes: 15 additions & 4 deletions lib/note-detail/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,17 @@ import SimplenoteCompactLogo from '../icons/simplenote-compact';
import renderToNode from './render-to-node';
import toggleTask from './toggle-task';

import * as S from '../state';

const syncDelay = 2000;

export class NoteDetail extends Component {
type StateProps = {
showNoteInfo: boolean;
};

type Props = StateProps;

export class NoteDetail extends Component<Props> {
static displayName = 'NoteDetail';

static propTypes = {
Expand All @@ -23,7 +31,6 @@ export class NoteDetail extends Component {
note: PropTypes.object,
noteBucket: PropTypes.object.isRequired,
previewingMarkdown: PropTypes.bool,
showNoteInfo: PropTypes.bool.isRequired,
spellCheckEnabled: PropTypes.bool.isRequired,
storeFocusEditor: PropTypes.func,
storeHasFocus: PropTypes.func,
Expand Down Expand Up @@ -226,10 +233,14 @@ export class NoteDetail extends Component {
}
}

const mapStateToProps = ({ appState: state, ui, settings }) => ({
const mapStateToProps: S.MapState<StateProps> = ({
appState: state,
ui,
settings,
}) => ({
dialogs: state.dialogs,
note: state.revision || ui.note,
showNoteInfo: state.showNoteInfo,
showNoteInfo: ui.showNoteInfo,
spellCheckEnabled: settings.spellCheckEnabled,
});

Expand Down
33 changes: 24 additions & 9 deletions lib/note-info/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,39 @@
import React, { Component } from 'react';
import { connect } from 'react-redux';
import PropTypes from 'prop-types';
import onClickOutside from 'react-onclickoutside';
import { includes, isEmpty } from 'lodash';
import format from 'date-fns/format';

import appState from '../flux/app-state';
import PanelTitle from '../components/panel-title';
import ToggleControl from '../controls/toggle';
import format from 'date-fns/format';
import CrossIcon from '../icons/cross';
import { connect } from 'react-redux';
import appState from '../flux/app-state';
import { setMarkdown } from '../state/settings/actions';

import actions from '../state/actions';

import * as S from '../state';
import * as T from '../types';

type OwnProps = {
noteBucket: T.Bucket<T.Note>;
};

type StateProps = {
isMarkdown: boolean;
isPinned: boolean;
note: T.NoteEntity | null;
};

type Props = StateProps;
type DispatchProps = {
onOutsideClick: () => any;
};

type Props = OwnProps & StateProps & DispatchProps;

export class NoteInfo extends Component<Props> {
static displayName = 'NoteInfo';

static propTypes = {
markdownEnabled: PropTypes.bool,
onPinNote: PropTypes.func.isRequired,
Expand Down Expand Up @@ -194,21 +206,24 @@ function characterCount(content) {
);
}

const { markdownNote, pinNote, toggleNoteInfo } = appState.actionCreators;
const { markdownNote, pinNote } = appState.actionCreators;

const mapStateToProps: S.MapState<StateProps> = ({ ui: { note } }) => ({
note,
isMarkdown: !!note && note.data.systemTags.includes('markdown'),
isPinned: !!note && note.data.systemTags.includes('pinned'),
});

const mapDispatchToProps = (dispatch, { noteBucket }) => ({
const mapDispatchToProps: S.MapDispatch<DispatchProps, OwnProps> = (
dispatch,
{ noteBucket }
) => ({
onMarkdownNote: (note, markdown = true) => {
dispatch(markdownNote({ markdown, note, noteBucket }));
// Update global setting to set markdown flag for new notes
dispatch(setMarkdown(markdown));
dispatch(actions.settings.setMarkdown(markdown));
},
onOutsideClick: () => dispatch(toggleNoteInfo()),
onOutsideClick: () => dispatch(actions.ui.toggleNoteInfo()),
onPinNote: (note, pin) => dispatch(pinNote({ noteBucket, note, pin })),
});

Expand Down
4 changes: 0 additions & 4 deletions lib/note-toolbar-container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ type DispatchProps = {
setIsViewingRevisions: (isViewingRevisions: boolean) => any;
shareNote: () => any;
toggleFocusMode: () => any;
toggleNoteInfo: () => any;
trashNote: (args: ListChanger) => any;
};

Expand Down Expand Up @@ -96,7 +95,6 @@ export class NoteToolbarContainer extends Component<Props> {
onCloseNote: this.onCloseNote,
onDeleteNoteForever: this.onDeleteNoteForever,
onRestoreNote: this.onRestoreNote,
onShowNoteInfo: this.props.toggleNoteInfo,
onShowRevisions: this.onShowRevisions,
onShareNote: this.onShareNote,
onTrashNote: this.onTrashNote,
Expand Down Expand Up @@ -132,7 +130,6 @@ const {
restoreNote,
setIsViewingRevisions,
showDialog,
toggleNoteInfo,
trashNote,
} = appState.actionCreators;

Expand All @@ -146,7 +143,6 @@ const mapDispatchToProps: S.MapDispatch<DispatchProps> = dispatch => ({
},
shareNote: () => dispatch(showDialog({ dialog: DialogTypes.SHARE })),
toggleFocusMode: () => dispatch(toggleFocusMode()),
toggleNoteInfo: () => dispatch(toggleNoteInfo()),
trashNote: args => dispatch(trashNote(args)),
});

Expand Down
16 changes: 9 additions & 7 deletions lib/note-toolbar/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@ import RevisionsIcon from '../icons/revisions';
import TrashIcon from '../icons/trash';
import ShareIcon from '../icons/share';
import SidebarIcon from '../icons/sidebar';

import { toggleEditMode } from '../state/ui/actions';
import { toggleNoteInfo } from '../state/ui/actions';

import * as S from '../state';
import * as T from '../types';

type DispatchProps = {
toggleEditMode: () => any;
toggleNoteInfo: () => any;
};

type StateProps = {
Expand All @@ -38,7 +41,6 @@ export class NoteToolbar extends Component<Props> {
onShowRevisions: PropTypes.func,
onShareNote: PropTypes.func,
onCloseNote: PropTypes.func,
onShowNoteInfo: PropTypes.func,
setIsViewingRevisions: PropTypes.func,
toggleFocusMode: PropTypes.func.isRequired,
markdownEnabled: PropTypes.bool,
Expand All @@ -48,7 +50,6 @@ export class NoteToolbar extends Component<Props> {
onCloseNote: noop,
onDeleteNoteForever: noop,
onRestoreNote: noop,
onShowNoteInfo: noop,
onShowRevisions: noop,
onShareNote: noop,
onTrashNote: noop,
Expand All @@ -73,7 +74,7 @@ export class NoteToolbar extends Component<Props> {
}

renderNormal = () => {
const { editMode, markdownEnabled, note } = this.props;
const { editMode, markdownEnabled, note, toggleNoteInfo } = this.props;
return !note ? (
<div className="note-toolbar-placeholder theme-color-border" />
) : (
Expand Down Expand Up @@ -128,7 +129,7 @@ export class NoteToolbar extends Component<Props> {
<div className="note-toolbar__button">
<IconButton
icon={<InfoIcon />}
onClick={this.props.onShowNoteInfo}
onClick={toggleNoteInfo}
title="Info"
/>
</div>
Expand Down Expand Up @@ -181,8 +182,9 @@ const mapStateToProps: S.MapState<StateProps> = ({
note,
});

const mapDispatchToProps: S.MapDispatch<DispatchProps> = dispatch => ({
toggleEditMode: () => dispatch(toggleEditMode()),
});
const mapDispatchToProps: S.MapDispatch<DispatchProps> = {
toggleEditMode,
toggleNoteInfo,
};

export default connect(mapStateToProps, mapDispatchToProps)(NoteToolbar);
5 changes: 3 additions & 2 deletions lib/state/action-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export type SetUnsyncedNoteIds = Action<
'SET_UNSYNCED_NOTE_IDS',
{ noteIds: T.EntityId[] }
>;
export type ToggleNoteInfo = Action<'NOTE_INFO_TOGGLE'>;
export type ToggleSimperiumConnectionStatus = Action<
'SIMPERIUM_CONNECTION_STATUS_TOGGLE',
{ simperiumConnected: boolean }
Expand Down Expand Up @@ -89,6 +90,7 @@ export type ActionType =
| SetUnsyncedNoteIds
| SetWPToken
| ToggleEditMode
| ToggleNoteInfo
| ToggleSimperiumConnectionStatus
| ToggleTagDrawer;

Expand Down Expand Up @@ -193,5 +195,4 @@ type LegacyAction =
| Action<'App.showAllNotesAndSelectFirst'>
| Action<'App.showDialog', { dialog: object }>
| Action<'App.tagsLoaded', { tags: T.TagEntity[]; sortTagsAlpha: boolean }>
| Action<'App.toggleNavigation'>
| Action<'App.toggleNoteInfo'>;
| Action<'App.toggleNavigation'>;
1 change: 0 additions & 1 deletion lib/state/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export type AppState = {
revision: T.NoteEntity | null;
searchFocus: boolean;
showNavigation: boolean;
showNoteInfo: boolean;
showTrash: boolean;
tags: T.TagEntity[];
tag?: T.TagEntity;
Expand Down
4 changes: 4 additions & 0 deletions lib/state/ui/actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ export const toggleEditMode: A.ActionCreator<A.ToggleEditMode> = () => ({
type: 'TOGGLE_EDIT_MODE',
});

export const toggleNoteInfo: A.ActionCreator<A.ToggleNoteInfo> = () => ({
type: 'NOTE_INFO_TOGGLE',
});

export const toggleTagDrawer: A.ActionCreator<A.ToggleTagDrawer> = (
show: boolean
) => ({
Expand Down
14 changes: 14 additions & 0 deletions lib/state/ui/reducer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,19 @@ const visiblePanes: A.Reducer<string[]> = (
return state;
};

const showNoteInfo: A.Reducer<boolean> = (state = false, action) => {
switch (action.type) {
case 'NOTE_INFO_TOGGLE':
return !state;

case 'App.toggleNavigation':
return false;

Copy link
Member Author

Choose a reason for hiding this comment

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

Again, I believe we don't need to handle these cases explicitly, but I hope someone can verify that

Copy link
Contributor

Choose a reason for hiding this comment

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

we can verify it!

Copy link
Contributor

Choose a reason for hiding this comment

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

in testing I can see that it's more of a choice to change the behavior or keep it. if we take this out then the note info stays open when we flip the navigation panel.

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't been able to make that happen, what are the steps? If I open note info, there's no way to open the navigation panel (any click is an "outside click" that closes the note info).

Copy link
Member Author

Choose a reason for hiding this comment

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

From Slack: "Use the keyboard shortcut Cmd + t and flip the tag drawer/nav panel open and closed"

I tried this and it does seem to close the info panel, so I'm gonna say.. it's fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry. I should have clarified that - obviously this code closes the note info panel when toggling the navigation panel. however, when I removed the case 'App.toggleNavigation' as it seemed like you were suggesting would be possible, then the note info would stay open on navigation.

That is, this is an answer to the question "can we skip handling these cases explicitly" and that answer is "if we want and decide to."

default:
return state;
}
};

const note: A.Reducer<T.NoteEntity | null> = (state = null, action) => {
switch (action.type) {
case 'App.selectNote':
Expand Down Expand Up @@ -92,6 +105,7 @@ export default combineReducers({
listTitle,
note,
searchQuery,
showNoteInfo,
simperiumConnected,
unsyncedNoteIds,
visiblePanes,
Expand Down