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

Commit

Permalink
Added close tab settings
Browse files Browse the repository at this point in the history
Resolves #6553

Auditors: @bbondy @bsclifton

Test Plan:
- Go to Preferences and set desired action
- Close tab
- New active tab should opened as defined in preferences
  • Loading branch information
NejcZdovc authored and bsclifton committed Jan 23, 2017
1 parent 36203c4 commit 2789018
Show file tree
Hide file tree
Showing 9 changed files with 182 additions and 44 deletions.
10 changes: 9 additions & 1 deletion app/common/constants/settingsEnums.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,16 @@ const bookmarksToolbarMode = {
FAVICONS_ONLY: 'faviconsOnly'
}

const tabCloseAction = {
LAST_ACTIVE: 'lastActive',
NEXT: 'next',
FIRST: 'first',
PARENT: 'parent'
}

module.exports = {
startsWithOption,
newTabMode,
bookmarksToolbarMode
bookmarksToolbarMode,
tabCloseAction
}
5 changes: 5 additions & 0 deletions app/extensions/brave/locales/en-US/preferences.properties
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ startsWith=Brave starts with
startsWithOptionLastTime=My windows / tabs from last time
startsWithOptionHomePage=Home page
startsWithOptionNewTabPage=Dashboard
tabCloseActionLastActive=Last active tab
tabCloseActionNext=Next tab
tabCloseActionFirst=First tab
tabCloseActionParent=Parent tab
newTabMode=A new tab shows
newTabBlank=blank
newTabNewTabPage=Dashboard
Expand All @@ -142,6 +146,7 @@ engineGoKey=Engine Go Key (Type First)
switchToNewTabs=Switch to new tabs immediately
paintTabs=Show tabs in page theme color
tabsPerTabPage=Number of tabs per tab set:
tabCloseAction=Which tab should be activated after the tab is closed:
showTabPreviews=Show tab previews on hover
showHistoryMatches=Show history matches
showBookmarkMatches=Show bookmark matches
Expand Down
1 change: 1 addition & 0 deletions docs/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ WindowStore
showFullScreenWarning: boolean, // true if a warning should be shown about full screen
startLoadTime: datetime,
endtLoadTime: datetime,
lastAccessedTime: datetime,
guestInstanceId: string, // not persisted
tabId: number, // session tab id not persisted
closedAtIndex: number, // Index the frame was last closed at, cleared unless the frame is inside of closedFrames
Expand Down
13 changes: 12 additions & 1 deletion js/about/preferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const messages = require('../constants/messages')
const settings = require('../constants/settings')
const coinbaseCountries = require('../constants/coinbaseCountries')
const {passwordManagers, extensionIds} = require('../constants/passwordManagers')
const {startsWithOption, newTabMode, bookmarksToolbarMode} = require('../../app/common/constants/settingsEnums')
const {startsWithOption, newTabMode, bookmarksToolbarMode, tabCloseAction} = require('../../app/common/constants/settingsEnums')
const {l10nErrorText} = require('../../app/common/lib/httpUtil')

