Auto Update with electron-updater (WIP) #808

Merged
alexliebowitz merged 36 commits from auto-update into master 2018-01-24 23:51:49 +01:00
8 changed files with 32 additions and 66 deletions
Showing only changes of commit b03623eb68 - Show all commits

View file

@ -8,7 +8,7 @@ import Https from 'https';
import Keytar from 'keytar';
import ChildProcess from 'child_process';
import Assert from 'assert';
import { app, BrowserWindow, globalShortcut, ipcMain, Menu, Tray } from 'electron';
import { app, dialog, BrowserWindow, globalShortcut, ipcMain, Menu, Tray } from 'electron';
import { autoUpdater } from 'electron-updater';
import log from 'electron-log';
import mainMenu from './menu/mainMenu';
@ -48,18 +48,14 @@ let daemonSubprocess;
// if it dies when we didn't ask it to shut down, we want to alert the user.
let daemonStopRequested = false;
// This keeps track of whether a new file has been downloaded. We mostly
// handle auto-update stuff in the render process, but need to know this
// in order to display the extra confirmation dialog we show on close
// on Windows.
let updateDownloaded;
if (process.platform === 'win32') {
updateDownloaded = false;
}
// This keeps track of whether the user has declined an update that was downloaded
// through the Electron auto-update system. When the user declines an update on Windows,
// they will get a confusing dialog, so we show our own dialog first.
let autoUpdateDeclined = false;
// This is used to keep track of whether we are showing he special dialog
// that we show on Windows after you decline an upgrade and close the app later.
let showingUpdateCloseAlert = false;
let showingAutoUpdateCloseAlert = false;
// When a quit is attempted, we cancel the quit, do some preparations, then
@ -420,10 +416,6 @@ if (isDevelopment) {
});
}
autoUpdater.on('update-downloaded', () => {
updateDownloaded = true;
});
app.setAsDefaultProtocolClient('lbry');
app.on('ready', () => {
@ -449,18 +441,25 @@ app.on('window-all-closed', () => {
});
app.on('before-quit', event => {
if (process.platform === 'darwin' && updateDownloaded && !showingUpdateCloseAlert) {
// We haven't shown the special dialog that we show on Windows
// if the user declines an update and then quits later
rendererWindow.webContents.send('quitRequested');
showingUpdateCloseAlert = true;
} else if (!readyToQuit) {
if (!readyToQuit) {
// We need to shutdown the daemon before we're ready to actually quit. This
// event will be triggered re-entrantly once preparation is done.
event.preventDefault();
shutdownDaemonAndQuit();
} else {
console.log('Quitting.');
} else if (process.platform == 'win32' && autoUpdateDeclined && !showingAutoUpdateCloseAlert) {
kauffj commented 2018-01-09 00:07:55 +01:00 (Migrated from github.com)
Review

"LBRY Will Upgrade"

"LBRY Will Upgrade"
kauffj commented 2018-01-09 00:08:51 +01:00 (Migrated from github.com)
Review

"LBRY has a pending upgrade. Please select "Yes" to install it on the prompt shown after this one."

(Assuming this is correct, and that prompt isn't shown until I ACK this one).

"LBRY has a pending upgrade. Please select "Yes" to install it on the prompt shown after this one." (Assuming this is correct, and that prompt isn't shown until I ACK this one).
alexliebowitz commented 2018-01-09 01:10:21 +01:00 (Migrated from github.com)
Review

You are correct, and I'll update the wording (along with all of the other wording changes you mentioned)

You are correct, and I'll update the wording (along with all of the other wording changes you mentioned)
// On Windows, if the user declined an update and closes the app later,
// they get a confusing permission escalation dialog, so we display a
// dialog to warn them.
event.preventDefault();
showingAutoUpdateCloseAlert = true;
dialog.showMessageBox({
type: "info",
title: "LBRY Will Upgrade",
message: "Please select \"Yes\" at the upgrade prompt shown after the app closes.",
}, () => {
// After the user approves the dialog, we can quit once and for all.
quitNow();
});
}
});
@ -549,7 +548,11 @@ ipcMain.on('version-info-requested', () => {
ipcMain.on('autoUpdate', () => {
minimize = false;
autoUpdater.quitAndInstall();
app.quit();
});
ipcMain.on('autoUpdateDeclined', () => {
autoUpdateDeclined = true;
});
ipcMain.on('get-auth-token', event => {

View file

@ -3,7 +3,6 @@ export const INCOMPATIBLE_DAEMON = 'incompatibleDaemon';
export const FILE_TIMEOUT = 'file_timeout';
export const DOWNLOADING = 'downloading';
export const AUTO_UPDATE_DOWNLOADED = "auto_update_downloaded";
export const UPDATE_CLOSE_ALERT = "updateCloseAlert";
export const ERROR = 'error';
export const INSUFFICIENT_CREDITS = 'insufficient_credits';
export const UPGRADE = 'upgrade';

View file

@ -9,7 +9,7 @@ import lbry from 'lbry';
import React from 'react';
import ReactDOM from 'react-dom';
import { Provider } from 'react-redux';
import { doConditionalAuthNavigate, doOpenModal, doDaemonReady, doShowSnackBar, doAutoUpdate } from 'redux/actions/app';
import { doConditionalAuthNavigate, doDaemonReady, doShowSnackBar, doAutoUpdate } from 'redux/actions/app';
import { doNavigate } from 'redux/actions/navigation';
import { doDownloadLanguages } from 'redux/actions/settings';
import { doUserEmailVerify } from 'redux/actions/user';
@ -62,7 +62,7 @@ ipcRenderer.on('window-is-focused', () => {
dock.setBadge('');
});
(function(history, ...args) {
((history, ...args) => {
const { replaceState } = history;
const newHistory = history;
newHistory.replaceState = (_, __, path) => {

View file

@ -2,7 +2,6 @@ import React from "react";
import { connect } from "react-redux";
import { doCloseModal } from "redux/actions/app";
import ModalAutoUpdateDownloaded from "./view";
import { doCloseModal } from "redux/actions/app";
const perform = dispatch => ({
closeModal: () => dispatch(doCloseModal()),

View file

@ -19,7 +19,10 @@ class ModalAutoUpdateDownloaded extends React.PureComponent {
kauffj commented 2018-01-09 00:14:26 +01:00 (Migrated from github.com)
Review

This isn't necessarily a blocker, but is there a strong reason we have to expose ipcRenderer inside of a component, as well as put all of the handling of these events inside of main/index.js? Could this component instead dispatch an APP_QUIT action, and a reducer could close the app? (Or the top-level component?)

This isn't necessarily a blocker, but is there a strong reason we have to expose `ipcRenderer` inside of a component, as well as put all of the handling of these events inside of `main/index.js`? Could this component instead dispatch an `APP_QUIT` action, and a reducer could close the app? (Or the top-level component?)
kauffj commented 2018-01-09 00:17:24 +01:00 (Migrated from github.com)
Review

"Use It Now"

"Use It Now"
kauffj commented 2018-01-09 00:20:18 +01:00 (Migrated from github.com)
Review

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.
kauffj commented 2018-01-09 00:22:04 +01:00 (Migrated from github.com)
Review

"Not Now" or "Upgrade on Restart"

"Not Now" or "Upgrade on Restart"
alexliebowitz commented 2018-01-09 01:27:36 +01:00 (Migrated from github.com)
Review

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed).

Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed). Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?
kauffj commented 2018-01-09 21:36:13 +01:00 (Migrated from github.com)
Review

I think this is fine to leave as-is, at least for now.

I think this is fine to leave as-is, at least for now.
kauffj commented 2018-01-09 00:14:26 +01:00 (Migrated from github.com)
Review

This isn't necessarily a blocker, but is there a strong reason we have to expose ipcRenderer inside of a component, as well as put all of the handling of these events inside of main/index.js? Could this component instead dispatch an APP_QUIT action, and a reducer could close the app? (Or the top-level component?)

This isn't necessarily a blocker, but is there a strong reason we have to expose `ipcRenderer` inside of a component, as well as put all of the handling of these events inside of `main/index.js`? Could this component instead dispatch an `APP_QUIT` action, and a reducer could close the app? (Or the top-level component?)
kauffj commented 2018-01-09 00:17:24 +01:00 (Migrated from github.com)
Review

"Use It Now"

"Use It Now"
kauffj commented 2018-01-09 00:20:18 +01:00 (Migrated from github.com)
Review

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.
kauffj commented 2018-01-09 00:22:04 +01:00 (Migrated from github.com)
Review

"Not Now" or "Upgrade on Restart"

"Not Now" or "Upgrade on Restart"
alexliebowitz commented 2018-01-09 01:27:36 +01:00 (Migrated from github.com)
Review

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed).

Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed). Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?
kauffj commented 2018-01-09 21:36:13 +01:00 (Migrated from github.com)
Review

I think this is fine to leave as-is, at least for now.

I think this is fine to leave as-is, at least for now.
onConfirmed={() => {
ipcRenderer.send("autoUpdate");
}}
onAborted={closeModal}
kauffj commented 2018-01-09 00:14:26 +01:00 (Migrated from github.com)
Review

This isn't necessarily a blocker, but is there a strong reason we have to expose ipcRenderer inside of a component, as well as put all of the handling of these events inside of main/index.js? Could this component instead dispatch an APP_QUIT action, and a reducer could close the app? (Or the top-level component?)

This isn't necessarily a blocker, but is there a strong reason we have to expose `ipcRenderer` inside of a component, as well as put all of the handling of these events inside of `main/index.js`? Could this component instead dispatch an `APP_QUIT` action, and a reducer could close the app? (Or the top-level component?)
kauffj commented 2018-01-09 00:17:24 +01:00 (Migrated from github.com)
Review

"Use It Now"

"Use It Now"
kauffj commented 2018-01-09 00:20:18 +01:00 (Migrated from github.com)
Review

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.
kauffj commented 2018-01-09 00:22:04 +01:00 (Migrated from github.com)
Review

"Not Now" or "Upgrade on Restart"

"Not Now" or "Upgrade on Restart"
alexliebowitz commented 2018-01-09 01:27:36 +01:00 (Migrated from github.com)
Review

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed).

Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed). Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?
kauffj commented 2018-01-09 21:36:13 +01:00 (Migrated from github.com)
Review

I think this is fine to leave as-is, at least for now.

I think this is fine to leave as-is, at least for now.
onAborted={() => {
kauffj commented 2018-01-09 00:14:26 +01:00 (Migrated from github.com)
Review

This isn't necessarily a blocker, but is there a strong reason we have to expose ipcRenderer inside of a component, as well as put all of the handling of these events inside of main/index.js? Could this component instead dispatch an APP_QUIT action, and a reducer could close the app? (Or the top-level component?)

This isn't necessarily a blocker, but is there a strong reason we have to expose `ipcRenderer` inside of a component, as well as put all of the handling of these events inside of `main/index.js`? Could this component instead dispatch an `APP_QUIT` action, and a reducer could close the app? (Or the top-level component?)
kauffj commented 2018-01-09 00:17:24 +01:00 (Migrated from github.com)
Review

"Use It Now"

"Use It Now"
kauffj commented 2018-01-09 00:20:18 +01:00 (Migrated from github.com)
Review

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.
kauffj commented 2018-01-09 00:22:04 +01:00 (Migrated from github.com)
Review

"Not Now" or "Upgrade on Restart"

"Not Now" or "Upgrade on Restart"
alexliebowitz commented 2018-01-09 01:27:36 +01:00 (Migrated from github.com)
Review

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed).

Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed). Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?
kauffj commented 2018-01-09 21:36:13 +01:00 (Migrated from github.com)
Review

I think this is fine to leave as-is, at least for now.

I think this is fine to leave as-is, at least for now.
ipcRenderer.send("autoUpdateDeclined");
kauffj commented 2018-01-09 00:14:26 +01:00 (Migrated from github.com)
Review

This isn't necessarily a blocker, but is there a strong reason we have to expose ipcRenderer inside of a component, as well as put all of the handling of these events inside of main/index.js? Could this component instead dispatch an APP_QUIT action, and a reducer could close the app? (Or the top-level component?)

This isn't necessarily a blocker, but is there a strong reason we have to expose `ipcRenderer` inside of a component, as well as put all of the handling of these events inside of `main/index.js`? Could this component instead dispatch an `APP_QUIT` action, and a reducer could close the app? (Or the top-level component?)
kauffj commented 2018-01-09 00:17:24 +01:00 (Migrated from github.com)
Review

"Use It Now"

"Use It Now"
kauffj commented 2018-01-09 00:20:18 +01:00 (Migrated from github.com)
Review

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.
kauffj commented 2018-01-09 00:22:04 +01:00 (Migrated from github.com)
Review

"Not Now" or "Upgrade on Restart"

"Not Now" or "Upgrade on Restart"
alexliebowitz commented 2018-01-09 01:27:36 +01:00 (Migrated from github.com)
Review

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed).

Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed). Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?
kauffj commented 2018-01-09 21:36:13 +01:00 (Migrated from github.com)
Review

I think this is fine to leave as-is, at least for now.

I think this is fine to leave as-is, at least for now.
closeModal();
kauffj commented 2018-01-09 00:14:26 +01:00 (Migrated from github.com)
Review

This isn't necessarily a blocker, but is there a strong reason we have to expose ipcRenderer inside of a component, as well as put all of the handling of these events inside of main/index.js? Could this component instead dispatch an APP_QUIT action, and a reducer could close the app? (Or the top-level component?)

This isn't necessarily a blocker, but is there a strong reason we have to expose `ipcRenderer` inside of a component, as well as put all of the handling of these events inside of `main/index.js`? Could this component instead dispatch an `APP_QUIT` action, and a reducer could close the app? (Or the top-level component?)
kauffj commented 2018-01-09 00:17:24 +01:00 (Migrated from github.com)
Review

"Use It Now"

"Use It Now"
kauffj commented 2018-01-09 00:20:18 +01:00 (Migrated from github.com)
Review

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.
kauffj commented 2018-01-09 00:22:04 +01:00 (Migrated from github.com)
Review

"Not Now" or "Upgrade on Restart"

"Not Now" or "Upgrade on Restart"
alexliebowitz commented 2018-01-09 01:27:36 +01:00 (Migrated from github.com)
Review

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed).

Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed). Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?
kauffj commented 2018-01-09 21:36:13 +01:00 (Migrated from github.com)
Review

I think this is fine to leave as-is, at least for now.

I think this is fine to leave as-is, at least for now.
}}
kauffj commented 2018-01-09 00:14:26 +01:00 (Migrated from github.com)
Review

This isn't necessarily a blocker, but is there a strong reason we have to expose ipcRenderer inside of a component, as well as put all of the handling of these events inside of main/index.js? Could this component instead dispatch an APP_QUIT action, and a reducer could close the app? (Or the top-level component?)

This isn't necessarily a blocker, but is there a strong reason we have to expose `ipcRenderer` inside of a component, as well as put all of the handling of these events inside of `main/index.js`? Could this component instead dispatch an `APP_QUIT` action, and a reducer could close the app? (Or the top-level component?)
kauffj commented 2018-01-09 00:17:24 +01:00 (Migrated from github.com)
Review

"Use It Now"

"Use It Now"
kauffj commented 2018-01-09 00:20:18 +01:00 (Migrated from github.com)
Review

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.
kauffj commented 2018-01-09 00:22:04 +01:00 (Migrated from github.com)
Review

"Not Now" or "Upgrade on Restart"

"Not Now" or "Upgrade on Restart"
alexliebowitz commented 2018-01-09 01:27:36 +01:00 (Migrated from github.com)
Review

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed).

Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed). Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?
kauffj commented 2018-01-09 21:36:13 +01:00 (Migrated from github.com)
Review

I think this is fine to leave as-is, at least for now.

I think this is fine to leave as-is, at least for now.
>
<section>
<h3 className="text-center">{__("LBRY Leveled Up")}</h3>

kauffj commented 2018-01-09 00:14:26 +01:00 (Migrated from github.com)
Review

This isn't necessarily a blocker, but is there a strong reason we have to expose ipcRenderer inside of a component, as well as put all of the handling of these events inside of main/index.js? Could this component instead dispatch an APP_QUIT action, and a reducer could close the app? (Or the top-level component?)

This isn't necessarily a blocker, but is there a strong reason we have to expose `ipcRenderer` inside of a component, as well as put all of the handling of these events inside of `main/index.js`? Could this component instead dispatch an `APP_QUIT` action, and a reducer could close the app? (Or the top-level component?)
kauffj commented 2018-01-09 00:17:24 +01:00 (Migrated from github.com)
Review

"Use It Now"

"Use It Now"
kauffj commented 2018-01-09 00:20:18 +01:00 (Migrated from github.com)
Review

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.
kauffj commented 2018-01-09 00:22:04 +01:00 (Migrated from github.com)
Review

"Not Now" or "Upgrade on Restart"

"Not Now" or "Upgrade on Restart"
alexliebowitz commented 2018-01-09 01:27:36 +01:00 (Migrated from github.com)
Review

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed).

Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed). Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?
kauffj commented 2018-01-09 21:36:13 +01:00 (Migrated from github.com)
Review

I think this is fine to leave as-is, at least for now.

I think this is fine to leave as-is, at least for now.
kauffj commented 2018-01-09 00:14:26 +01:00 (Migrated from github.com)
Review

This isn't necessarily a blocker, but is there a strong reason we have to expose ipcRenderer inside of a component, as well as put all of the handling of these events inside of main/index.js? Could this component instead dispatch an APP_QUIT action, and a reducer could close the app? (Or the top-level component?)

This isn't necessarily a blocker, but is there a strong reason we have to expose `ipcRenderer` inside of a component, as well as put all of the handling of these events inside of `main/index.js`? Could this component instead dispatch an `APP_QUIT` action, and a reducer could close the app? (Or the top-level component?)
kauffj commented 2018-01-09 00:17:24 +01:00 (Migrated from github.com)
Review

"Use It Now"

"Use It Now"
kauffj commented 2018-01-09 00:20:18 +01:00 (Migrated from github.com)
Review

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.

A new version of LBRY has been released, downloaded, and is ready for you to use pending a restart.
kauffj commented 2018-01-09 00:22:04 +01:00 (Migrated from github.com)
Review

"Not Now" or "Upgrade on Restart"

"Not Now" or "Upgrade on Restart"
alexliebowitz commented 2018-01-09 01:27:36 +01:00 (Migrated from github.com)
Review

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed).

Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?

