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

Commit

Permalink
Review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
NejcZdovc committed Apr 19, 2017
1 parent 24b5036 commit c5e6255
Show file tree
Hide file tree
Showing 13 changed files with 213 additions and 183 deletions.
15 changes: 8 additions & 7 deletions app/browser/reducers/tabsReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const windowAction = require('../../../js/actions/windowActions.js')
const {makeImmutable} = require('../../common/state/immutableUtil')
const {getFlashResourceId} = require('../../../js/flash')
const {l10nErrorText} = require('../../common/lib/httpUtil')
const windows = require('../windows')

const tabsReducer = (state, action) => {
action = makeImmutable(action)
Expand Down Expand Up @@ -72,20 +73,20 @@ const tabsReducer = (state, action) => {
case appConstants.APP_LOAD_URL_IN_ACTIVE_TAB_REQUESTED:
state = tabs.loadURLInActiveTab(state, action)
break
case appConstants.APP_ON_NAVIGATE_BACK:
case appConstants.APP_ON_GO_BACK:
state = tabs.goBack(state, action)
break
case appConstants.APP_ON_NAVIGATE_FORWARD:
case appConstants.APP_ON_GO_FORWARD:
state = tabs.goForward(state, action)
break
case appConstants.APP_ON_NAVIGATE_INDEX:
case appConstants.APP_ON_GO_TO_INDEX:
state = tabs.goToIndex(state, action)
break
case appConstants.APP_ON_NAVIGATE_BACK_LONG:
case appConstants.APP_ON_GO_BACK_LONG:
{
const history = tabs.getHistoryEntries(state, action)
const tabValue = tabState.getByTabId(state, action.get('tabId'))
const windowId = tabs.getWindowId()
const windowId = windows.getActiveWindowId()

if (history !== null) {
windowAction.onLongBackHistory(
Expand All @@ -99,11 +100,11 @@ const tabsReducer = (state, action) => {
}
break
}
case appConstants.APP_ON_NAVIGATE_FORWARD_LONG:
case appConstants.APP_ON_GO_FORWARD_LONG:
{
const history = tabs.getHistoryEntries(state, action)
const tabValue = tabState.getByTabId(state, action.get('tabId'))
const windowId = tabs.getWindowId()
const windowId = windows.getActiveWindowId()

if (history !== null) {
windowAction.onLongForwardHistory(
Expand Down
12 changes: 2 additions & 10 deletions app/browser/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const Immutable = require('immutable')
const tabState = require('../common/state/tabState')
const {app, BrowserWindow, extensions, session, ipcMain} = require('electron')
const {makeImmutable} = require('../common/state/immutableUtil')
const {getTargetAboutUrl, getSourceAboutUrl, isSourceAboutUrl, newFrameUrl} = require('../../js/lib/appUrlUtil')
const {getTargetAboutUrl, getSourceAboutUrl, isSourceAboutUrl, newFrameUrl, isTargetAboutUrl} = require('../../js/lib/appUrlUtil')
const {isURL, getUrlFromInput, toPDFJSLocation, getDefaultFaviconUrl} = require('../../js/lib/urlutil')
const {isSessionPartition} = require('../../js/state/frameStateUtil')
const {getOrigin} = require('../../js/state/siteUtil')
Expand Down Expand Up @@ -652,14 +652,6 @@ const api = {
return state
},

getWindowId: () => {
if (BrowserWindow.getFocusedWindow()) {
return BrowserWindow.getFocusedWindow().id
}

return -1
},

getHistoryEntries: (state, action) => {
const tab = api.getWebContents(action.get('tabId'))
const sites = state ? state.get('sites') : null
Expand All @@ -682,7 +674,7 @@ const api = {
icon: null
}

if (url.startsWith('chrome-extension://')) {
if (isTargetAboutUrl(url)) {
// TODO: return brave lion (or better: get icon from extension if possible as data URI)
} else {
if (sites) {
Expand Down
8 changes: 8 additions & 0 deletions app/browser/windows.js
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,14 @@ const api = {

getWindow: (windowId) => {
return currentWindows[windowId]
},

getActiveWindowId: () => {
if (BrowserWindow.getFocusedWindow()) {
return BrowserWindow.getFocusedWindow().id
}

return windowState.WINDOW_ID_NONE
}
}

Expand Down
132 changes: 5 additions & 127 deletions app/common/state/contextMenuState.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,155 +3,33 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */

const assert = require('assert')
const eventUtil = require('../../../js/lib/eventUtil.js')
const appActions = require('../../../js/actions/appActions.js')
const windowAction = require('../../../js/actions/windowActions.js')
const config = require('../../../js/constants/config.js')
const CommonMenu = require('../commonMenu.js')
const locale = require('../../../js/l10n.js')
const { makeImmutable, isMap } = require('./immutableUtil')

const validateAction = function (action) {
action = makeImmutable(action)
assert.ok(isMap(action), 'action must be an Immutable.Map')
return action
}

const validateState = function (state) {
state = makeImmutable(state)
assert.ok(isMap(state), 'state must be an Immutable.Map')
return state
}

const contextMenuState = {
setContextMenu: (state, action) => {
action = validateAction(action)
setContextMenu: (state, detail) => {
detail = makeImmutable(detail)
state = validateState(state)

if (!action.get('detail')) {
if (!detail) {
if (state.getIn(['contextMenuDetail', 'type']) === 'hamburgerMenu') {
state = state.set('hamburgerMenuWasOpen', true)
} else {
state = state.set('hamburgerMenuWasOpen', false)
}
state = state.delete('contextMenuDetail')
} else {
if (!(action.getIn(['detail', 'type']) === 'hamburgerMenu' && state.get('hamburgerMenuWasOpen'))) {
state = state.set('contextMenuDetail', action.get('detail'))
if (!(detail.get('type') === 'hamburgerMenu' && state.get('hamburgerMenuWasOpen'))) {
state = state.set('contextMenuDetail', detail)
}
state = state.set('hamburgerMenuWasOpen', false)
}

return state
},

onLongBacHistory: (state, action) => {
action = validateAction(action)
state = validateState(state)
const history = action.get('history')

const menuTemplate = []

if (action.get('tabId') > -1 && history && history.get('entries').size > 0) {
const stopIndex = Math.max(((history.get('currentIndex') - config.navigationBar.maxHistorySites) - 1), -1)
for (let index = (history.get('currentIndex') - 1); index > stopIndex; index--) {
const entry = history.getIn(['entries', index])
const url = entry.get('url')

menuTemplate.push({
label: entry.get('display'),
icon: entry.get('icon'),
click: function (e) {
if (eventUtil.isForSecondaryAction(e)) {
appActions.createTabRequested({
url,
partitionNumber: action.get('partitionNumber'),
active: !!e.shiftKey
})
} else {
appActions.onNavigateIndex(action.get('tabId'), index)
}
}
})
}

// Always display "Show History" link
menuTemplate.push(
CommonMenu.separatorMenuItem,
{
label: locale.translation('showAllHistory'),
click: function () {
appActions.createTabRequested({
url: 'about:history'
})
windowAction.setContextMenuDetail()
}
})

state = contextMenuState.setContextMenu(state, makeImmutable({
detail: {
left: action.get('left'),
top: action.get('top'),
template: menuTemplate
}
}))
}

return state
},

onLongForwardHistory: (state, action) => {
action = validateAction(action)
state = validateState(state)
const history = action.get('history')

const menuTemplate = []

if (action.get('tabId') > -1 && history && history.get('entries').size > 0) {
const stopIndex = Math.min(((history.get('currentIndex') + config.navigationBar.maxHistorySites) + 1), history.get('entries').size)
for (let index = (history.get('currentIndex') + 1); index < stopIndex; index++) {
const entry = history.getIn(['entries', index])
const url = entry.get('url')

menuTemplate.push({
label: entry.get('display'),
icon: entry.get('icon'),
click: function (e) {
if (eventUtil.isForSecondaryAction(e)) {
appActions.createTabRequested({
url,
partitionNumber: action.get('partitionNumber'),
active: !!e.shiftKey
})
} else {
appActions.onNavigateIndex(action.get('tabId'), index)
}
}
})
}

// Always display "Show History" link
menuTemplate.push(
CommonMenu.separatorMenuItem,
{
label: locale.translation('showAllHistory'),
click: function () {
appActions.createTabRequested({
url: 'about:history'
})
windowAction.setContextMenuDetail()
}
})

state = contextMenuState.setContextMenu(state, makeImmutable({
detail: {
left: action.get('left'),
top: action.get('top'),
template: menuTemplate
}
}))
}

return state
}
}
Expand Down
8 changes: 4 additions & 4 deletions app/renderer/components/navigation/navigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,17 @@ class Navigator extends ImmutableComponent {
}

onBack (e) {
this.onNav(e, 'canGoBack', 'back', appActions.onNavigateBack)
this.onNav(e, 'canGoBack', 'back', appActions.onGoBack)
}

onForward (e) {
this.onNav(e, 'canGoForward', 'forward', appActions.onNavigateForward)
this.onNav(e, 'canGoForward', 'forward', appActions.onGoForward)
}

onBackLongPress (target) {
const activeTab = this.props.activeTab
const rect = target.parentNode.getBoundingClientRect()
appActions.onNavigateBackLong(activeTab.get('tabId'), {
appActions.onGoBackLong(activeTab.get('tabId'), {
left: rect.left,
bottom: rect.bottom
})
Expand All @@ -140,7 +140,7 @@ class Navigator extends ImmutableComponent {
onForwardLongPress (target) {
const activeTab = this.props.activeTab
const rect = target.parentNode.getBoundingClientRect()
appActions.onNavigateForwardLong(activeTab.get('tabId'), {
appActions.onGoForwardLong(activeTab.get('tabId'), {
left: rect.left,
bottom: rect.bottom
})
Expand Down
Loading

0 comments on commit c5e6255

Please sign in to comment.