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

Optimize titlebar space on Windows #3854

Merged
merged 17 commits into from
Sep 21, 2016
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
25 changes: 0 additions & 25 deletions app/browser/lib/menuUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,31 +10,6 @@ const eventUtil = require('../../../js/lib/eventUtil')
const siteUtil = require('../../../js/state/siteUtil')
const locale = require('../../locale')

/**
* Get an electron MenuItem object for the PARENT menu (File, Edit, etc) based on its label
* @param {string} label - the text associated with the menu
* NOTE: label may be a localized string
*/
module.exports.getParentMenuDetails = (appMenu, label) => {
let menuIndex = -1
let menuItem = null

if (label && appMenu && appMenu.items && appMenu.items.length > 0) {
menuIndex = appMenu.items.findIndex(function (item, index) {
return item && item.label === label
})

if (menuIndex !== -1) {
menuItem = appMenu.items[menuIndex]
}
}

return {
menu: menuItem,
index: menuIndex
}
}

/**
* Get the an electron MenuItem object from a Menu based on its label
* @param {string} label - the text associated with the menu
Expand Down
12 changes: 12 additions & 0 deletions app/browser/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const messages = require('../../js/constants/messages')
const settings = require('../../js/constants/settings')
const siteTags = require('../../js/constants/siteTags')
const dialog = electron.dialog
const BrowserWindow = electron.BrowserWindow
const { fileUrl } = require('../../js/lib/appUrlUtil')
const menuUtil = require('./lib/menuUtil')
const getSetting = require('../../js/settings').getSetting
Expand Down Expand Up @@ -586,6 +587,8 @@ const createMenu = () => {
})
}

appActions.setMenubarTemplate(Immutable.fromJS(template))

let oldMenu = appMenu
appMenu = Menu.buildFromTemplate(template)
Menu.setApplicationMenu(appMenu)
Expand Down Expand Up @@ -643,6 +646,15 @@ const doAction = (action) => {
})
}
break
case windowConstants.WINDOW_CLICK_MENUBAR_SUBMENU:
appDispatcher.waitFor([appStore.dispatchToken], () => {
const clickedMenuItem = menuUtil.getMenuItem(appMenu, action.label)
if (clickedMenuItem) {
const focusedWindow = BrowserWindow.getFocusedWindow()
clickedMenuItem.click(clickedMenuItem, focusedWindow, focusedWindow.webContents)
}
})
break
default:
}
}
Expand Down
96 changes: 96 additions & 0 deletions app/common/lib/formatUtil.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/* 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/. */

'use strict'

const macOrderLookup = (value) => {
switch (value) {
case 'Alt':
case 'Option':
case 'AltGr':
return 0
case 'Shift':
return 1
case 'Control':
case 'Ctrl':
return 2
case 'Super':
case 'CmdOrCtrl':
case 'CommandOrControl':
case 'Command':
case 'Cmd':
return 3
default:
return 4
}
}
const defaultOrderLookup = (value) => {
switch (value) {
case 'CmdOrCtrl':
case 'CommandOrControl':
case 'Control':
case 'Ctrl':
return 0
case 'Alt':
case 'AltGr':
return 1
case 'Shift':
return 2
default:
return 3
}
}

/**
* Format an electron accelerator in the order you'd expect in a menu
* Accelerator reference: https://github.com/electron/electron/blob/master/docs/api/accelerator.md
*/
module.exports.formatAccelerator = (accelerator) => {
let result = accelerator
let splitResult = accelerator.split('+')
// sort in proper order, based on OS
// also, replace w/ name or symbol
if (process.platform === 'darwin') {
splitResult.sort(function (left, right) {
if (macOrderLookup(left) === macOrderLookup(right)) return 0
if (macOrderLookup(left) > macOrderLookup(right)) return 1
return -1
})
// NOTE: these characters might only show properly on Mac
result = splitResult.join('')
result = result.replace('CommandOrControl', '⌘')
result = result.replace('CmdOrCtrl', '⌘')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Please also support CommandOrControl, we don't use it yet but we might in the future.

result = result.replace('Command', '⌘')
result = result.replace('Cmd', '⌘')
result = result.replace('Alt', '⌥')
result = result.replace('AltGr', '⌥')
result = result.replace('Super', '⌘')
result = result.replace('Option', '⌥')
result = result.replace('Shift', '⇧')
result = result.replace('Control', '^')
result = result.replace('Ctrl', '^')
} else {
splitResult.sort(function (left, right) {
if (defaultOrderLookup(left) === defaultOrderLookup(right)) return 0
if (defaultOrderLookup(left) > defaultOrderLookup(right)) return 1
return -1
})
result = splitResult.join('+')
result = result.replace('CommandOrControl', 'Ctrl')
result = result.replace('CmdOrCtrl', 'Ctrl')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto CommandOrControl

result = result.replace('Control', 'Ctrl')
}
return result
}

