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

Commit

Permalink
Tabs should preview on tab page hover
Browse files Browse the repository at this point in the history
Fix #1424

Auditors: @diracdeltas
  • Loading branch information
bbondy committed Aug 9, 2016
1 parent 01198f1 commit 3cd705b
Show file tree
Hide file tree
Showing 12 changed files with 117 additions and 7 deletions.
4 changes: 4 additions & 0 deletions app/sessionStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ module.exports.cleanPerWindowData = (perWindowData) => {
delete perWindowData.contextMenuDetail
// Don't save preview frame since they are only related to hovering on a tab
delete perWindowData.previewFrameKey
// Don't save preview tab pages
if (perWindowData.ui && perWindowData.ui.tabs) {
delete perWindowData.ui.tabs.previewTabPageIndex
}
// Don't restore add/edit dialog
delete perWindowData.bookmarkDetail
// Don't restore bravery panel
Expand Down
10 changes: 10 additions & 0 deletions docs/windowActions.md
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,16 @@ Dispatches a message to the store to set the tab page index.



### 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.
Expand Down
12 changes: 12 additions & 0 deletions js/actions/windowActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,18 @@ 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.
*
Expand Down
2 changes: 2 additions & 0 deletions js/components/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -840,6 +840,7 @@ class Main extends ImmutableComponent {
nonPinnedFrames.size > tabsPerPage
? <TabPages frames={nonPinnedFrames}
tabsPerTabPage={tabsPerPage}
previewTabPage={getSetting(settings.SHOW_TAB_PREVIEWS)}
tabPageIndex={this.props.windowState.getIn(['ui', 'tabs', 'tabPageIndex'])} />
: null
}
Expand All @@ -851,6 +852,7 @@ class Main extends ImmutableComponent {
previewTabs={getSetting(settings.SHOW_TAB_PREVIEWS)}
tabsPerTabPage={tabsPerPage}
tabPageIndex={this.props.windowState.getIn(['ui', 'tabs', 'tabPageIndex'])}
previewTabPageIndex={this.props.windowState.getIn(['ui', 'tabs', 'previewTabPageIndex'])}
tabs={this.props.windowState.get('tabs')}
sites={this.props.appState.get('sites')}
key='tab-bar'
Expand Down
9 changes: 7 additions & 2 deletions js/components/tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ const dnd = require('../dnd')
const windowStore = require('../stores/windowStore')

class Tab extends ImmutableComponent {
constructor () {
super()
this.onMouseEnter = this.onMouseEnter.bind(this)
this.onMouseLeave = this.onMouseLeave.bind(this)
}
get frame () {
return windowStore.getFrame(this.props.tab.get('frameKey'))
}
Expand Down Expand Up @@ -177,8 +182,8 @@ class Tab extends ImmutableComponent {
isPinned: this.isPinned,
partOfFullPageSet: this.props.partOfFullPageSet
})}
onMouseEnter={this.props.previewTabs ? this.onMouseEnter.bind(this) : null}
onMouseLeave={this.props.previewTabs ? this.onMouseLeave.bind(this) : null}>
onMouseEnter={this.props.previewTabs ? this.onMouseEnter : null}
onMouseLeave={this.props.previewTabs ? this.onMouseLeave : null}>
<div className={cx({
tab: true,
isPinned: this.isPinned,
Expand Down
27 changes: 26 additions & 1 deletion js/components/tabPages.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,28 @@ const dnd = require('../dnd')
const dndData = require('../dndData')

class TabPage extends ImmutableComponent {
constructor () {
super()
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
// 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
this.hoverTimeout =
window.setTimeout(windowActions.setPreviewTabPageIndex.bind(null, this.props.index), previewMode ? 0 : 200)
}

onDrop (e) {
if (this.props.frames.size === 0) {
return
Expand Down Expand Up @@ -45,6 +67,8 @@ class TabPage extends ImmutableComponent {
frame.get('audioPlaybackActive') && !frame.get('audioMuted'))
return <span data-tab-page={this.props.index}
onDragOver={this.onDragOver.bind(this)}
onMouseEnter={this.props.previewTabPage ? this.onMouseEnter : null}
onMouseLeave={this.props.previewTabPage ? this.onMouseLeave : null}
onDrop={this.onDrop.bind(this)}
className={cx({
tabPage: true,
Expand All @@ -68,13 +92,14 @@ class TabPages extends ImmutableComponent {
sourceDragFromPageIndex /= this.props.tabsPerTabPage
}
}
return <div>
return <div className='tabPageWrap'>
{
tabPageCount > 1 &&
Array.from(new Array(tabPageCount)).map((x, i) =>
<TabPage
key={`tabPage-${i}`}
frames={this.props.frames.slice(i * this.props.tabsPerTabPage, i * this.props.tabsPerTabPage + this.props.tabsPerTabPage)}
previewTabPage={this.props.previewTabPage}
index={i}
sourceDragFromPageIndex={sourceDragFromPageIndex}
active={this.props.tabPageIndex === i} />)
Expand Down
7 changes: 5 additions & 2 deletions js/components/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,18 @@ class Tabs extends ImmutableComponent {

render () {
this.tabRefs = []
const index = this.props.previewTabPageIndex !== undefined
? this.props.previewTabPageIndex : this.props.tabPageIndex
return <div className='tabs'>
<span className={cx({
tabStripContainer: true,
isPreview: this.props.previewTabPageIndex !== undefined,
allowDragging: this.props.shouldAllowWindowDrag
})}
onDragOver={this.onDragOver}
onDrop={this.onDrop}>
{(() => {
if (this.props.tabPageIndex > 0) {
if (index > 0) {
return <span
className='prevTab fa fa-caret-left'
onClick={this.onPrevPage} />
Expand All @@ -116,7 +119,7 @@ class Tabs extends ImmutableComponent {
partOfFullPageSet={this.props.partOfFullPageSet} />)
}
{(() => {
if (this.props.currentTabs.size >= this.props.tabsPerTabPage && this.totalPages > this.props.tabPageIndex + 1) {
if (this.props.currentTabs.size >= this.props.tabsPerTabPage && this.totalPages > index + 1) {
return <span
className='nextTab fa fa-caret-right'
onClick={this.onNextPage} />
Expand Down
5 changes: 4 additions & 1 deletion js/components/tabsToolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ class TabsToolbar extends ImmutableComponent {
}

render () {
const startingFrameIndex = this.props.tabPageIndex * this.props.tabsPerTabPage
const index = this.props.previewTabPageIndex !== undefined
? this.props.previewTabPageIndex : this.props.tabPageIndex
const startingFrameIndex = index * this.props.tabsPerTabPage
const pinnedTabs = this.props.tabs.filter((tab) => tab.get('pinnedLocation'))
const unpinnedTabs = this.props.tabs.filter((tab) => !tab.get('pinnedLocation'))
const currentTabs = unpinnedTabs
Expand Down Expand Up @@ -60,6 +62,7 @@ class TabsToolbar extends ImmutableComponent {
activeFrameKey={this.props.activeFrameKey}
tabPageIndex={this.props.tabPageIndex}
currentTabs={currentTabs}
previewTabPageIndex={this.props.previewTabPageIndex}
startingFrameIndex={startingFrameIndex}
partOfFullPageSet={currentTabs.size === this.props.tabsPerTabPage}
/>
Expand Down
1 change: 1 addition & 0 deletions js/constants/windowConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const windowConstants = {
WINDOW_SET_ACTIVE_FRAME: _,
WINDOW_SET_FOCUSED_FRAME: _,
WINDOW_SET_PREVIEW_FRAME: _,
WINDOW_SET_PREVIEW_TAB_PAGE_INDEX: _,
WINDOW_SET_TAB_PAGE_INDEX: _,
WINDOW_SET_IS_BEING_DRAGGED_OVER_DETAIL: _,
WINDOW_TAB_MOVE: _,
Expand Down
11 changes: 11 additions & 0 deletions js/stores/windowStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ const updateTabPageIndex = (frameProps) => {
return
}
windowState = windowState.setIn(['ui', 'tabs', 'tabPageIndex'], index)
windowState = windowState.deleteIn(['ui', 'tabs', 'previewTabPageIndex'])
}

const focusWebview = (frameStatePath) => {
Expand Down Expand Up @@ -416,6 +417,7 @@ const doAction = (action) => {
activeFrameKey: action.frameProps.get('key'),
previewFrameKey: null
})
windowState = windowState.deleteIn(['ui', 'tabs', 'previewTabPageIndex'])
updateTabPageIndex(action.frameProps)
break
case WindowConstants.WINDOW_SET_PREVIEW_FRAME:
Expand All @@ -424,9 +426,17 @@ const doAction = (action) => {
? action.frameProps.get('key') : 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'])
}
break
case WindowConstants.WINDOW_SET_TAB_PAGE_INDEX:
if (action.index !== undefined) {
windowState = windowState.setIn(['ui', 'tabs', 'tabPageIndex'], action.index)
windowState = windowState.deleteIn(['ui', 'tabs', 'previewTabPageIndex'])
} else {
updateTabPageIndex(action.frameProps)
}
Expand Down Expand Up @@ -580,6 +590,7 @@ const doAction = (action) => {
windowState = windowState.merge({
previewFrameKey: null
})
windowState = windowState.deleteIn(['ui', 'tabs', 'previewTabPageIndex'])
// Pin changes need to happen right away or else a race condition could happen for app state
// change detection where it adds a second frame
windowStore.emitChanges()
Expand Down
6 changes: 6 additions & 0 deletions less/tabs.less
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@
height: 26px;
line-height: 22px;
}

&.isPreview {
opacity: 0.5;
animation: tabFadeIn 0.75s ease-in-out;
animation-fill-mode: forwards;
}
}

.tab {
Expand Down
30 changes: 29 additions & 1 deletion test/components/tabPagesTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const Brave = require('../lib/brave')
const appConfig = require('../../js/constants/appConfig')
const settings = require('../../js/constants/settings')
const {urlInput, newFrameButton, tabPage, tabPage1, tabPage2, closeTab, activeWebview} = require('../lib/selectors')
const {urlInput, newFrameButton, tabsTabs, tabPage, tabPage1, tabPage2, closeTab, activeWebview} = require('../lib/selectors')
const assert = require('assert')

describe('tab pages', function () {
Expand Down Expand Up @@ -77,4 +77,32 @@ describe('tab pages', function () {
})
})
})

describe('tab page previews', function () {
Brave.beforeAll(this)

before(function * () {
yield setup(this.app.client)
yield this.app.client.changeSetting(settings.TABS_PER_PAGE, appConfig.defaultSettings[settings.TABS_PER_PAGE])
// Make sure there are 2 tab pages
for (let i = 0; i < appConfig.defaultSettings[settings.TABS_PER_PAGE]; i++) {
yield this.app.client.windowByUrl(Brave.browserWindowUrl).click(newFrameButton)
}

yield this.app.client
.waitUntil(function () {
return this.elements(tabPage).then((res) => res.value.length === 2)
})
})

it('hovering over a tab page changes it', function * () {
yield this.app.client
.waitForExist(tabPage2 + '.active')
.moveToObject(tabPage1, 5, 5)
.waitForExist('.tabStripContainer.isPreview')
.waitUntil(function () {
return this.elements(tabsTabs).then((res) => res.value.length === appConfig.defaultSettings[settings.TABS_PER_PAGE])
})
})
})
})

0 comments on commit 3cd705b

Please sign in to comment.