Skip to content

Commit

Permalink
Removes non-primitive types from TabPage
Browse files Browse the repository at this point in the history
Resolves brave#9793

Auditors: @bsclifton @bridiver

Test Plan:
  • Loading branch information
NejcZdovc committed Jul 4, 2017
1 parent 5114cc3 commit ce69e5f
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 51 deletions.
22 changes: 16 additions & 6 deletions app/renderer/components/tabs/tabPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const settings = require('../../../../js/constants/settings')
// Utils
const {getSetting} = require('../../../../js/settings')
const cx = require('../../../../js/lib/classSet')
const {onTabPageContextMenu} = require('../../../../js/contextMenus')
const {getCurrentWindowId} = require('../../currentWindow')
const dndData = require('../../../../js/dndData')
const frameStateUtil = require('../../../../js/state/frameStateUtil')
Expand All @@ -30,6 +29,8 @@ class TabPage extends React.Component {
super(props)
this.onMouseEnter = this.onMouseEnter.bind(this)
this.onMouseLeave = this.onMouseLeave.bind(this)
this.onContextMenu = this.onContextMenu.bind(this)
this.onClick = this.onClick.bind(this)
}

onMouseLeave () {
Expand Down Expand Up @@ -69,6 +70,15 @@ class TabPage extends React.Component {
e.preventDefault()
}

onContextMenu (e) {
e.stopPropagation()
windowActions.onTabPageMenu(this.props.index)
}

onClick () {
windowActions.setTabPageIndex(this.props.index)
}

mergeProps (state, ownProps) {
const currentWindow = state.get('currentWindow')
const frames = frameStateUtil.getNonPinnedFrames(currentWindow) || Immutable.List()
Expand All @@ -90,7 +100,6 @@ class TabPage extends React.Component {
const props = {}
// used in renderer
props.index = ownProps.index
props.tabPageFrames = tabPageFrames // TODO (nejc) only primitives
props.isAudioPlaybackActive = isAudioPlaybackActive
props.previewTabPage = getSetting(settings.SHOW_TAB_PREVIEWS)
props.active = currentWindow.getIn(['ui', 'tabs', 'tabPageIndex']) === props.index
Expand All @@ -113,10 +122,11 @@ class TabPage extends React.Component {
className={cx({
tabPage: true,
audioPlaybackActive: this.props.isAudioPlaybackActive,
active: this.props.active})}
onContextMenu={onTabPageContextMenu.bind(this, this.props.tabPageFrames)}
onClick={windowActions.setTabPageIndex.bind(this, this.props.index)
} />
active: this.props.active
})}
onContextMenu={this.onContextMenu}
onClick={this.onClick}
/>
}
}

Expand Down
71 changes: 63 additions & 8 deletions app/renderer/reducers/contextMenuReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,31 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */

const assert = require('assert')
const Immutable = require('immutable')
const electron = require('electron')
const remote = electron.remote
const Menu = remote.Menu

// Constants
const config = require('../../../js/constants/config.js')
const windowConstants = require('../../../js/constants/windowConstants')
const settings = require('../../../js/constants/settings')

// State
const contextMenuState = require('../../common/state/contextMenuState.js')
const contextMenuState = require('../../common/state/contextMenuState')

// Actions
const appActions = require('../../../js/actions/appActions.js')
const windowAction = require('../../../js/actions/windowActions.js')
const appActions = require('../../../js/actions/appActions')
const windowActions = require('../../../js/actions/windowActions')

// Utils
const eventUtil = require('../../../js/lib/eventUtil.js')
const CommonMenu = require('../../common/commonMenu.js')
const eventUtil = require('../../../js/lib/eventUtil')
const CommonMenu = require('../../common/commonMenu')
const locale = require('../../../js/l10n.js')
const { makeImmutable, isMap } = require('../../common/state/immutableUtil')
const {makeImmutable, isMap} = require('../../common/state/immutableUtil')
const {getSetting} = require('../../../js/settings')
const frameStateUtil = require('../../../js/state/frameStateUtil')
const {getCurrentWindow} = require('../../renderer/currentWindow')