/**
* Clamp values down to a given range (min/max).
* Value is wrapped when out of bounds. ex:
* min-1 = max
* max+1 = min
*/
module.exports.wrappingClamp = (value, min, max) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just use const calmp = require('lodash/clamp') and remove this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and doh- can't use because it doesn't wrap

const range = (max - min) + 1
return value - Math.floor((value - min) / range) * range
}
4 changes: 4 additions & 0 deletions app/extensions/brave/locales/en-US/app.properties
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,7 @@ dismissDenyRunInsecureContent=Stay Insecure
denyRunInsecureContent=Stop Loading Unsafe Scripts
runInsecureContentWarning=This page is trying to load scripts from insecure sources. If you allow this content to run it will not be encrypted and it may transmit unencrypted data to other sites.
denyRunInsecureContentWarning=This page is currently loading scripts from insecure sources.
windowCaptionButtonMinimize=Minimize
windowCaptionButtonMaximize=Maximize
windowCaptionButtonRestore=Restore Down
windowCaptionButtonClose=Close
7 changes: 6 additions & 1 deletion app/locale.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,12 @@ var rendererIdentifiers = function () {
'downloadItemDelete',
'downloadItemClear',
'downloadToolbarHide',
'downloadItemClearCompleted'
'downloadItemClearCompleted',
// Caption buttons in titlebar (min/max/close - Windows only)
'windowCaptionButtonMinimize',
'windowCaptionButtonMaximize',
'windowCaptionButtonRestore',
'windowCaptionButtonClose'
]
}

