From c17247da586c00f9f3a0d54e01ed18b97f60355b Mon Sep 17 00:00:00 2001 From: Benedict Aas Date: Thu, 5 Apr 2018 17:26:16 +0100 Subject: [PATCH] feat(GUI): move drive selector warning to flash step (#1917) We move the drive selector warning to the flash step, and concatenate warning messages when more than one needs to be displayed at once. Change-Type: patch Changelog-Entry: Move the drive selector warning dialog to the flash step. --- .../controllers/drive-selector.js | 27 +---- .../drive-selector/drive-selector.js | 1 - .../warning-modal/styles/_warning-modal.scss | 5 + lib/gui/app/pages/main/controllers/flash.js | 110 +++++++++++++++--- lib/gui/app/pages/main/main.js | 1 + lib/gui/css/main.css | 4 + 6 files changed, 108 insertions(+), 40 deletions(-) diff --git a/lib/gui/app/components/drive-selector/controllers/drive-selector.js b/lib/gui/app/components/drive-selector/controllers/drive-selector.js index 6d2c686e..cf5dd1e3 100644 --- a/lib/gui/app/components/drive-selector/controllers/drive-selector.js +++ b/lib/gui/app/components/drive-selector/controllers/drive-selector.js @@ -18,7 +18,6 @@ const angular = require('angular') const _ = require('lodash') -const messages = require('../../../../../shared/messages') const constraints = require('../../../../../shared/drive-constraints') const analytics = require('../../../modules/analytics') const availableDrives = require('../../../../../shared/models/available-drives') @@ -27,8 +26,7 @@ const utils = require('../../../../../shared/utils') module.exports = function ( $q, - $uibModalInstance, - WarningModalService + $uibModalInstance ) { /** * @summary The drive selector state @@ -72,28 +70,7 @@ module.exports = function ( * }); */ const shouldChangeDriveSelectionState = (drive) => { - if (!constraints.isDriveValid(drive, selectionState.getImage())) { - return $q.resolve(false) - } - - if (!constraints.isDriveSizeRecommended(drive, selectionState.getImage())) { - return WarningModalService.display({ - confirmationLabel: 'Yes, continue', - description: [ - messages.warning.unrecommendedDriveSize(selectionState.getImage(), drive), - 'Are you sure you want to continue?' - ].join(' ') - }) - } - - if (constraints.isDriveSizeLarge(drive)) { - return WarningModalService.display({ - confirmationLabel: 'Yes, continue', - description: messages.warning.largeDriveSize(drive) - }) - } - - return $q.resolve(true) + return $q.resolve(constraints.isDriveValid(drive, selectionState.getImage())) } /** diff --git a/lib/gui/app/components/drive-selector/drive-selector.js b/lib/gui/app/components/drive-selector/drive-selector.js index f5dbeabb..f41f6e30 100644 --- a/lib/gui/app/components/drive-selector/drive-selector.js +++ b/lib/gui/app/components/drive-selector/drive-selector.js @@ -24,7 +24,6 @@ const angular = require('angular') const MODULE_NAME = 'Etcher.Components.DriveSelector' const DriveSelector = angular.module(MODULE_NAME, [ require('../modal/modal'), - require('../warning-modal/warning-modal'), require('../../utils/byte-size/byte-size') ]) diff --git a/lib/gui/app/components/warning-modal/styles/_warning-modal.scss b/lib/gui/app/components/warning-modal/styles/_warning-modal.scss index a1bdb84d..41d2f874 100644 --- a/lib/gui/app/components/warning-modal/styles/_warning-modal.scss +++ b/lib/gui/app/components/warning-modal/styles/_warning-modal.scss @@ -21,3 +21,8 @@ .modal-warning-modal .modal-title .glyphicon { color: $palette-theme-danger-background; } + +.modal-warning-modal .modal-body { + max-height: 200px; + overflow-y: auto; +} diff --git a/lib/gui/app/pages/main/controllers/flash.js b/lib/gui/app/pages/main/controllers/flash.js index e5d158da..1d7a8eba 100644 --- a/lib/gui/app/pages/main/controllers/flash.js +++ b/lib/gui/app/pages/main/controllers/flash.js @@ -16,6 +16,7 @@ 'use strict' +const _ = require('lodash') const messages = require('../../../../../shared/messages') const flashState = require('../../../../../shared/models/flash-state') const driveScanner = require('../../../modules/drive-scanner') @@ -25,19 +26,88 @@ const exceptionReporter = require('../../../modules/exception-reporter') const imageWriter = require('../../../modules/image-writer') const path = require('path') const store = require('../../../../../shared/store') +const constraints = require('../../../../../shared/drive-constraints') +const availableDrives = require('../../../../../shared/models/available-drives') module.exports = function ( + $q, $state, $timeout, - FlashErrorModalService + FlashErrorModalService, + WarningModalService, + DriveSelectorService ) { + /** + * @summary Spawn a confirmation warning modal + * @function + * @public + * + * @param {Array} warningMessages - warning messages + * @returns {Promise} warning modal promise + * + * @example + * confirmationWarningModal([ 'Hello, World!' ]) + */ + const confirmationWarningModal = (warningMessages) => { + return WarningModalService.display({ + confirmationLabel: 'Continue', + rejectionLabel: 'Change', + description: [ + warningMessages.join('\n\n'), + 'Are you sure you want to continue?' + ].join(' ') + }) + } + + /** + * @summary Display warning tailored to the warning of the current drives-image pair + * @function + * @public + * + * @param {Array} drives - list of drive objects + * @param {Object} image - image object + * @returns {Promise} + * + * @example + * displayTailoredWarning(drives, image).then(() => { + * console.log('Continue pressed') + * }).catch(() => { + * console.log('Change pressed') + * }) + */ + const displayTailoredWarning = (drives, image) => { + const warningMessages = _.reduce(drives, (accumMessages, drive) => { + if (constraints.isDriveSizeLarge(drive)) { + return accumMessages.concat(messages.warning.largeDriveSize(drive)) + } else if (!constraints.isDriveSizeRecommended(drive, image)) { + return accumMessages.concat(messages.warning.unrecommendedDriveSize(image, drive)) + } + + return accumMessages + }, []) + + if (!warningMessages.length) { + // TODO(Shou): we should consider adding the same warning dialog for system drives and remove unsafe mode + return $q.resolve() + } + + return confirmationWarningModal(warningMessages).then((value) => { + if (!value) { + DriveSelectorService.open() + return $q.reject() + } + + return $q.resolve() + }) + } + /** * @summary Flash image to drives * @function * @public * * @param {Object} image - image - * @param {Array} drives - drives + * @param {Array} devices - list of drive device paths * * @example * FlashController.flashImageToDrive({ @@ -49,22 +119,23 @@ module.exports = function ( * value: 1000000000 * } * } - * }, { - * device: '/dev/disk2', - * description: 'Foo', - * size: 99999, - * mountpoint: '/mnt/foo', - * system: false - * }) + * }, [ + * '/dev/disk2', + * '/dev/disk5' + * ]) */ - this.flashImageToDrive = (image, drives) => { + this.flashImageToDrive = (image, devices) => { if (flashState.isFlashing()) { return } - // Stop scanning drives when flashing - // otherwise Windows throws EPERM - driveScanner.stop() + const drives = _.filter(availableDrives.getDrives(), (drive) => { + return _.includes(devices, drive.device) + }) + + const hasDangerStatus = constraints.hasListDriveImageCompatibilityStatus(drives, image) + + const userConfirm = hasDangerStatus ? _.partial(displayTailoredWarning, drives, image) : $q.resolve // Trigger Angular digests along with store updates, as the flash state // updates. Without this there is essentially no progress to watch. @@ -72,7 +143,13 @@ module.exports = function ( const iconPath = '../../../assets/icon.png' - imageWriter.flash(image.path, drives).then(() => { + userConfirm().then(() => { + // Stop scanning drives when flashing + // otherwise Windows throws EPERM + driveScanner.stop() + + return imageWriter.flash(image.path, devices) + }).then(() => { if (!flashState.wasLastFlashCancelled()) { notification.send('Success!', { body: messages.info.flashComplete(path.basename(image.path), drives), @@ -81,6 +158,11 @@ module.exports = function ( $state.go('success') } }).catch((error) => { + // When flashing is cancelled before starting above there is no error + if (!error) { + return + } + notification.send('Oops! Looks like the flash failed.', { body: messages.error.flashFailure(path.basename(image.path), drives), icon: iconPath diff --git a/lib/gui/app/pages/main/main.js b/lib/gui/app/pages/main/main.js index 2d6ec4c9..609399be 100644 --- a/lib/gui/app/pages/main/main.js +++ b/lib/gui/app/pages/main/main.js @@ -37,6 +37,7 @@ const MainPage = angular.module(MODULE_NAME, [ require('../../components/tooltip-modal/tooltip-modal'), require('../../components/flash-error-modal/flash-error-modal'), require('../../components/progress-button/progress-button'), + require('../../components/warning-modal/warning-modal'), require('../../os/open-external/open-external'), require('../../os/dropzone/dropzone'), diff --git a/lib/gui/css/main.css b/lib/gui/css/main.css index 13e60ab0..e260d7d1 100644 --- a/lib/gui/css/main.css +++ b/lib/gui/css/main.css @@ -6483,6 +6483,10 @@ svg-icon { .modal-warning-modal .modal-title .glyphicon, .modal-warning-modal .modal-title .tick { color: #d9534f; } +.modal-warning-modal .modal-body { + max-height: 200px; + overflow-y: auto; } + /* * Copyright 2016 resin.io *