From d3881ebc5873140c51227ebc2a856c830f6177a5 Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Fri, 3 Feb 2017 11:37:18 +0100 Subject: [PATCH] Added default download path Resolves #2110 Auditors: @bradleyrichter @bsclifton Test Plan: - go to preferences, general tab - define your default download path - toggle show dialog - based on your settings, download dialog should be opened or not - if you set don't show dialog, filse should be downloaded to your default set path # Added hover effect --- app/browser/reducers/downloadsReducer.js | 19 +++++- .../locales/en-US/preferences.properties | 2 + app/filtering.js | 4 +- app/renderer/components/settings.js | 64 ++++++++++++++++++- docs/appActions.md | 6 ++ img/icon_pencil.svg | 16 +++++ js/about/preferences.js | 18 +++++- js/actions/appActions.js | 9 +++ js/constants/appConfig.js | 2 + js/constants/appConstants.js | 1 + js/constants/settings.js | 2 + test/unit/about/preferencesTest.js | 22 ++++++- .../browser/reducers/downloadsReducerTest.js | 30 +++++++++ test/unit/lib/fakeElectron.js | 6 +- 14 files changed, 193 insertions(+), 8 deletions(-) create mode 100644 img/icon_pencil.svg diff --git a/app/browser/reducers/downloadsReducer.js b/app/browser/reducers/downloadsReducer.js index 02547ab1200..5f4d16da032 100644 --- a/app/browser/reducers/downloadsReducer.js +++ b/app/browser/reducers/downloadsReducer.js @@ -6,17 +6,20 @@ const appConstants = require('../../../js/constants/appConstants') const downloadStates = require('../../../js/constants/downloadStates') -const {clipboard, BrowserWindow, shell} = require('electron') +const settings = require('../../../js/constants/settings') +const {clipboard, BrowserWindow, shell, dialog, app} = require('electron') const fs = require('fs') const path = require('path') const {cancelDownload, pauseDownload, resumeDownload} = require('../electronDownloadItem') const {CANCEL, PAUSE, RESUME} = require('../../common/constants/electronDownloadItemActions') +const appActions = require('../../../js/actions/appActions') const downloadsReducer = (state, action) => { const download = action.downloadId ? state.getIn(['downloads', action.downloadId]) : undefined if (!download && ![appConstants.APP_MERGE_DOWNLOAD_DETAIL, - appConstants.APP_CLEAR_COMPLETED_DOWNLOADS].includes(action.actionType)) { + appConstants.APP_CLEAR_COMPLETED_DOWNLOADS, + appConstants.APP_DOWNLOAD_DEFAULT_PATH].includes(action.actionType)) { return state } switch (action.actionType) { @@ -89,6 +92,18 @@ const downloadsReducer = (state, action) => { state = state.set('downloads', downloads) } break + case appConstants.APP_DOWNLOAD_DEFAULT_PATH: + const focusedWindow = BrowserWindow.getFocusedWindow() + + dialog.showOpenDialog(focusedWindow, { + defaultPath: app.getPath('downloads'), + properties: ['openDirectory'] + }, (folder) => { + if (Array.isArray(folder) && fs.lstatSync(folder[0]).isDirectory()) { + appActions.changeSetting(settings.DOWNLOAD_DEFAULT_PATH, folder[0]) + } + }) + break } return state } diff --git a/app/extensions/brave/locales/en-US/preferences.properties b/app/extensions/brave/locales/en-US/preferences.properties index 03d8f9c1cc7..7b887f35bf1 100644 --- a/app/extensions/brave/locales/en-US/preferences.properties +++ b/app/extensions/brave/locales/en-US/preferences.properties @@ -174,6 +174,8 @@ newTabDefaultSearchEngine=Default search engine newTabEmpty=Blank page myHomepage=My home page is multipleHomePages.title=Multiple home pages +downloadDefaultPath=Save my downloads here: +downloadAlwaysAsk=Always ask me where to save files aboutBlank=about:blank default=Default searchEngine=Search Engine diff --git a/app/filtering.js b/app/filtering.js index 0a581c20bae..5b25946508b 100644 --- a/app/filtering.js +++ b/app/filtering.js @@ -498,8 +498,8 @@ function registerForDownloadListener (session) { itemFilename = item.getFilename() } - const defaultPath = path.join(getSetting(settings.DEFAULT_DOWNLOAD_SAVE_PATH) || app.getPath('downloads'), itemFilename) - const savePath = (process.env.SPECTRON ? defaultPath : dialog.showSaveDialog(win, { defaultPath })) + const defaultPath = path.join(getSetting(settings.DOWNLOAD_DEFAULT_PATH) || getSetting(settings.DEFAULT_DOWNLOAD_SAVE_PATH) || app.getPath('downloads'), itemFilename) + const savePath = ((process.env.SPECTRON || !getSetting(settings.DOWNLOAD_ALWAYS_ASK)) ? defaultPath : dialog.showSaveDialog(win, { defaultPath })) // User cancelled out of save dialog prompt if (!savePath) { diff --git a/app/renderer/components/settings.js b/app/renderer/components/settings.js index 156b32ba898..c4106edd8f8 100644 --- a/app/renderer/components/settings.js +++ b/app/renderer/components/settings.js @@ -142,9 +142,71 @@ class SiteSettingCheckbox extends ImmutableComponent { } } +class SettingItemIcon extends ImmutableComponent { + render () { + const bg = StyleSheet.create({ + Icon: { + '-webkit-mask-image': `url(${this.props.icon})` + } + }) + + return
+ { + this.props.dataL10nId + ? + : null + } +
+ { + this.props.position === 'left' + ? + : null + } +
+ {this.props.children} +
+ { + this.props.position === 'right' + ? + : null + } +
+
+ } +} + +const styles = StyleSheet.create({ + icon: { + backgroundColor: '#5a5a5a', + height: '16px', + width: '16px', + display: 'inline-block', + verticalAlign: 'top', + padding: '0px', + '-webkit-mask-repeat': 'no-repeat', + + ':hover': { + backgroundColor: '#000' + } + }, + + iconLeft: { + margin: '8px 10px 0 0' + }, + + iconRight: { + margin: '8px 0 0 10px' + }, + + child: { + display: 'inline-block' + } +}) + module.exports = { SettingsList, SettingItem, SettingCheckbox, - SiteSettingCheckbox + SiteSettingCheckbox, + SettingItemIcon } diff --git a/docs/appActions.md b/docs/appActions.md index 1db6bd31460..7d18c235ebd 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -792,6 +792,12 @@ Action triggered by un-registering navigation handler +### defaultDownloadPath() + +Open dialog for default download path setting + + + * * * diff --git a/img/icon_pencil.svg b/img/icon_pencil.svg new file mode 100644 index 00000000000..db5ca30ec8c --- /dev/null +++ b/img/icon_pencil.svg @@ -0,0 +1,16 @@ + + + + + + + + + + diff --git a/js/about/preferences.js b/js/about/preferences.js index 63d9063566a..a57ddf44fae 100644 --- a/js/about/preferences.js +++ b/js/about/preferences.js @@ -10,7 +10,7 @@ const UrlUtil = require('../lib/urlutil') // Components const PreferenceNavigation = require('../../app/renderer/components/preferences/preferenceNavigation') -const {SettingsList, SettingItem, SettingCheckbox} = require('../../app/renderer/components/settings') +const {SettingsList, SettingItem, SettingCheckbox, SettingItemIcon} = require('../../app/renderer/components/settings') const {SettingTextbox} = require('../../app/renderer/components/textbox') const {SettingDropdown} = require('../../app/renderer/components/dropdown') const Button = require('../components/button') @@ -32,6 +32,7 @@ const {passwordManagers, extensionIds} = require('../constants/passwordManagers' const {startsWithOption, newTabMode, bookmarksToolbarMode, tabCloseAction, fullscreenOption} = require('../../app/common/constants/settingsEnums') const aboutActions = require('./aboutActions') +const appActions = require('../actions/appActions') const getSetting = require('../settings').getSetting const SortableTable = require('../components/sortableTable') const searchProviders = require('../data/searchProviders') @@ -114,6 +115,10 @@ class GeneralTab extends ImmutableComponent { return keyArray.every((key) => getSetting(key, this.props.settings) === true) } + openDownloadDialog () { + appActions.defaultDownloadPath() + } + render () { const languageOptions = this.props.languageCodes.map(function (lc) { return ( @@ -183,6 +188,17 @@ class GeneralTab extends ImmutableComponent { isDarwin ? null : } + + + + diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 2a442bc5edf..09d2268b3a7 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -970,6 +970,15 @@ const appActions = { protocol, location }) + }, + + /** + * Open dialog for default download path setting + */ + defaultDownloadPath: function () { + AppDispatcher.dispatch({ + actionType: appConstants.APP_DOWNLOAD_DEFAULT_PATH + }) } } diff --git a/js/constants/appConfig.js b/js/constants/appConfig.js index e065e8f9ba8..32edf5c3b8a 100644 --- a/js/constants/appConfig.js +++ b/js/constants/appConfig.js @@ -122,6 +122,8 @@ module.exports = { 'general.useragent.value': null, // Set at runtime 'general.autohide-menu': true, 'general.check-default-on-startup': true, + 'general.download-default-path': '', + 'general.download-always-ask': true, 'search.default-search-engine': 'Google', 'search.offer-search-suggestions': false, // false by default for privacy reasons 'tabs.switch-to-new-tabs': false, diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index 05f99bdc4de..2e3785d1203 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -80,6 +80,7 @@ const appConstants = { APP_DOWNLOAD_DELETED: _, APP_DOWNLOAD_CLEARED: _, APP_DOWNLOAD_REDOWNLOADED: _, + APP_DOWNLOAD_DEFAULT_PATH: _, APP_SHOW_DOWNLOAD_DELETE_CONFIRMATION: _, APP_HIDE_DOWNLOAD_DELETE_CONFIRMATION: _, APP_ALLOW_FLASH_ONCE: _, diff --git a/js/constants/settings.js b/js/constants/settings.js index 6d28b66e4fc..f42ab8d5de8 100644 --- a/js/constants/settings.js +++ b/js/constants/settings.js @@ -16,6 +16,8 @@ const settings = { LANGUAGE: 'general.language', CHECK_DEFAULT_ON_STARTUP: 'general.check-default-on-startup', IS_DEFAULT_BROWSER: 'general.is-default-browser', + DOWNLOAD_DEFAULT_PATH: 'general.download-default-path', + DOWNLOAD_ALWAYS_ASK: 'general.download-always-ask', // Search tab DEFAULT_SEARCH_ENGINE: 'search.default-search-engine', OFFER_SEARCH_SUGGESTIONS: 'search.offer-search-suggestions', diff --git a/test/unit/about/preferencesTest.js b/test/unit/about/preferencesTest.js index 5df9d1193b3..eaf9f788079 100644 --- a/test/unit/about/preferencesTest.js +++ b/test/unit/about/preferencesTest.js @@ -8,7 +8,7 @@ const {mount} = require('enzyme') const sinon = require('sinon') const assert = require('assert') const fakeElectron = require('../lib/fakeElectron') -let Preferences +let Preferences, appActions, SettingItemIcon require('../braveUnit') describe('Preferences component', function () { @@ -44,11 +44,14 @@ describe('Preferences component', function () { mockery.registerMock('../../../../extensions/brave/img/coinbase_logo.png') mockery.registerMock('../../../../extensions/brave/img/android_download.svg') mockery.registerMock('../../../../extensions/brave/img/ios_download.svg') + mockery.registerMock('../../img/icon_pencil.svg') window.chrome = fakeElectron window.CustomEvent = {} Preferences = require('../../../js/about/preferences').AboutPreferences + SettingItemIcon = require('../../../app/renderer/components/settings').SettingItemIcon + appActions = require('../../../js/actions/appActions') }) after(function () { mockery.disable() @@ -88,4 +91,21 @@ describe('Preferences component', function () { assert.equal(this.result.find('[data-l10n-id="searchSettings"]').length, 1) }) }) + + describe('General', function () { + describe('Default path', function () { + it('call appActions.defaultDownloadPath when pencil is clicked', function () { + const spy = sinon.spy(appActions, 'defaultDownloadPath') + const wrapper = mount( + + ) + wrapper.find('span[data-icon-position="right"]').simulate('click') + assert.equal(spy.calledOnce, true) + appActions.defaultDownloadPath.restore() + }) + }) + }) }) diff --git a/test/unit/app/browser/reducers/downloadsReducerTest.js b/test/unit/app/browser/reducers/downloadsReducerTest.js index dea890f5aa9..0162885ce44 100644 --- a/test/unit/app/browser/reducers/downloadsReducerTest.js +++ b/test/unit/app/browser/reducers/downloadsReducerTest.js @@ -231,4 +231,34 @@ describe('downloadsReducer', function () { }) }) }) + + describe('APP_DOWNLOAD_DEFAULT_PATH', function () { + describe('when showing folder selection dialog', function () { + let stub + let options + + before(function () { + stub = sinon.stub(fakeElectron.dialog, 'showOpenDialog', function (arg1, arg2) { + options = arg2 + }) + downloadsReducer({}, {actionType: appConstants.APP_DOWNLOAD_DEFAULT_PATH}) + }) + + after(function () { + stub.restore() + }) + + it('calls dialog.showOpenDialog', function () { + assert(stub.calledOnce) + }) + + it('passes the correct defaultPath', function () { + assert.equal(options.defaultPath, `${process.cwd()}/downloads`) + }) + + it('passes the correct properties object', function () { + assert.deepEqual(options.properties, ['openDirectory']) + }) + }) + }) }) diff --git a/test/unit/lib/fakeElectron.js b/test/unit/lib/fakeElectron.js index 68d7f33b5b4..c5e562ef038 100644 --- a/test/unit/lib/fakeElectron.js +++ b/test/unit/lib/fakeElectron.js @@ -27,12 +27,16 @@ const fakeElectron = { }, app: { on: function () { - } + }, + getPath: (param) => `${process.cwd()}/${param}` }, clipboard: { writeText: function () { } }, + dialog: { + showOpenDialog: function () { } + }, shell: { showItemInFolder: function () { },