From f345818f0baf1c95cecd0afd6276b5628c65bbbc Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Wed, 26 Apr 2017 15:51:07 -0400 Subject: [PATCH] Add a send URL by email menu item Fix #3121 Auditors: @bsclifton --- app/browser/menu.js | 2 + app/browser/reducers/shareReducer.js | 21 ++++++ app/browser/share.js | 18 +++++ app/common/commonMenu.js | 10 +++ .../brave/locales/en-US/menu.properties | 1 + app/locale.js | 1 + docs/appActions.md | 10 +++ js/actions/appActions.js | 11 +++ js/constants/appConstants.js | 1 + js/stores/appStore.js | 3 +- .../app/browser/reducers/shareReducerTest.js | 42 ++++++++++++ test/unit/app/browser/shareTest.js | 67 +++++++++++++++++++ test/unit/app/common/commonMenuTest.js | 6 ++ test/unit/lib/fakeElectron.js | 2 + 14 files changed, 194 insertions(+), 1 deletion(-) create mode 100644 app/browser/reducers/shareReducer.js create mode 100644 app/browser/share.js create mode 100644 test/unit/app/browser/reducers/shareReducerTest.js create mode 100644 test/unit/app/browser/shareTest.js diff --git a/app/browser/menu.js b/app/browser/menu.js index 3fa9fc1b164..891872eb010 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -108,6 +108,8 @@ const createFileSubmenu = () => { ] */ }, + // Move inside share menu when it's enabled + CommonMenu.emailPageLinkMenuItem(), CommonMenu.separatorMenuItem, CommonMenu.printMenuItem() ] diff --git a/app/browser/reducers/shareReducer.js b/app/browser/reducers/shareReducer.js new file mode 100644 index 00000000000..0e0c76c7cfe --- /dev/null +++ b/app/browser/reducers/shareReducer.js @@ -0,0 +1,21 @@ +/* This SourceCode 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/. */ + +'use strict' + +const appConstants = require('../../../js/constants/appConstants') +const {makeImmutable} = require('../../common/state/immutableUtil') +const {emailActiveTab} = require('../share') + +const shareReducer = (state, action) => { + action = makeImmutable(action) + switch (action.get('actionType')) { + case appConstants.APP_EMAIL_ACTIVE_TAB_REQUESTED: + state = emailActiveTab(state, action.get('windowId')) + break + } + return state +} + +module.exports = shareReducer diff --git a/app/browser/share.js b/app/browser/share.js new file mode 100644 index 00000000000..3e74b877319 --- /dev/null +++ b/app/browser/share.js @@ -0,0 +1,18 @@ +/* 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/. */ + +const tabState = require('../common/state/tabState') +const {shell} = require('electron') + +const emailActiveTab = (state, windowId) => { + const tabValue = tabState.getActiveTabValue(state, windowId) + shell.openExternal( + `mailto:?subject=${encodeURIComponent(tabValue.get('title') || '')}&body=${encodeURIComponent(tabValue.get('url'))}` + ) + return state +} + +module.exports = { + emailActiveTab +} diff --git a/app/common/commonMenu.js b/app/common/commonMenu.js index e8e15e055f1..b4b8cc2d2b0 100644 --- a/app/common/commonMenu.js +++ b/app/common/commonMenu.js @@ -148,6 +148,16 @@ module.exports.printMenuItem = () => { } } +module.exports.emailPageLinkMenuItem = () => { + return { + label: locale.translation('emailPageLink'), + accelerator: 'CmdOrCtrl+Shift+I', + click: function (item, focusedWindow) { + appActions.emailActiveTabRequested(getCurrentWindowId()) + } + } +} + module.exports.findOnPageMenuItem = () => { return { label: locale.translation('findOnPage'), diff --git a/app/extensions/brave/locales/en-US/menu.properties b/app/extensions/brave/locales/en-US/menu.properties index ff8d2879356..43110465511 100644 --- a/app/extensions/brave/locales/en-US/menu.properties +++ b/app/extensions/brave/locales/en-US/menu.properties @@ -80,6 +80,7 @@ newSessionTab=New Session Tab newWindow=New Window reopenLastClosedTab=Reopen Last Closed Tab print=Print… +emailPageLink=Email Page Link… findOnPage=Find on Page… find=Find… checkForUpdates=Check for Updates… diff --git a/app/locale.js b/app/locale.js index 952b180350a..5b00ce9b5bf 100644 --- a/app/locale.js +++ b/app/locale.js @@ -153,6 +153,7 @@ var rendererIdentifiers = function () { 'newWindow', 'reopenLastClosedTab', 'print', + 'emailPageLink', 'findOnPage', 'find', 'checkForUpdates', diff --git a/docs/appActions.md b/docs/appActions.md index a9f5ad2f808..0fcc17548cf 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -103,6 +103,16 @@ A request for a URL load for the active tab of the specified window +### emailActiveTabRequested(windowId) + +A request for a URL email share occurred + +**Parameters** + +**windowId**: `number`, the window ID to use for the active tab + + + ### maybeCreateTabRequested(createProperties) A request for a "maybe" new tab has been made with the specified createProperties diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 85232013239..a521622ffaa 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -148,6 +148,17 @@ const appActions = { }) }, + /** + * A request for a URL email share occurred + * @param {number} windowId - the window ID to use for the active tab + */ + emailActiveTabRequested: function (windowId) { + AppDispatcher.dispatch({ + actionType: appConstants.APP_EMAIL_ACTIVE_TAB_REQUESTED, + windowId + }) + }, + /** * A request for a "maybe" new tab has been made with the specified createProperties * If a tab is already opened it will instead set it as active. diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index 1e6b91f400a..05c57d7cb5e 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -70,6 +70,7 @@ const appConstants = { APP_MAYBE_CREATE_TAB_REQUESTED: _, APP_LOAD_URL_REQUESTED: _, APP_LOAD_URL_IN_ACTIVE_TAB_REQUESTED: _, + APP_EMAIL_ACTIVE_TAB_REQUESTED: _, APP_NEW_WEB_CONTENTS_ADDED: _, APP_TAB_DESTROYED: _, APP_SET_MENUBAR_TEMPLATE: _, diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 33b15c16970..6d217e444ab 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -386,7 +386,8 @@ const handleAppAction = (action) => { require('../../app/browser/reducers/passwordManagerReducer'), require('../../app/browser/reducers/tabMessageBoxReducer'), require('../../app/browser/reducers/dragDropReducer'), - require('../../app/browser/reducers/extensionsReducer') + require('../../app/browser/reducers/extensionsReducer'), + require('../../app/browser/reducers/shareReducer') ] initialized = true appState = action.appState diff --git a/test/unit/app/browser/reducers/shareReducerTest.js b/test/unit/app/browser/reducers/shareReducerTest.js new file mode 100644 index 00000000000..9e6e36d48f3 --- /dev/null +++ b/test/unit/app/browser/reducers/shareReducerTest.js @@ -0,0 +1,42 @@ +/* global describe, it, before, after */ +const mockery = require('mockery') +const sinon = require('sinon') +const Immutable = require('immutable') +const assert = require('assert') +const fakeElectron = require('../../../lib/fakeElectron') + +const appConstants = require('../../../../../js/constants/appConstants') +require('../../../braveUnit') + +describe('shareReducer', function () { + let shareReducer + before(function () { + mockery.enable({ + warnOnReplace: false, + warnOnUnregistered: false, + useCleanCache: true + }) + mockery.registerMock('electron', fakeElectron) + this.shareStub = { + emailActiveTab: sinon.stub() + } + mockery.registerMock('../share', this.shareStub) + shareReducer = require('../../../../../app/browser/reducers/shareReducer') + }) + + after(function () { + mockery.disable() + }) + + describe('APP_EMAIL_ACTIVE_TAB_REQUESTED', function () { + before(function () { + this.state = Immutable.Map() + this.windowId = 2 + this.newState = shareReducer(this.state, {actionType: appConstants.APP_EMAIL_ACTIVE_TAB_REQUESTED, windowId: this.windowId}) + }) + it('calls emailActiveTab once with the correct args', function () { + const callCount = this.shareStub.emailActiveTab.withArgs(this.state, this.windowId).callCount + assert.equal(callCount, 1) + }) + }) +}) diff --git a/test/unit/app/browser/shareTest.js b/test/unit/app/browser/shareTest.js new file mode 100644 index 00000000000..94cd2fcd217 --- /dev/null +++ b/test/unit/app/browser/shareTest.js @@ -0,0 +1,67 @@ +/* global describe, it, before, after, afterEach */ +const mockery = require('mockery') +const sinon = require('sinon') +const Immutable = require('immutable') +const assert = require('assert') +const fakeElectron = require('../../lib/fakeElectron') + +require('../../braveUnit') + +describe('share API', function () { + let share + before(function () { + mockery.enable({ + warnOnReplace: false, + warnOnUnregistered: false, + useCleanCache: true + }) + mockery.registerMock('electron', fakeElectron) + share = require('../../../../app/browser/share') + }) + + after(function () { + mockery.disable() + }) + + describe('emailActiveTab', function () { + before(function () { + this.state = Immutable.fromJS({ + tabs: [{ + windowId: 2, + tabId: 2, + url: 'https://www.brave.com/2', + title: 'title 2', + active: true + }, { + windowId: 3, + tabId: 3, + url: 'https://www.brave.com/3', + title: 'title 3' + }, { + windowId: 5, + tabId: 5, + url: 'https://www.brave.com/5', + title: 'title 5', + active: true + }] + }) + this.windowId = 2 + this.openExternalSpy = sinon.spy(fakeElectron.shell, 'openExternal') + }) + afterEach(function () { + this.openExternalSpy.reset() + }) + it('calls openExternal with the correct args', function () { + share.emailActiveTab(this.state, 2) + const expectedUrl = 'mailto:?subject=title%202&body=https%3A%2F%2Fwww.brave.com%2F2' + const callCount = this.openExternalSpy.withArgs(expectedUrl).callCount + assert.equal(callCount, 1) + }) + it('takes active tab windowId into consideration', function () { + share.emailActiveTab(this.state, 5) + const expectedUrl = 'mailto:?subject=title%205&body=https%3A%2F%2Fwww.brave.com%2F5' + const callCount = this.openExternalSpy.withArgs(expectedUrl).callCount + assert.equal(callCount, 1) + }) + }) +}) diff --git a/test/unit/app/common/commonMenuTest.js b/test/unit/app/common/commonMenuTest.js index 1fa3a6fbcd4..16001312501 100644 --- a/test/unit/app/common/commonMenuTest.js +++ b/test/unit/app/common/commonMenuTest.js @@ -102,6 +102,12 @@ describe('Common menu module unit tests', function () { }) }) + describe('emailPageLinkMenuItem', function () { + it('has the expected defaults set', function () { + checkExpectedDefaults(commonMenu.emailPageLinkMenuItem) + }) + }) + describe('findOnPageMenuItem', function () { it('has the expected defaults set', function () { checkExpectedDefaults(commonMenu.findOnPageMenuItem) diff --git a/test/unit/lib/fakeElectron.js b/test/unit/lib/fakeElectron.js index 7ddf1cc20e9..9be0b188294 100644 --- a/test/unit/lib/fakeElectron.js +++ b/test/unit/lib/fakeElectron.js @@ -49,6 +49,8 @@ const fakeElectron = { showOpenDialog: function () { } }, shell: { + openExternal: function () { + }, showItemInFolder: function () { }, openItem: function () {