diff --git a/app/renderer/reducers/frameReducer.js b/app/renderer/reducers/frameReducer.js index ddea1328a4f..a2ed68be7f9 100644 --- a/app/renderer/reducers/frameReducer.js +++ b/app/renderer/reducers/frameReducer.js @@ -128,7 +128,7 @@ const frameReducer = (state, action, immutableAction) => { } state = state.setIn(['frames', index, 'lastAccessedTime'], new Date().getTime()) - state = frameStateUtil.updateTabPageIndex(state, frame) + state = frameStateUtil.updateTabPageIndex(state, tabId) } } break diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js index ba6ae891c38..62a89e2e573 100644 --- a/js/state/frameStateUtil.js +++ b/js/state/frameStateUtil.js @@ -106,18 +106,18 @@ function getFrameKeysByDisplayIndex (state) { }, []) } -function getFrameKeysByNonPinnedDisplayIndex (state) { +function getTabIdsByNonPinnedDisplayIndex (state) { return state.get('frames') .filter((frame) => !frame.get('pinnedLocation')) - .map((frame) => frame.get('key')) + .map((frame) => frame.get('tabId')) } /** - * Obtains the display index for the specified frame key excluding pins + * Obtains the display index for the specified tab ID excluding pins */ -function findNonPinnedDisplayIndexForFrameKey (state, key) { - return getFrameKeysByNonPinnedDisplayIndex(state) - .findIndex((displayKey) => displayKey === key) +function findNonPinnedDisplayIndexForTabId (state, tabId) { + return getTabIdsByNonPinnedDisplayIndex(state) + .findIndex((displayKey) => displayKey === tabId) } function getFrameByDisplayIndex (state, i) { @@ -399,8 +399,8 @@ function removeFrame (state, frameProps, framePropsIndex) { } } -function getFrameTabPageIndex (state, frameKey, tabsPerTabPage = getSetting(settings.TABS_PER_PAGE)) { - const index = findNonPinnedDisplayIndexForFrameKey(state, frameKey) +function getFrameTabPageIndex (state, tabId, tabsPerTabPage = getSetting(settings.TABS_PER_PAGE)) { + const index = findNonPinnedDisplayIndexForTabId(state, tabId) if (index === -1) { return -1 } @@ -449,11 +449,12 @@ function isPinned (state, frameKey) { /** * Updates the tab page index to the specified frameProps - * @param frameProps Any frame belonging to the page + * @param state{Object} - Window state + * @param tabId{number} - Tab id for the frame + * @param tabsPerPage{string} - Current setting for tabs per page */ -function updateTabPageIndex (state, frameProps) { - frameProps = makeImmutable(frameProps) - const index = getFrameTabPageIndex(state, frameProps.get('key')) +function updateTabPageIndex (state, tabId, tabsPerPage = getSetting(settings.TABS_PER_PAGE)) { + const index = getFrameTabPageIndex(state, tabId, tabsPerPage) if (index === -1) { return state @@ -602,7 +603,7 @@ const setPreviewFrameKey = (state, frameKey, immediate = false) => { } } - const index = getFrameTabPageIndex(state, frame) + const index = frame ? getFrameTabPageIndex(state, frame.get('tabId')) : -1 if (index !== -1) { if (index !== state.getIn(['ui', 'tabs', 'tabPageIndex'])) { state = state.setIn(['ui', 'tabs', 'previewTabPageIndex'], index) diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index edd632f6d91..a8478a7c257 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -187,7 +187,7 @@ const newFrame = (state, frameOpts) => { if (openInForeground) { const tabId = frameOpts.tabId const frame = frameStateUtil.getFrameByTabId(state, tabId) - state = frameStateUtil.updateTabPageIndex(state, frame) + state = frameStateUtil.updateTabPageIndex(state, tabId) if (active) { // only set the activeFrameKey if the tab is already active state = state.set('activeFrameKey', frame.get('key')) @@ -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.get('tabId'), settingValue) + break + default: + } + + return state +} + const windowStore = new WindowStore() const emitChanges = debounce(windowStore.emitChanges.bind(windowStore), 5) @@ -365,7 +377,7 @@ const doAction = (action) => { windowState = windowState.setIn(['ui', 'tabs', 'tabPageIndex'], action.index) windowState = windowState.deleteIn(['ui', 'tabs', 'previewTabPageIndex']) } else { - windowState = frameStateUtil.updateTabPageIndex(windowState, action.frameProps) + windowState = frameStateUtil.updateTabPageIndex(windowState, action.frameProps.get('tabId')) } break case windowConstants.WINDOW_SET_TAB_BREAKPOINT: @@ -393,6 +405,7 @@ const doAction = (action) => { { const sourceFrameProps = frameStateUtil.getFrameByKey(windowState, action.sourceFrameKey) const sourceFrameIndex = frameStateUtil.getFrameIndex(windowState, action.sourceFrameKey) + const activeFrame = frameStateUtil.getActiveFrame(windowState) let newIndex = frameStateUtil.getFrameIndex(windowState, action.destinationFrameKey) + (action.prepend ? 0 : 1) let frames = frameStateUtil.getFrames(windowState).splice(sourceFrameIndex, 1) if (newIndex > sourceFrameIndex) { @@ -402,7 +415,7 @@ const doAction = (action) => { windowState = windowState.set('frames', frames) // Since the tab could have changed pages, update the tab page as well windowState = frameStateUtil.updateFramesInternalIndex(windowState, Math.min(sourceFrameIndex, newIndex)) - windowState = frameStateUtil.updateTabPageIndex(windowState, frameStateUtil.getActiveFrame(windowState)) + windowState = frameStateUtil.updateTabPageIndex(windowState, activeFrame.get('tabId')) break } case windowConstants.WINDOW_SET_LINK_HOVER_PREVIEW: @@ -712,6 +725,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 diff --git a/test/lib/selectors.js b/test/lib/selectors.js index 7ea7e454b61..0b429493afa 100644 --- a/test/lib/selectors.js +++ b/test/lib/selectors.js @@ -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', diff --git a/test/tab-components/tabPagesTest.js b/test/tab-components/tabPagesTest.js index 2221b959d35..1a8297302fc 100644 --- a/test/tab-components/tabPagesTest.js +++ b/test/tab-components/tabPagesTest.js @@ -12,7 +12,8 @@ const { tabPage1, tabPage2, activeWebview, - activeTab + activeTab, + tabsTab } = require('../lib/selectors') describe('tab pages', function () { @@ -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) + .tabByIndex(19) + .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) }) }) })