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

Commit

Permalink
Use new tab.on('tab-replaced-at') event instead of <webview>.tab-id-c…
Browse files Browse the repository at this point in the history
…hanged to ensure state gets updated correctly when a tab is discarded
  • Loading branch information
petemill committed Feb 14, 2018
1 parent ecddc07 commit 7d2fc77
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 14 deletions.
2 changes: 1 addition & 1 deletion app/browser/reducers/tabsReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ const tabsReducer = (state, action, immutableAction) => {
// tabs.debugTabs(state)
break
case appConstants.APP_TAB_REPLACED:
tabs.tabIdChanged(action.get('oldTabId'), action.get('newTabId'))
state = tabState.replaceTabValue(state, action.get('oldTabId'), action.get('newTabValue'))
break
case appConstants.APP_TAB_CLOSE_REQUESTED:
{
Expand Down
24 changes: 16 additions & 8 deletions app/browser/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,9 @@ const api = {
})

process.on('add-new-contents', (e, source, newTab, disposition, size, userGesture) => {
if (shouldDebugTabEvents) {
console.log('process:add-new-contents', newTab)
}
if (userGesture === false) {
e.preventDefault()
return
Expand Down Expand Up @@ -616,6 +619,19 @@ const api = {
}
})

tab.on('tab-replaced-at', (e, windowId, tabIndex, newContents) => {
console.log(`tab ${tab.getId()} changed to`, {newTabId: newContents.getId(), tabIndex})
const newTabId = newContents.getId()
if (shouldDebugTabEvents) {
console.log(`Tab [${tabId}] changed to tabId ${newTabId}. Updating state references...`)
}
// update state
appActions.tabReplaced(tabId, getTabValue(newTabId), getTabValue(tabId).get('windowId'))
// update in-memory caches
webContentsCache.tabIdChanged(tabId, newTabId)
activeTabHistory.tabIdChanged(tabId, newTabId)
})

tab.on('tab-strip-empty', () => {
// It's only safe to close a window when the last web-contents tab has been
// re-attached. A detach which already happens by this point is not enough.
Expand Down Expand Up @@ -854,14 +870,6 @@ const api = {
}
},

tabIdChanged: (oldTabId, newTabId) => {
if (shouldDebugTabEvents) {
console.log(`Tab [${oldTabId}] changed to tabId ${newTabId}. Updating state references...`)
}
webContentsCache.tabIdChanged(oldTabId, newTabId)
activeTabHistory.tabIdChanged(oldTabId, newTabId)
},

pin: (state, tabId, pinned) => {
const tab = webContentsCache.getWebContents(tabId)
if (tab && !tab.isDestroyed()) {
Expand Down
27 changes: 27 additions & 0 deletions app/common/state/tabState.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,31 @@ const tabState = {
return state.set('tabs', tabs.delete(index).insert(index, tabValue))
},

replaceTabValue: (state, tabId, newTabValue) => {
state = validateState(state)
newTabValue = makeImmutable(newTabValue)
// update tab
const index = getTabInternalIndexByTabId(state, tabId)
const oldTabValue = state.getIn(['tabs', index])
if (index == null || index === -1) {
console.error(`tabState: cannot replace tab ${tabId} as tab's index did not exist in state`, { index })
return state
}
let mergedTabValue = oldTabValue.mergeDeep(newTabValue)
if (mergedTabValue.has('frame')) {
mergedTabValue = mergedTabValue.mergeIn(['frame'], {
tabId: newTabValue.get('tabId'),
guestInstanceId: newTabValue.get('guestInstanceId')
})
}
mergedTabValue = mergedTabValue.set('windowId', oldTabValue.get('windowId'))
state = state.set('tabs', state.get('tabs').delete(index).insert(index, mergedTabValue))
// update tabId at tabsInternal index
state = deleteTabsInternalIndex(state, oldTabValue)
state = updateTabsInternalIndex(state, 0)
return state
},

removeTabField: (state, field) => {
state = makeImmutable(state)

Expand All @@ -421,12 +446,14 @@ const tabState = {
state = validateState(state)
action = validateAction(action)
const tabId = action.getIn(['frame', 'tabId'])
console.log(`frame changed ${tabId}`)
if (!tabId) {
return state
}

let tabValue = tabState.getByTabId(state, tabId)
if (!tabValue) {
console.log(`Frame changed - tab ${tabId} not found.`)
return state
}

Expand Down
1 change: 0 additions & 1 deletion app/renderer/components/frame/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,6 @@ class Frame extends React.Component {
return
}
if (this.props.tabId !== e.tabID) {
appActions.tabReplaced(this.props.tabId, e.tabID)
}
}, { passive: true })
this.webview.addEventListener('guest-ready', (e) => {
Expand Down
8 changes: 5 additions & 3 deletions js/actions/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,12 +302,14 @@ const appActions = {
})
},

tabReplaced: function (oldTabId, newTabId) {
console.log('tab replaced', {oldTabId, newTabId})
tabReplaced: function (oldTabId, newTabValue, windowId) {
dispatch({
actionType: appConstants.APP_TAB_REPLACED,
oldTabId,
newTabId
newTabValue,
queryInfo: {
windowId
}
})
},

Expand Down
5 changes: 4 additions & 1 deletion js/stores/windowStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,11 @@ const newFrame = (state, frameOpts) => {
const frameTabIdChanged = (state, action) => {
action = makeImmutable(action)
const oldTabId = action.get('oldTabId')
const newTabId = action.get('newTabId')
const newTabValue = action.get('newTabValue')
const newTabId = newTabValue.get('tabId')
console.log('frame id changed', oldTabId, newTabId)
if (newTabId == null || oldTabId === newTabId) {
console.error('Invalid action arguments for frameTabIdChanged')
return state
}

Expand Down

0 comments on commit 7d2fc77

Please sign in to comment.