const validateAction = function (action) {
action = makeImmutable(action)
Expand All @@ -33,6 +41,50 @@ const validateState = function (state) {
return state
}

function generateMuteFrameList (framePropsList, muted) {
return framePropsList.map((frameProp) => {
return {
frameKey: frameProp.get('key'),
tabId: frameProp.get('tabId'),
muted: muted && frameProp.get('audioPlaybackActive') && !frameProp.get('audioMuted')
}
})
}

const onTabPageMenu = function (state, action) {
action = validateAction(action)
state = validateState(state)

const index = action.get('index')
if (index == null || index < 0) {
return
}

const frames = frameStateUtil.getNonPinnedFrames(state) || Immutable.List()
const tabsPerPage = Number(getSetting(settings.TABS_PER_PAGE))
const tabPageFrames = frames.slice(index * tabsPerPage, (index * tabsPerPage) + tabsPerPage)

const template = [{
label: locale.translation('unmuteTabs'),
click: () => {
windowActions.muteAllAudio(generateMuteFrameList(tabPageFrames, false))
}
}, {
label: locale.translation('muteTabs'),
click: () => {
windowActions.muteAllAudio(generateMuteFrameList(tabPageFrames, true))
}
}, {
label: locale.translation('closeTabPage'),
click: () => {
windowActions.closeFrames(tabPageFrames)
}
}]

const tabPageMenu = Menu.buildFromTemplate(template)
tabPageMenu.popup(getCurrentWindow())
}

const onLongBackHistory = (state, action) => {
action = validateAction(action)
state = validateState(state)
Expand Down Expand Up @@ -72,7 +124,7 @@ const onLongBackHistory = (state, action) => {
appActions.createTabRequested({
url: 'about:history'
})
windowAction.setContextMenuDetail()
windowActions.setContextMenuDetail()
}
})

Expand Down Expand Up @@ -125,7 +177,7 @@ const onLongForwardHistory = (state, action) => {
appActions.createTabRequested({
url: 'about:history'
})
windowAction.setContextMenuDetail()
windowActions.setContextMenuDetail()
}
})

Expand All @@ -147,6 +199,9 @@ const contextMenuReducer = (windowState, action) => {
case windowConstants.WINDOW_ON_GO_FORWARD_LONG:
windowState = onLongForwardHistory(windowState, action)
break
case windowConstants.WINDOW_ON_TAB_PAGE_MENU:
onTabPageMenu(windowState, action)
break
}

return windowState
Expand Down
7 changes: 7 additions & 0 deletions js/actions/windowActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1142,6 +1142,13 @@ const windowActions = {
url,
error
})
},

onTabPageMenu: function (index) {
dispatch({
actionType: windowConstants.WINDOW_ON_TAB_PAGE_MENU,
index
})
}
}

Expand Down
3 changes: 2 additions & 1 deletion js/constants/windowConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ const windowConstants = {
WINDOW_ON_GO_BACK_LONG: _,
WINDOW_ON_GO_FORWARD_LONG: _,
WINDOW_CLOSE_OTHER_FRAMES: _,
WINDOW_ON_CERT_ERROR: _
WINDOW_ON_CERT_ERROR: _,
WINDOW_ON_TAB_PAGE_MENU: _
}

module.exports = mapValuesByKeys(windowConstants)
36 changes: 0 additions & 36 deletions js/contextMenus.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,35 +77,6 @@ const addFolderMenuItem = (closestDestinationDetail, isParent) => {
}
}

function tabPageTemplateInit (framePropsList) {
return [{
label: locale.translation('unmuteTabs'),
click: () => {
windowActions.muteAllAudio(generateMuteFrameList(framePropsList, false))
}
}, {
label: locale.translation('muteTabs'),
click: () => {
windowActions.muteAllAudio(generateMuteFrameList(framePropsList, true))
}
}, {
label: locale.translation('closeTabPage'),
click: () => {
windowActions.closeFrames(framePropsList)
}
}]
}

