From 54cf3111f92dd580b54e14d3f1fddeda98668fd3 Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Sat, 7 Jan 2017 10:48:17 +0100 Subject: [PATCH] Added close tab page Resolves #5489 Auditors: @bbondy @bsclifton Test Plan: - open two tab pages - right click on one and click close tab page - all tabs in tab pages should be closed --- .../brave/locales/en-US/menu.properties | 1 + app/locale.js | 1 + docs/windowActions.md | 10 +++ js/actions/windowActions.js | 28 ++++++++ js/components/tabPages.js | 10 +-- js/constants/windowConstants.js | 1 + js/contextMenus.js | 5 ++ js/state/frameStateUtil.js | 68 ++++++++++++++++++ js/stores/windowStore.js | 14 ++++ test/unit/state/frameStateUtilTest.js | 71 +++++++++++++++++++ 10 files changed, 204 insertions(+), 5 deletions(-) diff --git a/app/extensions/brave/locales/en-US/menu.properties b/app/extensions/brave/locales/en-US/menu.properties index 607a50a9370..ac656479e97 100644 --- a/app/extensions/brave/locales/en-US/menu.properties +++ b/app/extensions/brave/locales/en-US/menu.properties @@ -13,6 +13,7 @@ openLocation=Open Location… openSearch=Search for "{{selectedVariable}}" importFrom=Import from… closeWindow=Close Window +closeTabPage=Close tab page savePageAs=Save Page as… spreadTheWord=Spread the Word About Brave… share=Share… diff --git a/app/locale.js b/app/locale.js index badb22149f7..80c94997186 100644 --- a/app/locale.js +++ b/app/locale.js @@ -76,6 +76,7 @@ var rendererIdentifiers = function () { 'closeOtherTabs', 'closeTabsToRight', 'closeTabsToLeft', + 'closeTabPage', 'bookmarkPage', 'bookmarkLink', 'openFile', diff --git a/docs/windowActions.md b/docs/windowActions.md index cf581e9d463..f18e5c989d9 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -235,6 +235,16 @@ Dispatches a message to close a frame +### closeFrames(framePropsList) + +Dispatches a message to close multiple frames + +**Parameters** + +**framePropsList**: `Array.<Object>`, The properties of all frames to close + + + ### undoClosedFrame() Dispatches a message to the store to undo a closed frame diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index 22305f2c4c1..13b0ce41d9e 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -10,6 +10,7 @@ const appActions = require('../actions/appActions') const messages = require('../constants/messages') const siteTags = require('../constants/siteTags') const siteUtil = require('../state/siteUtil') +const frameStateUtil = require('../state/frameStateUtil') const UrlUtil = require('../lib/urlutil') const {currentWindow} = require('../../app/renderer/currentWindow') const windowStore = require('../stores/windowStore') @@ -346,6 +347,33 @@ const windowActions = { } }, + /** + * Dispatches a message to close multiple frames + * @param {Object[]} framePropsList - The properties of all frames to close + */ + closeFrames: function (framePropsList) { + const activeFrameKey = windowStore.getState().get('activeFrameKey') + const activeFrame = frameStateUtil.findFrameInList(framePropsList, activeFrameKey) + + if (activeFrame) { + const origin = siteUtil.getOrigin(activeFrame.get('location')) + if (origin) { + appActions.clearMessageBoxes(origin) + } + + // If the frame was full screen, exit + if (activeFrame && activeFrame.get('isFullScreen')) { + this.setFullScreen(activeFrame, false) + } + } + + dispatch({ + actionType: windowConstants.WINDOW_CLOSE_FRAMES, + framePropsList, + activeFrameRemoved: activeFrame + }) + }, + /** * Dispatches a message to the store to undo a closed frame * The new frame is expected to appear at the index it was last closed at diff --git a/js/components/tabPages.js b/js/components/tabPages.js index 93821736885..fc9fcedab7d 100644 --- a/js/components/tabPages.js +++ b/js/components/tabPages.js @@ -35,10 +35,10 @@ class TabPage extends ImmutableComponent { } onDrop (e) { - if (this.props.frames.size === 0) { + if (this.props.tabPageFrames.size === 0) { return } - const moveToFrame = this.props.frames.get(0) + const moveToFrame = this.props.tabPageFrames.get(0) const sourceDragData = dndData.getDragData(e.dataTransfer, dragTypes.TAB) const sourceDragFromPageIndex = this.props.sourceDragFromPageIndex // This must be executed async because the state change that this causes @@ -63,7 +63,7 @@ class TabPage extends ImmutableComponent { } render () { - const audioPlaybackActive = this.props.frames.find((frame) => + const audioPlaybackActive = this.props.tabPageFrames.find((frame) => frame.get('audioPlaybackActive') && !frame.get('audioMuted')) return } @@ -97,7 +97,7 @@ class TabPages extends ImmutableComponent { Array.from(new Array(tabPageCount)).map((x, i) => { windowActions.muteAllAudio(framePropsList, true) } + }, { + label: locale.translation('closeTabPage'), + click: () => { + windowActions.closeFrames(framePropsList) + } }] } diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js index e79553fd045..fbf619605e1 100644 --- a/js/state/frameStateUtil.js +++ b/js/state/frameStateUtil.js @@ -6,6 +6,7 @@ const Immutable = require('immutable') const config = require('../constants/config') const {tabCloseAction} = require('../../app/common/constants/settingsEnums') const urlParse = require('../../app/common/urlParse') +const {makeImmutable} = require('../../app/common/state/immutableUtil') const matchFrame = (queryInfo, frame) => { queryInfo = queryInfo.toJS ? queryInfo.toJS() : queryInfo @@ -49,6 +50,10 @@ function getFrameByIndex (windowState, i) { return windowState.getIn(['frames', i]) } +function findFrameInList (frames, key) { + return frames.find(matchFrame.bind(null, {key})) +} + // This will eventually go away fully when we replace frameKey by tabId function getFrameKeyByTabId (windowState, tabId) { let parentFrameKey @@ -493,6 +498,67 @@ function removeFrame (frames, tabs, closedFrames, frameProps, activeFrameKey, fr frames: newFrames } } +/** + * Removes a frames specified by framePropsList + * @return Immutable top level application state ready to merge back in + */ +function removeFrames (frames, tabs, closedFrames, framePropsList, activeFrameRemoved, activeFrameKey, closeAction) { + function getLastTab (newFrames) { + const sorted = newFrames + .filter((frame) => !frame.get('pinnedLocation')) + .sortBy((item) => item.get('key')) + + return (sorted.size === 0) ? 0 : sorted.last().get('key') + } + + function getLastActiveTab (newFrames) { + const sorted = newFrames + .filter((frame) => !frame.get('pinnedLocation')) + .sortBy((item) => item.get('lastAccessedTime') || 0) + + return (sorted.size === 0) ? 0 : sorted.last().get('key') + } + + let newFrames = makeImmutable(frames) + let newTabs = makeImmutable(tabs) + + framePropsList.forEach((frameProps) => { + if (!frameProps.get('isPrivate') && frameProps.get('location') !== 'about:newtab') { + frameProps = frameProps.set('isFullScreen', false) + closedFrames = closedFrames.push(frameProps) + if (frameProps.get('thumbnailBlob')) { + window.URL.revokeObjectURL(frameProps.get('thumbnailBlob')) + } + if (closedFrames.size > config.maxClosedFrames) { + closedFrames = closedFrames.shift() + } + } + + let framePropsIndex = getFramePropsIndex(newFrames, frameProps) + newFrames = newFrames.splice(framePropsIndex, 1) + newTabs = newTabs.splice(framePropsIndex, 1) + }) + + // return last non pinned frame index if active frame was removed + if (activeFrameRemoved) { + switch (closeAction) { + case tabCloseAction.LAST_ACTIVE: + activeFrameKey = getLastActiveTab(newFrames) + break + default: + activeFrameKey = getLastTab(newFrames) + break + } + } + + return { + previewFrameKey: null, + activeFrameKey, + closedFrames, + tabs: newTabs, + frames: newFrames + } +} /** * Removes all but the specified frameProps @@ -568,12 +634,14 @@ module.exports = { getFramePropPath, findIndexForFrameKey, findDisplayIndexForFrameKey, + findFrameInList, getFramePropsIndex, getFrameKeysByDisplayIndex, getPartition, addFrame, undoCloseFrame, removeFrame, + removeFrames, removeOtherFrames, tabFromFrame, getFrameKeyByTabId, diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 0f431a10a4b..eb5b63a8fea 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -368,6 +368,20 @@ const doAction = (action) => { }) } break + case windowConstants.WINDOW_CLOSE_FRAMES: + windowState = windowState.merge(frameStateUtil.removeFrames( + windowState.get('frames'), + windowState.get('tabs'), + windowState.get('closedFrames'), + action.framePropsList, + action.activeFrameRemoved, + windowState.get('activeFrameKey'), + getSetting(settings.TAB_CLOSE_ACTION) + )) + + updateTabPageIndex(frameStateUtil.getActiveFrame(windowState)) + windowState = windowState.deleteIn(['ui', 'tabs', 'fixTabWidth']) + break case windowConstants.WINDOW_UNDO_CLOSED_FRAME: windowState = windowState.merge(frameStateUtil.undoCloseFrame(windowState, windowState.get('closedFrames'))) focusWebview(activeFrameStatePath(windowState)) diff --git a/test/unit/state/frameStateUtilTest.js b/test/unit/state/frameStateUtilTest.js index e49241d0139..6ef666d78c3 100644 --- a/test/unit/state/frameStateUtilTest.js +++ b/test/unit/state/frameStateUtilTest.js @@ -283,4 +283,75 @@ describe('frameStateUtil', function () { }) }) }) + + describe('removeFrames', function () { + let frames, tabs, closedFrames, framePropsList, activeFrameKey, activeFrameRemoved, activeFrameKeyKeep + + beforeEach(function () { + frames = Immutable.fromJS([ + { key: 2 }, + { key: 3, parentFrameKey: 2 }, + { key: 4 }, + { key: 5, pinnedLocation: 'https://twitter.com/', lastAccessedTime: 1488184050731 }, + { key: 6, lastAccessedTime: 1488184050721 }, + { key: 7, lastAccessedTime: undefined }, + { key: 8, lastAccessedTime: 1488184050711 }, + { key: 9, pinnedLocation: 'https://brave.com/' } + ]) + tabs = Immutable.fromJS([ + { key: 2 }, + { key: 3 }, + { key: 4 }, + { key: 5, pinnedLocation: 'https://twitter.com/' }, + { key: 6 }, + { key: 7 }, + { key: 8 }, + { key: 9, pinnedLocation: 'https://brave.com/' } + ]) + closedFrames = Immutable.fromJS([ + { key: 1 } + ]) + framePropsList = Immutable.fromJS([ + { key: 2 }, + { key: 3 }, + { key: 4 } + ]) + activeFrameRemoved = Immutable.fromJS({ key: 2 }) + activeFrameKey = 2 + activeFrameKeyKeep = 6 + }) + + it('removes frames from `frames`', function () { + const result = frameStateUtil.removeFrames(frames, tabs, closedFrames, framePropsList, undefined, activeFrameKeyKeep) + assert.equal(5, result.frames.size) + }) + + it('removed frames are in `closedFrame`', function () { + const result = frameStateUtil.removeFrames(frames, tabs, closedFrames, framePropsList, activeFrameRemoved, activeFrameKey) + assert.equal((framePropsList.size + closedFrames.size), result.closedFrames.size) + }) + + it('active frame is the same', function () { + const result = frameStateUtil.removeFrames(frames, tabs, closedFrames, framePropsList, undefined, activeFrameKeyKeep) + assert.equal(activeFrameKeyKeep, result.activeFrameKey) + }) + + describe('when active frame is removed', function () { + it('removes frames from `frames`', function () { + const result = frameStateUtil.removeFrames(frames, tabs, closedFrames, framePropsList, activeFrameRemoved, activeFrameKey) + const inFrames = result.frames.find((frame) => frame.get('key') === activeFrameRemoved.get('key')) + assert.equal(true, inFrames === undefined) + }) + + it('new active frame (last tab)', function () { + const result = frameStateUtil.removeFrames(frames, tabs, closedFrames, framePropsList, activeFrameRemoved, activeFrameKey) + assert.equal(8, result.activeFrameKey) + }) + + it('new active frame (last active tab)', function () { + const result = frameStateUtil.removeFrames(frames, tabs, closedFrames, framePropsList, activeFrameRemoved, activeFrameKey, tabCloseAction.LAST_ACTIVE) + assert.equal(6, result.activeFrameKey) + }) + }) + }) })