From 4c452aaf4765f23a7c853b5b7a91db3d5d940726 Mon Sep 17 00:00:00 2001 From: Annie Date: Tue, 26 Jul 2016 15:01:52 -0700 Subject: [PATCH] fix(downloads): Add a check to see if last download directory is different, only show item in folder if true Summary: Each file downloaded would open finder and show the path to the file. Users were reporting that this felt excessive when downloading multiple files all to the same location #1044. I added a check to see if the path was the same as the previous file path, and only showed the item in the folder if these differed. Also added tests for this in file download store. test(downloads): Add tests for showing item in folder only if lastDownloadDirectory differs Test Plan: Added tests to file-download-store-spec Reviewers: bengotow Reviewed By: bengotow Differential Revision: https://phab.nylas.com/D3121 --- spec/stores/file-download-store-spec.coffee | 21 ++++++++++++++++++++- src/flux/stores/file-download-store.coffee | 7 +++++-- src/pro | 2 +- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/spec/stores/file-download-store-spec.coffee b/spec/stores/file-download-store-spec.coffee index fa02f6165..f2e765537 100644 --- a/spec/stores/file-download-store-spec.coffee +++ b/spec/stores/file-download-store-spec.coffee @@ -285,9 +285,28 @@ describe "FileDownloadStore", -> expect(shell.showItemInFolder).not.toHaveBeenCalled() @onEndEventCallback() advanceClock(1) + + it "should show file in folder if download path differs from previous download path", -> + spyOn(FileDownloadStore, '_saveDownload').andCallFake => + Promise.resolve(@testfile) + NylasEnv.savedState.lastDownloadDirectory = null + @userSelectedPath = "/Users/imaginary/.nylas/Another Random Folder/file.jpg" + FileDownloadStore._fetchAndSave(@testfile) + advanceClock(1) expect(shell.showItemInFolder).toHaveBeenCalledWith(@userSelectedPath) - it "should update the NylasEnv.savedState.lastDownloadDirectory", -> + it "should not show the file in the folder if the download path is the previous download path", -> + spyOn(FileDownloadStore, '_saveDownload').andCallFake => + Promise.resolve(@testfile) + @userSelectedPath = "/Users/imaginary/.nylas/Another Random Folder/123.png" + NylasEnv.savedState.lastDownloadDirectory = "/Users/imaginary/.nylas/Another Random Folder" + FileDownloadStore._fetchAndSave(@testfile) + advanceClock(1) + expect(shell.showItemInFolder).not.toHaveBeenCalled() + + it "should update the NylasEnv.savedState.lastDownloadDirectory if is has changed", -> + spyOn(FileDownloadStore, '_saveDownload').andCallFake => + Promise.resolve(@testfile) NylasEnv.savedState.lastDownloadDirectory = null @userSelectedPath = "/Users/imaginary/.nylas/Another Random Folder/file.jpg" FileDownloadStore._fetchAndSave(@testfile) diff --git a/src/flux/stores/file-download-store.coffee b/src/flux/stores/file-download-store.coffee index e45b615b9..d714f1bf3 100644 --- a/src/flux/stores/file-download-store.coffee +++ b/src/flux/stores/file-download-store.coffee @@ -231,8 +231,8 @@ FileDownloadStore = Reflux.createStore NylasEnv.showSaveDialog {defaultPath}, (savePath) => return unless savePath - NylasEnv.savedState.lastDownloadDirectory = path.dirname(savePath) + newDownloadDirectory = path.dirname(savePath) saveExtension = path.extname(savePath) didLoseExtension = defaultExtension isnt '' and saveExtension is '' if didLoseExtension @@ -240,7 +240,10 @@ FileDownloadStore = Reflux.createStore @_runDownload(file) .then (download) => @_saveDownload(download, savePath) - .then => shell.showItemInFolder(savePath) + .then => + if NylasEnv.savedState.lastDownloadDirectory isnt newDownloadDirectory + shell.showItemInFolder(savePath) + NylasEnv.savedState.lastDownloadDirectory = newDownloadDirectory .catch(@_catchFSErrors) .catch (error) => @_presentError({file, error}) diff --git a/src/pro b/src/pro index e04a35d00..aac7ee601 160000 --- a/src/pro +++ b/src/pro @@ -1 +1 @@ -Subproject commit e04a35d00a2320a915d4164c886c6d7dd3f0d8f3 +Subproject commit aac7ee6017a38af8a93d1a352e5fe3c0153db4a0