From ff49fdd9ff8921a3c3c5809632bbc1e3c03b2e91 Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Mon, 7 Aug 2017 17:52:26 +0200 Subject: [PATCH] Moves bookmark text calculation into the state Resolves #9517 Auditors: Test Plan: --- app/browser/menu.js | 2 + .../reducers/bookmarkFoldersReducer.js | 48 ++++- .../reducers/bookmarkToolbarReducer.js | 37 ++++ app/browser/reducers/bookmarksReducer.js | 52 ++++- app/browser/reducers/windowsReducer.js | 18 +- app/browser/tabs.js | 8 +- app/browser/windows.js | 10 +- app/common/lib/bookmarkFoldersUtil.js | 26 ++- app/common/lib/bookmarkToolbarUtil.js | 88 +++++++++ app/common/lib/bookmarkUtil.js | 185 +++++++----------- app/common/lib/textCalcUtil.js | 128 ++++++++++++ app/common/state/bookmarkFoldersState.js | 26 +-- app/common/state/bookmarkToolbarState.js | 63 ++++++ app/common/state/bookmarksState.js | 92 ++------- .../components/bookmarks/bookmarksToolbar.js | 8 +- docs/state.md | 10 +- js/actions/appActions.js | 33 ++++ js/constants/appConstants.js | 5 +- js/lib/textCalculator.js | 9 - js/stores/appStore.js | 27 ++- .../reducers/bookmarkFoldersReducerTest.js | 150 +++++++++++++- .../reducers/bookmarkToolbarReducerTest.js | 140 +++++++++++++ .../browser/reducers/bookmarksReducerTest.js | 158 ++++++++++++++- .../app/browser/reducers/windowReducerTest.js | 89 +++++++++ .../app/common/lib/bookmarkFoldersUtilTest.js | 64 +++++- .../app/common/lib/bookmarkToolbarUtilTest.js | 94 +++++++++ test/unit/app/common/lib/bookmarkUtilTest.js | 157 ++++++++++++++- 27 files changed, 1458 insertions(+), 269 deletions(-) create mode 100644 app/browser/reducers/bookmarkToolbarReducer.js create mode 100644 app/common/lib/bookmarkToolbarUtil.js create mode 100644 app/common/lib/textCalcUtil.js create mode 100644 app/common/state/bookmarkToolbarState.js delete mode 100644 js/lib/textCalculator.js create mode 100644 test/unit/app/browser/reducers/bookmarkToolbarReducerTest.js create mode 100644 test/unit/app/browser/reducers/windowReducerTest.js create mode 100644 test/unit/app/common/lib/bookmarkToolbarUtilTest.js diff --git a/app/browser/menu.js b/app/browser/menu.js index b3d8e62a8e2..40736d0aa19 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -701,8 +701,10 @@ const doAction = (state, action) => { } case appConstants.APP_ADD_BOOKMARK: case appConstants.APP_EDIT_BOOKMARK: + case appConstants.APP_MOVE_BOOKMARK: case appConstants.APP_REMOVE_BOOKMARK: case appConstants.APP_ADD_BOOKMARK_FOLDER: + case appConstants.APP_MOVE_BOOKMARK_FOLDER: case appConstants.APP_EDIT_BOOKMARK_FOLDER: case appConstants.APP_REMOVE_BOOKMARK_FOLDER: createMenu(state) diff --git a/app/browser/reducers/bookmarkFoldersReducer.js b/app/browser/reducers/bookmarkFoldersReducer.js index 0849fcd05f4..3241c7ddd98 100644 --- a/app/browser/reducers/bookmarkFoldersReducer.js +++ b/app/browser/reducers/bookmarkFoldersReducer.js @@ -6,14 +6,18 @@ const Immutable = require('immutable') // State const bookmarkFoldersState = require('../../common/state/bookmarkFoldersState') +const bookmarkToolbarState = require('../../common/state/bookmarkToolbarState') // Constants const appConstants = require('../../../js/constants/appConstants') +const siteTags = require('../../../js/constants/siteTags') const {STATE_SITES} = require('../../../js/constants/stateConstants') // Utils const {makeImmutable} = require('../../common/state/immutableUtil') const syncUtil = require('../../../js/state/syncUtil') +const textCalcUtil = require('../../common/lib/textCalcUtil') +const bookmarkFolderUtil = require('../../common/lib/bookmarkFoldersUtil') const bookmarkFoldersReducer = (state, action, immutableAction) => { action = immutableAction || makeImmutable(action) @@ -29,12 +33,16 @@ const bookmarkFoldersReducer = (state, action, immutableAction) => { if (Immutable.List.isList(folder)) { action.get('folderDetails', Immutable.List()).forEach((folder) => { - state = bookmarkFoldersState.addFolder(state, folder, closestKey) - state = syncUtil.updateObjectCache(state, folder, STATE_SITES.BOOKMARK_FOLDERS) + const folderDetails = bookmarkFolderUtil.buildFolder(folder, bookmarkFoldersState.getFolders(state)) + state = bookmarkFoldersState.addFolder(state, folderDetails, closestKey) + state = syncUtil.updateObjectCache(state, folderDetails, STATE_SITES.BOOKMARK_FOLDERS) + textCalcUtil.calcText(folderDetails, siteTags.BOOKMARK_FOLDER) }) } else { - state = bookmarkFoldersState.addFolder(state, folder, closestKey) - state = syncUtil.updateObjectCache(state, folder, STATE_SITES.BOOKMARK_FOLDERS) + const folderDetails = bookmarkFolderUtil.buildFolder(folder, bookmarkFoldersState.getFolders(state)) + state = bookmarkFoldersState.addFolder(state, folderDetails, closestKey) + state = syncUtil.updateObjectCache(state, folderDetails, STATE_SITES.BOOKMARK_FOLDERS) + textCalcUtil.calcText(folderDetails, siteTags.BOOKMARK_FOLDER) } break } @@ -49,7 +57,8 @@ const bookmarkFoldersReducer = (state, action, immutableAction) => { state = bookmarkFoldersState.editFolder(state, key, folder) state = syncUtil.updateObjectCache(state, folder, STATE_SITES.BOOKMARK_FOLDERS) - + const folderDetails = bookmarkFoldersState.getFolder(state, key) + textCalcUtil.calcText(folderDetails, siteTags.BOOKMARK_FOLDER) break } case appConstants.APP_MOVE_BOOKMARK_FOLDER: @@ -70,6 +79,9 @@ const bookmarkFoldersReducer = (state, action, immutableAction) => { const destinationDetail = bookmarkFoldersState.getFolder(state, action.get('destinationKey')) state = syncUtil.updateObjectCache(state, destinationDetail, STATE_SITES.BOOKMARK_FOLDERS) + if (destinationDetail.get('parentFolderId') === 0) { + state = bookmarkToolbarState.setToolbars(state) + } break } case appConstants.APP_REMOVE_BOOKMARK_FOLDER: @@ -82,16 +94,38 @@ const bookmarkFoldersReducer = (state, action, immutableAction) => { if (Immutable.List.isList(folderKey)) { action.get('folderKey', Immutable.List()).forEach((key) => { - const folder = state.getIn([STATE_SITES.BOOKMARK_FOLDERS, key]) + const folder = bookmarkFoldersState.getFolder(state, key) state = bookmarkFoldersState.removeFolder(state, key) state = syncUtil.updateObjectCache(state, folder, STATE_SITES.BOOKMARK_FOLDERS) }) + state = bookmarkToolbarState.setToolbars(state) } else { - const folder = state.getIn([STATE_SITES.BOOKMARK_FOLDERS, folderKey]) + const folder = bookmarkFoldersState.getFolder(state, folderKey) state = bookmarkFoldersState.removeFolder(state, folderKey) state = syncUtil.updateObjectCache(state, folder, STATE_SITES.BOOKMARK_FOLDERS) + if (folder.get('parentFolderId') === 0) { + state = bookmarkToolbarState.setToolbars(state) + } + } + break + } + case appConstants.APP_ON_BOOKMARK_FOLDER_WIDTH_CHANGED: + { + if (action.get('folderList', Immutable.List()).isEmpty()) { + break } + let updateToolbar = false + action.get('folderList').forEach(item => { + state = bookmarkFoldersState.setWidth(state, item.get('key'), item.get('width')) + if (item.get('parentFolderId') === 0) { + updateToolbar = true + } + }) + + if (updateToolbar) { + state = bookmarkToolbarState.setToolbars(state) + } break } } diff --git a/app/browser/reducers/bookmarkToolbarReducer.js b/app/browser/reducers/bookmarkToolbarReducer.js new file mode 100644 index 00000000000..7084f683cde --- /dev/null +++ b/app/browser/reducers/bookmarkToolbarReducer.js @@ -0,0 +1,37 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +// Constants +const appConstants = require('../../../js/constants/appConstants') + +// State +const bookmarksState = require('../../common/state/bookmarksState') +const bookmarkFoldersState = require('../../common/state/bookmarkFoldersState') + +// Util +const {makeImmutable} = require('../../common/state/immutableUtil') +const textCalcUtil = require('../../common/lib/textCalcUtil') + +const tabsReducer = (state, action, immutableAction) => { + action = immutableAction || makeImmutable(action) + switch (action.get('actionType')) { + case appConstants.APP_SET_STATE: + { + // update session for 0.21.x version + const bookmarks = bookmarksState.getBookmarks(state) + if (!bookmarks.first().has('width')) { + textCalcUtil.calcTextList(bookmarks.toList()) + } + + const bookmarkFolders = bookmarkFoldersState.getFolders(state) + if (!bookmarkFolders.first().has('width')) { + textCalcUtil.calcTextList(bookmarkFolders.toList()) + } + } + break + } + return state +} + +module.exports = tabsReducer diff --git a/app/browser/reducers/bookmarksReducer.js b/app/browser/reducers/bookmarksReducer.js index 867812930aa..e607cbb93e5 100644 --- a/app/browser/reducers/bookmarksReducer.js +++ b/app/browser/reducers/bookmarksReducer.js @@ -6,9 +6,11 @@ const Immutable = require('immutable') // State const bookmarksState = require('../../common/state/bookmarksState') +const bookmarkToolbarState = require('../../common/state/bookmarkToolbarState') // Constants const appConstants = require('../../../js/constants/appConstants') +const siteTags = require('../../../js/constants/siteTags') const {STATE_SITES} = require('../../../js/constants/stateConstants') // Utils @@ -16,6 +18,7 @@ const {makeImmutable} = require('../../common/state/immutableUtil') const syncUtil = require('../../../js/state/syncUtil') const bookmarkUtil = require('../../common/lib/bookmarkUtil') const bookmarkLocationCache = require('../../common/cache/bookmarkLocationCache') +const textCalcUtil = require('../../common/lib/textCalcUtil') const bookmarksReducer = (state, action, immutableAction) => { action = immutableAction || makeImmutable(action) @@ -34,12 +37,16 @@ const bookmarksReducer = (state, action, immutableAction) => { if (Immutable.List.isList(bookmark)) { action.get('siteDetail', Immutable.List()).forEach((bookmark) => { - state = bookmarksState.addBookmark(state, bookmark, closestKey) - state = syncUtil.updateObjectCache(state, bookmark, STATE_SITES.BOOKMARKS) + const bookmarkDetail = bookmarkUtil.buildBookmark(state, bookmark) + state = bookmarksState.addBookmark(state, bookmarkDetail, closestKey) + state = syncUtil.updateObjectCache(state, bookmarkDetail, STATE_SITES.BOOKMARKS) + textCalcUtil.calcText(bookmarkDetail, siteTags.BOOKMARK) }) } else { - state = bookmarksState.addBookmark(state, bookmark, closestKey) - state = syncUtil.updateObjectCache(state, bookmark, STATE_SITES.BOOKMARKS) + const bookmarkDetail = bookmarkUtil.buildBookmark(state, bookmark) + state = bookmarksState.addBookmark(state, bookmarkDetail, closestKey) + state = syncUtil.updateObjectCache(state, bookmarkDetail, STATE_SITES.BOOKMARKS) + textCalcUtil.calcText(bookmarkDetail, siteTags.BOOKMARK) } state = bookmarkUtil.updateActiveTabBookmarked(state) @@ -54,8 +61,15 @@ const bookmarksReducer = (state, action, immutableAction) => { break } - state = bookmarksState.editBookmark(state, key, bookmark) + const oldBookmark = bookmarksState.getBookmark(state, key) + if (oldBookmark.isEmpty()) { + break + } + + const bookmarkDetail = bookmarkUtil.buildEditBookmark(oldBookmark, bookmark) + state = bookmarksState.editBookmark(state, oldBookmark, bookmarkDetail) state = syncUtil.updateObjectCache(state, bookmark, STATE_SITES.BOOKMARKS) + textCalcUtil.calcText(bookmarkDetail, siteTags.BOOKMARK) state = bookmarkUtil.updateActiveTabBookmarked(state) break @@ -78,6 +92,10 @@ const bookmarksReducer = (state, action, immutableAction) => { const destinationDetail = bookmarksState.getBookmark(state, action.get('destinationKey')) state = syncUtil.updateObjectCache(state, destinationDetail, STATE_SITES.BOOKMARKS) + + if (destinationDetail.get('parentFolderId') === 0) { + state = bookmarkToolbarState.setToolbars(state) + } break } case appConstants.APP_REMOVE_BOOKMARK: @@ -91,12 +109,36 @@ const bookmarksReducer = (state, action, immutableAction) => { action.get('bookmarkKey', Immutable.List()).forEach((key) => { state = bookmarksState.removeBookmark(state, key) }) + state = bookmarkToolbarState.setToolbars(state) } else { + const bookmark = bookmarksState.getBookmark(state, bookmarkKey) + if (bookmark.get('parentFolderId') === 0) { + state = bookmarkToolbarState.setToolbars(state) + } state = bookmarksState.removeBookmark(state, bookmarkKey) } state = bookmarkUtil.updateActiveTabBookmarked(state) break } + case appConstants.APP_ON_BOOKMARK_WIDTH_CHANGED: + { + if (action.get('bookmarkList', Immutable.List()).isEmpty()) { + break + } + + let updateToolbar = false + action.get('bookmarkList').forEach(item => { + state = bookmarksState.setWidth(state, item.get('key'), item.get('width')) + if (item.get('parentFolderId') === 0) { + updateToolbar = true + } + }) + + if (updateToolbar) { + state = bookmarkToolbarState.setToolbars(state) + } + break + } } return state diff --git a/app/browser/reducers/windowsReducer.js b/app/browser/reducers/windowsReducer.js index 233bdc4132b..db63bb383c7 100644 --- a/app/browser/reducers/windowsReducer.js +++ b/app/browser/reducers/windowsReducer.js @@ -7,9 +7,15 @@ const path = require('path') const assert = require('assert') const Immutable = require('immutable') + +// Constants const appConstants = require('../../../js/constants/appConstants') const windowConstants = require('../../../js/constants/windowConstants') + +// State const windowState = require('../../common/state/windowState') + +// Utils const windows = require('../windows') const sessionStoreShutdown = require('../../sessionStoreShutdown') const {makeImmutable, isImmutable} = require('../../common/state/immutableUtil') @@ -305,12 +311,18 @@ const windowsReducer = (state, action, immutableAction) => { state = windowState.removeWindow(state, action) sessionStoreShutdown.removeWindowFromCache(action.getIn(['windowValue', 'windowId'])) break - case appConstants.APP_WINDOW_CREATED: - state = windowState.maybeCreateWindow(state, action) - break case appConstants.APP_WINDOW_UPDATED: state = windowState.maybeCreateWindow(state, action) break + case appConstants.APP_WINDOW_CREATED: + case appConstants.APP_WINDOW_RESIZED: + { + const bookmarkToolbarState = require('../../common/state/bookmarkToolbarState') + state = windowState.maybeCreateWindow(state, action) + const windowId = action.getIn(['windowValue', 'windowId'], windowState.WINDOW_ID_NONE) + state = bookmarkToolbarState.setToolbar(state, windowId) + break + } case appConstants.APP_DEFAULT_WINDOW_PARAMS_CHANGED: if (action.get('size')) { state = state.setIn(['defaultWindowParams', 'width'], action.getIn(['size', 0])) diff --git a/app/browser/tabs.js b/app/browser/tabs.js index bebe53607e4..8003cf0a2dc 100644 --- a/app/browser/tabs.js +++ b/app/browser/tabs.js @@ -699,9 +699,9 @@ const api = { }) }, - executeScriptInBackground: (script, cb) => { + executeScriptInBackground: (script, cb, debug = false) => { const win = new BrowserWindow({ - show: false, + show: debug, webPreferences: { partition: 'default' } @@ -709,7 +709,9 @@ const api = { win.webContents.on('did-finish-load', (e) => { win.webContents.executeScriptInTab(config.braveExtensionId, script, {}, (err, url, result) => { cb(err, url, result) - setImmediate(() => win.close()) + if (!debug) { + setImmediate(() => win.close()) + } }) }) win.loadURL('about:blank') diff --git a/app/browser/windows.js b/app/browser/windows.js index 99284906926..e710d24c7b1 100644 --- a/app/browser/windows.js +++ b/app/browser/windows.js @@ -62,6 +62,13 @@ const updateWindow = (windowId) => { } } +const onWindowResize = (windowId) => { + const windowValue = getWindowValue(windowId) + if (windowValue) { + appActions.onWindowResize(windowValue) + } +} + const updatePinnedTabs = (win) => { if (win.webContents.browserWindowOptions.disposition === 'new-popup') { return @@ -115,6 +122,7 @@ const api = { let windowId = -1 const updateWindowMove = debounce(updateWindow, 100) const updateWindowDebounce = debounce(updateWindow, 5) + const onWindowResizeDebounce = debounce(onWindowResize, 5) win.once('initialized', () => { windowId = win.id @@ -220,7 +228,7 @@ const api = { updateWindowDebounce(windowId) }) win.on('resize', () => { - updateWindowDebounce(windowId) + onWindowResizeDebounce(windowId) const size = win.getSize() const position = win.getPosition() // NOTE: the default window size is whatever the last window resize was diff --git a/app/common/lib/bookmarkFoldersUtil.js b/app/common/lib/bookmarkFoldersUtil.js index 8debb82caf1..fefc5af6d29 100644 --- a/app/common/lib/bookmarkFoldersUtil.js +++ b/app/common/lib/bookmarkFoldersUtil.js @@ -2,6 +2,7 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const siteTags = require('../../../js/constants/siteTags') +const {makeImmutable} = require('../state/immutableUtil') const isFolderNameValid = (title) => { return title != null && title.trim().length > 0 @@ -26,7 +27,7 @@ const getNextFolderIdItem = (folders) => { } const getNextFolderId = (folders) => { - const defaultFolderId = 0 + const defaultFolderId = 1 if (!folders) { return defaultFolderId } @@ -117,11 +118,32 @@ const isMoveAllowed = (folderList, sourceDetail, destinationDetail) => { return true } +const buildFolder = (folderDetails, folders) => { + folderDetails = makeImmutable(folderDetails) + let key = folderDetails.get('folderId') + + if (!folderDetails.has('folderId')) { + key = getNextFolderId(folders) + } + + return makeImmutable({ + title: folderDetails.get('title'), + folderId: ~~key, + key: key.toString(), + parentFolderId: ~~folderDetails.get('parentFolderId', 0), + partitionNumber: ~~folderDetails.get('partitionNumber', 0), + objectId: folderDetails.get('objectId', null), + type: siteTags.BOOKMARK_FOLDER, + skipSync: folderDetails.get('skipSync', null) + }) +} + module.exports = { isFolderNameValid, getNextFolderId, getNextFolderName, isFolder, getKey, - isMoveAllowed + isMoveAllowed, + buildFolder } diff --git a/app/common/lib/bookmarkToolbarUtil.js b/app/common/lib/bookmarkToolbarUtil.js new file mode 100644 index 00000000000..52e95197090 --- /dev/null +++ b/app/common/lib/bookmarkToolbarUtil.js @@ -0,0 +1,88 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +const Immutable = require('immutable') + +// Utils +const bookmarkUtil = require('../lib/bookmarkUtil') +const bookmarkFoldersUtil = require('../lib/bookmarkFoldersUtil') + +// Styles +const globalStyles = require('../../renderer/components/styles/global') +const {iconSize} = require('../../../js/constants/config') +const maxWidth = parseInt(globalStyles.spacing.bookmarksItemMaxWidth, 10) +const padding = parseInt(globalStyles.spacing.bookmarksItemPadding, 10) * 2 +const itemMargin = parseInt(globalStyles.spacing.bookmarksItemMargin, 10) +const toolbarPadding = parseInt(globalStyles.spacing.bookmarksToolbarPadding) +const overflowButtonWidth = parseInt(globalStyles.spacing.bookmarksToolbarOverflowButtonWidth, 10) + +const getBookmarkKeys = (width, bookmarks) => { + if (bookmarks == null) { + return { + toolbar: Immutable.List(), + other: Immutable.List() + } + } + + let widthAccountedFor = 0 + + const onlyText = bookmarkUtil.showOnlyText() + const textAndFavicon = bookmarkUtil.showTextAndFavicon() + const onlyFavicon = bookmarkUtil.showOnlyFavicon() + + // No margin for show only fav icons + const margin = onlyFavicon ? 0 : (itemMargin * 2) + const maximumBookmarksToolbarWidth = width - overflowButtonWidth + + widthAccountedFor += toolbarPadding + + // Loop through until we fill up the entire bookmark toolbar width + let i = 0 + for (let item of bookmarks) { + let iconWidth + + if (onlyText) { + iconWidth = 0 + } else if (textAndFavicon || bookmarkFoldersUtil.isFolder(item)) { + iconWidth = iconSize + itemMargin + } else if (onlyFavicon) { + iconWidth = iconSize + } + + let extraWidth = 0 + + if (onlyText) { + extraWidth = padding + item.get('width') + } else if (textAndFavicon) { + extraWidth = padding + iconWidth + item.get('width') + } else if (onlyFavicon) { + extraWidth = padding + iconWidth + + if (bookmarkFoldersUtil.isFolder(item)) { + extraWidth += item.get('width') + } + } + + extraWidth = Math.min(extraWidth, maxWidth) + widthAccountedFor += extraWidth + margin + + if (widthAccountedFor >= maximumBookmarksToolbarWidth) { + widthAccountedFor -= extraWidth + margin + i-- + + break + } + + i++ + } + + return { + toolbar: bookmarks.take(i).map((item) => item.get('key')).toList(), + other: bookmarks.skip(i).take(100).map((item) => item.get('key')).toList() + } +} + +module.exports = { + getBookmarkKeys +} diff --git a/app/common/lib/bookmarkUtil.js b/app/common/lib/bookmarkUtil.js index 44cf589fc21..a1f2110ca3c 100644 --- a/app/common/lib/bookmarkUtil.js +++ b/app/common/lib/bookmarkUtil.js @@ -6,24 +6,21 @@ const Immutable = require('immutable') // State const bookmarksState = require('../state/bookmarksState') -const bookmarkFoldersState = require('../state/bookmarkFoldersState') const tabState = require('../state/tabState') +const historyState = require('../state/historyState') // Constants const dragTypes = require('../../../js/constants/dragTypes') const {bookmarksToolbarMode} = require('../constants/settingsEnums') const settings = require('../../../js/constants/settings') const siteTags = require('../../../js/constants/siteTags') +const newTabData = require('../../../js/data/newTabData') // Utils const bookmarkLocationCache = require('../cache/bookmarkLocationCache') -const {calculateTextWidth} = require('../../../js/lib/textCalculator') -const {iconSize} = require('../../../js/constants/config') const {getSetting} = require('../../../js/settings') const UrlUtil = require('../../../js/lib/urlutil') - -// Styles -const globalStyles = require('../../renderer/components/styles/global') +const {makeImmutable} = require('../state/immutableUtil') const bookmarkHangerHeading = (editMode, isAdded) => { if (isAdded) { @@ -67,109 +64,6 @@ const getDNDBookmarkData = (state, bookmarkKey) => { return data.get('draggingOverKey') === bookmarkKey ? data : Immutable.Map() } -let oldBookmarks -let oldFolders -let lastValue -let lastWidth -const getToolbarBookmarks = (state) => { - const windowWidth = window.innerWidth - const allBookmarks = bookmarksState.getBookmarks(state) - const allFolders = bookmarkFoldersState.getFolders(state) - if ( - allBookmarks === oldBookmarks && - allFolders === oldFolders && - lastWidth === windowWidth && - lastValue - ) { - return lastValue - } - - oldBookmarks = allBookmarks - oldFolders = allFolders - lastWidth = windowWidth - - const bookmarks = bookmarksState.getBookmarksWithFolders(state) - let widthAccountedFor = 0 - - const onlyText = showOnlyText() - const textAndFavicon = showTextAndFavicon() - const onlyFavicon = showOnlyFavicon() - - const maxWidth = parseInt(globalStyles.spacing.bookmarksItemMaxWidth, 10) - const padding = parseInt(globalStyles.spacing.bookmarksItemPadding, 10) * 2 - const bookmarkItemMargin = parseInt(globalStyles.spacing.bookmarksItemMargin, 10) * 2 - const fontSize = parseInt(globalStyles.spacing.bookmarksItemFontSize) - const fontFamily = globalStyles.defaultFontFamily - const chevronMargin = parseInt(globalStyles.spacing.bookmarksItemChevronMargin) - const chevronFontSize = parseInt(globalStyles.spacing.bookmarksItemChevronFontSize) - const chevronWidth = chevronMargin + chevronFontSize - - // No margin for show only favicons - const margin = onlyFavicon ? 0 : bookmarkItemMargin - - // Toolbar padding is only on the left - const toolbarPadding = parseInt(globalStyles.spacing.bookmarksToolbarPadding) - - const overflowButtonWidth = parseInt(globalStyles.spacing.bookmarksToolbarOverflowButtonWidth, 10) - const maximumBookmarksToolbarWidth = windowWidth - overflowButtonWidth - - widthAccountedFor += toolbarPadding - - // Loop through until we fill up the entire bookmark toolbar width - let i = 0 - for (let bookmark of bookmarks) { - let iconWidth - - if (onlyText) { - iconWidth = 0 - } else if (textAndFavicon || bookmark.get('folderId')) { - iconWidth = iconSize + parseInt(globalStyles.spacing.bookmarksItemMargin, 10) - } else if (onlyFavicon) { - iconWidth = iconSize - } - - const currentChevronWidth = bookmark.get('folderId') ? chevronWidth : 0 - const text = bookmark.get('title') || bookmark.get('location') - let extraWidth - - if (onlyText) { - extraWidth = padding + calculateTextWidth(text, `${fontSize} ${fontFamily}`) - - if (bookmark.get('folderId')) { - extraWidth += currentChevronWidth - } - } else if (textAndFavicon) { - extraWidth = padding + iconWidth + calculateTextWidth(text, `${fontSize} ${fontFamily}`) + currentChevronWidth - } else if (onlyFavicon) { - extraWidth = padding + iconWidth + currentChevronWidth - - if (bookmark.get('folderId')) { - extraWidth += calculateTextWidth(text, `${fontSize} ${fontFamily}`) - } - } - - extraWidth = Math.min(extraWidth, maxWidth) - widthAccountedFor += extraWidth + margin - - if (widthAccountedFor >= maximumBookmarksToolbarWidth) { - widthAccountedFor -= extraWidth + margin - i-- - - break - } - - i++ - } - - lastValue = { - visibleBookmarks: bookmarks.take(i).map((item) => item.get('key')).toList(), - // Show at most 100 items in the overflow menu - hiddenBookmarks: bookmarks.skip(i).take(100).map((item) => item.get('key')).toList() - } - - return lastValue -} - const getDetailFromFrame = (frame) => { if (frame == null) { return null @@ -260,18 +154,87 @@ const getKey = (siteDetail) => { return null } +const buildBookmark = (state, bookmarkDetail) => { + bookmarkDetail = makeImmutable(bookmarkDetail) + let location + if (bookmarkDetail.has('location')) { + location = UrlUtil.getLocationIfPDF(bookmarkDetail.get('location')) + bookmarkDetail = bookmarkDetail.set('location', location) + } + + const key = getKey(bookmarkDetail) + const historyKey = key.slice(0, -2) + let dataItem = historyState.getSite(state, historyKey) + + if (dataItem.isEmpty()) { + // check if we have data in tabs + const tab = tabState.getActiveTab(state) || Immutable.Map() + const activeLocation = tab.get('url') || tab.getIn(['frame', 'location']) + + if (!tab.isEmpty() && bookmarkDetail.get('location') === activeLocation) { + dataItem = makeImmutable({ + partitionNumber: tab.getIn(['frame', 'partitionNumber'], 0), + favicon: tab.getIn(['frame', 'icon']), + themeColor: tab.getIn(['frame', 'themeColor']) + }) + } else { + // check if bookmark is in top sites + const topSites = Immutable.fromJS(newTabData.topSites.concat(newTabData.pinnedTopSites)) + const topSite = topSites.find(site => site.get('location') === bookmarkDetail.get('location')) || Immutable.Map() + + if (!topSite.isEmpty()) { + dataItem = topSite + } + } + } + + return makeImmutable({ + title: bookmarkDetail.get('title', ''), + location: bookmarkDetail.get('location'), + parentFolderId: ~~bookmarkDetail.get('parentFolderId', 0), + partitionNumber: ~~dataItem.get('partitionNumber', 0), + objectId: bookmarkDetail.get('objectId', null), + favicon: dataItem.get('favicon'), + themeColor: dataItem.get('themeColor'), + type: siteTags.BOOKMARK, + key: key, + skipSync: bookmarkDetail.get('skipSync', null), + width: 0 + }) +} + +const buildEditBookmark = (oldBookmark, bookmarkDetail) => { + let newBookmark = oldBookmark.merge(bookmarkDetail) + + let location + if (newBookmark.has('location')) { + location = UrlUtil.getLocationIfPDF(newBookmark.get('location')) + newBookmark = newBookmark.set('location', location) + } + + const newKey = getKey(newBookmark) + if (newKey === null) { + return oldBookmark + } + + return newBookmark.set('key', newKey) +} + module.exports = { bookmarkHangerHeading, isBookmarkNameValid, showOnlyFavicon, showFavicon, getDNDBookmarkData, - getToolbarBookmarks, getDetailFromFrame, isLocationBookmarked, toCreateProperties, isBookmark, updateTabBookmarked, updateActiveTabBookmarked, - getKey + getKey, + showOnlyText, + showTextAndFavicon, + buildBookmark, + buildEditBookmark } diff --git a/app/common/lib/textCalcUtil.js b/app/common/lib/textCalcUtil.js new file mode 100644 index 00000000000..645bbb6c3da --- /dev/null +++ b/app/common/lib/textCalcUtil.js @@ -0,0 +1,128 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +const Immutable = require('immutable') + +// Actions +const appActions = require('../../../js/actions/appActions') + +// Constant +const siteTags = require('../../../js/constants/siteTags') + +// Utils +const tabs = require('../../browser/tabs') + +// Styles +const globalStyles = require('../../renderer/components/styles/global') + +const fontSize = parseInt(globalStyles.spacing.bookmarksItemFontSize) +const fontFamily = globalStyles.defaultFontFamily + +const calcText = (item, type) => { + const title = type === siteTags.BOOKMARK + ? item.get('title') || item.get('location') + : item.get('title') + + const param = ` + const ctx = document.createElement('canvas').getContext('2d') + ctx.font = '${fontSize} ${fontFamily}' + ctx.measureText('${title}').width + ` + + tabs.executeScriptInBackground(param, (err, url, result) => { + if (err) { + throw err + } + + if (type === siteTags.BOOKMARK) { + appActions.onBookmarkWidthChanged(Immutable.fromJS([ + { + key: item.get('key'), + parentFolderId: item.get('parentFolderId'), + width: result[0] + } + ])) + } else { + appActions.onBookmarkFolderWidthChanged(Immutable.fromJS([ + { + key: item.get('key'), + parentFolderId: item.get('parentFolderId'), + width: result[0] + } + ])) + } + }) +} +const calcTextList = (list) => { + const take = 500 + if (!list.toJS) { + list = Immutable.fromJS(list) + } + + let paramList = JSON.stringify(list.take(take)) + .replace(/'/g, '!') + .replace(/\\"/g, '!') + .replace(/\\\\/g, '//') + + const param = ` + const ctx = document.createElement('canvas').getContext('2d') + ctx.font = '${fontSize} ${fontFamily}' + const bookmarks = [] + const folders = [] + const list = JSON.parse('${paramList}') + + list.forEach(item => { + if (item.type === '${siteTags.BOOKMARK}') { + bookmarks.push({ + key: item.key, + parentFolderId: item.parentFolderId, + width: ctx.measureText(item.title || item.location).width + }) + } else { + folders.push({ + key: item.key, + parentFolderId: item.parentFolderId, + width: ctx.measureText(item.title).width + }) + } + }) + + const result = { + bookmarks: bookmarks, + folders: folders + } + + JSON.stringify(result) + ` + + tabs.executeScriptInBackground(param, (err, url, result) => { + if (err) { + console.log('Error in executeScriptInBackground (textCalcUtil.js)') + } + + if (result[0]) { + const data = JSON.parse(result[0]) + if (data.bookmarks.length > 0) { + appActions.onBookmarkWidthChanged(Immutable.fromJS(data.bookmarks)) + } + + if (data.folders.length > 0) { + appActions.onBookmarkFolderWidthChanged(Immutable.fromJS(data.folders)) + } + } else { + console.log('Error, cant parse bookmarks in executeScriptInBackground') + } + + list = list.skip(take) + + if (list.size > 0) { + calcTextList(list) + } + }) +} + +module.exports = { + calcText, + calcTextList +} diff --git a/app/common/state/bookmarkFoldersState.js b/app/common/state/bookmarkFoldersState.js index 49046a08a03..618e7cc2938 100644 --- a/app/common/state/bookmarkFoldersState.js +++ b/app/common/state/bookmarkFoldersState.js @@ -49,27 +49,9 @@ const bookmarkFoldersState = { addFolder: (state, folderDetails, destinationKey) => { state = validateState(state) - folderDetails = makeImmutable(folderDetails) - let folders = bookmarkFoldersState.getFolders(state) - let key = folderDetails.get('folderId') - if (!folderDetails.has('folderId')) { - key = bookmarkFoldersUtil.getNextFolderId(folders) - } - - const newFolder = makeImmutable({ - title: folderDetails.get('title'), - folderId: ~~key, - key: key.toString(), - parentFolderId: ~~folderDetails.get('parentFolderId', 0), - partitionNumber: ~~folderDetails.get('partitionNumber', 0), - objectId: folderDetails.get('objectId', null), - type: siteTags.BOOKMARK_FOLDER, - skipSync: folderDetails.get('skipSync', null) - }) - - state = state.setIn([STATE_SITES.BOOKMARK_FOLDERS, key.toString()], newFolder) - state = bookmarkOrderCache.addFolderToCache(state, newFolder.get('parentFolderId'), key, destinationKey) + state = state.setIn([STATE_SITES.BOOKMARK_FOLDERS, folderDetails.get('key')], folderDetails) + state = bookmarkOrderCache.addFolderToCache(state, folderDetails.get('parentFolderId'), folderDetails.get('key'), destinationKey) return state }, @@ -184,6 +166,10 @@ const bookmarkFoldersState = { append ) return state + }, + + setWidth: (state, key, width) => { + return state.setIn([STATE_SITES.BOOKMARK_FOLDERS, key, 'width'], parseFloat(width)) } } diff --git a/app/common/state/bookmarkToolbarState.js b/app/common/state/bookmarkToolbarState.js new file mode 100644 index 00000000000..be6cc1a66e0 --- /dev/null +++ b/app/common/state/bookmarkToolbarState.js @@ -0,0 +1,63 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +const assert = require('assert') +const Immutable = require('immutable') + +// State +const bookmarksState = require('./bookmarksState') +const windowState = require('./windowState') + +// Utils +const {makeImmutable, isList, isMap} = require('./immutableUtil') +const bookmarkToolbarUtil = require('../lib/bookmarkToolbarUtil') + +const validateState = function (state) { + state = makeImmutable(state) + assert.ok(isMap(state), 'state must be an Immutable.Map') + assert.ok(isList(state.get('windows'), 'state must contain an Immutable.List of windows')) + return state +} + +const bookmarkToolbarState = { + setToolbars: (state) => { + validateState(state) + const bookmarks = bookmarksState.getBookmarksWithFolders(state, 0) + + state.get('windows').forEach((item, index) => { + const width = state.getIn(['windows', index, 'width']) + const data = bookmarkToolbarUtil.getBookmarkKeys(width, bookmarks) + + state = state + .setIn(['windows', index, 'bookmarksToolbar', 'toolbar'], data.toolbar) + .setIn(['windows', index, 'bookmarksToolbar', 'other'], data.other) + }) + + return state + }, + + setToolbar: (state, windowId) => { + validateState(state) + const bookmarks = bookmarksState.getBookmarksWithFolders(state, 0) + const windowIndex = windowState.getWindowIndexByWindowId(state, windowId) + const width = state.getIn(['windows', windowIndex, 'width']) + const data = bookmarkToolbarUtil.getBookmarkKeys(width, bookmarks) + + return state + .setIn(['windows', windowIndex, 'bookmarksToolbar', 'toolbar'], data.toolbar) + .setIn(['windows', windowIndex, 'bookmarksToolbar', 'other'], data.other) + }, + + getToolbar: (state, windowId) => { + const index = windowState.getWindowIndexByWindowId(state, windowId) + return state.getIn(['windows', index, 'bookmarksToolbar', 'toolbar'], Immutable.List()) + }, + + getOther: (state, windowId) => { + const index = windowState.getWindowIndexByWindowId(state, windowId) + return state.getIn(['windows', index, 'bookmarksToolbar', 'other'], Immutable.List()) + } +} + +module.exports = bookmarkToolbarState diff --git a/app/common/state/bookmarksState.js b/app/common/state/bookmarksState.js index 6d0b38c2a27..3ab20f78b60 100644 --- a/app/common/state/bookmarksState.js +++ b/app/common/state/bookmarksState.js @@ -9,13 +9,10 @@ const Immutable = require('immutable') const settings = require('../../../js/constants/settings') const siteTags = require('../../../js/constants/siteTags') const {STATE_SITES} = require('../../../js/constants/stateConstants') -const newTabData = require('../../../js/data/newTabData') // State -const historyState = require('./historyState') const bookmarkOrderCache = require('../cache/bookmarkOrderCache') const bookmarkFoldersState = require('./bookmarkFoldersState') -const tabState = require('./tabState') // Actions const syncActions = require('../../../js/actions/syncActions') @@ -78,99 +75,36 @@ const bookmarksState = { addBookmark: (state, bookmarkDetail, destinationKey) => { state = validateState(state) - const bookmarkUtil = require('../lib/bookmarkUtil') - - bookmarkDetail = makeImmutable(bookmarkDetail) - let location - if (bookmarkDetail.has('location')) { - location = UrlUtil.getLocationIfPDF(bookmarkDetail.get('location')) - bookmarkDetail = bookmarkDetail.set('location', location) - } - - const key = bookmarkUtil.getKey(bookmarkDetail) - const historyKey = key.slice(0, -2) - let dataItem = historyState.getSite(state, historyKey) - - if (dataItem.isEmpty()) { - // check if we have data in tabs - const tab = tabState.getActiveTab(state) || Immutable.Map() - const activeLocation = tab.get('url') || tab.getIn(['frame', 'location']) - - if (!tab.isEmpty() && bookmarkDetail.get('location') === activeLocation) { - dataItem = makeImmutable({ - partitionNumber: tab.getIn(['frame', 'partitionNumber'], 0), - favicon: tab.getIn(['frame', 'icon']), - themeColor: tab.getIn(['frame', 'themeColor']) - }) - } else { - // check if bookmark is in top sites - const topSites = Immutable.fromJS(newTabData.topSites.concat(newTabData.pinnedTopSites)) - const topSite = topSites.find(site => site.get('location') === bookmarkDetail.get('location')) || Immutable.Map() - - if (!topSite.isEmpty()) { - dataItem = topSite - } - } - } - - let bookmark = makeImmutable({ - title: bookmarkDetail.get('title', ''), - location: bookmarkDetail.get('location'), - parentFolderId: ~~bookmarkDetail.get('parentFolderId', 0), - partitionNumber: ~~dataItem.get('partitionNumber', 0), - objectId: bookmarkDetail.get('objectId', null), - favicon: dataItem.get('favicon'), - themeColor: dataItem.get('themeColor'), - type: siteTags.BOOKMARK, - key: key, - skipSync: bookmarkDetail.get('skipSync', null) - }) - + const key = bookmarkDetail.get('key') if (key === null) { return state } if (!state.hasIn([STATE_SITES.BOOKMARKS, key])) { - state = bookmarkLocationCache.addCacheKey(state, location, key) - state = bookmarkOrderCache.addBookmarkToCache(state, bookmark.get('parentFolderId'), key, destinationKey) + state = bookmarkLocationCache.addCacheKey(state, bookmarkDetail.get('location'), key) + state = bookmarkOrderCache.addBookmarkToCache(state, bookmarkDetail.get('parentFolderId'), key, destinationKey) } - state = state.setIn([STATE_SITES.BOOKMARKS, key], bookmark) + state = state.setIn([STATE_SITES.BOOKMARKS, key], bookmarkDetail) return state }, - editBookmark: (state, editKey, bookmarkDetail) => { + editBookmark: (state, oldBookmark, bookmarkDetail) => { state = validateState(state) - const bookmarkUtil = require('../lib/bookmarkUtil') - - const oldBookmark = bookmarksState.getBookmark(state, editKey) - - if (oldBookmark.isEmpty()) { - return state - } - let newBookmark = oldBookmark.merge(bookmarkDetail) - - let location - if (newBookmark.has('location')) { - location = UrlUtil.getLocationIfPDF(newBookmark.get('location')) - newBookmark = newBookmark.set('location', location) - } - const newKey = bookmarkUtil.getKey(newBookmark) - if (newKey === null) { - return state - } + const newKey = bookmarkDetail.get('key') + const editKey = oldBookmark.get('key') if (editKey !== newKey) { state = state.deleteIn([STATE_SITES.BOOKMARKS, editKey]) state = bookmarkOrderCache.removeCacheKey(state, oldBookmark.get('parentFolderId'), editKey) - state = bookmarkOrderCache.addBookmarkToCache(state, newBookmark.get('parentFolderId'), newKey) - newBookmark = newBookmark.set('key', newKey) + state = bookmarkOrderCache.addBookmarkToCache(state, bookmarkDetail.get('parentFolderId'), newKey) + bookmarkDetail = bookmarkDetail.set('key', newKey) } - state = state.setIn([STATE_SITES.BOOKMARKS, newKey], newBookmark) + state = state.setIn([STATE_SITES.BOOKMARKS, newKey], bookmarkDetail) state = bookmarkLocationCache.removeCacheKey(state, oldBookmark.get('location'), editKey) - state = bookmarkLocationCache.addCacheKey(state, location, newKey) + state = bookmarkLocationCache.addCacheKey(state, bookmarkDetail.get('location'), newKey) return state }, @@ -302,6 +236,10 @@ const bookmarksState = { const cache = bookmarkOrderCache.getBookmarksByParentId(state, folderKey) return cache.map((item) => bookmarksState.getBookmark(state, item.get('key'))) + }, + + setWidth: (state, key, width) => { + return state.setIn([STATE_SITES.BOOKMARKS, key, 'width'], parseFloat(width)) } } diff --git a/app/renderer/components/bookmarks/bookmarksToolbar.js b/app/renderer/components/bookmarks/bookmarksToolbar.js index 77e6e3317e7..210da6c1301 100644 --- a/app/renderer/components/bookmarks/bookmarksToolbar.js +++ b/app/renderer/components/bookmarks/bookmarksToolbar.js @@ -18,6 +18,7 @@ const windowActions = require('../../../../js/actions/windowActions') // State const windowState = require('../../../common/state/windowState') +const bookmarkToolbarState = require('../../../common/state/bookmarkToolbarState') // Constants const dragTypes = require('../../../../js/constants/dragTypes') @@ -32,6 +33,7 @@ const dndData = require('../../../../js/dndData') const isWindows = require('../../../common/lib/platformUtil').isWindows() const frameStateUtil = require('../../../../js/state/frameStateUtil') const bookmarkUtil = require('../../../common/lib/bookmarkUtil') +const {getCurrentWindowId} = require('../../currentWindow') // Styles const globalStyles = require('../styles/global') @@ -147,7 +149,7 @@ class BookmarksToolbar extends React.Component { mergeProps (state, ownProps) { const currentWindow = state.get('currentWindow') const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map() - const bookmarks = bookmarkUtil.getToolbarBookmarks(state) + const currentWindowId = getCurrentWindowId() const props = {} // used in renderer @@ -155,8 +157,8 @@ class BookmarksToolbar extends React.Component { props.showFavicon = bookmarkUtil.showFavicon() props.shouldAllowWindowDrag = windowState.shouldAllowWindowDrag(state, currentWindow, activeFrame, isFocused()) && !isWindows - props.visibleBookmarks = bookmarks.visibleBookmarks - props.hiddenBookmarks = bookmarks.hiddenBookmarks + props.visibleBookmarks = bookmarkToolbarState.getToolbar(state, currentWindowId) + props.hiddenBookmarks = bookmarkToolbarState.getOther(state, currentWindowId) // used in other functions props.activeFrameKey = activeFrame.get('key') diff --git a/docs/state.md b/docs/state.md index 87920066a14..4d0353993c3 100644 --- a/docs/state.md +++ b/docs/state.md @@ -63,7 +63,8 @@ AppStore parentFolderId: number, partitionNumber: number, // optionally specifies a specific session skipSync: boolean, - title: string + title: string, + width: float // bookmark text width } }, bookmarkFolders: { @@ -74,7 +75,8 @@ AppStore originalSeed: Array., // only set for bookmarks that have been synced before a sync profile reset parentFolderId: number, // set for bookmarks and bookmark folders only skipSync: boolean, // Set for objects FETCHed by sync - title: string + title: string, + width: float // bookmark folder text width } }, cache: { @@ -400,6 +402,10 @@ AppStore }, windows: [{ // persistent properties + bookmarksToolbar: { + toolbar: Array, // bookmark and folder keys that we want to display + other: Array // bookmark and folder keys that we display in more menu (limited to 100) + }, focused: boolean, height: number, left: number, diff --git a/js/actions/appActions.js b/js/actions/appActions.js index a8e0efe4777..e4dce0ff499 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -1600,6 +1600,39 @@ const appActions = { destinationKey, prepend }) + }, + + /** + * Dispatches a message that bookmark calculation was done + * @param bookmarkList {Object} - Object is a list of bookmarks with key, width and parentFolderId as a property + */ + onBookmarkWidthChanged: function (bookmarkList) { + dispatch({ + actionType: appConstants.APP_ON_BOOKMARK_WIDTH_CHANGED, + bookmarkList + }) + }, + + /** + * Dispatches a message that bookmark calculation was done + * @param folderList {Object} - Object is a list of folders with key, width and parentFolderId as a property + */ + onBookmarkFolderWidthChanged: function (folderList) { + dispatch({ + actionType: appConstants.APP_ON_BOOKMARK_FOLDER_WIDTH_CHANGED, + folderList + }) + }, + + /** + * Dispatches a message that window was resized + * @param windowValue - window properties + */ + onWindowResize: function (windowValue) { + dispatch({ + actionType: appConstants.APP_WINDOW_RESIZED, + windowValue + }) } } diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index 71e69d772a0..8479b2f328c 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -150,7 +150,10 @@ const appConstants = { APP_REMOVE_BOOKMARK_FOLDER: _, APP_REMOVE_BOOKMARK: _, APP_MOVE_BOOKMARK_FOLDER: _, - APP_MOVE_BOOKMARK: _ + APP_MOVE_BOOKMARK: _, + APP_ON_BOOKMARK_WIDTH_CHANGED: _, + APP_ON_BOOKMARK_FOLDER_WIDTH_CHANGED: _, + APP_WINDOW_RESIZED: _ } module.exports = mapValuesByKeys(appConstants) diff --git a/js/lib/textCalculator.js b/js/lib/textCalculator.js deleted file mode 100644 index c543d340b83..00000000000 --- a/js/lib/textCalculator.js +++ /dev/null @@ -1,9 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -module.exports.calculateTextWidth = (text, font = '11px Arial') => { - const ctx = document.createElement('canvas').getContext('2d') - ctx.font = font - return ctx.measureText(text).width -} diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 55313780011..acdddc03dfa 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -33,6 +33,7 @@ const assert = require('assert') const profiles = require('../../app/browser/profiles') const {zoomLevel} = require('../../app/common/constants/toolbarUserInterfaceScale') const {HrtimeLogger} = require('../../app/common/lib/logUtil') +const urlUtil = require('../lib/urlutil') // state helpers const {makeImmutable} = require('../../app/common/state/immutableUtil') @@ -44,7 +45,7 @@ const tabState = require('../../app/common/state/tabState') const bookmarksState = require('../../app/common/state/bookmarksState') const bookmarkFoldersState = require('../../app/common/state/bookmarkFoldersState') const historyState = require('../../app/common/state/historyState') -const urlUtil = require('../lib/urlutil') +const bookmarkToolbarState = require('../../app/common/state/bookmarkToolbarState') // Only used internally const CHANGE_EVENT = 'app-state-change' @@ -118,7 +119,7 @@ const emitChanges = debounce(appStore.emitChanges.bind(appStore), 5) * Useful for updating non-react preferences (electron properties, etc). * Called when any settings are modified (ex: via preferences). */ -function handleChangeSettingAction (settingKey, settingValue) { +function handleChangeSettingAction (state, settingKey, settingValue) { switch (settingKey) { case settings.AUTO_HIDE_MENU: BrowserWindow.getAllWindows().forEach(function (wnd) { @@ -129,14 +130,21 @@ function handleChangeSettingAction (settingKey, settingValue) { case settings.DEFAULT_ZOOM_LEVEL: filtering.setDefaultZoomLevel(settingValue) break - case settings.TOOLBAR_UI_SCALE: { - const newZoomLevel = zoomLevel[settingValue] || 0 - BrowserWindow.getAllWindows().forEach(function (wnd) { - wnd.webContents.setZoomLevel(newZoomLevel) - }) - } break + case settings.TOOLBAR_UI_SCALE: + { + const newZoomLevel = zoomLevel[settingValue] || 0 + BrowserWindow.getAllWindows().forEach(function (wnd) { + wnd.webContents.setZoomLevel(newZoomLevel) + }) + break + } + case settings.BOOKMARKS_TOOLBAR_MODE: + state = bookmarkToolbarState.setToolbars(state) + break default: } + + return state } let reducers = [] @@ -176,6 +184,7 @@ const handleAppAction = (action) => { require('../../app/browser/reducers/shareReducer'), require('../../app/browser/reducers/updatesReducer'), require('../../app/browser/reducers/topSitesReducer'), + require('../../app/browser/reducers/bookmarkToolbarReducer'), require('../../app/ledger').doAction, require('../../app/browser/menu') ] @@ -260,7 +269,7 @@ const handleAppAction = (action) => { break case appConstants.APP_CHANGE_SETTING: appState = appState.setIn(['settings', action.key], action.value) - handleChangeSettingAction(action.key, action.value) + appState = handleChangeSettingAction(appState, action.key, action.value) break case appConstants.APP_ALLOW_FLASH_ONCE: { diff --git a/test/unit/app/browser/reducers/bookmarkFoldersReducerTest.js b/test/unit/app/browser/reducers/bookmarkFoldersReducerTest.js index d58fdd573b7..3d93af6ad2b 100644 --- a/test/unit/app/browser/reducers/bookmarkFoldersReducerTest.js +++ b/test/unit/app/browser/reducers/bookmarkFoldersReducerTest.js @@ -7,6 +7,8 @@ const mockery = require('mockery') const Immutable = require('immutable') const assert = require('assert') const sinon = require('sinon') +const fakeElectron = require('../../../lib/fakeElectron') +const fakeAdBlock = require('../../../lib/fakeAdBlock') const appConstants = require('../../../../../js/constants/appConstants') const siteTags = require('../../../../../js/constants/siteTags') @@ -14,9 +16,10 @@ const {STATE_SITES} = require('../../../../../js/constants/stateConstants') require('../../../braveUnit') describe('bookmarkFoldersReducer unit test', function () { - let bookmarkFoldersReducer, bookmarkFoldersState + let bookmarkFoldersReducer, bookmarkFoldersState, bookmarkToolbarState const state = Immutable.fromJS({ + windows: [], bookmarks: {}, bookmarkFolders: {}, cache: { @@ -25,6 +28,7 @@ describe('bookmarkFoldersReducer unit test', function () { }) const stateWithData = Immutable.fromJS({ + windows: [], bookmarks: {}, bookmarkFolders: { '1': { @@ -44,6 +48,15 @@ describe('bookmarkFoldersReducer unit test', function () { partitionNumber: 0, objectId: null, type: siteTags.BOOKMARK_FOLDER + }, + '80': { + title: 'folder80', + folderId: 80, + key: '80', + parentFolderId: 69, + partitionNumber: 0, + objectId: null, + type: siteTags.BOOKMARK_FOLDER } }, cache: { @@ -59,19 +72,34 @@ describe('bookmarkFoldersReducer unit test', function () { order: 1, type: siteTags.BOOKMARK_FOLDER } + ], + '69': [ + { + key: '80', + order: 0, + type: siteTags.BOOKMARK_FOLDER + } ] } } }) + const fakeTextCalc = { + calcText: () => true + } + before(function () { mockery.enable({ warnOnReplace: false, warnOnUnregistered: false, useCleanCache: true }) + mockery.registerMock('electron', fakeElectron) + mockery.registerMock('ad-block', fakeAdBlock) + mockery.registerMock('../../common/lib/textCalcUtil', fakeTextCalc) bookmarkFoldersReducer = require('../../../../../app/browser/reducers/bookmarkFoldersReducer') bookmarkFoldersState = require('../../../../../app/common/state/bookmarkFoldersState') + bookmarkToolbarState = require('../../../../../app/common/state/bookmarkToolbarState') }) after(function () { @@ -79,23 +107,27 @@ describe('bookmarkFoldersReducer unit test', function () { }) describe('APP_ADD_BOOKMARK_FOLDER', function () { - let spy + let spy, spyCalc afterEach(function () { spy.restore() + spyCalc.restore() }) it('null case', function () { spy = sinon.spy(bookmarkFoldersState, 'addFolder') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarkFoldersReducer(state, { actionType: appConstants.APP_ADD_BOOKMARK_FOLDER }) assert.equal(spy.notCalled, true) + assert.equal(spyCalc.notCalled, true) assert.deepEqual(state, newState) }) it('folder data is map (single folder)', function () { spy = sinon.spy(bookmarkFoldersState, 'addFolder') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarkFoldersReducer(state, { actionType: appConstants.APP_ADD_BOOKMARK_FOLDER, folderDetails: { @@ -126,11 +158,13 @@ describe('bookmarkFoldersReducer unit test', function () { ] })) assert.equal(spy.calledOnce, true) + assert.equal(spyCalc.calledOnce, true) assert.deepEqual(newState.toJS(), expectedState.toJS()) }) it('folder data is list (multiple folders)', function () { spy = sinon.spy(bookmarkFoldersState, 'addFolder') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarkFoldersReducer(state, { actionType: appConstants.APP_ADD_BOOKMARK_FOLDER, folderDetails: [ @@ -201,38 +235,45 @@ describe('bookmarkFoldersReducer unit test', function () { ] })) assert.equal(spy.callCount, 3) + assert.equal(spyCalc.callCount, 3) assert.deepEqual(newState.toJS(), expectedState.toJS()) }) }) describe('APP_EDIT_BOOKMARK_FOLDER', function () { - let spy + let spy, spyCalc afterEach(function () { spy.restore() + spyCalc.restore() }) it('null case', function () { spy = sinon.spy(bookmarkFoldersState, 'editFolder') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarkFoldersReducer(stateWithData, { actionType: appConstants.APP_EDIT_BOOKMARK_FOLDER }) assert.equal(spy.notCalled, true) + assert.equal(spyCalc.notCalled, true) assert.deepEqual(stateWithData, newState) }) it('folder data is missing', function () { spy = sinon.spy(bookmarkFoldersState, 'editFolder') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarkFoldersReducer(stateWithData, { actionType: appConstants.APP_EDIT_BOOKMARK_FOLDER, editKey: '1' }) assert.equal(spy.notCalled, true) + assert.equal(spyCalc.notCalled, true) assert.deepEqual(stateWithData, newState) }) it('folder key is missing', function () { spy = sinon.spy(bookmarkFoldersState, 'editFolder') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarkFoldersReducer(stateWithData, { actionType: appConstants.APP_EDIT_BOOKMARK_FOLDER, folderDetails: { @@ -241,11 +282,13 @@ describe('bookmarkFoldersReducer unit test', function () { } }) assert.equal(spy.notCalled, true) + assert.equal(spyCalc.notCalled, true) assert.deepEqual(stateWithData, newState) }) it('folder data is correct', function () { spy = sinon.spy(bookmarkFoldersState, 'editFolder') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarkFoldersReducer(stateWithData, { actionType: appConstants.APP_EDIT_BOOKMARK_FOLDER, folderDetails: { @@ -256,28 +299,33 @@ describe('bookmarkFoldersReducer unit test', function () { }) const expectedState = stateWithData.setIn([STATE_SITES.BOOKMARK_FOLDERS, '1', 'title'], 'folder1 new') assert.equal(spy.calledOnce, true) + assert.equal(spyCalc.calledOnce, true) assert.deepEqual(newState.toJS(), expectedState.toJS()) }) }) describe('APP_MOVE_BOOKMARK_FOLDER', function () { - let spy + let spy, spyToolbar afterEach(function () { spy.restore() + spyToolbar.restore() }) it('null case', function () { spy = sinon.spy(bookmarkFoldersState, 'moveFolder') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') const newState = bookmarkFoldersReducer(state, { actionType: appConstants.APP_MOVE_BOOKMARK_FOLDER }) assert.equal(spy.notCalled, true) + assert.equal(spyToolbar.notCalled, true) assert.deepEqual(state, newState) }) it('check if move is working', function () { spy = sinon.spy(bookmarkFoldersState, 'moveFolder') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') const newState = bookmarkFoldersReducer(stateWithData, { actionType: appConstants.APP_MOVE_BOOKMARK_FOLDER, folderKey: '1', @@ -297,28 +345,43 @@ describe('bookmarkFoldersReducer unit test', function () { } ])) assert.equal(spy.calledOnce, true) + assert.equal(spyToolbar.calledOnce, true) assert.deepEqual(newState.toJS(), expectedState.toJS()) }) + + it('destination key is not on bookmark toolbar', function () { + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') + bookmarkFoldersReducer(stateWithData, { + actionType: appConstants.APP_MOVE_BOOKMARK_FOLDER, + folderKey: '1', + destinationKey: '80' + }) + assert.equal(spyToolbar.notCalled, true) + }) }) describe('APP_REMOVE_BOOKMARK_FOLDER', function () { - let spy + let spy, spyToolbar afterEach(function () { spy.restore() + spyToolbar.restore() }) it('null case', function () { spy = sinon.spy(bookmarkFoldersState, 'removeFolder') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') const newState = bookmarkFoldersReducer(state, { actionType: appConstants.APP_REMOVE_BOOKMARK_FOLDER }) assert.equal(spy.notCalled, true) + assert.equal(spyToolbar.notCalled, true) assert.deepEqual(state, newState) }) it('folder key is list (multiple folders)', function () { spy = sinon.spy(bookmarkFoldersState, 'removeFolder') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') const newState = bookmarkFoldersReducer(stateWithData, { actionType: appConstants.APP_REMOVE_BOOKMARK_FOLDER, folderKey: [ @@ -326,12 +389,14 @@ describe('bookmarkFoldersReducer unit test', function () { '69' ] }) - assert.equal(spy.callCount, 2) + assert.equal(spy.callCount, 3) + assert.equal(spyToolbar.calledOnce, true) assert.deepEqual(newState.toJS(), state.toJS()) }) it('folder key is map (single folder)', function () { spy = sinon.spy(bookmarkFoldersState, 'removeFolder') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') const newState = bookmarkFoldersReducer(stateWithData, { actionType: appConstants.APP_REMOVE_BOOKMARK_FOLDER, folderKey: '1' @@ -346,6 +411,79 @@ describe('bookmarkFoldersReducer unit test', function () { ])) .deleteIn([STATE_SITES.BOOKMARK_FOLDERS, '1']) assert.equal(spy.calledOnce, true) + assert.equal(spyToolbar.calledOnce, true) + assert.deepEqual(newState.toJS(), expectedState.toJS()) + }) + }) + + describe('APP_ON_BOOKMARK_FOLDER_WIDTH_CHANGED', function () { + let spy, spyToolbar + + afterEach(function () { + spy.restore() + spyToolbar.restore() + }) + + it('null case', function () { + spy = sinon.spy(bookmarkFoldersState, 'setWidth') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') + const newState = bookmarkFoldersReducer(state, { + actionType: appConstants.APP_ON_BOOKMARK_FOLDER_WIDTH_CHANGED + }) + assert.equal(spy.notCalled, true) + assert.equal(spyToolbar.notCalled, true) + assert.deepEqual(state, newState) + }) + + it('we update multiple items', function () { + spy = sinon.spy(bookmarkFoldersState, 'setWidth') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') + const newState = bookmarkFoldersReducer(stateWithData, { + actionType: appConstants.APP_ON_BOOKMARK_FOLDER_WIDTH_CHANGED, + folderList: Immutable.fromJS([ + { + key: '1', + width: 10, + parentFolderId: 0 + }, + { + key: '69', + width: 15, + parentFolderId: 0 + }, + { + key: '80', + width: 20, + parentFolderId: 69 + } + ]) + }) + assert.equal(spy.callCount, 3) + assert.equal(spyToolbar.calledOnce, true) + const expectedState = stateWithData + .setIn([STATE_SITES.BOOKMARK_FOLDERS, '1', 'width'], 10) + .setIn([STATE_SITES.BOOKMARK_FOLDERS, '69', 'width'], 15) + .setIn([STATE_SITES.BOOKMARK_FOLDERS, '80', 'width'], 20) + assert.deepEqual(newState.toJS(), expectedState.toJS()) + }) + + it('we update one and trigger toolbar update', function () { + spy = sinon.spy(bookmarkFoldersState, 'setWidth') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') + const newState = bookmarkFoldersReducer(stateWithData, { + actionType: appConstants.APP_ON_BOOKMARK_FOLDER_WIDTH_CHANGED, + folderList: Immutable.fromJS([ + { + key: '80', + width: 20, + parentFolderId: 69 + } + ]) + }) + assert.equal(spy.callCount, 1) + assert.equal(spyToolbar.notCalled, true) + const expectedState = stateWithData + .setIn([STATE_SITES.BOOKMARK_FOLDERS, '80', 'width'], 20) assert.deepEqual(newState.toJS(), expectedState.toJS()) }) }) diff --git a/test/unit/app/browser/reducers/bookmarkToolbarReducerTest.js b/test/unit/app/browser/reducers/bookmarkToolbarReducerTest.js new file mode 100644 index 00000000000..b93fb8bf14f --- /dev/null +++ b/test/unit/app/browser/reducers/bookmarkToolbarReducerTest.js @@ -0,0 +1,140 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/* global describe, it, before, after, afterEach */ +const mockery = require('mockery') +const Immutable = require('immutable') +const assert = require('assert') +const sinon = require('sinon') +const fakeElectron = require('../../../lib/fakeElectron') +const fakeAdBlock = require('../../../lib/fakeAdBlock') + +const appConstants = require('../../../../../js/constants/appConstants') +const siteTags = require('../../../../../js/constants/siteTags') +const {STATE_SITES} = require('../../../../../js/constants/stateConstants') +require('../../../braveUnit') + +describe('bookmarkToolbarReducer unit test', function () { + let bookmarkToolbarReducer + + const fakeTextCalc = { + calcTextList: () => true + } + + const stateWithData = Immutable.fromJS({ + windows: [], + bookmarks: { + 'https://brave.com/|0|0': { + favicon: undefined, + title: 'Brave', + location: 'https://brave.com/', + key: 'https://brave.com/|0|0', + parentFolderId: 0, + partitionNumber: 0, + objectId: null, + themeColor: undefined, + type: siteTags.BOOKMARK + }, + 'https://brianbondy.com/|0|1': { + favicon: undefined, + title: 'Clifton', + location: 'https://clifton.io/', + key: 'https://clifton.io/|0|1', + parentFolderId: 1, + partitionNumber: 0, + objectId: null, + themeColor: undefined, + type: siteTags.BOOKMARK + } + }, + bookmarkFolders: { + '1': { + title: 'folder1', + folderId: 1, + key: '1', + parentFolderId: 0, + partitionNumber: 0, + objectId: null, + type: siteTags.BOOKMARK_FOLDER + } + }, + cache: { + bookmarkOrder: { + '0': [ + { + key: 'https://brave.com/|0|0', + order: 0, + type: siteTags.BOOKMARK + }, + { + key: '1', + order: 1, + type: siteTags.BOOKMARK_FOLDER + } + ], + '1': [ + { + key: 'https://brianbondy.com/|0|1', + order: 0, + type: siteTags.BOOKMARK + } + ] + }, + bookmarkLocation: { + 'https://brave.com/': [ + 'https://brave.com/|0|0' + ], + 'https://brianbondy.com/': [ + 'https://brianbondy.com/|0|1' + ] + } + }, + historySites: {}, + tabs: [] + }) + + before(function () { + mockery.enable({ + warnOnReplace: false, + warnOnUnregistered: false, + useCleanCache: true + }) + mockery.registerMock('electron', fakeElectron) + mockery.registerMock('ad-block', fakeAdBlock) + mockery.registerMock('../../common/lib/textCalcUtil', fakeTextCalc) + bookmarkToolbarReducer = require('../../../../../app/browser/reducers/bookmarkToolbarReducer') + }) + + after(function () { + mockery.disable() + }) + + describe('APP_SET_STATE', function () { + let spyCalc + + afterEach(function () { + spyCalc.restore() + }) + + it('we are upgrading from version 0.20 to 0.21', function () { + spyCalc = sinon.spy(fakeTextCalc, 'calcTextList') + bookmarkToolbarReducer(stateWithData, { + actionType: appConstants.APP_SET_STATE + }) + assert.equal(spyCalc.callCount, 2) + }) + + it('we are on version 0.21', function () { + spyCalc = sinon.spy(fakeTextCalc, 'calcTextList') + const newState = stateWithData + .setIn([STATE_SITES.BOOKMARKS, 'https://brave.com/|0|0', 'width'], 10) + .setIn([STATE_SITES.BOOKMARKS, 'https://brianbondy.com/|0|1', 'width'], 10) + .setIn([STATE_SITES.BOOKMARK_FOLDERS, '1', 'width'], 10) + bookmarkToolbarReducer(newState, { + actionType: appConstants.APP_SET_STATE + }) + assert.equal(spyCalc.notCalled, true) + }) + }) +}) diff --git a/test/unit/app/browser/reducers/bookmarksReducerTest.js b/test/unit/app/browser/reducers/bookmarksReducerTest.js index 7f9ebaf6f2d..2ca8a0f184b 100644 --- a/test/unit/app/browser/reducers/bookmarksReducerTest.js +++ b/test/unit/app/browser/reducers/bookmarksReducerTest.js @@ -7,13 +7,15 @@ const mockery = require('mockery') const Immutable = require('immutable') const assert = require('assert') const sinon = require('sinon') +const fakeElectron = require('../../../lib/fakeElectron') +const fakeAdBlock = require('../../../lib/fakeAdBlock') const appConstants = require('../../../../../js/constants/appConstants') const siteTags = require('../../../../../js/constants/siteTags') require('../../../braveUnit') describe('bookmarksReducer unit test', function () { - let bookmarksReducer, bookmarksState, bookmarkLocationCache + let bookmarksReducer, bookmarksState, bookmarkLocationCache, bookmarkToolbarState const state = Immutable.fromJS({ windows: [], @@ -51,6 +53,17 @@ describe('bookmarksReducer unit test', function () { objectId: null, themeColor: undefined, type: siteTags.BOOKMARK + }, + 'https://brianbondy.com/|0|1': { + favicon: undefined, + title: 'Clifton', + location: 'https://clifton.io/', + key: 'https://clifton.io/|0|1', + parentFolderId: 1, + partitionNumber: 0, + objectId: null, + themeColor: undefined, + type: siteTags.BOOKMARK } }, bookmarkFolders: {}, @@ -67,6 +80,13 @@ describe('bookmarksReducer unit test', function () { order: 1, type: siteTags.BOOKMARK } + ], + '1': [ + { + key: 'https://brianbondy.com/|0|1', + order: 0, + type: siteTags.BOOKMARK + } ] }, bookmarkLocation: { @@ -75,6 +95,9 @@ describe('bookmarksReducer unit test', function () { ], 'https://clifton.io/': [ 'https://clifton.io/|0|0' + ], + 'https://brianbondy.com/': [ + 'https://brianbondy.com/|0|1' ] } }, @@ -82,15 +105,23 @@ describe('bookmarksReducer unit test', function () { tabs: [] }) + const fakeTextCalc = { + calcText: () => true + } + before(function () { mockery.enable({ warnOnReplace: false, warnOnUnregistered: false, useCleanCache: true }) + mockery.registerMock('electron', fakeElectron) + mockery.registerMock('ad-block', fakeAdBlock) + mockery.registerMock('../../common/lib/textCalcUtil', fakeTextCalc) bookmarksReducer = require('../../../../../app/browser/reducers/bookmarksReducer') bookmarksState = require('../../../../../app/common/state/bookmarksState') bookmarkLocationCache = require('../../../../../app/common/cache/bookmarkLocationCache') + bookmarkToolbarState = require('../../../../../app/common/state/bookmarkToolbarState') }) after(function () { @@ -116,23 +147,27 @@ describe('bookmarksReducer unit test', function () { }) describe('APP_ADD_BOOKMARK', function () { - let spy + let spy, spyCalc afterEach(function () { spy.restore() + spyCalc.restore() }) it('null case', function () { spy = sinon.spy(bookmarksState, 'addBookmark') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarksReducer(state, { actionType: appConstants.APP_ADD_BOOKMARK }) assert.equal(spy.notCalled, true) + assert.equal(spyCalc.notCalled, true) assert.deepEqual(state, newState) }) it('bookmark data is map (single bookmark)', function () { spy = sinon.spy(bookmarksState, 'addBookmark') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarksReducer(state, { actionType: appConstants.APP_ADD_BOOKMARK, siteDetail: { @@ -153,7 +188,8 @@ describe('bookmarksReducer unit test', function () { themeColor: undefined, title: 'Clifton', type: siteTags.BOOKMARK, - key: 'https://clifton.io/|0|0' + key: 'https://clifton.io/|0|0', + width: 0 } })) .setIn(['cache', 'bookmarkLocation'], Immutable.fromJS({ @@ -171,11 +207,13 @@ describe('bookmarksReducer unit test', function () { ] })) assert.equal(spy.calledOnce, true) + assert.equal(spyCalc.calledOnce, true) assert.deepEqual(newState.toJS(), expectedState.toJS()) }) it('bookmark data is list (multiple bookmarks)', function () { spy = sinon.spy(bookmarksState, 'addBookmark') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarksReducer(state, { actionType: appConstants.APP_ADD_BOOKMARK, siteDetail: [ @@ -204,7 +242,8 @@ describe('bookmarksReducer unit test', function () { themeColor: undefined, title: 'Clifton', type: siteTags.BOOKMARK, - key: 'https://clifton.io/|0|0' + key: 'https://clifton.io/|0|0', + width: 0 }, 'https://brianbondy.com/|0|0': { favicon: undefined, @@ -216,7 +255,8 @@ describe('bookmarksReducer unit test', function () { themeColor: undefined, title: 'Bondy', type: siteTags.BOOKMARK, - key: 'https://brianbondy.com/|0|0' + key: 'https://brianbondy.com/|0|0', + width: 0 } })) .setIn(['cache', 'bookmarkLocation'], Immutable.fromJS({ @@ -242,38 +282,45 @@ describe('bookmarksReducer unit test', function () { ] })) assert.equal(spy.callCount, 2) + assert.equal(spyCalc.callCount, 2) assert.deepEqual(newState.toJS(), expectedState.toJS()) }) }) describe('APP_EDIT_BOOKMARK', function () { - let spy + let spy, spyCalc afterEach(function () { spy.restore() + spyCalc.restore() }) it('null case', function () { spy = sinon.spy(bookmarksState, 'editBookmark') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarksReducer(state, { actionType: appConstants.APP_EDIT_BOOKMARK }) assert.equal(spy.notCalled, true) + assert.equal(spyCalc.notCalled, true) assert.deepEqual(state, newState) }) it('bookmark data is missing', function () { spy = sinon.spy(bookmarksState, 'editBookmark') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarksReducer(state, { actionType: appConstants.APP_EDIT_BOOKMARK, editKey: 'https://clifton.io|0|0' }) assert.equal(spy.notCalled, true) + assert.equal(spyCalc.notCalled, true) assert.deepEqual(state, newState) }) it('bookmark key is missing', function () { spy = sinon.spy(bookmarksState, 'editBookmark') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarksReducer(state, { actionType: appConstants.APP_EDIT_BOOKMARK, siteDetail: { @@ -282,11 +329,13 @@ describe('bookmarksReducer unit test', function () { } }) assert.equal(spy.notCalled, true) + assert.equal(spyCalc.notCalled, true) assert.deepEqual(state, newState) }) it('bookmark data is correct', function () { spy = sinon.spy(bookmarksState, 'editBookmark') + spyCalc = sinon.spy(fakeTextCalc, 'calcText') const newState = bookmarksReducer(stateWithData, { actionType: appConstants.APP_EDIT_BOOKMARK, siteDetail: { @@ -297,28 +346,33 @@ describe('bookmarksReducer unit test', function () { const expectedState = stateWithData .setIn(['bookmarks', 'https://clifton.io/|0|0', 'title'], 'Bondy') assert.equal(spy.calledOnce, true) + assert.equal(spyCalc.calledOnce, true) assert.deepEqual(newState.toJS(), expectedState.toJS()) }) }) describe('APP_MOVE_BOOKMARK', function () { - let spy + let spy, spyToolbar afterEach(function () { spy.restore() + spyToolbar.restore() }) it('null case', function () { spy = sinon.spy(bookmarksState, 'moveBookmark') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') const newState = bookmarksReducer(state, { actionType: appConstants.APP_MOVE_BOOKMARK }) assert.equal(spy.notCalled, true) + assert.equal(spyToolbar.notCalled, true) assert.deepEqual(state, newState) }) it('data is correct', function () { spy = sinon.spy(bookmarksState, 'moveBookmark') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') const newState = bookmarksReducer(stateWithData, { actionType: appConstants.APP_MOVE_BOOKMARK, bookmarkKey: 'https://clifton.io/|0|0', @@ -339,28 +393,43 @@ describe('bookmarksReducer unit test', function () { } ])) assert.equal(spy.calledOnce, true) + assert.equal(spyToolbar.calledOnce, true) assert.deepEqual(newState.toJS(), expectedState.toJS()) }) + + it('destination key is not on bookmark toolbar', function () { + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') + bookmarksReducer(stateWithData, { + actionType: appConstants.APP_MOVE_BOOKMARK, + bookmarkKey: 'https://clifton.io/|0|0', + destinationKey: 'https://brianbondy.com/|0|1' + }) + assert.equal(spyToolbar.notCalled, true) + }) }) describe('APP_REMOVE_BOOKMARK', function () { - let spy + let spy, spyToolbar afterEach(function () { spy.restore() + spyToolbar.restore() }) it('null case', function () { spy = sinon.spy(bookmarksState, 'removeBookmark') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') const newState = bookmarksReducer(state, { actionType: appConstants.APP_REMOVE_BOOKMARK }) assert.equal(spy.notCalled, true) + assert.equal(spyToolbar.notCalled, true) assert.deepEqual(state, newState) }) it('check if delete is working', function () { spy = sinon.spy(bookmarksState, 'removeBookmark') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') const newState = bookmarksReducer(stateWithData, { actionType: appConstants.APP_REMOVE_BOOKMARK, bookmarkKey: 'https://clifton.io/|0|0' @@ -376,6 +445,79 @@ describe('bookmarksReducer unit test', function () { .deleteIn(['bookmarks', 'https://clifton.io/|0|0']) .deleteIn(['cache', 'bookmarkLocation', 'https://clifton.io/']) assert.equal(spy.calledOnce, true) + assert.equal(spyToolbar.calledOnce, true) + assert.deepEqual(newState.toJS(), expectedState.toJS()) + }) + }) + + describe('APP_ON_BOOKMARK_WIDTH_CHANGED', function () { + let spy, spyToolbar + + afterEach(function () { + spy.restore() + spyToolbar.restore() + }) + + it('null case', function () { + spy = sinon.spy(bookmarksState, 'setWidth') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') + const newState = bookmarksReducer(state, { + actionType: appConstants.APP_ON_BOOKMARK_WIDTH_CHANGED + }) + assert.equal(spy.notCalled, true) + assert.equal(spyToolbar.notCalled, true) + assert.deepEqual(state, newState) + }) + + it('we update multiple items', function () { + spy = sinon.spy(bookmarksState, 'setWidth') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') + const newState = bookmarksReducer(stateWithData, { + actionType: appConstants.APP_ON_BOOKMARK_WIDTH_CHANGED, + bookmarkList: Immutable.fromJS([ + { + key: 'https://brave.com/|0|0', + width: 10, + parentFolderId: 0 + }, + { + key: 'https://clifton.io/|0|0', + width: 15, + parentFolderId: 0 + }, + { + key: 'https://brianbondy.com/|0|1', + width: 20, + parentFolderId: 69 + } + ]) + }) + assert.equal(spy.callCount, 3) + assert.equal(spyToolbar.calledOnce, true) + const expectedState = stateWithData + .setIn(['bookmarks', 'https://brave.com/|0|0', 'width'], 10) + .setIn(['bookmarks', 'https://clifton.io/|0|0', 'width'], 15) + .setIn(['bookmarks', 'https://brianbondy.com/|0|1', 'width'], 20) + assert.deepEqual(newState.toJS(), expectedState.toJS()) + }) + + it('we update one and trigger toolbar update', function () { + spy = sinon.spy(bookmarksState, 'setWidth') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbars') + const newState = bookmarksReducer(stateWithData, { + actionType: appConstants.APP_ON_BOOKMARK_WIDTH_CHANGED, + bookmarkList: Immutable.fromJS([ + { + key: 'https://brianbondy.com/|0|1', + width: 20, + parentFolderId: 1 + } + ]) + }) + assert.equal(spy.callCount, 1) + assert.equal(spyToolbar.notCalled, true) + const expectedState = stateWithData + .setIn(['bookmarks', 'https://brianbondy.com/|0|1', 'width'], 20) assert.deepEqual(newState.toJS(), expectedState.toJS()) }) }) diff --git a/test/unit/app/browser/reducers/windowReducerTest.js b/test/unit/app/browser/reducers/windowReducerTest.js new file mode 100644 index 00000000000..a50751484bf --- /dev/null +++ b/test/unit/app/browser/reducers/windowReducerTest.js @@ -0,0 +1,89 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/* global describe, it, before, after, afterEach */ +const mockery = require('mockery') +const sinon = require('sinon') +const Immutable = require('immutable') +const assert = require('assert') +const fakeAdBlock = require('../../../lib/fakeAdBlock') + +const appConstants = require('../../../../../js/constants/appConstants') +require('../../../braveUnit') + +describe('windowsReducer unit test', function () { + let windowsReducer, bookmarkToolbarState + const fakeElectron = Object.assign({}, require('../../../lib/fakeElectron')) + + const fakeWindowState = { + maybeCreateWindow: (state, action) => state + } + + const state = Immutable.fromJS({ + windows: [], + defaultWindowParams: {}, + bookmarks: {} + }) + + before(function () { + mockery.enable({ + warnOnReplace: false, + warnOnUnregistered: false, + useCleanCache: true + }) + mockery.registerMock('electron', fakeElectron) + mockery.registerMock('ad-block', fakeAdBlock) + mockery.registerMock('../../common/state/windowState', fakeWindowState) + windowsReducer = require('../../../../../app/browser/reducers/windowsReducer') + bookmarkToolbarState = require('../../../../../app/common/state/bookmarkToolbarState') + }) + + after(function () { + mockery.disable() + }) + + describe('APP_WINDOW_CREATED', function () { + let spy, spyToolbar + + afterEach(function () { + spy.restore() + spyToolbar.restore() + }) + + it('check if functions are called', function () { + spy = sinon.spy(fakeWindowState, 'maybeCreateWindow') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbar') + windowsReducer(state, { + actionType: appConstants.APP_WINDOW_CREATED, + windowValue: Immutable.fromJS({ + windowId: 1 + }) + }) + assert.equal(spy.calledOnce, true) + assert.equal(spyToolbar.calledOnce, true) + }) + }) + + describe('APP_WINDOW_RESIZED', function () { + let spy, spyToolbar + + afterEach(function () { + spy.restore() + spyToolbar.restore() + }) + + it('check if functions are called', function () { + spy = sinon.spy(fakeWindowState, 'maybeCreateWindow') + spyToolbar = sinon.spy(bookmarkToolbarState, 'setToolbar') + windowsReducer(state, { + actionType: appConstants.APP_WINDOW_RESIZED, + windowValue: Immutable.fromJS({ + windowId: 1 + }) + }) + assert.equal(spy.calledOnce, true) + assert.equal(spyToolbar.calledOnce, true) + }) + }) +}) diff --git a/test/unit/app/common/lib/bookmarkFoldersUtilTest.js b/test/unit/app/common/lib/bookmarkFoldersUtilTest.js index d8f3167ac29..2abdba9d9ab 100644 --- a/test/unit/app/common/lib/bookmarkFoldersUtilTest.js +++ b/test/unit/app/common/lib/bookmarkFoldersUtilTest.js @@ -29,7 +29,7 @@ describe('bookmarkFoldersUtil unit test', function () { describe('getNextFolderId', function () { it('null check', function () { const id = bookmarkFoldersUtil.getNextFolderId() - assert.equal(id, 0) + assert.equal(id, 1) }) it('folders list is empty', function () { @@ -195,4 +195,66 @@ describe('bookmarkFoldersUtil unit test', function () { assert.equal(valid, true) }) }) + + describe('buildFolder', function () { + it('uses all default values', function () { + const folderDetails = Immutable.fromJS({ + title: 'Brave' + }) + const folder = bookmarkFoldersUtil.buildFolder(folderDetails) + const expectedFolder = { + title: 'Brave', + folderId: 1, + key: '1', + parentFolderId: 0, + partitionNumber: 0, + objectId: null, + type: siteTags.BOOKMARK_FOLDER, + skipSync: null + } + assert.deepEqual(folder.toJS(), expectedFolder) + }) + + it('folderId is provided', function () { + const folderDetails = Immutable.fromJS({ + title: 'Brave', + folderId: 3 + }) + const folder = bookmarkFoldersUtil.buildFolder(folderDetails) + const expectedFolder = { + title: 'Brave', + folderId: 3, + key: '3', + parentFolderId: 0, + partitionNumber: 0, + objectId: null, + type: siteTags.BOOKMARK_FOLDER, + skipSync: null + } + assert.deepEqual(folder.toJS(), expectedFolder) + }) + + it('all values are provided', function () { + const folderDetails = Immutable.fromJS({ + title: 'Brave', + folderId: 3, + parentFolderId: 1, + partitionNumber: 1, + objectId: [1], + skipSync: true + }) + const folder = bookmarkFoldersUtil.buildFolder(folderDetails) + const expectedFolder = { + title: 'Brave', + folderId: 3, + key: '3', + parentFolderId: 1, + partitionNumber: 1, + objectId: [1], + type: siteTags.BOOKMARK_FOLDER, + skipSync: true + } + assert.deepEqual(folder.toJS(), expectedFolder) + }) + }) }) diff --git a/test/unit/app/common/lib/bookmarkToolbarUtilTest.js b/test/unit/app/common/lib/bookmarkToolbarUtilTest.js new file mode 100644 index 00000000000..6d4e47e6b3e --- /dev/null +++ b/test/unit/app/common/lib/bookmarkToolbarUtilTest.js @@ -0,0 +1,94 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/* global describe, it, before, after */ +const assert = require('assert') +const mockery = require('mockery') +const sinon = require('sinon') +const Immutable = require('immutable') +const siteTags = require('../../../../../js/constants/siteTags') + +require('../../../braveUnit') + +describe('bookmarkToolbarUtil unit test', function () { + let bookmarkToolbarUtil, bookmarkUtil + + const generateBookmarks = (num) => { + return Immutable.fromJS(new Array(num).fill().map((_, i) => { + return { + type: siteTags.BOOKMARK, + title: `Bookmark ${i}`, + key: `bookmark-${i}|0|0`, + width: 10 * i + } + })) + } + + const generateBookmarksKeys = (num, skip = 0) => { + return Immutable.fromJS(new Array(num).fill().map((_, i) => { + const id = skip + i + return `bookmark-${id}|0|0` + })) + } + + before(function () { + mockery.enable({ + warnOnReplace: false, + warnOnUnregistered: false, + useCleanCache: true + }) + bookmarkToolbarUtil = require('../../../../../app/common/lib/bookmarkToolbarUtil') + bookmarkUtil = require('../../../../../app/common/lib/bookmarkUtil') + }) + + after(function () { + mockery.disable() + }) + + describe('getToolbarBookmarks', function () { + let showOnlyText + + before(function () { + showOnlyText = sinon.stub(bookmarkUtil, 'showOnlyText', () => true) + }) + + after(function () { + showOnlyText.restore() + }) + + it('null scenario', function () { + assert.deepEqual(bookmarkToolbarUtil.getBookmarkKeys(), { + toolbar: Immutable.List(), + other: Immutable.List() + }) + }) + + it('we only have bookmark for the toolbar', function () { + const bookmarks = generateBookmarks(5) + + assert.deepEqual(bookmarkToolbarUtil.getBookmarkKeys(500, bookmarks), { + toolbar: generateBookmarksKeys(5), + other: Immutable.List() + }) + }) + + it('we have bookmarks for toolbar and other', function () { + const bookmarks = generateBookmarks(50) + + assert.deepEqual(bookmarkToolbarUtil.getBookmarkKeys(500, bookmarks), { + toolbar: generateBookmarksKeys(7), + other: generateBookmarksKeys(43, 7) + }) + }) + + it('other limit is set to 100', function () { + const bookmarks = generateBookmarks(500) + + assert.deepEqual(bookmarkToolbarUtil.getBookmarkKeys(500, bookmarks), { + toolbar: generateBookmarksKeys(7), + other: generateBookmarksKeys(100, 7) + }) + }) + }) +}) diff --git a/test/unit/app/common/lib/bookmarkUtilTest.js b/test/unit/app/common/lib/bookmarkUtilTest.js index 317ea19d7c0..c86b87e730e 100644 --- a/test/unit/app/common/lib/bookmarkUtilTest.js +++ b/test/unit/app/common/lib/bookmarkUtilTest.js @@ -95,7 +95,12 @@ describe('bookmarkUtil unit test', function () { url: 'https://brave.com/', title: 'Brave', active: true, - bookmarked: false + bookmarked: false, + frame: { + partitionNumber: 2, + icon: 'ico', + themeColor: '#FFF' + } }], tabsInternal: { index: { @@ -446,4 +451,154 @@ describe('bookmarkUtil unit test', function () { }) }) }) + + describe('buildBookmark', function () { + it('use only defaults', function () { + const bookmark = Immutable.fromJS({ + title: 'Brave', + location: 'https://brave.com' + }) + + const expectedResult = { + title: 'Brave', + location: 'https://brave.com', + parentFolderId: 0, + partitionNumber: 0, + objectId: null, + favicon: undefined, + themeColor: undefined, + type: siteTags.BOOKMARK, + key: 'https://brave.com|0|0', + skipSync: null, + width: 0 + } + + assert.deepEqual(bookmarkUtil.buildBookmark(state, bookmark).toJS(), expectedResult) + }) + + it('bookmark data is in history', function () { + const newState = state + .setIn(['historySites', 'https://brave.com|0'], Immutable.fromJS({ + partitionNumber: 1, + favicon: 'icon', + themeColor: '#000' + })) + + const bookmark = Immutable.fromJS({ + title: 'Brave', + location: 'https://brave.com' + }) + + const expectedResult = { + title: 'Brave', + location: 'https://brave.com', + parentFolderId: 0, + partitionNumber: 1, + objectId: null, + favicon: 'icon', + themeColor: '#000', + type: siteTags.BOOKMARK, + key: 'https://brave.com|0|0', + skipSync: null, + width: 0 + } + + assert.deepEqual(bookmarkUtil.buildBookmark(newState, bookmark).toJS(), expectedResult) + }) + + it('bookmark data is in active tab', function () { + const bookmark = Immutable.fromJS({ + title: 'Brave', + location: 'https://brave.com/' + }) + + const expectedResult = { + title: 'Brave', + location: 'https://brave.com/', + parentFolderId: 0, + partitionNumber: 2, + objectId: null, + favicon: 'ico', + themeColor: '#FFF', + type: siteTags.BOOKMARK, + key: 'https://brave.com/|0|0', + skipSync: null, + width: 0 + } + + assert.deepEqual(bookmarkUtil.buildBookmark(stateWithData, bookmark).toJS(), expectedResult) + }) + + it('bookmark data is in topSites', function () { + const bookmark = Immutable.fromJS({ + title: 'Brave', + location: 'https://www.facebook.com/BraveSoftware/' + }) + + const expectedResult = { + title: 'Brave', + location: 'https://www.facebook.com/BraveSoftware/', + parentFolderId: 0, + partitionNumber: 0, + objectId: null, + favicon: 'chrome-extension://mnojpmjdmbbfmejpflffifhffcmidifd/img/newtab/defaultTopSitesIcon/facebook.png', + themeColor: 'rgb(59, 89, 152)', + type: siteTags.BOOKMARK, + key: 'https://www.facebook.com/BraveSoftware/|0|0', + skipSync: null, + width: 0 + } + + assert.deepEqual(bookmarkUtil.buildBookmark(stateWithData, bookmark).toJS(), expectedResult) + }) + }) + + describe('buildEditBookmark', function () { + it('bookmarkDetail is null', function () { + const bookmark = Immutable.fromJS({ + title: 'Brave', + type: siteTags.BOOKMARK + }) + assert.deepEqual(bookmarkUtil.buildEditBookmark(bookmark).toJS(), bookmark.toJS()) + }) + + it('old and new are merged, but key is the same', function () { + const oldBookmark = Immutable.fromJS({ + title: 'Brave', + location: 'http://brave.com', + type: siteTags.BOOKMARK, + parentFolderId: 0, + key: 'http://brave.com|0|0' + }) + + const newBookmark = Immutable.fromJS({ + title: 'Brave 1', + location: 'http://brave.com', + type: siteTags.BOOKMARK, + parentFolderId: 0 + }) + + const expectedBookmark = newBookmark.set('key', oldBookmark.get('key')) + assert.deepEqual(bookmarkUtil.buildEditBookmark(oldBookmark, newBookmark).toJS(), expectedBookmark.toJS()) + }) + + it('old and new data is merged and new key is generated', function () { + const oldBookmark = Immutable.fromJS({ + title: 'Brave', + location: 'http://brave.com', + type: siteTags.BOOKMARK, + parentFolderId: 0 + }) + + const newBookmark = Immutable.fromJS({ + title: 'Brave 1', + location: 'http://new.brave.com', + type: siteTags.BOOKMARK, + parentFolderId: 1 + }) + + const expectedBookmark = newBookmark.set('key', 'http://new.brave.com|0|1') + assert.deepEqual(bookmarkUtil.buildEditBookmark(oldBookmark, newBookmark).toJS(), expectedBookmark.toJS()) + }) + }) })