Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Added default download path #7023

Merged
merged 2 commits into from
Mar 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions app/browser/reducers/downloadsReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -68,7 +71,7 @@ const downloadsReducer = (state, action) => {
case appConstants.APP_DOWNLOAD_REDOWNLOADED:
const win = BrowserWindow.getFocusedWindow()
if (win) {
win.webContents.downloadURL(download.get('url'))
win.webContents.downloadURL(download.get('url'), true)
state = state.deleteIn(['downloads', action.downloadId])
} else {
shell.beep()
Expand All @@ -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])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine as-is... but it could be better as it's own discreet action (in the event that we'd like other reducers to be listening). Perhaps an action like defaultDownloadPathChanged

}
})
break
}
return state
}
Expand Down
2 changes: 2 additions & 0 deletions app/extensions/brave/locales/en-US/preferences.properties
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,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
Expand Down
4 changes: 2 additions & 2 deletions app/filtering.js
Original file line number Diff line number Diff line change
Expand Up @@ -501,8 +501,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) && !item.promptForSaveLocation())) ? defaultPath : dialog.showSaveDialog(win, { defaultPath }))

// User cancelled out of save dialog prompt
if (!savePath) {
Expand Down
2 changes: 1 addition & 1 deletion app/ledger.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ var backupKeys = (appState, action) => {
if (action.backupAction === 'print') {
webContents.print({silent: false, printBackground: false})
} else {
webContents.downloadURL(fileUrl(filePath))
webContents.downloadURL(fileUrl(filePath), true)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion app/pdf.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const renderUrlToPdf = (appState, action, testingMode) => {
let listeners = wv.session.listeners('will-download')
wv.session.removeAllListeners('will-download')

wv.downloadURL(pdfDataURI)
wv.downloadURL(pdfDataURI, true)
wv.session.once('will-download', function (event, item) {
if (savePath) {
item.setSavePath(savePath)
Expand Down
64 changes: 63 additions & 1 deletion app/renderer/components/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,71 @@ class SiteSettingCheckbox extends ImmutableComponent {
}
}

class SettingItemIcon extends ImmutableComponent {
render () {
const bg = StyleSheet.create({
Icon: {
'-webkit-mask-image': `url(${this.props.icon})`
}
})

return <div className='settingItem'>
{
this.props.dataL10nId
? <span data-l10n-id={this.props.dataL10nId} />
: null
}
<div>
{
this.props.position === 'left'
? <span className={css(bg.Icon, styles.icon, styles.iconLeft)} data-icon-position='left' onClick={this.props.clickAction} />
: null
}
<div className={css(styles.child)}>
{this.props.children}
</div>
{
this.props.position === 'right'
? <span className={css(bg.Icon, styles.icon, styles.iconRight)} data-icon-position='right' onClick={this.props.clickAction} />
: null
}
</div>
</div>
}
}

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
}
6 changes: 6 additions & 0 deletions docs/appActions.md
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,12 @@ Action triggered by un-registering navigation handler



### defaultDownloadPath()

Open dialog for default download path setting




* * *

Expand Down
16 changes: 16 additions & 0 deletions img/icon_pencil.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
18 changes: 17 additions & 1 deletion js/about/preferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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')
Expand Down Expand Up @@ -115,6 +116,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 (
Expand Down Expand Up @@ -184,6 +189,17 @@ class GeneralTab extends ImmutableComponent {
isDarwin ? null : <SettingCheckbox dataL10nId='autoHideMenuBar' prefKey={settings.AUTO_HIDE_MENU} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
}
<SettingCheckbox dataL10nId='disableTitleMode' prefKey={settings.DISABLE_TITLE_MODE} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
<SettingItemIcon dataL10nId='downloadDefaultPath' position='right' icon={require('../../img/icon_pencil.svg')} clickAction={this.openDownloadDialog}>
<SettingTextbox
spellCheck='false'
readOnly='true'
data-l10n-id='downloadDefaultPathInput'
value={getSetting(settings.DOWNLOAD_DEFAULT_PATH, this.props.settings)}
onClick={this.openDownloadDialog} />
</SettingItemIcon>
<SettingCheckbox dataL10nId='downloadAlwaysAsk' prefKey={settings.DOWNLOAD_ALWAYS_ASK}
settings={this.props.settings}
onChangeSetting={this.props.onChangeSetting} />
<SettingItem dataL10nId='bookmarkToolbarSettings'>
<SettingDropdown id='bookmarksBarSelect' value={getSetting(settings.BOOKMARKS_TOOLBAR_MODE, this.props.settings)}
onChange={changeSetting.bind(null, this.props.onChangeSetting, settings.BOOKMARKS_TOOLBAR_MODE)}>
Expand Down
9 changes: 9 additions & 0 deletions js/actions/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}
}

Expand Down
2 changes: 1 addition & 1 deletion js/components/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ class Frame extends ImmutableComponent {
? UrlUtil.getLocationIfPDF(this.tab.get('url'))
: this.tab.get('url')
// TODO: Sometimes this tries to save in a non-existent directory
this.webview.downloadURL(downloadLocation)
this.webview.downloadURL(downloadLocation, true)
break
case 'print':
this.webview.print()
Expand Down
2 changes: 2 additions & 0 deletions js/constants/appConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,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,
Expand Down
1 change: 1 addition & 0 deletions js/constants/appConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: _,
Expand Down
2 changes: 2 additions & 0 deletions js/constants/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion js/contextMenus.js
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,7 @@ const saveAsMenuItem = (label, location) => {
label: locale.translation(label),
click: (item, focusedWindow) => {
if (focusedWindow && location) {
focusedWindow.webContents.downloadURL(location)
focusedWindow.webContents.downloadURL(location, true)
}
}
}
Expand Down
22 changes: 21 additions & 1 deletion test/unit/about/preferencesTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -46,11 +46,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()
Expand Down Expand Up @@ -90,4 +93,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(
<SettingItemIcon
clickAction={appActions.defaultDownloadPath()}
position='right'
/>
)
wrapper.find('span[data-icon-position="right"]').simulate('click')
assert.equal(spy.calledOnce, true)
appActions.defaultDownloadPath.restore()
})
})
})
})
30 changes: 30 additions & 0 deletions test/unit/app/browser/reducers/downloadsReducerTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
})
})
})
})
6 changes: 5 additions & 1 deletion test/unit/lib/fakeElectron.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,16 @@ const fakeElectron = {
},
app: {
on: function () {
}
},
getPath: (param) => `${process.cwd()}/${param}`
},
clipboard: {
writeText: function () {
}
},
dialog: {
showOpenDialog: function () { }
},
shell: {
showItemInFolder: function () {
},
Expand Down