From 07fc91bc2b02fd956b0d29d20e829be4f3d87975 Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Sun, 11 Jun 2017 12:29:06 -0700 Subject: [PATCH] Refactors TabPage and TabPages into redux Resolves #9339 Auditors: @bsclifton @bridiver Test Plan: - try to open enaugh tabs to trigger tab page (default is 20) - see if audio indicator is displayed if you have video playing in tab page - check if you can close tab page --- app/renderer/components/main/main.js | 5 +- app/renderer/components/tabs/tabPage.js | 66 +++++++++++++---- app/renderer/components/tabs/tabPages.js | 37 +++++----- js/state/frameStateUtil.js | 10 ++- test/unit/state/frameStateUtilTest.js | 94 +++++++++++++++++++++++- 5 files changed, 172 insertions(+), 40 deletions(-) diff --git a/app/renderer/components/main/main.js b/app/renderer/components/main/main.js index 350c4accb90..1c5aa837e74 100644 --- a/app/renderer/components/main/main.js +++ b/app/renderer/components/main/main.js @@ -849,10 +849,7 @@ class Main extends ImmutableComponent { onContextMenu={this.onTabContextMenu}> { nonPinnedFrames.size > tabsPerPage - ? + ? : null } diff --git a/app/renderer/components/tabs/tabPage.js b/app/renderer/components/tabs/tabPage.js index 3ead0076179..6a222d3eeeb 100644 --- a/app/renderer/components/tabs/tabPage.js +++ b/app/renderer/components/tabs/tabPage.js @@ -3,9 +3,10 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const React = require('react') +const Immutable = require('immutable') // Components -const ImmutableComponent = require('../immutableComponent') +const ReduxComponent = require('../reduxComponent') // Actions const appActions = require('../../../../js/actions/appActions') @@ -13,28 +14,33 @@ const windowActions = require('../../../../js/actions/windowActions') // Constants const dragTypes = require('../../../../js/constants/dragTypes') +const settings = require('../../../../js/constants/settings') // Utils +const {getSetting} = require('../../../../js/settings') const cx = require('../../../../js/lib/classSet') const {onTabPageContextMenu} = require('../../../../js/contextMenus') const {getCurrentWindowId} = require('../../currentWindow') const dndData = require('../../../../js/dndData') +const frameStateUtil = require('../../../../js/state/frameStateUtil') +const dnd = require('../../../../js/dnd') -class TabPage extends ImmutableComponent { - constructor () { - super() +class TabPage extends React.Component { + constructor (props) { + super(props) this.onMouseEnter = this.onMouseEnter.bind(this) this.onMouseLeave = this.onMouseLeave.bind(this) } + onMouseLeave () { window.clearTimeout(this.hoverTimeout) windowActions.setPreviewTabPageIndex() } onMouseEnter (e) { - // relatedTarget inside mouseenter checks which element before this event was the pointer on + // 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 sequency of tabs. Called here as previewMode. + // 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 @@ -44,13 +50,12 @@ class TabPage extends ImmutableComponent { } onDrop (e) { - if (this.props.tabPageFrames.size === 0) { + if (this.props.isPageEmpty) { return } appActions.dataDropped(getCurrentWindowId()) - const moveToFrame = this.props.tabPageFrames.get(0) const sourceDragData = dndData.getDragData(e.dataTransfer, dragTypes.TAB) const sourceDragFromPageIndex = this.props.sourceDragFromPageIndex // This must be executed async because the state change that this causes @@ -58,7 +63,7 @@ class TabPage extends ImmutableComponent { setTimeout(() => { // If we're moving to a right page, then we're already shifting everything to the left by one, so we want // to drop it on the right. - windowActions.moveTab(sourceDragData.get('key'), moveToFrame.get('key'), + windowActions.moveTab(sourceDragData.get('key'), this.props.moveToFrameKey, // Has -1 value for pinned tabs sourceDragFromPageIndex === -1 || sourceDragFromPageIndex >= this.props.index) @@ -73,17 +78,50 @@ class TabPage extends ImmutableComponent { e.preventDefault() } + mergeProps (state, ownProps) { + const currentWindow = state.get('currentWindow') + const frames = frameStateUtil.getNonPinnedFrames(currentWindow) || Immutable.List() + const tabsPerPage = Number(getSetting(settings.TABS_PER_PAGE)) + const tabPageFrames = frames.slice(ownProps.index * tabsPerPage, (ownProps.index * tabsPerPage) + tabsPerPage) + const isAudioPlaybackActive = tabPageFrames.find((frame) => + frame.get('audioPlaybackActive') && !frame.get('audioMuted')) + + let sourceDragFromPageIndex + const sourceDragData = dnd.getInterBraveDragData() + if (sourceDragData) { + sourceDragFromPageIndex = frames.findIndex((frame) => frame.get('key') === sourceDragData.get('key')) + + if (sourceDragFromPageIndex !== -1) { + sourceDragFromPageIndex /= tabsPerPage + } + } + + const props = {} + // used in renderer + props.index = ownProps.index + props.tabPageFrames = tabPageFrames // TODO (nejc) only primitives + props.isAudioPlaybackActive = isAudioPlaybackActive + props.previewTabPage = getSetting(settings.SHOW_TAB_PREVIEWS) + props.active = currentWindow.getIn(['ui', 'tabs', 'tabPageIndex']) === props.index + + // used in other functions + props.sourceDragFromPageIndex = sourceDragFromPageIndex + props.isPageEmpty = tabPageFrames.size === 0 + props.moveToFrameKey = tabPageFrames.getIn([0, 'key']) + + return props + } + render () { - const audioPlaybackActive = this.props.tabPageFrames.find((frame) => - frame.get('audioPlaybackActive') && !frame.get('audioMuted')) - return frame.get('key') === sourceDragData.get('key')) - if (sourceDragFromPageIndex !== -1) { - sourceDragFromPageIndex /= this.props.tabsPerTabPage - } - } return
{ - tabPageCount > 1 && - Array.from(new Array(tabPageCount)).map((x, i) => + this.props.tabPageCount > 1 && + Array.from(new Array(this.props.tabPageCount)).map((x, i) => ) + index={i} /> + ) }
} } -module.exports = TabPages +module.exports = ReduxComponent.connect(TabPages) diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js index 0c93c09df3d..3c2c7269bfe 100644 --- a/js/state/frameStateUtil.js +++ b/js/state/frameStateUtil.js @@ -525,6 +525,13 @@ const isValidClosedFrame = (frame) => { return !frame.get('isPrivate') } +const getTabPageCount = (state) => { + const frames = getNonPinnedFrames(state) || Immutable.List() + const tabsPerPage = Number(getSetting(settings.TABS_PER_PAGE)) + + return Math.ceil(frames.size / tabsPerPage) +} + module.exports = { deleteTabInternalIndex, deleteFrameInternalIndex, @@ -574,5 +581,6 @@ module.exports = { getTotalBlocks, isPinned, updateTabPageIndex, - isValidClosedFrame + isValidClosedFrame, + getTabPageCount } diff --git a/test/unit/state/frameStateUtilTest.js b/test/unit/state/frameStateUtilTest.js index 2205be2cb82..4752268dd8c 100644 --- a/test/unit/state/frameStateUtilTest.js +++ b/test/unit/state/frameStateUtilTest.js @@ -13,15 +13,19 @@ const defaultWindowStore = Immutable.fromJS({ }) describe('frameStateUtil', function () { - let frameStateUtil + let frameStateUtil, getSettingsValue before(function () { + getSettingsValue = 20 mockery.enable({ warnOnReplace: false, warnOnUnregistered: false, useCleanCache: true }) mockery.registerMock('electron', fakeElectron) + mockery.registerMock('../settings', { + getSetting: () => getSettingsValue + }) frameStateUtil = require('../../../js/state/frameStateUtil') this.windowState = Immutable.fromJS(Object.assign({}, defaultWindowStore.toJS())) }) @@ -319,4 +323,92 @@ describe('frameStateUtil', function () { assert.equal(result, '99+') }) }) + + describe('getTabPageCount', function () { + before(function () { + getSettingsValue = 6 + }) + + it('returns two pages when we have more tabs then the tab page limit', function () { + let state = Immutable.fromJS({ + activeFrameKey: 1, + frames: [ + { key: 1 }, + { key: 2 }, + { key: 3 }, + { key: 4 }, + { key: 5 }, + { key: 6 }, + { key: 7 }, + { key: 8 } + ], + framesInternal: { + index: { + 1: 0, + 2: 1, + 3: 2, + 4: 3, + 5: 4, + 6: 5, + 7: 6, + 8: 7 + } + } + }) + const result = frameStateUtil.getTabPageCount(state) + assert.equal(result, 2) + }) + + it('returns one pages when we have exactly the same tabs as the tab page limit', function () { + let state = Immutable.fromJS({ + activeFrameKey: 1, + frames: [ + { key: 1 }, + { key: 2 }, + { key: 3 }, + { key: 4 }, + { key: 5 }, + { key: 6 } + ], + framesInternal: { + index: { + 1: 0, + 2: 1, + 3: 2, + 4: 3, + 5: 4, + 6: 5 + } + } + }) + const result = frameStateUtil.getTabPageCount(state) + assert.equal(result, 1) + }) + + it('returns one pages when we have less tabs then the tab page limit', function () { + let state = Immutable.fromJS({ + activeFrameKey: 1, + frames: [ + { key: 1 }, + { key: 2 }, + { key: 3 }, + { key: 4 }, + { key: 5 }, + { key: 6 } + ], + framesInternal: { + index: { + 1: 0, + 2: 1, + 3: 2, + 4: 3, + 5: 4, + 6: 5 + } + } + }) + const result = frameStateUtil.getTabPageCount(state) + assert.equal(result, 1) + }) + }) })