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

Ensuring File > Share context menu items are disabled when no windows are open #13933

Merged
merged 2 commits into from
May 1, 2018
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
35 changes: 28 additions & 7 deletions app/browser/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down
1 change: 0 additions & 1 deletion app/browser/share.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}'
}

Expand Down
3 changes: 2 additions & 1 deletion app/common/commonMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}),
Expand Down
12 changes: 8 additions & 4 deletions app/common/lib/menuUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions test/unit/app/common/lib/menuUtilTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe('menuUtil tests', function () {
})
})

describe('setTemplateItemChecked', function () {
describe('setTemplateItemAttribute', function () {
const defaultTemplate = Immutable.fromJS([
{
'label': 'Bookmarks',
Expand All @@ -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)
})
})
Expand Down