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

Warn users when before signing out with unsynced notes #702

Merged
merged 10 commits into from
Feb 7, 2018
8 changes: 7 additions & 1 deletion lib/app.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,13 @@ export const App = connect(mapStateToProps, mapDispatchToProps)(
throw new Error('Unknown dialog type.');
}

return <DialogComponent {...this.props} {...{ key, dialog, params }} />;
return (
<DialogComponent
isElectron={isElectron()}
{...this.props}
{...{ key, dialog, params }}
/>
);
},
})
);
Expand Down
71 changes: 70 additions & 1 deletion lib/dialogs/settings.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const SettingsDialog = React.createClass({
propTypes: {
actions: PropTypes.object.isRequired,
onSignOut: PropTypes.func.isRequired,
isElectron: PropTypes.bool.isRequired,
},

onDone() {
Expand All @@ -37,6 +38,74 @@ export const SettingsDialog = React.createClass({
});
},

onSignOutRequested() {
// Safety first! Check for any unsynced notes before signing out.
const { onSignOut, noteBucket } = this.props;
const { notes } = this.props.appState;
const { getVersion } = noteBucket;

if (
noteBucket.hasLocalChanges((error, hasChanges) => {
if (hasChanges) {
this.showUnsyncedWarning();
return;
}

// Also check persisted store for any notes with version 0
const noteHasSynced = note =>
new Promise((resolve, reject) =>
getVersion(note.id, (e, v) => (e || v === 0 ? reject() : resolve()))
);

Promise.race(notes.map(noteHasSynced)).then(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be Promise.all not Promise.race? You want to make sure that none of the notes are unsynced right?

() => onSignOut(), // All good, sign out now!
() => this.showUnsyncedWarning() // Show a warning to the user
);
})
);
Copy link
Member

Choose a reason for hiding this comment

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

what's going on here? is there a missing if statement body? something is wrong.

Copy link
Contributor Author

@roundhill roundhill Feb 7, 2018

Choose a reason for hiding this comment

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

Whoops, I had an if there that didn't belong. Callbacks are silly.

Copy link
Member

Choose a reason for hiding this comment

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

😄 looks better now

},

showUnsyncedWarning() {
const { isElectron } = this.props;
isElectron ? this.showElectronWarningDialog() : this.showWebWarningDialog();
},

showElectronWarningDialog() {
const { onSignOut } = this.props;
const dialog = __non_webpack_require__('electron').remote.dialog; // eslint-disable-line no-undef
dialog.showMessageBox(
{
type: 'warning',
buttons: ['Delete Notes', 'Cancel', 'Visit Web App'],
title: 'Unsynced Notes Detected',
message:
'Logging out will delete any unsynced notes. You can verify your ' +
'synced notes by logging in to the Web App.',
},
response => {
if (response === 0) {
onSignOut();
} else if (response === 2) {
viewExternalUrl('https://app.simplenote.com');
}
}
);
},

showWebWarningDialog() {
const { onSignOut } = this.props;
const shouldReallySignOut = confirm(
'Warning: Unsynced notes were detected.\n\n' +
'Logging out will delete any notes that have not synced. ' +
'Check your connection and visit app.simplenote.com to verify synced notes.' +
'\n\nClick OK to delete unsynced notes and Log Out.'
);

if (shouldReallySignOut) {
onSignOut();
}
},

render() {
var dialog = this.props.dialog;

Expand Down Expand Up @@ -91,7 +160,7 @@ export const SettingsDialog = React.createClass({
<button
type="button"
className="button button-primary"
onClick={this.props.onSignOut}
onClick={this.onSignOutRequested}
>
Log Out
</button>
Expand Down
12 changes: 6 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@
"build": "NODE_ENV=development npm run build:dll && npm run build:app",
"build:app": "webpack --config ./webpack.config.js",
"build:dll": "webpack --config ./webpack.config.dll.js",
"build:prod":
"NODE_ENV=production webpack -p --config ./webpack.config.dll.js && webpack -p --config ./webpack.config.js",
"build:prod": "NODE_ENV=production webpack -p --config ./webpack.config.dll.js && webpack -p --config ./webpack.config.js",
"format": "prettier --write {desktop,lib,sass}/{**/,*}.{js,json,jsx,sass}",
"lint": "eslint --ext .js --ext .jsx lib",
"start":
"NODE_ENV=hot npm run build && webpack-dev-server --config ./webpack.config.js --content-base dist --host 0.0.0.0 --port 4000 --hot --inline"
"start": "NODE_ENV=hot npm run build && webpack-dev-server --config ./webpack.config.js --content-base dist --host 0.0.0.0 --port 4000 --hot --inline"
},
"license": "ISC",
"engines": {
"node": "7.9.0",
"npm": "4.2.0"
},
"browserslist": ["Chrome >= 58"],
"browserslist": [
"Chrome >= 58"
],
"jest": {
"rootDir": "lib",
"testRegex": "(/test/.*\\.jsx?)|(test\\.jsx?)$"
Expand Down Expand Up @@ -99,7 +99,7 @@
"serve-favicon": "2.4.3",
"showdown": "1.7.2",
"showdown-xss-filter": "0.2.0",
"simperium": "0.2.9"
"simperium": "0.3.1"
},
"optionalDependencies": {
"appdmg": "0.4.5"
Expand Down