diff --git a/app/common/state/contextMenuState.js b/app/common/state/contextMenuState.js index 0f88ce240c1..d278e679561 100644 --- a/app/common/state/contextMenuState.js +++ b/app/common/state/contextMenuState.js @@ -11,27 +11,36 @@ const validateState = function (state) { return state } -const contextMenuState = { - setContextMenu: (state, detail) => { +const api = { + setContextMenu: (windowState, detail) => { detail = makeImmutable(detail) - state = validateState(state) + windowState = validateState(windowState) if (!detail) { - if (state.getIn(['contextMenuDetail', 'type']) === 'hamburgerMenu') { - state = state.set('hamburgerMenuWasOpen', true) + if (windowState.getIn(['contextMenuDetail', 'type']) === 'hamburgerMenu') { + windowState = windowState.set('hamburgerMenuWasOpen', true) } else { - state = state.set('hamburgerMenuWasOpen', false) + windowState = windowState.set('hamburgerMenuWasOpen', false) } - state = state.delete('contextMenuDetail') + windowState = windowState.delete('contextMenuDetail') } else { - if (!(detail.get('type') === 'hamburgerMenu' && state.get('hamburgerMenuWasOpen'))) { - state = state.set('contextMenuDetail', detail) + if (!(detail.get('type') === 'hamburgerMenu' && windowState.get('hamburgerMenuWasOpen'))) { + windowState = windowState.set('contextMenuDetail', detail) } - state = state.set('hamburgerMenuWasOpen', false) + windowState = windowState.set('hamburgerMenuWasOpen', false) } - return state + return windowState + }, + + selectedIndex: (windowState) => { + const selectedIndex = windowState.getIn(['ui', 'contextMenu', 'selectedIndex']) + return (typeof selectedIndex === 'object' && + Array.isArray(selectedIndex) && + selectedIndex.length > 0) + ? selectedIndex + : null } } -module.exports = contextMenuState +module.exports = api diff --git a/app/common/state/menuBarState.js b/app/common/state/menuBarState.js new file mode 100644 index 00000000000..9da54ae3374 --- /dev/null +++ b/app/common/state/menuBarState.js @@ -0,0 +1,15 @@ +/* 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 {isWindows} = require('../lib/platformUtil') +const {getSetting} = require('../../../js/settings') +const settings = require('../../../js/constants/settings') + +const api = { + isMenuBarVisible: (windowState) => { + return isWindows() && (!getSetting(settings.AUTO_HIDE_MENU) || windowState.getIn(['ui', 'menubar', 'isVisible'])) + } +} + +module.exports = api diff --git a/app/renderer/components/navigation/menuBar.js b/app/renderer/components/navigation/menuBar.js index 6733f5b6e3a..29a8f2064bc 100644 --- a/app/renderer/components/navigation/menuBar.js +++ b/app/renderer/components/navigation/menuBar.js @@ -5,7 +5,7 @@ const React = require('react') // Components -const ImmutableComponent = require('../immutableComponent') +const ReduxComponent = require('../reduxComponent') const MenuBarItem = require('./menuBarItem') // Constants @@ -14,6 +14,9 @@ const keyCodes = require('../../../common/constants/keyCodes') // Actions const windowActions = require('../../../../js/actions/windowActions') +// State +const contextMenuState = require('../../../common/state/contextMenuState.js') + // Utils const {separatorMenuItem} = require('../../../common/commonMenu') const {showContextMenu} = require('../../../common/lib/menuUtil') @@ -24,7 +27,7 @@ const {wrappingClamp} = require('../../../common/lib/formatUtil') * 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 { +class MenuBar extends React.Component { constructor () { super() this.onKeyDown = this.onKeyDown.bind(this) @@ -43,11 +46,15 @@ class MenuBar extends ImmutableComponent { * Used to position the context menu object. */ getMenubarItemBounds (index) { - if (typeof index !== 'number') index = this.props.selectedIndex + if (typeof index !== 'number') { + index = this.props.selectedIndex + } + const selected = document.querySelectorAll('.menubar .menubarItem[data-index=\'' + index + '\']') if (selected.length === 1) { return selected.item(0).getBoundingClientRect() } + return null } @@ -112,31 +119,32 @@ class MenuBar extends ImmutableComponent { } } - shouldComponentUpdate (nextProps, nextState) { - return this.props.selectedIndex !== nextProps.selectedIndex || - this.props.template !== nextProps.template + mergeProps (state, dispatchProps, ownProps) { + const currentWindow = state.get('currentWindow') + const contextMenuDetail = currentWindow.get('contextMenuDetail') + + const props = {} + // used in renderer + props.template = state.getIn(['menu', 'template']) + + // used in other functions + props.selectedIndex = currentWindow.getIn(['ui', 'menubar', 'selectedIndex']) + props.contextMenuSelectedIndex = contextMenuState.selectedIndex(currentWindow) + props.contextMenuDetail = !!contextMenuDetail + props.lastFocusedSelector = currentWindow.getIn(['ui', 'menubar', 'lastFocusedSelector']) + + return Object.assign({}, ownProps, props) } render () { - let i = 0 return
{ - this.props.template.map((menubarItem) => { - let props = { - label: menubarItem.get('label'), - index: i++, - submenu: menubarItem.get('submenu').toJS(), - lastFocusedSelector: this.props.lastFocusedSelector, - menubar: this - } - if (props.index === this.props.selectedIndex) { - props.selected = true - } - return + this.props.template.map((item, i) => { + return }) }
} } -module.exports = MenuBar +module.exports = ReduxComponent.connect(MenuBar) diff --git a/app/renderer/components/navigation/menuBarItem.js b/app/renderer/components/navigation/menuBarItem.js index df94467dfe8..1e569184cf8 100644 --- a/app/renderer/components/navigation/menuBarItem.js +++ b/app/renderer/components/navigation/menuBarItem.js @@ -5,7 +5,7 @@ const React = require('react') // Components -const ImmutableComponent = require('../immutableComponent') +const ReduxComponent = require('../reduxComponent') // Actions const windowActions = require('../../../../js/actions/windowActions') @@ -13,7 +13,7 @@ const windowActions = require('../../../../js/actions/windowActions') // Utils const {showContextMenu} = require('../../../common/lib/menuUtil') -class MenuBarItem extends ImmutableComponent { +class MenuBarItem extends React.Component { constructor () { super() this.onClick = this.onClick.bind(this) @@ -24,27 +24,44 @@ class MenuBarItem extends ImmutableComponent { if (e && e.stopPropagation) { e.stopPropagation() } + // If clicking on an already selected item, deselect it - const selected = this.props.menubar.props.selectedIndex - if (selected && selected === this.props.index) { + if (this.props.selected) { windowActions.setContextMenuDetail() windowActions.setMenuBarSelectedIndex() return } + // Otherwise, mark item as selected and show its context menu + const rect = e.target.getBoundingClientRect() windowActions.setMenuBarSelectedIndex(this.props.index) windowActions.setContextMenuSelectedIndex() - const rect = e.target.getBoundingClientRect() showContextMenu(rect, this.props.submenu, this.props.lastFocusedSelector) } onMouseOver (e) { - const selected = this.props.menubar.props.selectedIndex - if (typeof selected === 'number' && selected !== this.props.index) { + if (!this.props.selected) { this.onClick(e) } } + mergeProps (state, dispatchProps, ownProps) { + const currentWindow = state.get('currentWindow') + const selectedIndex = currentWindow.getIn(['ui', 'menubar', 'selectedIndex']) + const template = state.getIn(['menu', 'template', ownProps.index]) + + const props = {} + // used in renderer + props.selected = ownProps.index === selectedIndex + props.label = template.get('label') + + // used in other functions + props.submenu = template.get('submenu') && template.get('submenu').toJS() + props.lastFocusedSelector = currentWindow.getIn(['ui', 'menubar', 'lastFocusedSelector']) + + return Object.assign({}, ownProps, props) + } + render () { return - + : null diff --git a/js/components/main.js b/js/components/main.js index d0a21197eb1..9987ad69ac1 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -47,7 +47,6 @@ const settings = require('../constants/settings') const dragTypes = require('../constants/dragTypes') const keyCodes = require('../../app/common/constants/keyCodes') const keyLocations = require('../../app/common/constants/keyLocations') -const isWindows = process.platform === 'win32' const {bookmarksToolbarMode} = require('../../app/common/constants/settingsEnums') // State handling @@ -59,6 +58,7 @@ const defaultBrowserState = require('../../app/common/state/defaultBrowserState' const shieldState = require('../../app/common/state/shieldState') const tabState = require('../../app/common/state/tabState') const siteSettingsState = require('../../app/common/state/siteSettingsState') +const menuBarState = require('../../app/common/state/menuBarState') // Util const _ = require('underscore') @@ -67,7 +67,7 @@ const eventUtil = require('../lib/eventUtil') const siteSettings = require('../state/siteSettings') const debounce = require('../lib/debounce') const {getCurrentWindowId, isMaximized, isFocused, isFullScreen} = require('../../app/renderer/currentWindow') -const platformUtil = require('../../app/common/lib/platformUtil') +const {isDarwin, isWindows} = require('../../app/common/lib/platformUtil') class Main extends ImmutableComponent { constructor () { @@ -93,14 +93,13 @@ class Main extends ImmutableComponent { } registerWindowLevelShortcuts () { // For window level shortcuts that don't work as local shortcuts - const isDarwin = platformUtil.isDarwin() document.addEventListener('keydown', (e) => { switch (e.which) { case keyCodes.ESC: this.exitFullScreen() break case keyCodes.F12: - if (!isDarwin) { + if (!isDarwin()) { ipc.emit(messages.SHORTCUT_ACTIVE_FRAME_TOGGLE_DEV_TOOLS) } break @@ -653,13 +652,11 @@ class Main extends ImmutableComponent { } get customTitlebar () { - const customTitlebarEnabled = isWindows - const captionButtonsVisible = customTitlebarEnabled - const menubarVisible = customTitlebarEnabled && (!getSetting(settings.AUTO_HIDE_MENU) || this.props.windowState.getIn(['ui', 'menubar', 'isVisible'])) + const menubarVisible = menuBarState.isMenuBarVisible(this.props.windowState) const selectedIndex = this.props.windowState.getIn(['ui', 'contextMenu', 'selectedIndex']) return { - enabled: customTitlebarEnabled, - captionButtonsVisible: captionButtonsVisible, + enabled: isWindows(), + captionButtonsVisible: isWindows(), menubarVisible: menubarVisible, menubarTemplate: menubarVisible ? this.props.appState.getIn(['menu', 'template']) : null, menubarSelectedIndex: this.props.windowState.getIn(['ui', 'menubar', 'selectedIndex']), @@ -858,7 +855,7 @@ class Main extends ImmutableComponent { draggingOverData={this.props.appState.getIn(['dragData', 'dragOverData', 'draggingOverType']) === dragTypes.BOOKMARK && this.props.appState.getIn(['dragData', 'dragOverData'])} showFavicon={showFavicon} showOnlyFavicon={showOnlyFavicon} - shouldAllowWindowDrag={shouldAllowWindowDrag && !isWindows} + shouldAllowWindowDrag={shouldAllowWindowDrag && !isWindows()} activeFrameKey={(activeFrame && activeFrame.get('key')) || undefined} windowWidth={window.innerWidth} contextMenuDetail={contextMenuDetail} diff --git a/test/unit/app/common/state/contextMenuStateTest.js b/test/unit/app/common/state/contextMenuStateTest.js new file mode 100644 index 00000000000..f85760eb7e2 --- /dev/null +++ b/test/unit/app/common/state/contextMenuStateTest.js @@ -0,0 +1,35 @@ +/* global describe, it */ +const Immutable = require('immutable') +const contextMenuState = require('../../../../../app/common/state/contextMenuState.js') +const assert = require('chai').assert + +const defaultWindowState = Immutable.fromJS({ + ui: { + contextMenu: { + selectedIndex: null + } + } +}) + +describe('contextMenuState', function () { + describe('selectedIndex', function () { + it('returns null if selectedIndex is not set', function () { + const result = contextMenuState.selectedIndex(defaultWindowState) + assert.equal(result, null) + }) + + it('returns null if selectedIndex is only number', function () { + const index = 0 + const newState = defaultWindowState.setIn(['ui', 'contextMenu', 'selectedIndex'], index) + const result = contextMenuState.selectedIndex(newState) + assert.equal(result, null) + }) + + it('returns array of selected index', function () { + const index = [0] + const newState = defaultWindowState.setIn(['ui', 'contextMenu', 'selectedIndex'], index) + const result = contextMenuState.selectedIndex(newState) + assert.equal(result, index) + }) + }) +})