const WidevineInfo = require('../../app/renderer/components/widevineInfo')
Expand Down Expand Up @@ -858,6 +858,17 @@ class TabsTab extends ImmutableComponent {
}
</select>
</SettingItem>
<SettingItem dataL10nId='tabCloseAction'>
<select
className='form-control'
value={getSetting(settings.TAB_CLOSE_ACTION, this.props.settings)}
onChange={changeSetting.bind(null, this.props.onChangeSetting, settings.TAB_CLOSE_ACTION)}>
<option data-l10n-id='tabCloseActionLastActive' value={tabCloseAction.LAST_ACTIVE} />
<option data-l10n-id='tabCloseActionNext' value={tabCloseAction.NEXT} />
<option data-l10n-id='tabCloseActionFirst' value={tabCloseAction.FIRST} />
<option data-l10n-id='tabCloseActionParent' value={tabCloseAction.PARENT} />
</select>
</SettingItem>
<SettingCheckbox dataL10nId='switchToNewTabs' prefKey={settings.SWITCH_TO_NEW_TABS} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
<SettingCheckbox dataL10nId='paintTabs' prefKey={settings.PAINT_TABS} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
<SettingCheckbox dataL10nId='showTabPreviews' prefKey={settings.SHOW_TAB_PREVIEWS} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
Expand Down
1 change: 1 addition & 0 deletions js/constants/appConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ module.exports = {
'tabs.switch-to-new-tabs': false,
'tabs.paint-tabs': true,
'tabs.tabs-per-page': 10,
'tabs.close-action': 'parent',
'tabs.show-tab-previews': 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 @@ -21,6 +21,7 @@ const settings = {
OFFER_SEARCH_SUGGESTIONS: 'search.offer-search-suggestions',
// Tabs tab
SWITCH_TO_NEW_TABS: 'tabs.switch-to-new-tabs',
TAB_CLOSE_ACTION: 'tabs.close-action',
PAINT_TABS: 'tabs.paint-tabs',
TABS_PER_PAGE: 'tabs.tabs-per-page',
SHOW_TAB_PREVIEWS: 'tabs.show-tab-previews',
Expand Down
79 changes: 54 additions & 25 deletions js/state/frameStateUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

const Immutable = require('immutable')
const config = require('../constants/config')
const {tabCloseAction} = require('../../app/common/constants/settingsEnums')
const urlParse = require('url').parse

const matchFrame = (queryInfo, frame) => {
Expand Down Expand Up @@ -185,6 +186,23 @@ function getFramePropsIndex (frames, frameProps) {
return frames.findIndex(matchFrame.bind(null, queryInfo))
}

/**
* Find frame that was accessed last
*/
function getFrameByLastAccessedTime (frames) {
const frameProps = frames.toJS()
.reduce((pre, cur) => {
return (cur.pinnedLocation === undefined &&
cur.lastAccessedTime &&
cur.lastAccessedTime > pre.lastAccessedTime
) ? cur : pre
}, {
lastAccessedTime: 0
})

return (frameProps.lastAccessedTime === 0) ? -1 : getFramePropsIndex(frames, frameProps)
}

/**
* Determines if the specified frame was opened from the specified
* ancestorFrameKey.
Expand Down Expand Up @@ -328,6 +346,7 @@ function addFrame (windowState, tabs, frameOpts, newKey, partitionNumber, active
loading: !!delayedLoadUrl,
startLoadTime: delayedLoadUrl ? new Date().getTime() : null,
endLoadTime: null,
lastAccessedTime: (activeFrameKey === newKey) ? new Date().getTime() : null,
isPrivate: false,
partitionNumber,
pinnedLocation: isPinned ? url : undefined,
Expand Down Expand Up @@ -391,32 +410,32 @@ function undoCloseFrame (windowState, closedFrames) {
* Removes a frame specified by frameProps
* @return Immutable top level application state ready to merge back in
*/
function removeFrame (frames, tabs, closedFrames, frameProps, activeFrameKey) {
function removeFrame (frames, tabs, closedFrames, frameProps, activeFrameKey, framePropsIndex, closeAction) {
function getNewActiveFrame (activeFrameIndex) {
// Go to the next frame if it exists.
let index = activeFrameIndex
let nextFrame = frames.get(index)
let nextFrame = newFrames.get(index)
do {
if (nextFrame) {
if (nextFrame.get('pinnedLocation') === undefined) {
return nextFrame.get('key')
}
nextFrame = frames.get(++index)
nextFrame = newFrames.get(++index)
}
} while (nextFrame)
// Otherwise go to the frame right before the active tab.
index = activeFrameIndex
while (index > 0) {
const prevFrame = frames.get(--index)
const prevFrame = newFrames.get(--index)
if (prevFrame && prevFrame.get('pinnedLocation') === undefined) {
return prevFrame.get('key')
}
}
// Fall back on the original logic.
return Math.max(
frames.get(activeFrameIndex)
? frames.get(activeFrameIndex).get('key')
: frames.get(activeFrameIndex - 1).get('key'),
newFrames.get(activeFrameIndex)
? newFrames.get(activeFrameIndex).get('key')
: newFrames.get(activeFrameIndex - 1).get('key'),
0)
}

Expand All @@ -431,33 +450,43 @@ function removeFrame (frames, tabs, closedFrames, frameProps, activeFrameKey) {
}
}

// If the frame being removed IS ACTIVE, then try to replace activeFrameKey with parentFrameKey
let isActiveFrameBeingRemoved = frameProps.get('key') === activeFrameKey
let parentFrameIndex = findIndexForFrameKey(frames, frameProps.get('parentFrameKey'))
let activeFrameIndex

if (!isActiveFrameBeingRemoved || parentFrameIndex === -1) {
activeFrameIndex = findIndexForFrameKey(frames, activeFrameKey)
} else {
activeFrameIndex = parentFrameIndex
}
const newFrames = frames.splice(framePropsIndex, 1)
const newTabs = tabs.splice(framePropsIndex, 1)
let newActiveFrameKey = activeFrameKey

const framePropsIndex = getFramePropsIndex(frames, frameProps)
frames = frames.splice(framePropsIndex, 1)
tabs = tabs.splice(framePropsIndex, 1)
// If the frame being removed IS ACTIVE
let isActiveFrameBeingRemoved = frameProps.get('key') === activeFrameKey
if (isActiveFrameBeingRemoved && newFrames.size > 0) {
let activeFrameIndex

switch (closeAction) {
case tabCloseAction.LAST_ACTIVE:
const lastActive = getFrameByLastAccessedTime(newFrames)
activeFrameIndex = (lastActive > -1) ? lastActive : frames.count() - 1
break
case tabCloseAction.NEXT:
activeFrameIndex = ((frames.count() - 1) === framePropsIndex) ? (framePropsIndex - 1) : framePropsIndex
break
case tabCloseAction.FIRST:
activeFrameIndex = 0
break
// Default is a parent tab
default:
let parentFrameIndex = findIndexForFrameKey(frames, frameProps.get('parentFrameKey'))
activeFrameIndex = (parentFrameIndex === -1) ? findIndexForFrameKey(frames, activeFrameKey) : parentFrameIndex
break
}

let newActiveFrameKey = activeFrameKey
if (isActiveFrameBeingRemoved && frames.size > 0) {
// Frame with focus was closed; let's find new active NON-PINNED frame.
// let's find new active NON-PINNED frame.
newActiveFrameKey = getNewActiveFrame(activeFrameIndex)
}

return {
previewFrameKey: null,
activeFrameKey: newActiveFrameKey,
closedFrames,
tabs,
frames
tabs: newTabs,
frames: newFrames
}
}

Expand Down
15 changes: 12 additions & 3 deletions js/stores/windowStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -338,9 +338,15 @@ const doAction = (action) => {
const frameProps = action.frameProps || frameStateUtil.getActiveFrame(windowState)
const index = frameStateUtil.getFramePropsIndex(windowState.get('frames'), frameProps)
const activeFrameKey = frameStateUtil.getActiveFrame(windowState).get('key')
windowState = windowState.merge(frameStateUtil.removeFrame(windowState.get('frames'), windowState.get('tabs'),
windowState.get('closedFrames'), frameProps.set('closedAtIndex', index),
activeFrameKey))
windowState = windowState.merge(frameStateUtil.removeFrame(
windowState.get('frames'),
windowState.get('tabs'),
windowState.get('closedFrames'),
frameProps.set('closedAtIndex', index),
activeFrameKey,
index,
getSetting(settings.TAB_CLOSE_ACTION)
))
// If we reach the limit of opened tabs per page while closing tabs, switch to
// the active tab's page otherwise the user will hang on empty page
let totalOpenTabs = windowState.get('frames').filter((frame) => !frame.get('pinnedLocation')).size
Expand All @@ -363,6 +369,9 @@ const doAction = (action) => {
activeFrameKey: action.frameProps.get('key'),
previewFrameKey: null
})
windowState = windowState.mergeIn(['frames', frameStateUtil.getFramePropsIndex(windowState.get('frames'), action.frameProps)], {
lastAccessedTime: new Date().getTime()
})
windowState = windowState.deleteIn(['ui', 'tabs', 'previewTabPageIndex'])
updateTabPageIndex(action.frameProps)
break
Expand Down
Loading

0 comments on commit 2789018

Please sign in to comment.