Expand Down
213 changes: 213 additions & 0 deletions app/renderer/components/menubar.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
/* 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 React = require('react')
const Immutable = require('immutable')
const ImmutableComponent = require('../../../js/components/immutableComponent')
const windowActions = require('../../../js/actions/windowActions')
const separatorMenuItem = require('../../common/commonMenu').separatorMenuItem
const keyCodes = require('../../../js/constants/keyCodes')
const { wrappingClamp } = require('../../common/lib/formatUtil')

const showContextMenu = (rect, submenu, lastFocusedSelector) => {
windowActions.setContextMenuDetail(Immutable.fromJS({
left: rect.left,
top: rect.bottom,
template: submenu.map((submenuItem) => {
if (submenuItem.type === separatorMenuItem.type) {
return submenuItem
}
submenuItem.click = function (e) {
e.preventDefault()
if (lastFocusedSelector) {
// Send focus back to the active web frame
const results = document.querySelectorAll(lastFocusedSelector)
if (results.length === 1) {
results[0].focus()
}
}
windowActions.clickMenubarSubmenu(submenuItem.label)
}
return submenuItem
})
}))
}

class MenubarItem extends ImmutableComponent {
constructor () {
super()
this.onClick = this.onClick.bind(this)
this.onMouseOver = this.onMouseOver.bind(this)
}
onClick (e) {
if (e && e.stopPropagation) {
e.stopPropagation()
}
// If clicking on an already selected item, deselect it
const selected = this.props.menubar.props.selectedLabel
if (selected && selected === this.props.label) {
windowActions.setContextMenuDetail()
windowActions.setMenubarSelectedLabel()
return
}
// Otherwise, mark item as selected and show its context menu
windowActions.setMenubarSelectedLabel(this.props.label)
const rect = e.target.getBoundingClientRect()
showContextMenu(rect, this.props.submenu, this.props.lastFocusedSelector)
}
onMouseOver (e) {
const selected = this.props.menubar.props.selectedLabel
if (selected && selected !== this.props.label) {
this.onClick(e)
}
}
render () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently you aren't respecting the visible: false for menu templates which makes find next / find prev show up int he Edit menu as items but not work. These should be hidden but the shortcuts should still work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really good find! 😄

return <span
className={'menubarItem' + (this.props.selected ? ' selected' : '')}
onClick={this.onClick}
onMouseOver={this.onMouseOver}
data-label={this.props.label}>
{ this.props.label }
</span>
}
}

/**
* Menubar that can be optionally be displayed at the top of a window (in favor of the system menu).
* First intended use is with Windows to enable a slim titlebar.
* NOTE: the system menu is still created and used in order to keep the accelerators working.
*/
class Menubar extends ImmutableComponent {
constructor () {
super()
this.onKeyDown = this.onKeyDown.bind(this)
}
componentWillMount () {
document.addEventListener('keydown', this.onKeyDown)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to make it symmetric with componentWillMount since you do it on unmount.

}
componentWillUnmount () {
document.removeEventListener('keydown', this.onKeyDown)
}
getTemplateByLabel (label) {
const element = this.props.template.find((element) => {
return element.get('label') === label
})
return element ? element.get('submenu') : null
}
get selectedTemplate () {
return this.getTemplateByLabel(this.props.selectedLabel)
}
get selectedTemplateItemsOnly () {
// exclude the separators AND items that are not visible
return this.selectedTemplate.filter((element) => {
if (element.get('type') === separatorMenuItem.type) return false
if (element.has('visible')) return element.get('visible')
return true
})
}
get selectedIndexMax () {
const result = this.selectedTemplateItemsOnly
if (result && result.size && result.size > 0) {
return result.size
}
return 0
}
getRectByLabel (label) {
const selected = document.querySelectorAll('.menubar .menubarItem[data-label=\'' + label + '\']')
if (selected.length === 1) {
return selected.item(0).getBoundingClientRect()
}
return null
}
get selectedRect () {
return this.getRectByLabel(this.props.selectedLabel)
}
onKeyDown (e) {
switch (e.which) {
case keyCodes.ENTER:
e.preventDefault()
if (this.selectedTemplate) {
const selectedLabel = this.selectedTemplateItemsOnly.getIn([this.props.selectedIndex, 'label'])
windowActions.clickMenubarSubmenu(selectedLabel)
windowActions.resetMenuState()
}
break

case keyCodes.LEFT:
case keyCodes.RIGHT:
if (!this.props.autohide && !this.props.selectedLabel) break

e.preventDefault()
if (this.props.template.size > 0) {
const selectedIndex = this.props.template.findIndex((element) => {
return element.get('label') === this.props.selectedLabel
})
const nextIndex = selectedIndex === -1
? 0
: wrappingClamp(
selectedIndex + (e.which === keyCodes.LEFT ? -1 : 1),
0,
this.props.template.size - 1)

// BSCTODO: consider submenus (ex: for bookmark folders)

const nextLabel = this.props.template.getIn([nextIndex, 'label'])
const nextRect = this.getRectByLabel(nextLabel)

windowActions.setMenubarSelectedLabel(nextLabel)

// Context menu already being displayed; auto-open the next one
if (this.props.contextMenuDetail && this.selectedTemplate && nextRect) {
windowActions.setSubmenuSelectedIndex(0)
showContextMenu(nextRect, this.getTemplateByLabel(nextLabel).toJS(), this.props.lastFocusedSelector)
}
}
break

case keyCodes.UP:
case keyCodes.DOWN:
if (!this.props.autohide && !this.props.selectedLabel) break

e.preventDefault()
if (this.props.selectedLabel && this.selectedTemplate) {
if (!this.props.contextMenuDetail && this.selectedRect) {
// First time hitting up/down; popup the context menu
windowActions.setSubmenuSelectedIndex(0)
showContextMenu(this.selectedRect, this.selectedTemplate.toJS(), this.props.lastFocusedSelector)
} else {
// Context menu already visible; move selection up or down
const nextIndex = wrappingClamp(
this.props.selectedIndex + (e.which === keyCodes.UP ? -1 : 1),
0,
this.selectedIndexMax - 1)
windowActions.setSubmenuSelectedIndex(nextIndex)
}
}
break
}
}
shouldComponentUpdate (nextProps, nextState) {
return this.props.selectedLabel !== nextProps.selectedLabel
}
render () {
return <div className='menubar'>
{
this.props.template.map((menubarItem) => {
let props = {
label: menubarItem.get('label'),
submenu: menubarItem.get('submenu').toJS(),
menubar: this,
lastFocusedSelector: this.props.lastFocusedSelector
}
if (props.label === this.props.selectedLabel) {
props.selected = true
}
return <MenubarItem {...props} />
})
}
</div>
}
}

module.exports = Menubar
Loading