refactor preview handling to make the view dumber

Author: bridiver

app/renderer/components/tabs/tab.js

@@ -27,7 +27,6 @@ const tabContentState = require('../../../common/state/tabContentState')
 
 // Constants
 const dragTypes = require('../../../../js/constants/dragTypes')
-const settings = require('../../../../js/constants/settings')
 
 // Styles
 const styles = require('../styles/tab')
@@ -43,7 +42,6 @@ const frameStateUtil = require('../../../../js/state/frameStateUtil')
 const {getTabBreakpoint, tabUpdateFrameRate} = require('../../lib/tabUtil')
 const {isWindows} = require('../../../common/lib/platformUtil')
 const {getCurrentWindowId} = require('../../currentWindow')
-const {getSetting} = require('../../../../js/settings')
 const UrlUtil = require('../../../../js/lib/urlutil')
 const {hasBreakpoint} = require('../../lib/tabUtil')
 
@@ -130,25 +128,10 @@ class Tab extends React.Component {
   }
 
   onMouseLeave () {
-    if (this.props.previewTabs) {
-      window.clearTimeout(this.hoverTimeout)
-      windowActions.setPreviewFrame(null)
-    }
     windowActions.setTabHoverState(this.props.frameKey, false)
   }
 
   onMouseEnter (e) {
-    // relatedTarget inside mouseenter checks which element before this event was the pointer on
-    // if this element has a tab-like class, then it's likely that the user was previewing
-    // a sequency of tabs. Called here as previewMode.
-    const previewMode = /tab(?!pages)/i.test(e.relatedTarget.classList)
-
-    // If user isn't in previewMode, we add a bit of delay to avoid tab from flashing out
-    // as reported here: https://github.com/brave/browser-laptop/issues/1434
-    if (this.props.previewTabs) {
-      this.hoverTimeout =
-        window.setTimeout(windowActions.setPreviewFrame.bind(null, this.props.frameKey), previewMode ? 0 : 200)
-    }
     windowActions.setTabHoverState(this.props.frameKey, true)
   }
 
