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
19 changed files with 366 additions and 45 deletions

View file

@ -79,6 +79,12 @@ if [ "$FULL_BUILD" == "true" ]; then
yarn build yarn build
# Workaround: TeamCity expects the dmg to be in dist/mac, but in the new electron-builder
# it's put directly in dist/ (the right way to solve this is to update the TeamCity config)
if $OSX; then
cp dist/*.dmg dist/mac
IGassmann commented 2018-01-23 19:35:36 +01:00 (Migrated from github.com)
Review

Wouldn't be better to change TeamCity config directly?

Wouldn't be better to change TeamCity config directly?
alexliebowitz commented 2018-01-23 20:52:21 +01:00 (Migrated from github.com)
Review

It would, but I figured we would worry about it later. It's already going to be a little complicated to roll out auto update.

It would, but I figured we would worry about it later. It's already going to be a little complicated to roll out auto update.
fi
# electron-build has a publish feature, but I had a hard time getting # electron-build has a publish feature, but I had a hard time getting
# it to reliably work and it also seemed difficult to configure. Not proud of # it to reliably work and it also seemed difficult to configure. Not proud of
# this, but it seemed better to write my own. # this, but it seemed better to write my own.

View file

@ -9,48 +9,42 @@ import github
import uritemplate import uritemplate
import boto3 import boto3
def main(): def main():
upload_to_github_if_tagged('lbryio/lbry-app') upload_to_github_if_tagged('lbryio/lbry-app')
upload_to_s3('app')
def get_asset_filename(): def get_asset_path():
this_dir = os.path.dirname(os.path.realpath(__file__)) this_dir = os.path.dirname(os.path.realpath(__file__))
system = platform.system() system = platform.system()
if system == 'Darwin': if system == 'Darwin':
return glob.glob(this_dir + '/../dist/LBRY*.dmg')[0] suffix = 'dmg'
elif system == 'Linux': elif system == 'Linux':
return glob.glob(this_dir + '/../dist/LBRY*.deb')[0] suffix = 'deb'
elif system == 'Windows': elif system == 'Windows':
return glob.glob(this_dir + '/../dist/LBRY*.exe')[0] suffix = 'exe'
else: else:
raise Exception("I don't know about any artifact on {}".format(system)) raise Exception("I don't know about any artifact on {}".format(system))
return os.path.realpath(glob.glob(this_dir + '/../dist/LBRY*.' + suffix)[0])
def upload_to_s3(folder): def get_update_asset_path():
tag = subprocess.check_output(['git', 'describe', '--always', '--abbrev=8', 'HEAD']).strip() # Get the asset used used for updates. On Mac, this is a .zip; on
commit_date = subprocess.check_output([ # Windows it's just the installer file.
'git', 'show', '-s', '--format=%cd', '--date=format:%Y%m%d-%H%I%S', 'HEAD']).strip() if platform.system() == 'Darwin':
this_dir = os.path.dirname(os.path.realpath(__file__))
return os.path.realpath(glob.glob(this_dir + '/../dist/LBRY*.zip')[0])
else:
return get_asset_path()
asset_path = get_asset_filename()
bucket = 'releases.lbry.io'
key = folder + '/' + commit_date + '-' + tag + '/' + os.path.basename(asset_path)
print "Uploading " + asset_path + " to s3://" + bucket + '/' + key + '' def get_latest_file_path():
# The update metadata file is called latest.yml on Windows, latest-mac.yml on
# Mac, latest-linux.yml on Linux
this_dir = os.path.dirname(os.path.realpath(__file__))
if 'AWS_ACCESS_KEY_ID' not in os.environ or 'AWS_SECRET_ACCESS_KEY' not in os.environ: latestfilematches = glob.glob(this_dir + '/../dist/latest*.yml')
print 'Must set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY to publish assets to s3'
return 1
s3 = boto3.resource(
's3',
aws_access_key_id=os.environ['AWS_ACCESS_KEY_ID'],
aws_secret_access_key=os.environ['AWS_SECRET_ACCESS_KEY'],
config=boto3.session.Config(signature_version='s3v4')
)
s3.meta.client.upload_file(asset_path, bucket, key)
return latestfilematches[0] if latestfilematches else None
def upload_to_github_if_tagged(repo_name): def upload_to_github_if_tagged(repo_name):
try: try:
@ -75,7 +69,7 @@ def upload_to_github_if_tagged(repo_name):
# TODO: maybe this should be an error # TODO: maybe this should be an error
return 1 return 1
asset_path = get_asset_filename() asset_path = get_asset_path()
print "Uploading " + asset_path + " to Github tag " + current_tag print "Uploading " + asset_path + " to Github tag " + current_tag
release = get_github_release(repo, current_tag) release = get_github_release(repo, current_tag)
upload_asset_to_github(release, asset_path, gh_token) upload_asset_to_github(release, asset_path, gh_token)

View file

@ -1,6 +1,10 @@
{ {
"appId": "io.lbry.LBRY", "appId": "io.lbry.LBRY",
"productName": "LBRY", "publish": {
"provider": "s3",
"bucket": "releases.lbry.io",
"path": "app/latest"
},
"mac": { "mac": {
"category": "public.app-category.entertainment" "category": "public.app-category.entertainment"
}, },

View file

@ -1,6 +1,6 @@
{ {
"name": "LBRY", "name": "LBRY",
"version": "0.19.4", "version": "0.19.4-dev",
"description": "A browser for the LBRY network, a digital marketplace controlled by its users.", "description": "A browser for the LBRY network, a digital marketplace controlled by its users.",
"homepage": "https://lbry.io/", "homepage": "https://lbry.io/",
"bugs": { "bugs": {
@ -38,6 +38,9 @@
"classnames": "^2.2.5", "classnames": "^2.2.5",
"country-data": "^0.0.31", "country-data": "^0.0.31",
"electron-dl": "^1.6.0", "electron-dl": "^1.6.0",
"electron-log": "^2.2.12",
"electron-publisher-s3": "^19.47.0",
"electron-updater": "^2.16.1",
"formik": "^0.10.4", "formik": "^0.10.4",
"from2": "^2.3.0", "from2": "^2.3.0",
"install": "^0.10.2", "install": "^0.10.2",

View file

@ -5,10 +5,25 @@ import SemVer from 'semver';
import url from 'url'; import url from 'url';
import https from 'https'; import https from 'https';
import { shell, app, ipcMain, dialog } from 'electron'; import { shell, app, ipcMain, dialog } from 'electron';
import { autoUpdater } from 'electron-updater';
import Daemon from './Daemon'; import Daemon from './Daemon';
import Tray from './Tray'; import Tray from './Tray';
import createWindow from './createWindow'; import createWindow from './createWindow';
autoUpdater.autoDownload = true;
// This is set to true if an auto update has been downloaded through the Electron
// auto-update system and is ready to install. If the user declined an update earlier,
// it will still install on shutdown.
let autoUpdateDownloaded = false;
// Keeps track of whether the user has accepted an auto-update through the interface.
let autoUpdateAccepted = false;
// This is used to keep track of whether we are showing the special dialog
// that we show on Windows after you decline an upgrade and close the app later.
let showingAutoUpdateCloseAlert = false;
// Keep a global reference, if you don't, they will be closed automatically when the JavaScript // Keep a global reference, if you don't, they will be closed automatically when the JavaScript
// object is garbage collected. // object is garbage collected.
let rendererWindow; let rendererWindow;
@ -68,7 +83,26 @@ app.on('activate', () => {
if (!rendererWindow) rendererWindow = createWindow(); if (!rendererWindow) rendererWindow = createWindow();
}); });
IGassmann commented 2018-01-23 19:57:04 +01:00 (Migrated from github.com)
Review

nit: use a more descriptive name (http://airbnb.io/javascript/#naming--descriptive)

nit: use a more descriptive name (http://airbnb.io/javascript/#naming--descriptive)
alexliebowitz commented 2018-01-23 20:51:25 +01:00 (Migrated from github.com)
Review

Does this apply even to e? That has a standard meaning of "event," similar to x, y, i, etc.

If we want to be even more specific than "event," it would have to be willQuitEvent or something, which sounds redundant

Does this apply even to `e`? That has a standard meaning of "event," similar to `x`, `y`, `i`, etc. If we want to be even more specific than "event," it would have to be `willQuitEvent` or something, which sounds redundant
IGassmann commented 2018-01-23 21:11:00 +01:00 (Migrated from github.com)
Review

I think event would fit best here. We don't have multiple events in this callback function so no need to specify precisely the event, and it would also be redundant as you pointed it out. Anyway, this is a nit.

I think `event` would fit best here. We don't have multiple events in this callback function so no need to specify precisely the event, and it would also be redundant as you pointed it out. Anyway, this is a nit.
app.on('will-quit', () => { app.on('will-quit', (event) => {
if (process.platform === 'win32' && autoUpdateDownloaded && !autoUpdateAccepted && !showingAutoUpdateCloseAlert) {
// We're on Win and have an update downloaded, but the user declined it (or closed
// the app without accepting it). Now the user is closing the app, so the new update
// will install. On Mac this is silent, but on Windows they get a confusing permission
// escalation dialog, so we show Windows users a warning dialog first.
showingAutoUpdateCloseAlert = true;
dialog.showMessageBox({
type: 'info',
title: 'LBRY Will Upgrade',
message: 'LBRY has a pending upgrade. Please select "Yes" to install it on the prompt shown after this one.',
}, () => {
app.quit();
});
event.preventDefault();
return;
}
isQuitting = true; isQuitting = true;
if (daemon) daemon.quit(); if (daemon) daemon.quit();
}); });
@ -108,6 +142,15 @@ ipcMain.on('upgrade', (event, installerPath) => {
app.quit(); app.quit();
}); });
autoUpdater.on('update-downloaded', () => {
autoUpdateDownloaded = true;
})
ipcMain.on('autoUpdateAccepted', () => {
autoUpdateAccepted = true;
autoUpdater.quitAndInstall();
});
ipcMain.on('version-info-requested', () => { ipcMain.on('version-info-requested', () => {
function formatRc(ver) { function formatRc(ver) {
// Adds dash if needed to make RC suffix SemVer friendly // Adds dash if needed to make RC suffix SemVer friendly
@ -195,4 +238,4 @@ const isSecondInstance = app.makeSingleInstance(argv => {
if (isSecondInstance) { if (isSecondInstance) {
app.exit(); app.exit();
} }

View file

@ -5,13 +5,14 @@ import { selectIsBackDisabled, selectIsForwardDisabled } from 'redux/selectors/n
import { selectBalance } from 'redux/selectors/wallet'; import { selectBalance } from 'redux/selectors/wallet';
import { doNavigate, doHistoryBack, doHistoryForward } from 'redux/actions/navigation'; import { doNavigate, doHistoryBack, doHistoryForward } from 'redux/actions/navigation';
import Header from './view'; import Header from './view';
import { selectIsUpgradeAvailable } from 'redux/selectors/app'; import { selectIsUpgradeAvailable, selectAutoUpdateDownloaded } from 'redux/selectors/app';
import { doDownloadUpgrade } from 'redux/actions/app'; import { doDownloadUpgradeRequested } from 'redux/actions/app';
const select = state => ({ const select = state => ({
isBackDisabled: selectIsBackDisabled(state), isBackDisabled: selectIsBackDisabled(state),
isForwardDisabled: selectIsForwardDisabled(state), isForwardDisabled: selectIsForwardDisabled(state),
isUpgradeAvailable: selectIsUpgradeAvailable(state), isUpgradeAvailable: selectIsUpgradeAvailable(state),
autoUpdateDownloaded: selectAutoUpdateDownloaded(state),
balance: formatCredits(selectBalance(state) || 0, 2), balance: formatCredits(selectBalance(state) || 0, 2),
}); });
@ -19,7 +20,7 @@ const perform = dispatch => ({
navigate: path => dispatch(doNavigate(path)), navigate: path => dispatch(doNavigate(path)),
back: () => dispatch(doHistoryBack()), back: () => dispatch(doHistoryBack()),
forward: () => dispatch(doHistoryForward()), forward: () => dispatch(doHistoryForward()),
downloadUpgrade: () => dispatch(doDownloadUpgrade()), downloadUpgradeRequested: () => dispatch(doDownloadUpgradeRequested()),
}); });
export default connect(select, perform)(Header); export default connect(select, perform)(Header);

View file

@ -10,8 +10,9 @@ export const Header = props => {
isBackDisabled, isBackDisabled,
isForwardDisabled, isForwardDisabled,
isUpgradeAvailable, isUpgradeAvailable,
autoUpdateDownloaded,
navigate, navigate,
downloadUpgrade, downloadUpgradeRequested,
} = props; } = props;
return ( return (
<header id="header"> <header id="header">
@ -86,9 +87,9 @@ export const Header = props => {
title={__('Settings')} title={__('Settings')}
/> />
</div> </div>
{isUpgradeAvailable && ( {(autoUpdateDownloaded || (process.platform === 'linux' && isUpgradeAvailable)) && (
<Link <Link
onClick={() => downloadUpgrade()} onClick={() => downloadUpgradeRequested()}
button="primary button--flat" button="primary button--flat"
icon="icon-arrow-up" icon="icon-arrow-up"
label={__('Upgrade App')} label={__('Upgrade App')}

View file

@ -28,6 +28,8 @@ export const UPDATE_VERSION = 'UPDATE_VERSION';
export const UPDATE_REMOTE_VERSION = 'UPDATE_REMOTE_VERSION'; export const UPDATE_REMOTE_VERSION = 'UPDATE_REMOTE_VERSION';
export const SKIP_UPGRADE = 'SKIP_UPGRADE'; export const SKIP_UPGRADE = 'SKIP_UPGRADE';
export const START_UPGRADE = 'START_UPGRADE'; export const START_UPGRADE = 'START_UPGRADE';
export const AUTO_UPDATE_DECLINED = 'AUTO_UPDATE_DECLINED';
export const AUTO_UPDATE_DOWNLOADED = 'AUTO_UPDATE_DOWNLOADED';
// Wallet // Wallet
export const GET_NEW_ADDRESS_STARTED = 'GET_NEW_ADDRESS_STARTED'; export const GET_NEW_ADDRESS_STARTED = 'GET_NEW_ADDRESS_STARTED';

View file

@ -2,6 +2,8 @@ export const CONFIRM_FILE_REMOVE = 'confirmFileRemove';
export const INCOMPATIBLE_DAEMON = 'incompatibleDaemon'; export const INCOMPATIBLE_DAEMON = 'incompatibleDaemon';
export const FILE_TIMEOUT = 'file_timeout'; export const FILE_TIMEOUT = 'file_timeout';
export const DOWNLOADING = 'downloading'; export const DOWNLOADING = 'downloading';
export const AUTO_UPDATE_DOWNLOADED = "auto_update_downloaded";
export const AUTO_UPDATE_CONFIRM = 'auto_update_confirm';
export const ERROR = 'error'; export const ERROR = 'error';
export const INSUFFICIENT_CREDITS = 'insufficient_credits'; export const INSUFFICIENT_CREDITS = 'insufficient_credits';
export const UPGRADE = 'upgrade'; export const UPGRADE = 'upgrade';

View file

@ -9,7 +9,7 @@ import lbry from 'lbry';
import React from 'react'; import React from 'react';
import ReactDOM from 'react-dom'; import ReactDOM from 'react-dom';
import { Provider } from 'react-redux'; import { Provider } from 'react-redux';
import { doConditionalAuthNavigate, doDaemonReady, doShowSnackBar } from 'redux/actions/app'; import { doConditionalAuthNavigate, doDaemonReady, doShowSnackBar, doAutoUpdate } from 'redux/actions/app';
import { doUpdateIsNightAsync } from 'redux/actions/settings'; import { doUpdateIsNightAsync } from 'redux/actions/settings';
import { doNavigate } from 'redux/actions/navigation'; import { doNavigate } from 'redux/actions/navigation';
import { doDownloadLanguages } from 'redux/actions/settings'; import { doDownloadLanguages } from 'redux/actions/settings';
@ -18,6 +18,15 @@ import 'scss/all.scss';
import store from 'store'; import store from 'store';
import app from './app'; import app from './app';
const { autoUpdater } = remote.require('electron-updater');
autoUpdater.logger = remote.require("electron-log");
window.addEventListener('contextmenu', event => {
contextMenu(remote.getCurrentWindow(), event.x, event.y, app.env === 'development');
event.preventDefault();
});
ipcRenderer.on('open-uri-requested', (event, uri, newSession) => { ipcRenderer.on('open-uri-requested', (event, uri, newSession) => {
if (uri && uri.startsWith('lbry://')) { if (uri && uri.startsWith('lbry://')) {
if (uri.startsWith('lbry://?verify=')) { if (uri.startsWith('lbry://?verify=')) {
@ -91,6 +100,22 @@ document.addEventListener('click', event => {
}); });
const init = () => { const init = () => {
autoUpdater.on("update-downloaded", () => {
app.store.dispatch(doAutoUpdate());
});
if (["win32", "darwin"].includes(process.platform)) {
autoUpdater.on("update-available", () => {
console.log("Update available");
});
autoUpdater.on("update-not-available", () => {
console.log("Update not available");
});
autoUpdater.on("update-downloaded", () => {
console.log("Update downloaded");
app.store.dispatch(doAutoUpdate());
});
}
app.store.dispatch(doUpdateIsNightAsync()); app.store.dispatch(doUpdateIsNightAsync());
app.store.dispatch(doDownloadLanguages()); app.store.dispatch(doDownloadLanguages());

View file

@ -0,0 +1,11 @@
import React from "react";
import { connect } from "react-redux";
import { doCloseModal, doAutoUpdateDeclined } from "redux/actions/app";
import ModalAutoUpdateConfirm from "./view";
const perform = dispatch => ({
closeModal: () => dispatch(doCloseModal()),
declineAutoUpdate: () => dispatch(doAutoUpdateDeclined()),
});
export default connect(null, perform)(ModalAutoUpdateConfirm);

View file

@ -0,0 +1,44 @@
import React from "react";
import { Modal } from "modal/modal";
import { Line } from "rc-progress";
import Link from "component/link/index";
const { ipcRenderer } = require("electron");
class ModalAutoUpdateConfirm extends React.PureComponent {
render() {
const { closeModal, declineAutoUpdate } = this.props;
return (
<Modal
isOpen={true}
type="confirm"
contentLabel={__("Update Downloaded")}
confirmButtonLabel={__("Upgrade")}
abortButtonLabel={__("Not now")}
onConfirmed={() => {
ipcRenderer.send("autoUpdateAccepted");
}}
onAborted={() => {
declineAutoUpdate();
closeModal();
}}
>
<section>
<h3 className="text-center">{__("LBRY Update Ready")}</h3>
<p>
{__(
'Your LBRY update is ready. Restart LBRY now to use it!'
)}
</p>
<p className="meta text-center">
{__('Want to know what has changed?')} See the{' '}
<Link label={__('release notes')} href="https://github.com/lbryio/lbry-app/releases" />.
</p>
</section>
</Modal>
);
}
}
export default ModalAutoUpdateConfirm;

View file

@ -0,0 +1,11 @@
import React from "react";
import { connect } from "react-redux";
import { doCloseModal, doAutoUpdateDeclined } from "redux/actions/app";
import ModalAutoUpdateDownloaded from "./view";
const perform = dispatch => ({
closeModal: () => dispatch(doCloseModal()),
declineAutoUpdate: () => dispatch(doAutoUpdateDeclined()),
});
export default connect(null, perform)(ModalAutoUpdateDownloaded);

View file

@ -0,0 +1,41 @@
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.
import React from "react";
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.
import { Modal } from "modal/modal";
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.
import { Line } from "rc-progress";
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.
import Link from "component/link/index";
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.
const { ipcRenderer } = require("electron");
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.
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.
render() {
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.
const { closeModal, declineAutoUpdate } = this.props;
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.
return (
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.
<Modal
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.
isOpen={true}
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.
type="confirm"
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.
contentLabel={__("Update Downloaded")}
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.
confirmButtonLabel={__("Use it 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.
abortButtonLabel={__("Upgrade on Close")}
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={() => {
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("autoUpdateAccepted");
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.
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.
declineAutoUpdate();
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.
>
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>
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.
<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:16:01 +01:00 (Migrated from github.com)
Review

I noticed a <br/> was dropped here.

I noticed a `<br/>` was dropped here.
alexliebowitz commented 2018-01-09 01:09:43 +01:00 (Migrated from github.com)
Review

There are other modals (most?) with no <br />, and it's not semantic, and seemed to look fine this way. What did you think of the look?

There are other modals (most?) with no `<br />`, and it's not semantic, and seemed to look fine this way. What did you think of the look?
<p>
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.
'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: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.
</p>
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>
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.
</Modal>
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.
}
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.
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.
export default ModalAutoUpdateDownloaded;
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

@ -2,7 +2,7 @@ import React from 'react';
import { connect } from 'react-redux'; import { connect } from 'react-redux';
import { doOpenModal } from 'redux/actions/app'; import { doOpenModal } from 'redux/actions/app';
import * as settings from 'constants/settings'; import * as settings from 'constants/settings';
import { selectCurrentModal, selectModalProps } from 'redux/selectors/app'; import { selectCurrentModal, selectModalProps, selectModalsAllowed } from 'redux/selectors/app';
import { selectCurrentPage } from 'redux/selectors/navigation'; import { selectCurrentPage } from 'redux/selectors/navigation';
import { selectCostForCurrentPageUri } from 'redux/selectors/cost_info'; import { selectCostForCurrentPageUri } from 'redux/selectors/cost_info';
import { makeSelectClientSetting } from 'redux/selectors/settings'; import { makeSelectClientSetting } from 'redux/selectors/settings';
@ -24,6 +24,7 @@ const select = (state, props) => ({
), ),
isWelcomeAcknowledged: makeSelectClientSetting(settings.NEW_USER_ACKNOWLEDGED)(state), isWelcomeAcknowledged: makeSelectClientSetting(settings.NEW_USER_ACKNOWLEDGED)(state),
user: selectUser(state), user: selectUser(state),
modalsAllowed: selectModalsAllowed(state),
}); });
const perform = dispatch => ({ const perform = dispatch => ({

View file

@ -2,6 +2,8 @@ import React from 'react';
import ModalError from 'modal/modalError'; import ModalError from 'modal/modalError';
import ModalAuthFailure from 'modal/modalAuthFailure'; import ModalAuthFailure from 'modal/modalAuthFailure';
import ModalDownloading from 'modal/modalDownloading'; import ModalDownloading from 'modal/modalDownloading';
import ModalAutoUpdateDownloaded from 'modal/modalAutoUpdateDownloaded';
import ModalAutoUpdateConfirm from 'modal/modalAutoUpdateConfirm';
import ModalUpgrade from 'modal/modalUpgrade'; import ModalUpgrade from 'modal/modalUpgrade';
import ModalWelcome from 'modal/modalWelcome'; import ModalWelcome from 'modal/modalWelcome';
import ModalFirstReward from 'modal/modalFirstReward'; import ModalFirstReward from 'modal/modalFirstReward';
@ -96,13 +98,17 @@ class ModalRouter extends React.PureComponent {
} }
render() { render() {
const { modal, modalProps } = this.props; const { modal, modalsAllowed, modalProps } = this.props;
switch (modal) { switch (modal) {
case modals.UPGRADE: case modals.UPGRADE:
return <ModalUpgrade {...modalProps} />; return <ModalUpgrade {...modalProps} />;
case modals.DOWNLOADING: case modals.DOWNLOADING:
return <ModalDownloading {...modalProps} />; return <ModalDownloading {...modalProps} />;
case modals.AUTO_UPDATE_DOWNLOADED:
return <ModalAutoUpdateDownloaded {...modalProps} />;
case modals.AUTO_UPDATE_CONFIRM:
return <ModalAutoUpdateConfirm {...modalProps} />;
case modals.ERROR: case modals.ERROR:
return <ModalError {...modalProps} />; return <ModalError {...modalProps} />;
case modals.FILE_TIMEOUT: case modals.FILE_TIMEOUT:

View file

@ -10,6 +10,8 @@ import { doAuthNavigate } from 'redux/actions/navigation';
import { doFetchDaemonSettings } from 'redux/actions/settings'; import { doFetchDaemonSettings } from 'redux/actions/settings';
import { doAuthenticate } from 'redux/actions/user'; import { doAuthenticate } from 'redux/actions/user';
import { doBalanceSubscribe } from 'redux/actions/wallet'; import { doBalanceSubscribe } from 'redux/actions/wallet';
import { doPause } from "redux/actions/media";
import { import {
selectCurrentModal, selectCurrentModal,
selectIsUpgradeSkipped, selectIsUpgradeSkipped,
@ -18,8 +20,10 @@ import {
selectUpgradeDownloadItem, selectUpgradeDownloadItem,
selectUpgradeDownloadPath, selectUpgradeDownloadPath,
selectUpgradeFilename, selectUpgradeFilename,
selectAutoUpdateDeclined,
} from 'redux/selectors/app'; } from 'redux/selectors/app';
const { autoUpdater } = remote.require('electron-updater');
const { download } = remote.require('electron-dl'); const { download } = remote.require('electron-dl');
const Fs = remote.require('fs'); const Fs = remote.require('fs');
const { lbrySettings: config } = require('package.json'); const { lbrySettings: config } = require('package.json');
@ -66,6 +70,41 @@ export function doStartUpgrade() {
}; };
} }
export function doDownloadUpgradeRequested() {
// This means the user requested an upgrade by clicking the "upgrade" button in the navbar.
// If on Mac and Windows, we do some new behavior for the auto-update system.
// This will probably be reorganized once we get auto-update going on Linux and remove
// the old logic.
return (dispatch, getState) => {
const state = getState();
// Pause video if needed
dispatch(doPause());
const autoUpdateDeclined = selectAutoUpdateDeclined(state);
if (['win32', 'darwin'].includes(process.platform)) { // electron-updater behavior
if (autoUpdateDeclined) {
// The user declined an update before, so show the "confirm" dialog
dispatch({
type: ACTIONS.OPEN_MODAL,
data: { modal: MODALS.AUTO_UPDATE_CONFIRM },
});
} else {
// The user was never shown the original update dialog (e.g. because they were
// watching a video). So show the inital "update downloaded" dialog.
dispatch({
type: ACTIONS.OPEN_MODAL,
data: { modal: MODALS.AUTO_UPDATE_DOWNLOADED },
});
}
} else { // Old behavior for Linux
dispatch(doDownloadUpgrade());
}
};
}
export function doDownloadUpgrade() { export function doDownloadUpgrade() {
return (dispatch, getState) => { return (dispatch, getState) => {
const state = getState(); const state = getState();
@ -105,6 +144,29 @@ export function doDownloadUpgrade() {
}; };
} }
export function doAutoUpdate() {
return function(dispatch, getState) {
const state = getState();
dispatch({
type: ACTIONS.AUTO_UPDATE_DOWNLOADED,
});
dispatch({
type: ACTIONS.OPEN_MODAL,
data: { modal: MODALS.AUTO_UPDATE_DOWNLOADED },
});
};
}
export function doAutoUpdateDeclined() {
return function(dispatch, getState) {
const state = getState();
dispatch({
type: ACTIONS.AUTO_UPDATE_DECLINED,
});
}
}
export function doCancelUpgrade() { export function doCancelUpgrade() {
return (dispatch, getState) => { return (dispatch, getState) => {
const state = getState(); const state = getState();
@ -135,6 +197,17 @@ export function doCheckUpgradeAvailable() {
type: ACTIONS.CHECK_UPGRADE_START, type: ACTIONS.CHECK_UPGRADE_START,
}); });
if (["win32", "darwin"].includes(process.platform)) {
// On Windows and Mac, updates happen silently through
// electron-updater.
const autoUpdateDeclined = selectAutoUpdateDeclined(state);
if (!autoUpdateDeclined) {
autoUpdater.checkForUpdates();
}
return;
}
const success = ({ remoteVersion, upgradeAvailable }) => { const success = ({ remoteVersion, upgradeAvailable }) => {
dispatch({ dispatch({
type: ACTIONS.CHECK_UPGRADE_SUCCESS, type: ACTIONS.CHECK_UPGRADE_SUCCESS,

View file

@ -26,6 +26,8 @@ export type AppState = {
hasSignature: boolean, hasSignature: boolean,
badgeNumber: number, badgeNumber: number,
volume: number, volume: number,
autoUpdateDeclined: boolean,
modalsAllowed: boolean,
downloadProgress: ?number, downloadProgress: ?number,
upgradeDownloading: ?boolean, upgradeDownloading: ?boolean,
upgradeDownloadComplete: ?boolean, upgradeDownloadComplete: ?boolean,
@ -46,6 +48,9 @@ const defaultState: AppState = {
hasSignature: false, hasSignature: false,
badgeNumber: 0, badgeNumber: 0,
volume: Number(sessionStorage.getItem('volume')) || 1, volume: Number(sessionStorage.getItem('volume')) || 1,
autoUpdateDownloaded: false,
autoUpdateDeclined: false,
modalsAllowed: true,
downloadProgress: undefined, downloadProgress: undefined,
upgradeDownloading: undefined, upgradeDownloading: undefined,
@ -79,6 +84,17 @@ reducers[ACTIONS.UPGRADE_CANCELLED] = state =>
modal: null, modal: null,
}); });
reducers[ACTIONS.AUTO_UPDATE_DOWNLOADED] = state =>
Object.assign({}, state, {
autoUpdateDownloaded: true,
});
reducers[ACTIONS.AUTO_UPDATE_DECLINED] = state => {
return Object.assign({}, state, {
autoUpdateDeclined: true,
});
}
reducers[ACTIONS.UPGRADE_DOWNLOAD_COMPLETED] = (state, action) => reducers[ACTIONS.UPGRADE_DOWNLOAD_COMPLETED] = (state, action) =>
Object.assign({}, state, { Object.assign({}, state, {
downloadPath: action.data.path, downloadPath: action.data.path,
@ -91,6 +107,11 @@ reducers[ACTIONS.UPGRADE_DOWNLOAD_STARTED] = state =>
upgradeDownloading: true, upgradeDownloading: true,
}); });
reducers[ACTIONS.CHANGE_MODALS_ALLOWED] = (state, action) =>
Object.assign({}, state, {
modalsAllowed: action.data.modalsAllowed,
});
reducers[ACTIONS.SKIP_UPGRADE] = state => { reducers[ACTIONS.SKIP_UPGRADE] = state => {
sessionStorage.setItem('upgradeSkipped', 'true'); sessionStorage.setItem('upgradeSkipped', 'true');
@ -100,6 +121,28 @@ reducers[ACTIONS.SKIP_UPGRADE] = state => {
}); });
}; };
reducers[ACTIONS.MEDIA_PLAY] = state => {
return Object.assign({}, state, {
modalsAllowed: false,
});
};
reducers[ACTIONS.MEDIA_PAUSE] = state => {
return Object.assign({}, state, {
modalsAllowed: true,
});
};
reducers[ACTIONS.SET_PLAYING_URI] = (state, action) => {
if (action.data.uri === null) {
return Object.assign({}, state, {
modalsAllowed: true,
});
} else {
return state;
}
};
reducers[ACTIONS.UPDATE_VERSION] = (state, action) => reducers[ACTIONS.UPDATE_VERSION] = (state, action) =>
Object.assign({}, state, { Object.assign({}, state, {
version: action.data.version, version: action.data.version,
@ -116,12 +159,16 @@ reducers[ACTIONS.CHECK_UPGRADE_SUBSCRIBE] = (state, action) =>
checkUpgradeTimer: action.data.checkUpgradeTimer, checkUpgradeTimer: action.data.checkUpgradeTimer,
}); });
reducers[ACTIONS.OPEN_MODAL] = (state, action) => reducers[ACTIONS.OPEN_MODAL] = (state, action) => {
Object.assign({}, state, { if (!state.modalsAllowed) {
modal: action.data.modal, return state;
modalProps: action.data.modalProps || {}, } else {
}); return Object.assign({}, state, {
modal: action.data.modal,
modalProps: action.data.modalProps || {},
});
}
};
reducers[ACTIONS.CLOSE_MODAL] = state => reducers[ACTIONS.CLOSE_MODAL] = state =>
Object.assign({}, state, { Object.assign({}, state, {
modal: undefined, modal: undefined,

View file

@ -56,6 +56,12 @@ export const selectUpgradeDownloadPath = createSelector(selectState, state => st
export const selectUpgradeDownloadItem = createSelector(selectState, state => state.downloadItem); export const selectUpgradeDownloadItem = createSelector(selectState, state => state.downloadItem);
export const selectAutoUpdateDownloaded = createSelector(selectState, state => state.autoUpdateDownloaded);
export const selectAutoUpdateDeclined = createSelector(selectState, state => state.autoUpdateDeclined);
export const selectModalsAllowed = createSelector(selectState, state => state.modalsAllowed);
export const selectModalProps = createSelector(selectState, state => state.modalProps); export const selectModalProps = createSelector(selectState, state => state.modalProps);
export const selectDaemonVersionMatched = createSelector( export const selectDaemonVersionMatched = createSelector(