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

Commit

Permalink
Merge pull request #5867 from brave/revert-5846-fix-context-menu
Browse files Browse the repository at this point in the history
Revert "Prevent duplicate separators for all context menu items."
  • Loading branch information
bbondy authored Nov 28, 2016
2 parents b77dc66 + ff1eaf7 commit 803a7ac
Show file tree
Hide file tree
Showing 15 changed files with 99 additions and 169 deletions.
48 changes: 6 additions & 42 deletions app/common/lib/menuUtil.js → app/browser/lib/menuUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ module.exports.setTemplateItemChecked = (template, label, checked) => {
return null
}

const createBookmarkTemplateItems = (bookmarks, parentFolderId) => {
const createBookmarkMenuItems = (bookmarks, parentFolderId) => {
const filteredBookmarks = parentFolderId
? bookmarks.filter((bookmark) => bookmark.get('parentFolderId') === parentFolderId)
: bookmarks.filter((bookmark) => !bookmark.get('parentFolderId'))
Expand Down Expand Up @@ -99,26 +99,23 @@ const createBookmarkTemplateItems = (bookmarks, parentFolderId) => {
const submenuItems = bookmarks.filter((bookmark) => bookmark.get('parentFolderId') === folderId)
payload.push({
label: site.get('customTitle') || site.get('title'),
submenu: submenuItems.count() > 0 ? createBookmarkTemplateItems(bookmarks, folderId) : null
submenu: submenuItems.count() > 0 ? createBookmarkMenuItems(bookmarks, folderId) : null
})
}
})
return payload
}

/**
* Used to create bookmarks and bookmark folder entries for the "Bookmarks" menu
* Add bookmarks / folders to "Bookmarks" menu
*
* @param sites The application state's Immutable sites list
*/
module.exports.createBookmarkTemplateItems = (sites) => {
return createBookmarkTemplateItems(siteUtil.getBookmarks(sites))
module.exports.createBookmarkMenuItems = (sites) => {
return createBookmarkMenuItems(siteUtil.getBookmarks(sites))
}

/**
* Create "recently closed" history entries for the "History" menu
*/
module.exports.createRecentlyClosedTemplateItems = (lastClosedFrames) => {
module.exports.createRecentlyClosedMenuItems = (lastClosedFrames) => {
const payload = []
if (lastClosedFrames && lastClosedFrames.size > 0) {
payload.push(
Expand All @@ -144,36 +141,3 @@ module.exports.createRecentlyClosedTemplateItems = (lastClosedFrames) => {
}
return payload
}

const isItemValid = (currentItem, previousItem) => {
if (previousItem && previousItem === CommonMenu.separatorMenuItem) {
if (currentItem === CommonMenu.separatorMenuItem) {
return false
}
}
return currentItem && (typeof currentItem.label === 'string' || typeof currentItem.type === 'string')
}

/**
* Remove invalid entries from a menu template:
* - null or falsey entries
* - extra menu separators
* - entries which don't have a label or type
*/
module.exports.sanitizeTemplateItems = (template) => {
return template.reduce((previousValue, currentValue, currentIndex, array) => {
const result = currentIndex === 1 ? [] : previousValue
if (currentIndex === 1) {
if (isItemValid(previousValue)) {
result.push(previousValue)
}
}
const previousItem = result.length > 0
? result[result.length - 1]
: undefined
if (isItemValid(currentValue, previousItem)) {
result.push(currentValue)
}
return result
})
}
6 changes: 3 additions & 3 deletions app/browser/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const siteTags = require('../../js/constants/siteTags')
const dialog = electron.dialog
const BrowserWindow = electron.BrowserWindow
const {fileUrl} = require('../../js/lib/appUrlUtil')
const menuUtil = require('../common/lib/menuUtil')
const menuUtil = require('./lib/menuUtil')
const getSetting = require('../../js/settings').getSetting
const locale = require('../locale')
const {isSiteBookmarked} = require('../../js/state/siteUtil')
Expand Down Expand Up @@ -329,7 +329,7 @@ const createHistorySubmenu = () => {
}
}
]
submenu = submenu.concat(menuUtil.createRecentlyClosedTemplateItems(Immutable.fromJS(Object.keys(closedFrames).map(key => closedFrames[key]))))
submenu = submenu.concat(menuUtil.createRecentlyClosedMenuItems(Immutable.fromJS(Object.keys(closedFrames).map(key => closedFrames[key]))))

submenu.push(
// TODO: recently visited
Expand Down Expand Up @@ -374,7 +374,7 @@ const createBookmarksSubmenu = () => {
CommonMenu.importBrowserDataMenuItem()
]

const bookmarks = menuUtil.createBookmarkTemplateItems(appStore.getState().get('sites'))
const bookmarks = menuUtil.createBookmarkMenuItems(appStore.getState().get('sites'))
if (bookmarks.length > 0) {
submenu.push(CommonMenu.separatorMenuItem)
submenu = submenu.concat(bookmarks)
Expand Down
2 changes: 1 addition & 1 deletion app/common/lib/historyUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ module.exports.groupEntriesByDay = (history, locale) => {
const reduced = history.reduce((previousValue, currentValue, currentIndex, array) => {
const result = currentIndex === 1 ? [] : previousValue
if (currentIndex === 1) {
const firstDate = getDayString(previousValue, locale)
const firstDate = getDayString(currentValue, locale)
result.push({date: firstDate, entries: [previousValue]})
}
const date = getDayString(currentValue, locale)
Expand Down
Loading

0 comments on commit 803a7ac

Please sign in to comment.