From 86d1a11ee77347869a3fbebf5a4933a4e26b4d55 Mon Sep 17 00:00:00 2001 From: ryanml Date: Tue, 24 Apr 2018 18:10:06 -0700 Subject: [PATCH 1/2] Ensuring share context menu items are disabled when no windows are open Refactoring menu.js item attribute functions --- app/browser/menu.js | 35 +++++++++++++++++++----- app/common/commonMenu.js | 3 +- app/common/lib/menuUtil.js | 12 +++++--- test/unit/app/common/lib/menuUtilTest.js | 6 ++-- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/app/browser/menu.js b/app/browser/menu.js index 0ca202669a7..37fece5610b 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -39,6 +39,8 @@ const bookmarkUtil = require('../common/lib/bookmarkUtil') const isDarwin = platformUtil.isDarwin() const isLinux = platformUtil.isLinux() const isWindows = platformUtil.isWindows() +const {templateUrls} = require('./share') +const {getAllRendererWindows} = require('./windows') let appMenu = null let closedFrames = new Immutable.OrderedMap() @@ -596,21 +598,29 @@ const createMenu = (state) => { } } -const setMenuItemChecked = (state, label, checked) => { - // Update electron menu (Mac / Linux) +const setMenuItemAttribute = (state, label, key, value) => { const systemMenuItem = menuUtil.getMenuItem(appMenu, label) - systemMenuItem.checked = checked + systemMenuItem[key] = value // Update in-memory menu template (Windows) if (isWindows) { const oldTemplate = state.getIn(['menu', 'template']) - const newTemplate = menuUtil.setTemplateItemChecked(oldTemplate, label, checked) + const newTemplate = menuUtil.setTemplateItemAttribute(oldTemplate, label, key, value) if (newTemplate) { appActions.setMenubarTemplate(newTemplate) } } } +const updateShareMenuItems = (state, enabled) => { + for (let key of Object.keys(templateUrls)) { + const siteName = menuUtil.extractSiteName(key) + const l10nId = key === 'email' ? 'emailPageLink' : 'sharePageLink' + const label = locale.translation(l10nId, {siteName: siteName}) + setMenuItemAttribute(state, label, 'enabled', enabled) + } +} + const doAction = (state, action) => { switch (action.actionType) { case appConstants.APP_SET_STATE: @@ -622,17 +632,28 @@ const doAction = (state, action) => { const frame = frameStateUtil.getFrameByTabId(state, action.tabId) if (frame) { currentLocation = frame.location - setMenuItemChecked(state, locale.translation('bookmarkPage'), isCurrentLocationBookmarked(state)) + setMenuItemAttribute(state, locale.translation('bookmarkPage'), 'checked', isCurrentLocationBookmarked(state)) + } + break + } + case appConstants.APP_WINDOW_CLOSED: + case appConstants.APP_WINDOW_CREATED: + { + const windowCount = getAllRendererWindows().length + if (action.actionType === appConstants.APP_WINDOW_CLOSED && windowCount === 0) { + updateShareMenuItems(state, false) + } else if (action.actionType === appConstants.APP_WINDOW_CREATED && windowCount === 1) { + updateShareMenuItems(state, true) } break } case appConstants.APP_CHANGE_SETTING: if (action.key === settings.SHOW_BOOKMARKS_TOOLBAR) { // Update the checkbox next to "Bookmarks Toolbar" (Bookmarks menu) - setMenuItemChecked(state, locale.translation('bookmarksToolbar'), action.value) + setMenuItemAttribute(state, locale.translation('bookmarksToolbar'), 'checked', action.value) } if (action.key === settings.DEBUG_ALLOW_MANUAL_TAB_DISCARD) { - setMenuItemChecked(state, 'Allow manual tab discarding', action.value) + setMenuItemAttribute(state, 'Allow manual tab discarding', 'checked', action.value) } break case windowConstants.WINDOW_UNDO_CLOSED_FRAME: diff --git a/app/common/commonMenu.js b/app/common/commonMenu.js index ef90b23ab4c..ede60939542 100644 --- a/app/common/commonMenu.js +++ b/app/common/commonMenu.js @@ -13,6 +13,7 @@ const getSetting = require('../../js/settings').getSetting const communityURL = 'https://community.brave.com/' const isDarwin = process.platform === 'darwin' const electron = require('electron') +const menuUtil = require('./lib/menuUtil') const ensureAtLeastOneWindow = (frameOpts) => { // Handle no new tab requested, but need a window @@ -136,7 +137,7 @@ module.exports.printMenuItem = () => { } module.exports.simpleShareActiveTabMenuItem = (l10nId, type, accelerator) => { - const siteName = type.charAt(0).toUpperCase() + type.slice(1) + const siteName = menuUtil.extractSiteName(type) return { label: locale.translation(l10nId, {siteName: siteName}), diff --git a/app/common/lib/menuUtil.js b/app/common/lib/menuUtil.js index a6329f98bf8..894fd460bae 100644 --- a/app/common/lib/menuUtil.js +++ b/app/common/lib/menuUtil.js @@ -67,16 +67,20 @@ const getTemplateItem = (template, label) => { return null } +module.exports.extractSiteName = (type) => { + return type.charAt(0).toUpperCase() + type.slice(1) +} + /** - * Search a menu template and update the checked status + * Searches a menu template and updates a passed item key * * @return the new template OR null if no change was made (no update needed) */ -module.exports.setTemplateItemChecked = (template, label, checked) => { +module.exports.setTemplateItemAttribute = (template, label, key, value) => { const menu = template.toJS() const menuItem = getTemplateItem(menu, label) - if (menuItem.checked !== checked) { - menuItem.checked = checked + if (menuItem[key] !== value) { + menuItem[key] = value return makeImmutable(menu) } return null diff --git a/test/unit/app/common/lib/menuUtilTest.js b/test/unit/app/common/lib/menuUtilTest.js index cce7b0475f6..b445a34a8c0 100644 --- a/test/unit/app/common/lib/menuUtilTest.js +++ b/test/unit/app/common/lib/menuUtilTest.js @@ -86,7 +86,7 @@ describe('menuUtil tests', function () { }) }) - describe('setTemplateItemChecked', function () { + describe('setTemplateItemAttribute', function () { const defaultTemplate = Immutable.fromJS([ { 'label': 'Bookmarks', @@ -113,11 +113,11 @@ describe('menuUtil tests', function () { ] } ]) - const newTemplate = menuUtil.setTemplateItemChecked(defaultTemplate, 'Bookmarks Toolbar', true) + const newTemplate = menuUtil.setTemplateItemAttribute(defaultTemplate, 'Bookmarks Toolbar', 'checked', true) assert.deepEqual(newTemplate.toJS(), expectedTemplate.toJS()) }) it('returns null when no change is made', function () { - const newTemplate = menuUtil.setTemplateItemChecked(defaultTemplate, 'Bookmarks Toolbar', false) + const newTemplate = menuUtil.setTemplateItemAttribute(defaultTemplate, 'Bookmarks Toolbar', 'checked', false) assert.equal(newTemplate, null) }) }) From f371e515fba17979dad2947198cd59850db0d303 Mon Sep 17 00:00:00 2001 From: ryanml Date: Tue, 1 May 2018 15:09:31 -0700 Subject: [PATCH 2/2] Removing digg from templateUrls --- app/browser/share.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/browser/share.js b/app/browser/share.js index 99e0a35164f..4bbc196ea26 100644 --- a/app/browser/share.js +++ b/app/browser/share.js @@ -15,7 +15,6 @@ const templateUrls = { googlePlus: 'https://plus.google.com/share?url={url}', linkedIn: 'https://www.linkedin.com/shareArticle?url={url}&title={title}', buffer: 'https://buffer.com/add?text={title}&url={url}', - digg: 'https://digg.com/submit?url={url}&title={title}', reddit: 'https://reddit.com/submit?url={url}&title={title}' }