Merge pull request #10008 from brave/tabpages--9922

Update tabPage based on active tab state
brave · Jul 18, 2017 · 52a43e5 · 52a43e5
2 parents 9fdd83e + 0333c59
commit 52a43e5
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 21 deletions.
diff --git a/app/renderer/components/tabs/content/tabIcon.js b/app/renderer/components/tabs/content/tabIcon.js
@@ -30,12 +30,22 @@ class TabIcon extends ImmutableComponent {
       }
     })
 
+    let altProps
+    if (!this.props.symbol) {
+      altProps = {
+        'data-test-id': this.props['data-test-id'],
+        'data-test2-id': this.props['data-test2-id']
+      }
+    }
+
     return <div
       className={this.props.className}
       data-test-favicon={this.props['data-test-favicon']}
       onDragStart={this.props.onDragStart}
       draggable={this.props.draggable}
-      onClick={this.props.onClick}>
+      onClick={this.props.onClick}
+      {...altProps}
+    >
       {
         this.props.symbol
           ? <span

diff --git a/app/renderer/reducers/frameReducer.js b/app/renderer/reducers/frameReducer.js
@@ -83,7 +83,10 @@ const getLocation = (location) => {
 const frameReducer = (state, action, immutableAction) => {
   switch (action.actionType) {
     case appConstants.APP_TAB_UPDATED:
+      // This case will be fired for both tab creation and tab update.
+      // being `tabValue` set for tab creation and `changeInfo` set for tab update
       const tab = immutableAction.get('tabValue')
+      const changeInfo = immutableAction.get('changeInfo')
       if (!tab) {
         break
       }
@@ -112,26 +115,28 @@ const frameReducer = (state, action, immutableAction) => {
       }
 
       // TODO fix race condition in Muon more info in #9000
-      const title = immutableAction.getIn(['tabValue', 'title'])
+      const title = tab.get('title')
       if (title != null) {
         state = state.setIn(['frames', index, 'title'], title)
       }
 
       // TODO fix race condition in Muon more info in #9000
-      const active = immutableAction.getIn(['tabValue', 'active'])
-      const hasTabInHoverState = state.getIn(['ui', 'tabs', 'hoverTabIndex'])
+      const active = tab.get('active')
       if (active != null) {
         if (active) {
-          state = state.set('activeFrameKey', frame.get('key'))
-          if (hasTabInHoverState != null) {
+          state = frameStateUtil.setActiveFrameKey(state, frame.get('key'))
+          state = frameStateUtil.setFrameLastAccessedTime(state, index)
+
+          // Handle tabPage updates and preview cancelation based on tab updated
+          // otherwise tabValue will fire those events each time a tab finish loading
+          // see bug #8429
+          const isNewTab = changeInfo.isEmpty()
+          const activeTabHasUpdated = changeInfo.get('active') != null
+
+          if (!isNewTab && activeTabHasUpdated) {
+            state = frameStateUtil.updateTabPageIndex(state, tabId)
             state = state.set('previewFrameKey', null)
           }
-          if (frame.getIn(['ui', 'tabs', 'hoverTabPageIndex']) == null) {
-            state = state.deleteIn(['ui', 'tabs', 'previewTabPageIndex'])
-          }
-          state = state.setIn(['frames', index, 'lastAccessedTime'], new Date().getTime())
-
-          state = frameStateUtil.updateTabPageIndex(state, tabId)
         }
       }
       break

diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js
@@ -219,6 +219,14 @@ function getActiveFrameKey (state) {
   return state.get('activeFrameKey')
 }
 
+const setActiveFrameKey = (state, frameKey) => {
+  return state.set('activeFrameKey', frameKey)
+}
+
+const setFrameLastAccessedTime = (state, index) => {
+  return state.setIn(['frames', index, 'lastAccessedTime'], new Date().getTime())
+}
+
 function getNextFrame (state) {
   const activeFrameIndex = findDisplayIndexForFrameKey(state, getActiveFrameKey(state))
   const index = (activeFrameIndex + 1) % state.get('frames').size
@@ -461,16 +469,11 @@ function isPinned (state, frameKey) {
  */
 function updateTabPageIndex (state, tabId, tabsPerPage = getSetting(settings.TABS_PER_PAGE)) {
   const index = getFrameTabPageIndex(state, tabId, tabsPerPage)
-  const isTabInHoverState = !!getHoverTabIndex(state)
 
   if (index === -1) {
     return state
   }
 
-  // Do not update tabPageIndex if user is in hover mode
-  if (isTabInHoverState) {
-    return state
-  }
   return state.setIn(['ui', 'tabs', 'tabPageIndex'], index)
 }
 const frameStatePath = (state, frameKey) => {
@@ -735,6 +738,8 @@ module.exports = {
   getFrameByTabId,
   getIndexByTabId,
   getPartitionNumber,
+  setFrameLastAccessedTime,
+  setActiveFrameKey,
   getActiveFrame,
   getNextFrame,
   getPreviousFrame,

diff --git a/test/tab-components/tabPagesTest.js b/test/tab-components/tabPagesTest.js
@@ -14,7 +14,8 @@ const {
   tabPage2,
   activeWebview,
   activeTab,
-  tabsTab
+  tabsTab,
+  closeTab
 } = require('../lib/selectors')
 
 describe('tab pages', function () {
@@ -59,11 +60,10 @@ describe('tab pages', function () {
 
     it('shows the right number of tabs after closing with mouse', function * () {
       const numTabsPerPage = appConfig.defaultSettings[settings.TABS_PER_PAGE]
-      const firstTabOfNewPageIndex = numTabsPerPage
       yield this.app.client.click(newFrameButton)
         .waitForElementCount(tabPage, 2)
-        .closeTabWithMouse()
-        .closeTabByIndex(firstTabOfNewPageIndex)
+        .moveToObject(activeTab)
+        .click(closeTab)
         // No tab page indicator elements when 1 page
         .waitForElementCount(tabPage, 0)
         .waitForElementCount(tabsTabs, numTabsPerPage)