Download Item Removal Confirmation

Fixes brave#2604

Updated test to handle confirm click
- rename downloadDeleteConfirmationVisible to deleteConfirmationVisible
- wrap spy code with try/finally to ensure proper cleanup
- confirmDeleteButton is checked independently from deleteButton

Auditors: @michalbe

app/renderer/components/downloadItem.js

@@ -21,6 +21,8 @@ class DownloadItem extends ImmutableComponent {
  this.onResumeDownload = this.onDownloadActionPerformed.bind(this, RESUME)
  this.onCancelDownload = this.onDownloadActionPerformed.bind(this, CANCEL)
  this.onClearDownload = this.onClearDownload.bind(this)
+ this.onShowDeleteConfirmation = this.onShowDeleteConfirmation.bind(this)
+ this.onHideDeleteConfirmation = this.onHideDeleteConfirmation.bind(this)
  this.onDeleteDownload = this.onDeleteDownload.bind(this)
  this.onRedownload = this.onRedownload.bind(this)
  this.onCopyLinkToClipboard = this.onCopyLinkToClipboard.bind(this)
@@ -34,7 +36,14 @@ class DownloadItem extends ImmutableComponent {
  onClearDownload () {
  appActions.downloadCleared(this.props.downloadId)
  }
+ onShowDeleteConfirmation () {
+ appActions.showDownloadDeleteConfirmation()
+ }
+ onHideDeleteConfirmation () {
+ appActions.hideDownloadDeleteConfirmation()
+ }
  onDeleteDownload () {
+ appActions.hideDownloadDeleteConfirmation()
  appActions.downloadDeleted(this.props.downloadId)
  }
  onDownloadActionPerformed (downloadAction) {
@@ -74,10 +83,17 @@ class DownloadItem extends ImmutableComponent {
  return <span
  onContextMenu={contextMenus.onDownloadsToolbarContextMenu.bind(null, this.props.downloadId, this.props.download)}
  onDoubleClick={this.onOpenDownload}
+ onMouseLeave={this.onHideDeleteConfirmation}
  className={cx({
  downloadItem: true,
+ deleteConfirmationVisible: this.props.deleteConfirmationVisible,
  [this.props.download.get('state')]: true
  })}>
+ {
+ this.props.deleteConfirmationVisible
+ ? <div className='deleteConfirmation'>Delete?<Button l10nId='ok' className='primaryButton confirmDeleteButton' onClick={this.onDeleteDownload} /></div>
+ : null
+ }
  <div className='downloadActions'>
  {
  downloadUtil.shouldAllowPause(this.props.download)
@@ -111,7 +127,7 @@ class DownloadItem extends ImmutableComponent {
  }
  {
  downloadUtil.shouldAllowDelete(this.props.download)
- ? <Button className='deleteButton' l10nId='downloadDelete' iconClass='fa-trash-o' onClick={this.onDeleteDownload} />
+ ? <Button className='deleteButton' l10nId='downloadDelete' iconClass='fa-trash-o' onClick={this.onShowDeleteConfirmation} />
  : null
  }
  {

app/renderer/components/downloadsBar.js

@@ -37,6 +37,7 @@ class DownloadsBar extends ImmutableComponent {
  .map((download, downloadId) =>
  <DownloadItem download={download}
  windowWidth={this.props.windowWidth}
+ deleteConfirmationVisible={this.props.deleteConfirmationVisible}
  downloadId={downloadId}
  downloadsSize={this.props.downloads.size} />)
  }

docs/appActions.md

@@ -628,6 +628,18 @@ Dispatches a message when a download is being redownloaded
 
 
 
+### showDownloadDeleteConfirmation() 
+
+Shows delete confirmation bar in download item panel
+
+
+
+### hideDownloadDeleteConfirmation() 
+
+Hides delete confirmation bar in download item panel
+
+
+
 ### clipboardTextCopied(text) 
 
 Dispatches a message when text is updated to the clipboard

js/actions/appActions.js

@@ -775,6 +775,24 @@ const appActions = {
  })
  },
 
+ /**
+ * Shows delete confirmation bar in download item panel
+ */
+ showDownloadDeleteConfirmation: function () {
+ AppDispatcher.dispatch({
+ actionType: appConstants.APP_SHOW_DOWNLOAD_DELETE_CONFIRMATION
+ })
+ },
+
+ /**
+ * Hides delete confirmation bar in download item panel
+ */
+ hideDownloadDeleteConfirmation: function () {
+ AppDispatcher.dispatch({
+ actionType: appConstants.APP_HIDE_DOWNLOAD_DELETE_CONFIRMATION
+ })
+ },
+
  /**
  * Dispatches a message when text is updated to the clipboard
  * @param {string} text - clipboard text which is copied

js/components/main.js

@@ -1298,6 +1298,7 @@ class Main extends ImmutableComponent {
  this.props.windowState.getIn(['ui', 'downloadsToolbar', 'isVisible']) && this.props.appState.get('downloads') && this.props.appState.get('downloads').size > 0
  ? <DownloadsBar
  windowWidth={window.innerWidth}
+ deleteConfirmationVisible={this.props.appState.get('deleteConfirmationVisible')}
  downloads={this.props.appState.get('downloads')} />
  : null
  }

js/constants/appConstants.js

@@ -78,6 +78,8 @@ const appConstants = {
  APP_DOWNLOAD_DELETED: _,
  APP_DOWNLOAD_CLEARED: _,
  APP_DOWNLOAD_REDOWNLOADED: _,
+ APP_SHOW_DOWNLOAD_DELETE_CONFIRMATION: _,
+ APP_HIDE_DOWNLOAD_DELETE_CONFIRMATION: _,
  APP_ALLOW_FLASH_ONCE: _,
  APP_ALLOW_FLASH_ALWAYS: _,
  APP_FLASH_PERMISSION_REQUESTED: _,

js/stores/appStore.js

@@ -885,6 +885,12 @@ const handleAppAction = (action) => {
  appState = appState.setIn(['sync', 'seedQr'], action.seedQr)
  }
  break
+ case appConstants.APP_SHOW_DOWNLOAD_DELETE_CONFIRMATION:
+ appState = appState.set('deleteConfirmationVisible', true)
+ break
+ case appConstants.APP_HIDE_DOWNLOAD_DELETE_CONFIRMATION:
+ appState = appState.set('deleteConfirmationVisible', false)
+ break
  default:
  }
 

less/downloadBar.less

@@ -45,7 +45,7 @@
 
  &:hover {
  height: 62px;
- top: -24px;
+ top: -23px;
 
  .downloadInfo .downloadArrow {
  display: none;
@@ -60,6 +60,10 @@
  .downloadActions {
  display: none;
  }
+
+ .deleteConfirmation {
+ display: none;
+ }
  }
 
  &.paused {
@@ -75,6 +79,11 @@
  }
  }
 
+ &.deleteConfirmationVisible:hover {
+ height: 97px;
+ top: -58px;
+ }
+
  .downloadProgress {
  background-color: @highlightBlue;
  left: 0;
@@ -125,6 +134,22 @@
  }
  }
  }
+
+ .deleteConfirmation {
+ line-height: 2;
+ border-bottom: 1px solid #CCC;
+ padding: 5px 0;
+ margin-bottom: auto 0 10px 0;
+ font-size: 12px;
+
+ .confirmDeleteButton {
+ font-weight: normal;
+ padding: 1px;
+ min-width: 50px;
+ float: right;
+ margin-right: -5px;
+ }
+ }
  }
  }
 
@@ -150,5 +175,3 @@
  }
  }
 }
-
-

test/unit/app/renderer/downloadItemTest.js

@@ -1,7 +1,7 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
-/* global describe, before, after, it */
+/* global describe, before, beforeEach, after, it */
 
 const mockery = require('mockery')
 const {mount} = require('enzyme')
@@ -25,6 +25,7 @@ const newDownload = (state) => Immutable.fromJS({
  url: downloadUrl,
  totalBytes: 104729,
  receivedBytes: 96931,
+ deleteConfirmationVisible: false,
  state
 })
 
@@ -39,6 +40,7 @@ describe('downloadItem component', function () {
  DownloadItem = require('../../../../app/renderer/components/downloadItem')
  appActions = require('../../../../js/actions/appActions')
  })
+
  after(function () {
  mockery.disable()
  })
@@ -48,13 +50,19 @@ describe('downloadItem component', function () {
  before(function () {
  this.downloadId = uuid.v4()
  this.download = newDownload(state)
- this.result = mount(<DownloadItem downloadId={this.downloadId} download={newDownload(state)} />)
+ this.result = mount(
+ <DownloadItem
+ downloadId={this.downloadId}
+ download={this.download}
+ deleteConfirmationVisible={this.download.get('deleteConfirmationVisible')} />
+ )
  })
 
- const shouldProgressBarExist = [downloadStates.IN_PROGRESS, downloadStates.PAUSED].includes(state)
  it('filename exists and matches download filename', function () {
  assert.equal(this.result.find('.downloadFilename').text(), this.download.get('filename'))
  })
+
+ const shouldProgressBarExist = [downloadStates.IN_PROGRESS, downloadStates.PAUSED].includes(state)
  it(shouldProgressBarExist ? 'progress bar should exist' : 'progress bar should not exist', function () {
  assert.equal(this.result.find('.downloadProgress').length, shouldProgressBarExist ? 1 : 0)
  })
@@ -114,11 +122,39 @@ describe('downloadItem component', function () {
  })
 
  testButton('.deleteButton', [downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.COMPLETED], function (button) {
- const spy = sinon.spy(appActions, 'downloadDeleted')
- button.simulate('click')
- assert(spy.withArgs(this.downloadId).calledOnce)
- appActions.downloadDeleted.restore()
+ const spy = sinon.spy(appActions, 'showDownloadDeleteConfirmation')
+ try {
+ // Confirmation should NOT be visible by default
+ assert.equal(this.result.find('.confirmDeleteButton').length, 0)
+
+ // Clicking delete should show confirmation
+ button.simulate('click')
+ assert(spy.called)
+ } finally {
+ appActions.showDownloadDeleteConfirmation.restore()
+ }
  })
+
+ if ([downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.COMPLETED].includes(state)) {
+ describe('when delete button has been clicked', function () {
+ beforeEach(function () {
+ this.result.setProps({
+ deleteConfirmationVisible: true
+ })
+ })
+
+ testButton('.confirmDeleteButton', [downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.COMPLETED], function (button) {
+ const spy = sinon.spy(appActions, 'downloadDeleted')
+ try {
+ // Accepting confirmation should delete the item
+ button.simulate('click')
+ assert(spy.withArgs(this.downloadId).calledOnce)
+ } finally {
+ appActions.downloadDeleted.restore()
+ }
+ })
+ })
+ }
  })
  })
 })