Skip to content

Commit

Permalink
Fixes tab pages after setting change
Browse files Browse the repository at this point in the history
Resolves brave#7806

Auditors: @bsclifton @cezaraugusto

Test Plan:
- Open about:preferences#tabs
- Set the number of tabs per tab set to 6
- Open new tabs to create a new tab set, which has just 2 tabs
- Increase the number to 8
  • Loading branch information
NejcZdovc committed Jun 29, 2017
1 parent 4147821 commit 6c27646
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 6 deletions.
4 changes: 2 additions & 2 deletions js/state/frameStateUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -451,9 +451,9 @@ function isPinned (state, frameKey) {
* Updates the tab page index to the specified frameProps
* @param frameProps Any frame belonging to the page
*/
function updateTabPageIndex (state, frameProps) {
function updateTabPageIndex (state, frameProps, tabsPerPage = getSetting(settings.TABS_PER_PAGE)) {
frameProps = makeImmutable(frameProps)
const index = getFrameTabPageIndex(state, frameProps.get('key'))
const index = getFrameTabPageIndex(state, frameProps.get('key'), tabsPerPage)

if (index === -1) {
return state
Expand Down
15 changes: 15 additions & 0 deletions js/stores/windowStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,18 @@ const frameGuestInstanceIdChanged = (state, action) => {
})
}

function handleChangeSettingAction (state, settingKey, settingValue) {
switch (settingKey) {
case settings.TABS_PER_PAGE:
const activeFrame = frameStateUtil.getActiveFrame(state)
state = frameStateUtil.updateTabPageIndex(state, activeFrame, settingValue)
break
default:
}

return state
}

const windowStore = new WindowStore()
const emitChanges = debounce(windowStore.emitChanges.bind(windowStore), 5)

Expand Down Expand Up @@ -712,6 +724,9 @@ const doAction = (action) => {
}
windowState = newFrame(windowState, action.frameOpts)
break
case appConstants.APP_CHANGE_SETTING:
windowState = handleChangeSettingAction(windowState, action.key, action.value)
break
case windowConstants.WINDOW_FRAME_MOUSE_ENTER:
windowState = windowState.setIn(['ui', 'mouseInFrame'], true)
break
Expand Down
1 change: 1 addition & 0 deletions test/lib/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ module.exports = {
securityTab: '[data-l10n-id="security"]',
paymentsTab: '[data-l10n-id="payments"]',
syncTab: '[data-l10n-id="sync"]',
tabsTab: '[data-l10n-id="tabs"]',
doneButton: '[data-l10n-id="done"]',
homepageInput: '[data-l10n-id="homepageInput"]',
compactBraveryPanelSwitch: '[data-test-id="compactBraveryPanelSwitch"] .switchBackground',
Expand Down
19 changes: 15 additions & 4 deletions test/tab-components/tabPagesTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ const {
tabPage1,
tabPage2,
activeWebview,
activeTab
activeTab,
tabsTab
} = require('../lib/selectors')

describe('tab pages', function () {
Expand Down Expand Up @@ -90,10 +91,20 @@ describe('tab pages', function () {

describe('tabs per page setting', function () {
it('takes effect immediately', function * () {
const defaultTabsPerPage = appConfig.defaultSettings[settings.TABS_PER_PAGE]
const newValue = 6
const tabs = appConfig.defaultSettings[settings.TABS_PER_PAGE]
const numberOfPages = Math.ceil(tabs / newValue)
const numberOfTabs = tabs - (Math.floor(tabs / newValue) * newValue)
yield this.app.client
.changeSetting(settings.TABS_PER_PAGE, 1)
.waitForElementCount(tabPage, defaultTabsPerPage)
.waitForBrowserWindow()
.loadUrl('about:preferences')
.waitForVisible(tabsTab)
.click(tabsTab)
.waitForElementCount('[data-test-id="tabsPerTabPage"]', 1)
.click('[data-test-id="tabsPerTabPage"]')
.click(`option[value="${newValue}"]`)
.waitForElementCount(tabPage, numberOfPages)
.waitForElementCount('.tabArea', numberOfTabs)
})
})
})
Expand Down

0 comments on commit 6c27646

Please sign in to comment.