function generateMuteFrameList (framePropsList, muted) {
return framePropsList.map((frameProp) => {
return {
frameKey: frameProp.get('key'),
tabId: frameProp.get('tabId'),
muted: muted && frameProp.get('audioPlaybackActive') && !frameProp.get('audioMuted')
}
})
}

function urlBarTemplateInit (searchDetail, activeFrame, e) {
const items = getEditableItems(window.getSelection().toString())
const clipboardText = clipboard.readText()
Expand Down Expand Up @@ -1378,12 +1349,6 @@ function onDownloadsToolbarContextMenu (downloadId, downloadItem, e) {
downloadsToolbarMenu.popup(getCurrentWindow())
}

function onTabPageContextMenu (framePropsList, e) {
e.stopPropagation()
const tabPageMenu = Menu.buildFromTemplate(tabPageTemplateInit(framePropsList))
tabPageMenu.popup(getCurrentWindow())
}

function onUrlBarContextMenu (e) {
e.stopPropagation()
const searchDetail = appStoreRenderer.state.get('searchDetail')
Expand Down Expand Up @@ -1478,7 +1443,6 @@ module.exports = {
onNewTabContextMenu,
onTabsToolbarContextMenu,
onDownloadsToolbarContextMenu,
onTabPageContextMenu,
onUrlBarContextMenu,
onFindBarContextMenu,
onSiteDetailContextMenu,
Expand Down
86 changes: 86 additions & 0 deletions test/unit/app/renderer/reducers/contextMenuReducerTest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/* 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/. */

/* global describe, it, before, beforeEach, after, afterEach */
const mockery = require('mockery')
const assert = require('assert')
const sinon = require('sinon')
const Immutable = require('immutable')
const fakeElectron = require('../../../lib/fakeElectron')
const windowConstants = require('../../../../../js/constants/windowConstants')

describe('contextMenuReducer', function () {
let fakeElectronMenu, contextMenuReducer

const fakeLocale = {
translation: (token) => { return token }
}

before(function () {
mockery.enable({
warnOnReplace: false,
warnOnUnregistered: false,
useCleanCache: true
})
mockery.registerMock('electron', fakeElectron)
mockery.registerMock('../../../js/l10n.js', fakeLocale)
fakeElectronMenu = require('../../../lib/fakeElectronMenu')
contextMenuReducer = require('../../../../../app/renderer/reducers/contextMenuReducer')
})

after(function () {
mockery.disable()
})

describe('WINDOW_ON_TAB_PAGE_MENU', function () {
let menuBuildFromTemplateSpy, menuPopupSpy, state

before(function () {
state = Immutable.fromJS({
frames: [{
tabId: 1,
key: 1
}],
framesInternal: {
index: {
1: 0
},
tabIndex: {
1: 0
}
}
})
})

beforeEach(function () {
menuBuildFromTemplateSpy = sinon.spy(fakeElectron.remote.Menu, 'buildFromTemplate')
menuPopupSpy = sinon.spy(fakeElectronMenu, 'popup')
})

afterEach(function () {
menuBuildFromTemplateSpy.restore()
menuPopupSpy.restore()
})

it('index is outside the scope', function () {
const action = {
actionType: windowConstants.WINDOW_ON_TAB_PAGE_MENU,
index: -1
}
contextMenuReducer(state, action)
assert.equal(menuBuildFromTemplateSpy.calledOnce, false)
assert.equal(menuPopupSpy.calledOnce, false)
})

it('index is correct', function () {
const action = {
actionType: windowConstants.WINDOW_ON_TAB_PAGE_MENU,
index: 0
}
contextMenuReducer(state, action)
assert.equal(menuBuildFromTemplateSpy.calledOnce, true)
assert.equal(menuPopupSpy.calledOnce, true)
})
})
})

0 comments on commit ce69e5f

Please sign in to comment.