From c291a9067d8c5e8205b971226a01319b7a95a8a6 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Thu, 23 Jun 2016 18:39:15 -0400 Subject: [PATCH] fix: emit progress even when not in the main screen (#523) We have logic that displays useful log messages about the state of the flashing, as well as update the window progress in each oeprating system (dock in OS X, task bar in Windows, etc), however since this logic lives in the controller, the progress reports are completely frozen if the user navigates away from the main screen (to the settings page for example). As a solution, we move the code that subscribes to the state change events to a global `.run()` so it can persist page changes. Since making sure the listeners are unregistered is not relevant anymore (since the code is not running in a controller anymore), we get rid of `NotifierService`, a module we built for this purpose, and directly subscribe to the Redux store instead. Signed-off-by: Juan Cruz Viotti --- lib/gui/app.js | 33 +++++++--- lib/gui/modules/image-writer.js | 7 +-- lib/gui/utils/notifier/notifier.js | 31 ---------- lib/gui/utils/notifier/services/notifier.js | 67 --------------------- 4 files changed, 28 insertions(+), 110 deletions(-) delete mode 100644 lib/gui/utils/notifier/notifier.js delete mode 100644 lib/gui/utils/notifier/services/notifier.js diff --git a/lib/gui/app.js b/lib/gui/app.js index 450244a6..3b012988 100644 --- a/lib/gui/app.js +++ b/lib/gui/app.js @@ -22,6 +22,7 @@ var angular = require('angular'); const _ = require('lodash'); +const Store = require('./models/store'); const app = angular.module('Etcher', [ require('angular-ui-router'), @@ -60,7 +61,6 @@ const app = angular.module('Etcher', [ require('./os/dialog/dialog'), // Utils - require('./utils/notifier/notifier'), require('./utils/path/path'), require('./utils/manifest-bind/manifest-bind'), require('./utils/byte-size/byte-size') @@ -89,6 +89,31 @@ app.run((AnalyticsService, UpdateNotifierService, SelectionStateModel) => { }); +app.run((AnalyticsService, OSWindowProgressService, ImageWriterService) => { + Store.subscribe(() => { + const state = Store.getState().toJS(); + + // There is usually a short time period between the `isFlashing()` + // property being set, and the flashing actually starting, which + // might cause some non-sense flashing state logs including + // `undefined` values. + // + // We use the presence of `.eta` to determine that the actual + // writing started. + if (!ImageWriterService.isFlashing() || !state.flashState.eta) { + return; + } + + AnalyticsService.log([ + `Progress (${state.flashState.type}):`, + `${state.flashState.progress}% at ${state.flashState.speed} MB/s`, + `(eta ${state.flashState.eta}s)` + ].join(' ')); + + OSWindowProgressService.set(state.flashState.progress); + }); +}); + app.config(($stateProvider, $urlRouterProvider) => { $urlRouterProvider.otherwise('/main'); @@ -103,7 +128,6 @@ app.config(($stateProvider, $urlRouterProvider) => { app.controller('AppController', function( $state, $scope, - NotifierService, DriveScannerService, SelectionStateModel, SettingsModel, @@ -148,11 +172,6 @@ app.controller('AppController', function( }); } - NotifierService.subscribe($scope, 'image-writer:state', (state) => { - AnalyticsService.log(`Progress (${state.type}): ${state.progress}% at ${state.speed} MB/s (eta ${state.eta}s)`); - OSWindowProgressService.set(state.progress); - }); - DriveScannerService.start(); DriveScannerService.on('error', this.handleError); diff --git a/lib/gui/modules/image-writer.js b/lib/gui/modules/image-writer.js index bd48df07..aeaa94d3 100644 --- a/lib/gui/modules/image-writer.js +++ b/lib/gui/modules/image-writer.js @@ -26,11 +26,10 @@ const childWriter = require('../../src/child-writer'); const MODULE_NAME = 'Etcher.image-writer'; const imageWriter = angular.module(MODULE_NAME, [ - require('../models/settings'), - require('../utils/notifier/notifier') + require('../models/settings') ]); -imageWriter.service('ImageWriterService', function($q, $timeout, SettingsModel, NotifierService) { +imageWriter.service('ImageWriterService', function($q, $timeout, SettingsModel) { /** * @summary Reset flash state @@ -136,8 +135,6 @@ imageWriter.service('ImageWriterService', function($q, $timeout, SettingsModel, speed: Math.floor(state.speed / 1e+6 * 100) / 100 || 0 } }); - - NotifierService.emit('image-writer:state', this.state); }; /** diff --git a/lib/gui/utils/notifier/notifier.js b/lib/gui/utils/notifier/notifier.js deleted file mode 100644 index eb19e2f8..00000000 --- a/lib/gui/utils/notifier/notifier.js +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright 2016 Resin.io - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -/** - * @module Etcher.Utils.Notifier - * - * The purpose of this module is to provide a safe way - * to emit and receive events from the $rootScope. - */ - -const angular = require('angular'); -const MODULE_NAME = 'Etcher.Utils.Notifier'; -const Notifier = angular.module(MODULE_NAME, []); -Notifier.service('NotifierService', require('./services/notifier')); - -module.exports = MODULE_NAME; diff --git a/lib/gui/utils/notifier/services/notifier.js b/lib/gui/utils/notifier/services/notifier.js deleted file mode 100644 index bf7e0c56..00000000 --- a/lib/gui/utils/notifier/services/notifier.js +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Copyright 2016 Resin.io - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -/* - * Based on: - * http://www.codelord.net/2015/05/04/angularjs-notifying-about-changes-from-services-to-controllers/ - */ - -module.exports = function($rootScope) { - - /** - * @summary Safely subscribe to an event - * @function - * @public - * - * @description - * We say "safely" since this subscribe function will listen - * to the scope's `$destroy` event and unbind itself automatically. - * - * @param {Object} scope - angular scope - * @param {String} name - event name - * @param {Function} callback - callback - * - * @example - * NotifierService.subscribe($scope, 'my-event', () => { - * console.log('Event received!'); - * }); - */ - this.subscribe = (scope, name, callback) => { - const handler = $rootScope.$on(name, (event, data) => { - return callback(data); - }); - - scope.$on('$destroy', handler); - }; - - /** - * @summary Emit an event - * @function - * @public - * - * @param {String} name - event name - * @param {*} data - event data - * - * @example - * NotifierService.emit('my-event', 'Foo'); - */ - this.emit = (name, data) => { - $rootScope.$emit(name, data); - }; - -};