From 1a91cc6204ef8924ab3c3cf1ce2a7df4f3734dff Mon Sep 17 00:00:00 2001 From: Alex Robinson Date: Fri, 23 Mar 2018 00:27:14 +1100 Subject: [PATCH 1/3] Add feature window.prompt This is a first pass at implementing the JavaScript function window.prompt. closes #94 - Add translations for message box 'ok' and 'cancel' - Introduce PromptTextbox and use it in messageBox for window.prompt --- app/browser/tabMessageBox.js | 29 ++++++++++++++----- .../brave/locales/en-US/menu.properties | 2 ++ app/locale.js | 2 ++ app/renderer/components/common/messageBox.js | 22 ++++++++++++++ app/renderer/components/common/textbox.js | 12 ++++++++ 5 files changed, 60 insertions(+), 7 deletions(-) diff --git a/app/browser/tabMessageBox.js b/app/browser/tabMessageBox.js index 03c6286851d..32e74eff4de 100644 --- a/app/browser/tabMessageBox.js +++ b/app/browser/tabMessageBox.js @@ -1,6 +1,7 @@ const appActions = require('../../js/actions/appActions') const tabMessageBoxState = require('../common/state/tabMessageBoxState') const {makeImmutable} = require('../common/state/immutableUtil') +const locale = require('../../app/locale') // callbacks for alert, confirm, etc. let messageBoxCallbacks = {} @@ -21,7 +22,7 @@ const tabMessageBox = { const detail = { message, title, - buttons: ['ok'], + buttons: [locale.translation('messageBoxOk')], suppress: false, showSuppress: shouldDisplaySuppressCheckbox } @@ -35,7 +36,7 @@ const tabMessageBox = { const detail = { message, title, - buttons: ['ok', 'cancel'], + buttons: [locale.translation('messageBoxOk'), locale.translation('messageBoxCancel')], cancelId: 1, suppress: false, showSuppress: shouldDisplaySuppressCheckbox @@ -45,10 +46,20 @@ const tabMessageBox = { }) process.on('window-prompt', (webContents, extraData, title, message, defaultPromptText, - shouldDisplaySuppressCheckbox, isBeforeUnloadDialog, isReload, muonCb) => { - console.warn('window.prompt is not supported yet') - let suppress = false - muonCb(null, '', suppress) + shouldDisplaySuppressCheckbox, isBeforeUnloadDialog, isReload, muonCb) => { + const tabId = webContents.getId() + const detail = { + message, + title, + buttons: [locale.translation('messageBoxOk'), locale.translation('messageBoxCancel')], + cancelId: 1, + suppress: false, + allowInput: true, + defaultPromptText, + showSuppress: shouldDisplaySuppressCheckbox + } + + tabMessageBox.show(tabId, detail, muonCb) }) return state @@ -70,6 +81,7 @@ const tabMessageBox = { const muonCb = messageBoxCallbacks[tabId] let suppress = false let result = true + let input = '' state = tabMessageBoxState.removeDetail(state, action) if (muonCb) { cleanupCallback(tabId) @@ -80,7 +92,10 @@ const tabMessageBox = { if (detail.has('result')) { result = detail.get('result') } - muonCb(result, '', suppress) + if (detail.has('input')) { + input = detail.get('input') + } + muonCb(result, input, suppress) } else { muonCb(false, '', false) } diff --git a/app/extensions/brave/locales/en-US/menu.properties b/app/extensions/brave/locales/en-US/menu.properties index ebcd1bc352d..dac4977850c 100644 --- a/app/extensions/brave/locales/en-US/menu.properties +++ b/app/extensions/brave/locales/en-US/menu.properties @@ -93,6 +93,8 @@ learnSpelling=Learn Spelling licenseText=This software uses libraries from the FFmpeg project under the LGPLv2.1 lookupSelection=Look Up “{{selectedVariable}}” mergeAllWindows=Merge All Windows +messageBoxOk=ok +messageBoxCancel=cancel minimize=Minimize moveTabToNewWindow=Move to New Window muteOtherTabs=Mute other Tabs diff --git a/app/locale.js b/app/locale.js index 1dcfbfe1404..d6416b8d5e5 100644 --- a/app/locale.js +++ b/app/locale.js @@ -235,6 +235,8 @@ var rendererIdentifiers = function () { 'dappDismiss', 'dappEnableExtension', 'banSiteConfirmation', + 'messageBoxOk', + 'messageBoxCancel', // other 'passwordsManager', 'extensionsManager', diff --git a/app/renderer/components/common/messageBox.js b/app/renderer/components/common/messageBox.js index fa937cecf57..e0e01d14a7f 100644 --- a/app/renderer/components/common/messageBox.js +++ b/app/renderer/components/common/messageBox.js @@ -12,6 +12,7 @@ const Dialog = require('./dialog') const FlyoutDialog = require('./flyoutDialog') const BrowserButton = require('../common/browserButton') const SwitchControl = require('./switchControl') +const {PromptTextBox} = require('./textbox') // Actions const appActions = require('../../../../js/actions/appActions') @@ -36,6 +37,9 @@ class MessageBox extends React.Component { super(props) this.onKeyDown = this.onKeyDown.bind(this) this.onSuppressChanged = this.onSuppressChanged.bind(this) + this.state = { + textInput: props.defaultPromptText + } } componentWillMount () { @@ -82,6 +86,10 @@ class MessageBox extends React.Component { response.result = buttonId !== this.props.cancelId } + if (this.props.allowInput) { + response.input = this.state.textInput + } + appActions.tabMessageBoxDismissed(tabId, response) } @@ -112,6 +120,8 @@ class MessageBox extends React.Component { // used in renderer props.tabId = tabId props.message = messageBoxDetail.get('message') + props.allowInput = messageBoxDetail.get('allowInput') + props.defaultPromptText = messageBoxDetail.get('defaultPromptText') props.suppress = tabMessageBoxState.getSuppress(state, tabId) props.title = tabMessageBoxState.getTitle(state, tabId) props.showSuppress = tabMessageBoxState.getShowSuppress(state, tabId) @@ -151,6 +161,18 @@ class MessageBox extends React.Component { /> : null } + { + this.props.allowInput && ( + { + this.setState({ + textInput: e.target.value + }) + }} + /> + ) + }
{this.messageBoxButtons}
diff --git a/app/renderer/components/common/textbox.js b/app/renderer/components/common/textbox.js index 76f0714cdec..34b46e57842 100644 --- a/app/renderer/components/common/textbox.js +++ b/app/renderer/components/common/textbox.js @@ -21,6 +21,7 @@ class Textbox extends ImmutableComponent { (this.props.readonly || this.props.readOnly) ? styles.readOnly : styles.outlineable, this.props['data-isCommonForm'] && commonStyles.isCommonForm, this.props['data-isSettings'] && styles.isSettings, + this.props['data-isPrompt'] && styles.isPrompt, this.props.customClass && this.props.customClass ) @@ -66,6 +67,12 @@ class SettingTextbox extends ImmutableComponent { } } +class PromptTextBox extends ImmutableComponent { + render () { + return + } +} + // TextArea class TextArea extends ImmutableComponent { render () { @@ -160,6 +167,10 @@ const styles = StyleSheet.create({ isSettings: { width: '280px' }, + isPrompt: { + width: '100%', + marginBottom: '20px' + }, readOnly: { background: globalStyles.color.lightGray, boxShadow: 'none', @@ -248,6 +259,7 @@ module.exports = { FormTextbox, GroupedFormTextbox, SettingTextbox, + PromptTextBox, TextArea, DefaultTextArea, WordCountTextArea From a2873fa7f31f022560f0719f8afe1362c4e7c9be Mon Sep 17 00:00:00 2001 From: Alex Robinson Date: Tue, 17 Apr 2018 22:01:50 +1000 Subject: [PATCH 2/3] Add unit tests for rendering PromptTextBox in MessageBox when input is allowed Extract out and export onWindowPrompt function and unit test it --- app/browser/tabMessageBox.js | 35 ++++++------ test/unit/app/browser/tabMessageBoxTest.js | 48 ++++++++++++++++ .../components/common/messageBoxTest.js | 56 ++++++++++++++++++- 3 files changed, 122 insertions(+), 17 deletions(-) diff --git a/app/browser/tabMessageBox.js b/app/browser/tabMessageBox.js index 32e74eff4de..fe95a5924a4 100644 --- a/app/browser/tabMessageBox.js +++ b/app/browser/tabMessageBox.js @@ -14,6 +14,23 @@ const cleanupCallback = (tabId) => { return false } +const onWindowPrompt = show => (webContents, extraData, title, message, defaultPromptText, + shouldDisplaySuppressCheckbox, isBeforeUnloadDialog, isReload, muonCb) => { + const tabId = webContents.getId() + const detail = { + message, + title, + buttons: [locale.translation('messageBoxOk'), locale.translation('messageBoxCancel')], + cancelId: 1, + suppress: false, + allowInput: true, + defaultPromptText, + showSuppress: shouldDisplaySuppressCheckbox + } + + show(tabId, detail, muonCb) +} + const tabMessageBox = { init: (state, action) => { process.on('window-alert', (webContents, extraData, title, message, defaultPromptText, @@ -45,22 +62,7 @@ const tabMessageBox = { tabMessageBox.show(tabId, detail, muonCb) }) - process.on('window-prompt', (webContents, extraData, title, message, defaultPromptText, - shouldDisplaySuppressCheckbox, isBeforeUnloadDialog, isReload, muonCb) => { - const tabId = webContents.getId() - const detail = { - message, - title, - buttons: [locale.translation('messageBoxOk'), locale.translation('messageBoxCancel')], - cancelId: 1, - suppress: false, - allowInput: true, - defaultPromptText, - showSuppress: shouldDisplaySuppressCheckbox - } - - tabMessageBox.show(tabId, detail, muonCb) - }) + process.on('window-prompt', onWindowPrompt(tabMessageBox.show)) return state }, @@ -145,3 +147,4 @@ const tabMessageBox = { } module.exports = tabMessageBox +module.exports.onWindowPrompt = onWindowPrompt diff --git a/test/unit/app/browser/tabMessageBoxTest.js b/test/unit/app/browser/tabMessageBoxTest.js index 17d405c8842..d9b704d71ed 100644 --- a/test/unit/app/browser/tabMessageBoxTest.js +++ b/test/unit/app/browser/tabMessageBoxTest.js @@ -38,8 +38,13 @@ describe('tabMessageBox unit tests', function () { useCleanCache: true }) + const fakeLocale = { + translation: (token) => { return token } + } + mockery.registerMock('electron', require('../../lib/fakeElectron')) mockery.registerMock('../common/state/tabMessageBoxState', fakeMessageBoxState) + mockery.registerMock('../../../js/l10n', fakeLocale) tabMessageBox = require('../../../../app/browser/tabMessageBox') appActions = require('../../../../js/actions/appActions') @@ -224,4 +229,47 @@ describe('tabMessageBox unit tests', function () { }) }) }) + + describe('onWindowPrompt', () => { + const tabId = '123' + const webContents = { + getId: () => tabId + } + const extraData = undefined + const title = 'some title' + const message = 'some message' + const defaultPromptText = 'some prompt text' + const shouldDisplaySuppressCheckbox = true + const isBeforeUnloadDialog = undefined + const isReload = undefined + const muonCb = 'muonCb' + + it('calls tabMessageBox.show', () => { + const mockShow = sinon.stub() + const expectecDetail = { + message, + title, + buttons: ['MESSAGEBOXOK', 'MESSAGEBOXCANCEL'], + cancelId: 1, + suppress: false, + allowInput: true, + defaultPromptText, + showSuppress: shouldDisplaySuppressCheckbox + } + + tabMessageBox.onWindowPrompt(mockShow)( + webContents, + extraData, + title, + message, + defaultPromptText, + shouldDisplaySuppressCheckbox, + isBeforeUnloadDialog, + isReload, + muonCb + ) + + assert.equal(mockShow.withArgs(tabId, expectecDetail, muonCb).calledOnce, true) + }) + }) }) diff --git a/test/unit/app/renderer/components/common/messageBoxTest.js b/test/unit/app/renderer/components/common/messageBoxTest.js index 15277c896ea..d497d076060 100644 --- a/test/unit/app/renderer/components/common/messageBoxTest.js +++ b/test/unit/app/renderer/components/common/messageBoxTest.js @@ -41,6 +41,25 @@ let appState = Immutable.fromJS({ } }) +const createAppState = detail => Immutable.fromJS({ + windows: [{ + windowId: 1, + windowUUID: 'uuid' + }], + tabs: [{ + tabId: tabId, + windowId: 1, + windowUUID: 'uuid', + url: 'https://brave.com', + messageBoxDetail: detail + }], + tabsInternal: { + index: { + 1: 0 + } + } +}) + describe('MessageBox component unit tests', function () { before(function () { mockery.enable({ @@ -60,7 +79,7 @@ describe('MessageBox component unit tests', function () { describe('Rendering', function () { before(function () { - appStoreRenderer.state = Immutable.fromJS(appState) + appStoreRenderer.state = createAppState(detail1) }) it('renders itself inside a dialog component', function () { const wrapper = mount( @@ -98,6 +117,19 @@ describe('MessageBox component unit tests', function () { assert.equal(wrapper.find('button[data-l10n-id="Cancel"][data-test-id="secondaryColor"]').length, 1) }) + it('renders the PromptTextBox when input is allowed', function () { + appStoreRenderer.state = createAppState(Object.assign({}, detail1, { + allowInput: true + })) + const wrapper = mount( + + ) + assert.equal(wrapper.find('PromptTextBox').length, 1) + }) + it('hides the suppress checkbox if showSuppress is false', function () { const appState2 = appState.setIn(['tabs', 0, 'messageBoxDetail', 'showSuppress'], false) appStoreRenderer.state = Immutable.fromJS(appState2) @@ -158,5 +190,27 @@ describe('MessageBox component unit tests', function () { assert.equal(spy.withArgs(tabId, response).calledOnce, true) appActions.tabMessageBoxDismissed.restore() }) + + it('calls appActions.tabMessageBoxDismissed with input input is allowed', function () { + const expectedInput = 'some input' + appStoreRenderer.state = createAppState(Object.assign({}, detail1, { + allowInput: true, + defaultPromptText: expectedInput + })) + const spy = sinon.spy(appActions, 'tabMessageBoxDismissed') + const wrapper = mount( + + ) + const response = { + suppress: detail1.suppress, + result: false, + input: expectedInput + } + wrapper.find('button[data-l10n-id="Cancel"][data-test-id="secondaryColor"]').simulate('click') + assert.equal(spy.withArgs(tabId, response).calledOnce, true) + appActions.tabMessageBoxDismissed.restore() + }) }) }) From 489b1a55ce459dc1eca8ee5094fe3e970d9c4498 Mon Sep 17 00:00:00 2001 From: Alex Robinson Date: Sat, 5 May 2018 14:26:46 +1000 Subject: [PATCH 3/3] Highlight input in window.prompt when it is displayed: --- app/renderer/components/common/messageBox.js | 9 +++++++++ app/renderer/components/common/textbox.js | 4 +++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/renderer/components/common/messageBox.js b/app/renderer/components/common/messageBox.js index e0e01d14a7f..7f234cb1bce 100644 --- a/app/renderer/components/common/messageBox.js +++ b/app/renderer/components/common/messageBox.js @@ -46,6 +46,12 @@ class MessageBox extends React.Component { document.addEventListener('keydown', this.onKeyDown) } + componentDidMount () { + if (this.props.allowInput) { + this.inputRef.select() + } + } + componentWillUnmount () { document.removeEventListener('keydown', this.onKeyDown) } @@ -165,6 +171,9 @@ class MessageBox extends React.Component { this.props.allowInput && ( { + this.inputRef = ref + }} onChange={e => { this.setState({ textInput: e.target.value diff --git a/app/renderer/components/common/textbox.js b/app/renderer/components/common/textbox.js index 34b46e57842..d9c6d8fc5a0 100644 --- a/app/renderer/components/common/textbox.js +++ b/app/renderer/components/common/textbox.js @@ -26,9 +26,11 @@ class Textbox extends ImmutableComponent { ) const props = Object.assign({}, this.props) + const ref = this.props.inputRef delete props.customClass + delete props.inputRef - return + return } }