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.

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

Add sync indicator #1201

Merged
merged 15 commits into from
Feb 19, 2019
37 changes: 26 additions & 11 deletions lib/app.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import DevBadge from './components/dev-badge';
import DialogRenderer from './dialog-renderer';
import { getIpcRenderer } from './utils/electron';
import exportZipArchive from './utils/export';
import { activityHooks, nudgeUnsynced } from './utils/sync';
import { activityHooks, getUnsyncedNoteIds, nudgeUnsynced } from './utils/sync';
import { setLastSyncedTime } from './utils/sync/last-synced-time';
import analytics from './analytics';
import classNames from 'classnames';
import {
Expand Down Expand Up @@ -152,11 +153,16 @@ export const App = connect(mapStateToProps, mapDispatchToProps)(
.on('update', debounce(this.props.loadTags, 200))
.on('remove', this.props.loadTags);

const { actions: { setConnectionStatus } } = this.props;

this.props.client
.on('authorized', this.onAuthChanged)
.on('unauthorized', this.onAuthChanged)
.on('message', setLastSyncedTime)
.on('message', this.syncActivityHooks)
.on('send', this.syncActivityHooks);
.on('send', this.syncActivityHooks)
.on('connect', () => setConnectionStatus({ isOffline: false }))
.on('disconnect', () => setConnectionStatus({ isOffline: true }));

this.onLoadPreferences(() =>
// Make sure that tracking starts only after preferences are loaded
Expand Down Expand Up @@ -262,8 +268,13 @@ export const App = connect(mapStateToProps, mapDispatchToProps)(
this.props.loadTags();
};

onNotesIndex = () =>
this.props.actions.loadNotes({ noteBucket: this.props.noteBucket });
onNotesIndex = () => {
const { noteBucket } = this.props;
const { loadNotes, setUnsyncedNoteIds } = this.props.actions;

loadNotes({ noteBucket });
setUnsyncedNoteIds({ noteIds: getUnsyncedNoteIds(noteBucket) });
};

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

Expand Down Expand Up @@ -316,10 +327,16 @@ export const App = connect(mapStateToProps, mapDispatchToProps)(
syncActivityHooks = data => {
activityHooks(data, {
onIdle: () => {
nudgeUnsynced({
client: this.props.client,
noteBucket: this.props.noteBucket,
notes: this.props.appState.notes,
const {
actions: { setUnsyncedNoteIds },
appState: { notes },
client,
noteBucket,
} = this.props;

nudgeUnsynced({ client, noteBucket, notes });
setUnsyncedNoteIds({
noteIds: getUnsyncedNoteIds(noteBucket),
});
},
});
Expand Down Expand Up @@ -372,9 +389,7 @@ export const App = connect(mapStateToProps, mapDispatchToProps)(
{isDevConfig && <DevBadge />}
{isAuthorized ? (
<div className={mainClasses}>
{state.showNavigation && (
<NavigationBar noteBucket={noteBucket} tagBucket={tagBucket} />
)}
{state.showNavigation && <NavigationBar />}
<AppLayout
isFocusMode={settings.focusModeEnabled}
isNavigationOpen={state.showNavigation}
Expand Down
24 changes: 24 additions & 0 deletions lib/components/sync-status/get-note-titles.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { compact } from 'lodash';
import noteTitleAndPreview from '../../utils/note-utils';

const getNoteTitles = (ids, notes, limit = Infinity) => {
const matchedNotes = ids.map((id, i) => {
if (i >= limit) {
return;
}

const note = notes.find(thisNote => thisNote.id === id);

if (!note) {
// eslint-disable-next-line no-console
console.log(`Could not find note with id '${id}'`);
return null;
}

return { id, title: noteTitleAndPreview(note).title };
});

return compact(matchedNotes);
};

export default getNoteTitles;
46 changes: 46 additions & 0 deletions lib/components/sync-status/get-note-titles.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import getNoteTitles from './get-note-titles';

describe('getNoteTitles', () => {
const originalConsoleLog = console.log; // eslint-disable-line no-console

afterEach(() => {
global.console.log = originalConsoleLog;
});

it('should return the titles for the given note ids', () => {
const result = getNoteTitles(
['foo', 'baz'],
[
{ id: 'foo', data: { content: 'title\nexcerpt', systemTags: [] } },
{ id: 'bar' },
{ id: 'baz', data: { content: 'title\nexcerpt', systemTags: [] } },
]
);
expect(result).toEqual([
{ id: 'foo', title: 'title' },
{ id: 'baz', title: 'title' },
]);
});

it('should not choke on invalid ids', () => {
global.console.log = jest.fn();
const result = getNoteTitles(
['foo', 'bar'],
[{ id: 'foo', data: { content: 'title', systemTags: [] } }]
);
expect(result).toEqual([{ id: 'foo', title: 'title' }]);
});

it('should return no more than `limit` items', () => {
const limit = 1;
const result = getNoteTitles(
['foo', 'bar'],
[
{ id: 'foo', data: { content: 'title', systemTags: [] } },
{ id: 'bar' },
],
limit
);
expect(result).toHaveLength(limit);
});
});
66 changes: 66 additions & 0 deletions lib/components/sync-status/index.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import React, { Component } from 'react';
import PropTypes from 'prop-types';

import AlertIcon from '../../icons/alert';
import SyncIcon from '../../icons/sync';
import SyncStatusPopover from './popover';

class SyncStatus extends Component {
static propTypes = {
isOffline: PropTypes.bool.isRequired,
unsyncedNoteIds: PropTypes.array.isRequired,
};

state = {
anchorEl: null,
};

handlePopoverOpen = event => {
this.setState({ anchorEl: event.currentTarget });
};

handlePopoverClose = () => {
this.setState({ anchorEl: null });
};

render() {
const { isOffline, unsyncedNoteIds } = this.props;
const { anchorEl } = this.state;

const popoverId = 'sync-status__popover';

const unsyncedChangeCount = unsyncedNoteIds.length;
const unit = unsyncedChangeCount === 1 ? 'change' : 'changes';
const text = unsyncedChangeCount
? `${unsyncedChangeCount} unsynced ${unit}`
: isOffline ? 'No connection' : 'All changes synced';

return (
<div>
<div
className="sync-status"
aria-owns={anchorEl ? popoverId : undefined}
aria-haspopup="true"
onFocus={this.handlePopoverOpen}
onMouseEnter={this.handlePopoverOpen}
onMouseLeave={this.handlePopoverClose}
tabIndex="0"
>
<span className="sync-status__icon">
{isOffline ? <AlertIcon /> : <SyncIcon />}
</span>
{text}
</div>

<SyncStatusPopover
anchorEl={anchorEl}
id={popoverId}
onClose={this.handlePopoverClose}
unsyncedNoteIds={unsyncedNoteIds}
/>
</div>
);
}
}

export default SyncStatus;
110 changes: 110 additions & 0 deletions lib/components/sync-status/popover.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import React from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import classnames from 'classnames';
import distanceInWordsToNow from 'date-fns/distance_in_words_to_now';
import Popover from '@material-ui/core/Popover';

import { getLastSyncedTime } from '../../utils/sync/last-synced-time';
import getNoteTitles from './get-note-titles';

class SyncStatusPopover extends React.Component {
render() {
const {
anchorEl,
classes = {},
id,
notes,
onClose,
theme,
unsyncedNoteIds,
} = this.props;
const themeClass = `theme-${theme}`;
const open = Boolean(anchorEl);
const hasUnsyncedChanges = unsyncedNoteIds.length > 0;

const QUERY_LIMIT = 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this limit so that the popup doesn't become too tall? What if we put the note titles in a scrollable div?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. A few reasons for the limit, in my mind:

  1. The note titles are taken by regex-ing the note contents on each sync indicator hover, and not cached.
  2. I feel like having a scrollable area inside a hover-activated popover is a little too much complexity. That would warrant a proper click-activated popover.
  3. The intent of this feature is not to provide an exhaustive list of unsynced changes, but rather a way to self-diagnose if something's going wrong. Providing a list of 100 note titles doesn't add much utility over 10 notes. If we really wanted to show every unsynced note in a useful way, we should consider a different UI.

const noteTitles = hasUnsyncedChanges
? getNoteTitles(unsyncedNoteIds, notes, QUERY_LIMIT)
: [];
const overflowCount = unsyncedNoteIds.length - noteTitles.length;
const unit = overflowCount === 1 ? 'note' : 'notes';

const lastSyncedTime = distanceInWordsToNow(getLastSyncedTime(), {
addSuffix: true,
});

return (
<Popover
id={id}
className={classnames(
'sync-status-popover',
classes.popover,
themeClass
)}
classes={{
paper: classnames(
'sync-status-popover__paper',
'theme-color-bg',
'theme-color-border',
'theme-color-fg-dim',
{ 'has-unsynced-changes': hasUnsyncedChanges },
classes.paper
),
}}
open={open}
anchorEl={anchorEl}
anchorOrigin={{
vertical: 'center',
horizontal: 'left',
}}
transformOrigin={{
vertical: 'bottom',
horizontal: 'center',
}}
onBlur={onClose}
onClose={onClose}
PaperProps={{ square: true }}
disableRestoreFocus
>
{hasUnsyncedChanges && (
<div className="sync-status-popover__unsynced theme-color-border">
<h2 className="sync-status-popover__heading">
Notes with unsynced changes
</h2>
<ul className="sync-status-popover__notes theme-color-fg">
{noteTitles.map(note => <li key={note.id}>{note.title}</li>)}
</ul>
{!!overflowCount && (
<p>
and {overflowCount} more {unit}
</p>
)}
<div>
If a note isn’t syncing, try switching networks or editing the
note again.
</div>
</div>
)}
<span>Last synced: {lastSyncedTime}</span>
</Popover>
);
}
}

SyncStatusPopover.propTypes = {
anchorEl: PropTypes.object,
classes: PropTypes.object,
id: PropTypes.string,
notes: PropTypes.array,
onClose: PropTypes.func.isRequired,
theme: PropTypes.string.isRequired,
unsyncedNoteIds: PropTypes.array.isRequired,
};

const mapStateToProps = ({ appState, settings }) => ({
notes: appState.notes,
theme: settings.theme,
});

export default connect(mapStateToProps)(SyncStatusPopover);
61 changes: 61 additions & 0 deletions lib/components/sync-status/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
.sync-status {
display: flex;
align-items: center;
padding: 1.25em 1.75em;
font-size: .75rem;
line-height: 1;
}

.sync-status__icon {
width: 18px;
margin-right: .5em;
text-align: center;
}

.sync-status-popover {
pointer-events: none;

&.theme-light,
&.theme-dark {
background: transparent;
}
}

.sync-status-popover__paper {
padding: .5em 1em;
border-radius: $border-radius;
border: 1px solid lighten($gray, 30%);
font-size: .75rem;

&.has-unsynced-changes {
padding: 1em 1.5em;
}
}

.sync-status-popover__unsynced {
max-width: 18em;
margin-bottom: .75em;
padding-bottom: 1em;
border-bottom: 1px solid lighten($gray, 30%);
line-height: 1.45;
}

.sync-status-popover__heading {
margin: .5em 0 0;
font-size: .75rem;
font-weight: $bold;
text-transform: uppercase;
}

.sync-status-popover__notes {
margin: 1em 0;
padding-left: .5em;
list-style-position: inside;
font-size: .875rem;

li {
overflow: hidden;
white-space: nowrap;
text-overflow: ellipsis;
}
}
Loading