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

make tab previews based on mouse idle time #9887

Merged
merged 2 commits into from
Aug 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions app/common/constants/settingsEnums.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ const bookmarksToolbarMode = {
FAVICONS_ONLY: 'faviconsOnly'
}

const tabPreviewTiming = {
LONG: 2000,
NORMAL: 1000,
SHORT: 500
}

const tabCloseAction = {
LAST_ACTIVE: 'lastActive',
NEXT: 'next',
Expand All @@ -41,6 +47,7 @@ module.exports = {
startsWithOption,
newTabMode,
bookmarksToolbarMode,
tabPreviewTiming,
tabCloseAction,
fullscreenOption,
autoplayOption
Expand Down
4 changes: 4 additions & 0 deletions app/extensions/brave/locales/en-US/preferences.properties
Original file line number Diff line number Diff line change
Expand Up @@ -381,3 +381,7 @@ urlBarOptions=URL Bar Options
disableTitleMode=Always show the URL bar
wideURLbar=Use wide URL bar
autoplay=Autoplay Media
tabPreviewTiming=Time to wait before previewing a tab
long=Long
normal=Normal
short=Short
38 changes: 37 additions & 1 deletion app/renderer/components/preferences/tabsTab.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const {SettingsList, SettingItem, SettingCheckbox} = require('../common/settings

const {SettingDropdown} = require('../common/dropdown')

const {tabCloseAction} = require('../../../common/constants/settingsEnums')
const {tabCloseAction, tabPreviewTiming} = require('../../../common/constants/settingsEnums')
const {changeSetting} = require('../../lib/settingsUtil')
const getSetting = require('../../../../js/settings').getSetting
const settings = require('../../../../js/constants/settings')
Expand All @@ -36,6 +36,22 @@ class TabsTab extends ImmutableComponent {
}
]
}
get tabPreviewTimingOptions () {
return [
{
id: 'long',
action: tabPreviewTiming.LONG
},
{
id: 'normal',
action: tabPreviewTiming.NORMAL
},
{
id: 'short',
action: tabPreviewTiming.SHORT
}
]
}
render () {
return (
<div>
Expand Down Expand Up @@ -107,6 +123,26 @@ class TabsTab extends ImmutableComponent {
settings={this.props.settings}
onChangeSetting={this.props.onChangeSetting}
/>
{
getSetting(settings.SHOW_TAB_PREVIEWS, this.props.settings)
? <SettingItem dataL10nId='tabPreviewTiming'>
<SettingDropdown
value={getSetting(settings.TAB_PREVIEW_TIMING, this.props.settings)}
onChange={changeSetting.bind(null, this.props.onChangeSetting, settings.TAB_PREVIEW_TIMING)}>
{this.tabPreviewTimingOptions.map(option =>
<option
data-l10n-id={option.id}
data-test-id='tabPreviewTimingOption'
data-test-active={
getSetting(settings.TAB_PREVIEW_TIMING, this.props.settings) === option.action
}
value={option.action}
/>
)}
</SettingDropdown>
</SettingItem>
: null
}
<SettingItem dataL10nId='dashboardSettingsTitle'>
<SettingCheckbox
dataL10nId='dashboardShowImages'
Expand Down
4 changes: 3 additions & 1 deletion app/renderer/components/styles/tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ const styles = StyleSheet.create({
alignItems: 'center',
display: 'flex',
flex: '1',
minWidth: '0' // @see https://bugzilla.mozilla.org/show_bug.cgi?id=1108514#c5
minWidth: '0', // @see https://bugzilla.mozilla.org/show_bug.cgi?id=1108514#c5
// prevent the icons wrapper from being the target of mouse events.
pointerEvents: 'none'
},

isPinned: {
Expand Down
4 changes: 3 additions & 1 deletion app/renderer/components/tabs/content/tabTitle.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ const styles = StyleSheet.create({
lineHeight: '1.6',
padding: globalStyles.spacing.defaultTabPadding,
color: 'transparent',
WebkitBackgroundClip: 'text'
WebkitBackgroundClip: 'text',
// prevents the title from being the target of mouse events.
pointerEvents: 'none'
},

enforceFontVisibility: {
Expand Down
29 changes: 24 additions & 5 deletions app/renderer/components/tabs/tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,20 @@ const contextMenus = require('../../../../js/contextMenus')
const dnd = require('../../../../js/dnd')
const throttle = require('../../../../js/lib/throttle')
const frameStateUtil = require('../../../../js/state/frameStateUtil')
const {getTabBreakpoint, tabUpdateFrameRate} = require('../../lib/tabUtil')
const {
getTabBreakpoint,
tabUpdateFrameRate,
hasBreakpoint,
hasTabAsRelatedTarget
} = require('../../lib/tabUtil')
const isWindows = require('../../../common/lib/platformUtil').isWindows()
const {getCurrentWindowId} = require('../../currentWindow')
const UrlUtil = require('../../../../js/lib/urlutil')
const {hasBreakpoint} = require('../../lib/tabUtil')

class Tab extends React.Component {
constructor (props) {
super(props)
this.onMouseMove = this.onMouseMove.bind(this)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bridiver before sending this I debounced mouse move but even with 10ms it was leading to bad behavior and tabs were previewing without user action, given they were debounced. The event triggered inside onmousemove is canceled with both mouseenter and mouseleave, which I guess cover the event's cost.

this.onMouseEnter = this.onMouseEnter.bind(this)
this.onMouseLeave = this.onMouseLeave.bind(this)
this.onUpdateTabSize = this.onUpdateTabSize.bind(this)
Expand Down Expand Up @@ -129,12 +134,22 @@ class Tab extends React.Component {
dnd.onDragOver(dragTypes.TAB, this.tabNode.getBoundingClientRect(), this.props.frameKey, this.draggingOverData, e)
}

onMouseLeave () {
windowActions.setTabHoverState(this.props.frameKey, false)
onMouseLeave (e) {
// mouseleave will keep the previewMode
// as long as the related target is another tab
windowActions.setTabHoverState(this.props.frameKey, false, hasTabAsRelatedTarget(e))
}

onMouseEnter (e) {
windowActions.setTabHoverState(this.props.frameKey, true)
// if mouse entered a tab we only trigger a new preview
// if user is in previewMode, which is defined by mouse move
windowActions.setTabHoverState(this.props.frameKey, true, this.props.previewMode)
}

onMouseMove () {
// dispatch a message to the store so it can delay
// and preview the tab based on mouse idle time
windowActions.onTabMouseMove(this.props.frameKey)
}

onAuxClick (e) {
Expand Down Expand Up @@ -258,6 +273,7 @@ class Tab extends React.Component {
props.dragData = state.getIn(['dragData', 'type']) === dragTypes.TAB && state.get('dragData')
props.hasTabInFullScreen = tabContentState.hasTabInFullScreen(currentWindow)
props.tabId = tabId
props.previewMode = currentWindow.getIn(['ui', 'tabs', 'previewMode'])

return props
}
Expand All @@ -275,6 +291,7 @@ class Tab extends React.Component {
}
})
return <div
data-tab-area
className={cx({
tabArea: true,
draggingOverLeft: this.isDraggingOverLeft && !this.isDraggingOverSelf,
Expand All @@ -284,6 +301,7 @@ class Tab extends React.Component {
partOfFullPageSet: this.props.partOfFullPageSet || !!this.props.tabWidth
})}
style={this.props.tabWidth ? { flex: `0 0 ${this.props.tabWidth}px` } : {}}
onMouseMove={this.onMouseMove}
onMouseEnter={this.onMouseEnter}
onMouseLeave={this.onMouseLeave}>
{
Expand All @@ -292,6 +310,7 @@ class Tab extends React.Component {
: null
}
<div
data-tab
ref={(node) => { this.tabNode = node }}
className={css(
styles.tab,
Expand Down
14 changes: 14 additions & 0 deletions app/renderer/lib/tabUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,17 @@ module.exports.hasBreakpoint = (breakpoint, arr) => {
arr = Array.isArray(arr) ? arr : [arr]
return arr.includes(breakpoint)
}

/**
* Check whether or not the related target is a tab
* by checking the parentNode dataset
* @param {Object} event - The mouse event
* @returns {Boolean} Whether or not the related target is a tab
*/
module.exports.hasTabAsRelatedTarget = (event) => {
const relatedTarget = event.relatedTarget
const hasDataset = relatedTarget.parentNode && relatedTarget.parentNode.dataset
const tabAsRelatedTarget = hasDataset.tab || hasDataset.tabArea

return hasDataset && tabAsRelatedTarget
}
4 changes: 3 additions & 1 deletion app/renderer/reducers/frameReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const closeFrame = (state, action) => {

const frameProps = frameStateUtil.getFrameByKey(state, action.frameKey)
const hoverState = frameStateUtil.getTabHoverState(state, action.frameKey)
const previewMode = state.getIn(['ui', 'tabs', 'previewMode'])

state = state.merge(frameStateUtil.removeFrame(
state,
Expand All @@ -57,7 +58,8 @@ const closeFrame = (state, action) => {
// This allow us to have closeTab button visible for sequential frames closing,
// until onMouseLeave event happens.
if (hoverState) {
state = frameStateUtil.setTabHoverState(state, nextFrame.get('key'), hoverState)
state = frameStateUtil
.setTabHoverState(state, nextFrame.get('key'), hoverState, previewMode)
}
} else if (hoverState && frameStateUtil.getPreviewFrameKey(state) === action.frameKey) {
state = frameStateUtil.setPreviewFrameKey(state, null)
Expand Down
5 changes: 3 additions & 2 deletions docs/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -705,8 +705,9 @@ WindowStore
size: array, // last known window size [x, y]
tabs: {
hoverTabIndex: number, // index of the current hovered tab
tabPageIndex: number, // index of the current tab page
previewTabPageIndex: number // index of the tab being previewed
previewMode: boolean, // whether or not tab preview should be fired based on mouse idle time
previewTabPageIndex: number, // index of the tab being previewed
tabPageIndex: number // index of the current tab page
},
},
widevinePanelDetail: {
Expand Down
28 changes: 12 additions & 16 deletions js/actions/windowActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,20 +242,6 @@ const windowActions = {
})
},

/**
* Dispatches a message to the store to set a preview frame.
* 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.
*/
setPreviewFrame: function (frameKey) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't use it anymore. frameStateUtilsetPreviewFrameKey(..args) now checks for preview mode and returns if there is none. I guess the only use case for that was to delay tab previews for one use case, which we don't have anymore

dispatch({
actionType: windowConstants.WINDOW_SET_PREVIEW_FRAME,
frameKey
})
},

/**
* Dispatches a message to the store to set the tab page index.
*
Expand Down Expand Up @@ -287,12 +273,15 @@ const windowActions = {
*
* @param {Object} frameKey - the frame key for the webview in question.
* @param {boolean} hoverState - whether or not mouse is over tab
* @param {boolean} previewMode - whether or not the next tab should be previewed
* based on mouse idle time
*/
setTabHoverState: function (frameKey, hoverState) {
setTabHoverState: function (frameKey, hoverState, previewMode) {
dispatch({
actionType: windowConstants.WINDOW_SET_TAB_HOVER_STATE,
frameKey,
hoverState
hoverState,
previewMode
})
},

Expand Down Expand Up @@ -1056,6 +1045,13 @@ const windowActions = {
})
},

onTabMouseMove: function (data) {
dispatch({
actionType: windowConstants.WINDOW_TAB_MOUSE_MOVE,
data
})
},

onTabMouseLeave: function (data) {
dispatch({
actionType: windowConstants.WINDOW_TAB_MOUSE_LEAVE,
Expand Down
1 change: 1 addition & 0 deletions js/constants/appConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ module.exports = {
'tabs.tabs-per-page': 20,
'tabs.close-action': 'parent',
'tabs.show-tab-previews': true,
'tabs.preview-timing': 1000,
'tabs.show-dashboard-images': true,
'privacy.history-suggestions': true,
'privacy.bookmark-suggestions': true,
Expand Down
1 change: 1 addition & 0 deletions js/constants/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const settings = {
PAINT_TABS: 'tabs.paint-tabs',
TABS_PER_PAGE: 'tabs.tabs-per-page',
SHOW_TAB_PREVIEWS: 'tabs.show-tab-previews',
TAB_PREVIEW_TIMING: 'tabs.preview-timing',
SHOW_DASHBOARD_IMAGES: 'tabs.show-dashboard-images',
// Privacy Tab
HISTORY_SUGGESTIONS: 'privacy.history-suggestions',
Expand Down
2 changes: 1 addition & 1 deletion js/constants/windowConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ const windowConstants = {
WINDOW_CLOSE_FRAME: _,
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: _,
Expand Down Expand Up @@ -82,6 +81,7 @@ const windowConstants = {
WINDOW_AUTOFILL_SELECTION_CLICKED: _,
WINDOW_AUTOFILL_POPUP_HIDDEN: _,
WINDOW_TAB_CLOSED_WITH_MOUSE: _,
WINDOW_TAB_MOUSE_MOVE: _,
WINDOW_TAB_MOUSE_LEAVE: _,
WINDOW_FRAME_MOUSE_ENTER: _,
WINDOW_FRAME_MOUSE_LEAVE: _,
Expand Down
Loading