I was going back and forth on this. Technically, neither of these IPC calls have anything to do with the store, and they do not lead to DOM updates. The "autoUpdateAccepted" IPC call tells the main process that it's time to shut down and restart (not UI state) and the "autoUpdateDeclined" IPC call just tells the main process to remember that the update was declined so that it can show the extra dialog on Windows (again, not UI state; the dialog is created by the main process and only appears after the app window is closed). Is it typical to put code in Redux-land when it is only used for its side effects and doesn't affect anything going on in the browser window?
kauffj commented 2018-01-09 21:36:13 +01:00 (Migrated from github.com)
Review

I think this is fine to leave as-is, at least for now.

I think this is fine to leave as-is, at least for now.

View file

@ -3,7 +3,6 @@ import ModalError from 'modal/modalError';
import ModalAuthFailure from 'modal/modalAuthFailure';
import ModalDownloading from 'modal/modalDownloading';
import ModalAutoUpdateDownloaded from "modal/modalAutoUpdateDownloaded";
import ModalUpdateCloseAlert from "modal/modalUpdateCloseAlert";
import ModalUpgrade from 'modal/modalUpgrade';
import ModalWelcome from 'modal/modalWelcome';
import ModalFirstReward from 'modal/modalFirstReward';
@ -106,8 +105,6 @@ class ModalRouter extends React.PureComponent {
return <ModalDownloading {...modalProps} />;
case modals.AUTO_UPDATE_DOWNLOADED:
return <ModalAutoUpdateDownloaded {...modalProps} />;
case modals.UPDATE_CLOSE_ALERT:
return <ModalUpdateCloseAlert {...modalProps} />;
case modals.ERROR:
return <ModalError {...modalProps} />;
case modals.FILE_TIMEOUT:

View file

@ -1,12 +0,0 @@
import React from 'react';
import { connect } from 'react-redux';
import { doQuit } from 'redux/actions/app';
import ModalUpdateCloseAlert from './view';
const select = state => ({});
const perform = dispatch => ({
quit: () => dispatch(doSkipUpgrade()),
});
export default connect(select, perform)(ModalUpdateCloseAlert);

View file

@ -1,23 +0,0 @@
import React from 'react';
import { Modal } from 'modal/modal';
import Link from 'component/link';
class ModalUpdateCloseAlert extends React.PureComponent {
render() {
const { quit } = this.props;
return (
<Modal
isOpen
type="alert"
confirmButtonLabel={__('Close Now')}
onConfirmed={quit}
>
<h3 className="text-center">{__('LBRY Will Upgrade')}</h3>
<p>{__('Please select yes to the upgrade prompt shown after the app closes.')}</p>
</Modal>
);
}
}
export default ModalUpdateCloseAlert;