@@ -279,7 +262,6 @@ class Tab extends React.Component {
 
     // used in other functions
     props.totalTabs = state.get('tabs').size
-    props.previewTabs = getSetting(settings.SHOW_TAB_PREVIEWS)
     props.dragData = state.getIn(['dragData', 'type']) === dragTypes.TAB && state.get('dragData')
     props.hasTabInFullScreen = tabContentState.hasTabInFullScreen(currentWindow)
     props.tabId = frame.get('tabId')

app/renderer/components/tabs/tabPage.js

@@ -32,23 +32,6 @@ class TabPage extends React.Component {
     this.onMouseLeave = this.onMouseLeave.bind(this)
   }
 
-  onMouseLeave () {
-    window.clearTimeout(this.hoverTimeout)
-    windowActions.setPreviewTabPageIndex()
-  }
-
-  onMouseEnter (e) {
-    // relatedTarget inside mouse enter checks which element before this event was the pointer on
-    // if this element has a tab-like class, then it's likely that the user was previewing
-    // a sequence of tabs. Called here as previewMode.
-    const previewMode = /tab(?!pages)/i.test(e.relatedTarget.classList)
-
-    // If user isn't in previewMode, we add a bit of delay to avoid tab from flashing out
-    // as reported here: https://github.com/brave/browser-laptop/issues/1434
-    this.hoverTimeout =
-      window.setTimeout(windowActions.setPreviewTabPageIndex.bind(null, this.props.index), previewMode ? 0 : 200)
-  }
-
   onDrop (e) {
     if (this.props.isPageEmpty) {
       return

app/renderer/reducers/frameReducer.js

@@ -20,8 +20,6 @@ const frameStateUtil = require('../../../js/state/frameStateUtil')
 const {getCurrentWindowId} = require('../currentWindow')
 const {getSourceAboutUrl, getSourceMagnetUrl} = require('../../../js/lib/appUrlUtil')
 const {isURL, isPotentialPhishingUrl, getUrlFromInput} = require('../../../js/lib/urlutil')
-const settings = require('../../../js/constants/settings')
-const {getSetting} = require('../../../js/settings')
 
 const setFullScreen = (state, action) => {
   const index = frameStateUtil.getFrameIndex(state, action.frameProps.get('key'))
@@ -32,15 +30,13 @@ const setFullScreen = (state, action) => {
 }
 
 const closeFrame = (state, action) => {
-  const activeFrameIndex = frameStateUtil.getActiveFrameIndex(state)
   const index = frameStateUtil.getFrameIndex(state, action.frameKey)
   if (index === -1) {
     return state
   }
 
   const frameProps = frameStateUtil.getFrameByKey(state, action.frameKey)
   const hoverState = state.getIn(['frames', index, 'hoverState'])
-  const framePreviewEnabled = getSetting(settings.SHOW_TAB_PREVIEWS)
 
   state = state.merge(frameStateUtil.removeFrame(
     state,
@@ -56,14 +52,12 @@ const closeFrame = (state, action) => {
 
   const nextFrame = frameStateUtil.getFrameByIndex(state, index)
 
-  if (nextFrame && hoverState) {
+  if (nextFrame) {
     // Copy the hover state if tab closed with mouse as long as we have a next frame
     // This allow us to have closeTab button visible for sequential frames closing,
     // until onMouseLeave event happens.
-    windowActions.setTabHoverState(nextFrame.get('key'), hoverState)
-    if (framePreviewEnabled && index !== activeFrameIndex) {
-      // After closing a tab, preview the next frame as long as there is one
-      windowActions.setPreviewFrame(nextFrame.get('key'))
+    if (hoverState) {
+      windowActions.setTabHoverState(nextFrame.get('key'), hoverState)
     }
   }
 

docs/windowActions.md

@@ -192,7 +192,8 @@ Dispatches a message to the store when the frame is active and the window is foc
 ### setPreviewFrame(frameKey) 
 
 Dispatches a message to the store to set a preview frame.
-This is done when hovering over a tab.
+This should only be called internally by `WINDOW_SET_TAB_HOVER_STATE`
+when we need to delay updating the preview frame value
 
 **Parameters**
 
@@ -234,16 +235,6 @@ Dispatches a message to the store to set the current tab hover state.
 
 
 
-### setPreviewTabPageIndex(previewTabPageIndex) 
-
-Dispatches a message to the store to set the tab page index being previewed.
-
-**Parameters**
-
-**previewTabPageIndex**: `number`, The tab page index to preview
-
-
-
 ### setTabPageIndexByFrame(frameProps) 
 
 Dispatches a message to the store to set the tab page index.

js/actions/windowActions.js

@@ -229,7 +229,8 @@ const windowActions = {
 
   /**
    * Dispatches a message to the store to set a preview frame.
-   * This is done when hovering over a tab.
+   * This should only be called internally by `WINDOW_SET_TAB_HOVER_STATE`
+   * when we need to delay updating the preview frame value
    *
    * @param {Object} frameKey - the frame key for the webview in question.
    */
@@ -280,18 +281,6 @@ const windowActions = {
     })
   },
 
-  /**
-   * Dispatches a message to the store to set the tab page index being previewed.
-   *
-   * @param {number} previewTabPageIndex - The tab page index to preview
-   */
-  setPreviewTabPageIndex: function (previewTabPageIndex) {
-    dispatch({
-      actionType: windowConstants.WINDOW_SET_PREVIEW_TAB_PAGE_INDEX,
-      previewTabPageIndex
-    })
-  },
-
   /**
    * Dispatches a message to the store to set the tab page index.
    *

js/constants/windowConstants.js

@@ -11,7 +11,6 @@ const windowConstants = {
   WINDOW_CLOSE_FRAMES: _,
   WINDOW_SET_FOCUSED_FRAME: _,
   WINDOW_SET_PREVIEW_FRAME: _,
-  WINDOW_SET_PREVIEW_TAB_PAGE_INDEX: _,
   WINDOW_SET_TAB_PAGE_INDEX: _,
   WINDOW_SET_TAB_BREAKPOINT: _,
   WINDOW_SET_TAB_HOVER_STATE: _,

js/state/frameStateUtil.js

@@ -21,6 +21,8 @@ const {getSetting} = require('../settings')
 const {isIntermediateAboutPage} = require('../lib/appUrlUtil')
 const urlParse = require('../../app/common/urlParse')
 
+let hoverTimeout = null
+
 const comparatorByKeyAsc = (a, b) => a.get('key') > b.get('key')
   ? 1 : b.get('key') > a.get('key') ? -1 : 0
 
@@ -397,9 +399,8 @@ function removeFrame (state, frameProps, framePropsIndex) {
   }
 }
 
-function getFrameTabPageIndex (state, frameProps, tabsPerTabPage) {
-  frameProps = makeImmutable(frameProps)
-  const index = findNonPinnedDisplayIndexForFrameKey(state, frameProps.get('key'))
+function getFrameTabPageIndex (state, frameKey, tabsPerTabPage = getSetting(settings.TABS_PER_PAGE)) {
+  const index = findNonPinnedDisplayIndexForFrameKey(state, frameKey)
   if (index === -1) {
     return -1
   }
@@ -451,7 +452,8 @@ function isPinned (state, frameKey) {
  * @param frameProps Any frame belonging to the page
  */
 function updateTabPageIndex (state, frameProps) {
-  const index = getFrameTabPageIndex(state, frameProps, getSetting(settings.TABS_PER_PAGE))
+  frameProps = makeImmutable(frameProps)
+  const index = getFrameTabPageIndex(state, frameProps.get('key'))
 
   if (index === -1) {
     return state
@@ -532,7 +534,51 @@ const getTabPageCount = (state) => {
   return Math.ceil(frames.size / tabsPerPage)
 }
 
+const getPreviewFrameKey = (state) => {
+  return state.get('previewFrameKey')
+}
+
+const setPreviewFrameKey = (state, frameKey) => {
+  clearTimeout(hoverTimeout)
+  const frame = getFrameByKey(state, frameKey)
+  const isActive = isFrameKeyActive(state, frameKey)
+  const previewTabs = getSetting(settings.SHOW_TAB_PREVIEWS)
+  let newPreviewFrameKey = frameKey
+
+  if (!previewTabs || frame == null || !frame.get('hoverState') || isActive) {
+    newPreviewFrameKey = null
+  }
+
+  // if there is an existing preview frame key then we're already in preview mode
+  // we use actions here because that is the only way to delay updating the state
+  const previewMode = getPreviewFrameKey(state) != null
+  if (previewMode && newPreviewFrameKey == null) {
+    // add a small delay when we are clearing the preview frame key so we don't lose
+    // previewMode if the user mouses over another tab - see below
+    this.hoverTimeout = setTimeout(windowActions.setPreviewFrame.bind(null, null), 200)
+    return state
+  }
+
+  if (!previewMode) {
+    // If user isn't in previewMode so we add a bit of delay to avoid tab from flashing out
+    // as reported here: https://github.com/brave/browser-laptop/issues/1434
+    // using an action here because that is the only way we can do a delayed state update
+    hoverTimeout = setTimeout(windowActions.setPreviewFrame.bind(null, newPreviewFrameKey), 200)
+    return state
+  }
+
+  const index = getFrameTabPageIndex(state, frame)
+  if (index !== state.getIn(['ui', 'tabs', 'tabPageIndex'])) {
+    state = state.setIn(['ui', 'tabs', 'previewTabPageIndex'], index)
+  } else {
+    state = state.deleteIn(['ui', 'tabs', 'previewTabPageIndex'])
+  }
+  return state.set('previewFrameKey', newPreviewFrameKey)
+}
+
 module.exports = {
+  setPreviewFrameKey,
+  getPreviewFrameKey,
   deleteTabInternalIndex,
   deleteFrameInternalIndex,
   updateFramesInternalIndex,

js/stores/windowStore.js

@@ -355,19 +355,7 @@ const doAction = (action) => {
       windowState = windowState.set('closedFrames', new Immutable.List())
       break
     case windowConstants.WINDOW_SET_PREVIEW_FRAME:
-      windowState = windowState.merge({
-        previewFrameKey:
-          action.frameKey != null && !frameStateUtil.isFrameKeyActive(windowState, action.frameKey)
-          ? action.frameKey
-          : null
-      })
-      break
-    case windowConstants.WINDOW_SET_PREVIEW_TAB_PAGE_INDEX:
-      if (action.previewTabPageIndex !== windowState.getIn(['ui', 'tabs', 'tabPageIndex'])) {
-        windowState = windowState.setIn(['ui', 'tabs', 'previewTabPageIndex'], action.previewTabPageIndex)
-      } else {
-        windowState = windowState.deleteIn(['ui', 'tabs', 'previewTabPageIndex'])
-      }
+      windowState = frameStateUtil.setPreviewFrameKey(windowState, action.frameKey)
       break
     case windowConstants.WINDOW_SET_TAB_PAGE_INDEX:
       if (action.index !== undefined) {
@@ -393,6 +381,7 @@ const doAction = (action) => {
         const frameIndex = frameStateUtil.getFrameIndex(windowState, action.frameKey)
         if (frameIndex !== -1) {
           windowState = windowState.setIn(['frames', frameIndex, 'hoverState'], action.hoverState)
+          windowState = frameStateUtil.setPreviewFrameKey(windowState, action.frameKey)
         }
         break
       }