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

Avoid serializing Function in array template #13432

Merged
merged 1 commit into from
Mar 14, 2018
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
12 changes: 8 additions & 4 deletions app/common/state/contextMenuState.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,38 +5,42 @@
const Immutable = require('immutable')
const assert = require('assert')
const { makeImmutable, isMap } = require('./immutableUtil')
const uuid = require('uuid')

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

let contextMenuDetail = Immutable.Map()

const api = {
setContextMenu: (windowState, detail) => {
detail = makeImmutable(detail)
windowState = validateState(windowState)

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

return windowState
},

getContextMenu: (windowState) => {
windowState = validateState(windowState)
return windowState.get('contextMenuDetail', Immutable.Map())
return contextMenuDetail
},

selectedIndex: (windowState) => {
Expand Down
5 changes: 4 additions & 1 deletion app/renderer/components/common/contextMenu/contextMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ const windowActions = require('../../../../../js/actions/windowActions')
// Constants
const keyCodes = require('../../../../common/constants/keyCodes')

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

// Utils
const cx = require('../../../../../js/lib/classSet')
const frameStateUtil = require('../../../../../js/state/frameStateUtil')
Expand Down Expand Up @@ -215,7 +218,7 @@ class ContextMenu extends React.Component {
const currentWindow = state.get('currentWindow')
const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map()
const selectedIndex = currentWindow.getIn(['ui', 'contextMenu', 'selectedIndex'], null)
const contextMenuDetail = currentWindow.get('contextMenuDetail', Immutable.Map())
const contextMenuDetail = contextMenuState.getContextMenu(currentWindow)

const props = {}
props.lastZoomPercentage = activeFrame.get('lastZoomPercentage')
Expand Down
3 changes: 2 additions & 1 deletion app/renderer/components/frame/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const windowStore = require('../../../../js/stores/windowStore')
const appStoreRenderer = require('../../../../js/stores/appStoreRenderer')

// State
const contextMenuState = require('../../../common/state/contextMenuState')
const siteSettings = require('../../../../js/state/siteSettings')
const siteSettingsState = require('../../../common/state/siteSettingsState')
const tabState = require('../../../common/state/tabState')
Expand Down Expand Up @@ -856,7 +857,7 @@ class Frame extends React.Component {
const allSiteSettings = siteSettingsState.getAllSiteSettings(state, isPrivate)
const frameSiteSettings = siteSettings.getSiteSettingsForURL(allSiteSettings, location) || Immutable.Map()

const contextMenu = currentWindow.get('contextMenuDetail')
const contextMenu = contextMenuState.getContextMenu(currentWindow)
const tab = tabId && tabId > -1 && tabState.getByTabId(state, tabId)

const props = {}
Expand Down
2 changes: 1 addition & 1 deletion docs/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ WindowStore
},
cleanedOnShutdown: boolean, // whether app data was successfully cleared on shutdown
closedFrames: [], // holds the same type of frame objects as frames
contextMenuDetail: {
contextMenuDetail: { // currently using uuid hack to avoid serializing click function in template
bottom: number, // the bottom position of the context menu
left: number, // the left position of the context menu
maxHeight: number, // the maximum height of the context menu
Expand Down
9 changes: 8 additions & 1 deletion js/actions/windowActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -536,9 +536,16 @@ const windowActions = {
* @param {Object} detail - The context menu detail
*/
setContextMenuDetail: function (detail) {
// TODO(darkdh): This is a hack to prevent dispatch from serializing
// click function in template. `contextMenuDetail` is just a uuid to trigger
// state update for new menu
const Immutable = require('immutable')
const contextMenuState = require('../../app/common/state/contextMenuState')
let state = contextMenuState.setContextMenu(Immutable.Map(), detail)
const contextMenuDetail = state.get('contextMenuDetail')
dispatch({
actionType: windowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL,
detail
contextMenuDetail
})
},

Expand Down
25 changes: 16 additions & 9 deletions js/stores/windowStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -465,21 +465,28 @@ const doAction = (action) => {
windowState = windowState.delete('bookmarkFolderDetail')
break
case windowConstants.WINDOW_AUTOFILL_SELECTION_CLICKED:
windowState = contextMenuState.setContextMenu(windowState)
ipc.send('autofill-selection-clicked', action.tabId, action.value, action.frontEndId, action.index)
windowState = windowState.delete('contextMenuDetail')
break
case windowConstants.WINDOW_AUTOFILL_POPUP_HIDDEN:
if (!action.detail &&
windowState.getIn(['contextMenuDetail', 'type']) === 'autofill' &&
windowState.getIn(['contextMenuDetail', 'tabId']) === action.tabId) {
windowState = windowState.delete('contextMenuDetail')
if (action.notify) {
ipc.send('autofill-popup-hidden', action.tabId)
{
const contextMenuDetail = contextMenuState.getContextMenu(windowState)
if (!action.detail &&
contextMenuDetail.get('type') === 'autofill' &&
contextMenuDetail.get('tabId') === action.tabId) {
windowState = contextMenuState.setContextMenu(windowState)
if (action.notify) {
ipc.send('autofill-popup-hidden', action.tabId)
}
}
break
}
break
case windowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL:
windowState = contextMenuState.setContextMenu(windowState, action.detail)
if (action.contextMenuDetail) {
windowState = windowState.set('contextMenuDetail', action.contextMenuDetail)
} else {
windowState = windowState.delete('contextMenuDetail')
}
break
case windowConstants.WINDOW_SET_POPUP_WINDOW_DETAIL:
if (!action.detail) {
Expand Down