diff --git a/app/browser/bookmarksExporter.js b/app/browser/bookmarksExporter.js index f9b88eee608..598501ed3da 100644 --- a/app/browser/bookmarksExporter.js +++ b/app/browser/bookmarksExporter.js @@ -52,7 +52,7 @@ function createBookmarkArray (sites, parentFolderId, first = true, depth = 1) { if (first) payload.push(`${indentFirst}

`) - filteredBookmarks.toList().sort(siteUtil.siteSort).forEach((site) => { + filteredBookmarks.forEach((site) => { if (site.get('tags').includes(siteTags.BOOKMARK) && site.get('location')) { title = site.get('customTitle') || site.get('title') || site.get('location') payload.push(`${indentNext}

${title}`) diff --git a/app/browser/menu.js b/app/browser/menu.js index eb38bb04735..b38f70986b5 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -26,7 +26,7 @@ const menuUtil = require('../common/lib/menuUtil') const {getByTabId} = require('../common/state/tabState') const getSetting = require('../../js/settings').getSetting const locale = require('../locale') -const {isLocationBookmarked, siteSort} = require('../../js/state/siteUtil') +const {isLocationBookmarked} = require('../../js/state/siteUtil') const tabState = require('../../app/common/state/tabState') const isDarwin = process.platform === 'darwin' const isLinux = process.platform === 'linux' @@ -381,7 +381,7 @@ const createBookmarksSubmenu = () => { CommonMenu.exportBookmarksMenuItem() ] - const bookmarks = menuUtil.createBookmarkTemplateItems(appStore.getState().get('sites').toList().sort(siteSort)) + const bookmarks = menuUtil.createBookmarkTemplateItems(appStore.getState().get('sites')) if (bookmarks.length > 0) { submenu.push(CommonMenu.separatorMenuItem) submenu = submenu.concat(bookmarks) diff --git a/app/browser/reducers/sitesReducer.js b/app/browser/reducers/sitesReducer.js index 65f412c592b..c7123b49ec4 100644 --- a/app/browser/reducers/sitesReducer.js +++ b/app/browser/reducers/sitesReducer.js @@ -69,6 +69,7 @@ const sitesReducer = (state, action, immutableAction) => { sourceKey, destinationKey, false, false, true) } } + state = state.set('sites', state.get('sites').sort(siteUtil.siteSort)) if (syncEnabled()) { state = syncUtil.updateSiteCache(state, action.destinationDetail || action.siteDetail) } @@ -77,6 +78,7 @@ const sitesReducer = (state, action, immutableAction) => { case appConstants.APP_REMOVE_SITE: const removeSiteSyncCallback = action.skipSync ? undefined : syncActions.removeSite state = siteUtil.removeSite(state, action.siteDetail, action.tag, true, removeSiteSyncCallback) + state = state.set('sites', state.get('sites').sort(siteUtil.siteSort)) if (syncEnabled()) { state = syncUtil.updateSiteCache(state, action.siteDetail) } @@ -86,6 +88,7 @@ const sitesReducer = (state, action, immutableAction) => { state = siteUtil.moveSite(state, action.sourceKey, action.destinationKey, action.prepend, action.destinationIsParent, false) + state = state.set('sites', state.get('sites').sort(siteUtil.siteSort)) if (syncEnabled()) { const destinationDetail = state.getIn(['sites', action.destinationKey]) state = syncUtil.updateSiteCache(state, destinationDetail) diff --git a/app/browser/tabs.js b/app/browser/tabs.js index 317e80f62f3..2c6b8f5ec08 100644 --- a/app/browser/tabs.js +++ b/app/browser/tabs.js @@ -18,7 +18,6 @@ const settings = require('../../js/constants/settings') const {getBaseUrl, aboutUrls} = require('../../js/lib/appUrlUtil') const siteSettings = require('../../js/state/siteSettings') const messages = require('../../js/constants/messages') -const siteUtil = require('../../js/state/siteUtil') const aboutHistoryState = require('../common/state/aboutHistoryState') const appStore = require('../../js/stores/appStore') const appConfig = require('../../js/constants/appConfig') @@ -135,8 +134,8 @@ ipcMain.on(messages.ABOUT_COMPONENT_INITIALIZED, (e) => { }) const getBookmarksData = function (state) { - let bookmarkSites = new Immutable.Map() - let bookmarkFolderSites = new Immutable.Map() + let bookmarkSites = new Immutable.OrderedMap() + let bookmarkFolderSites = new Immutable.OrderedMap() state.get('sites').forEach((site, siteKey) => { const tags = site.get('tags') if (tags.includes(siteTags.BOOKMARK)) { @@ -146,8 +145,8 @@ const getBookmarksData = function (state) { bookmarkFolderSites = bookmarkFolderSites.set(siteKey, site) } }) - const bookmarks = bookmarkSites.toList().sort(siteUtil.siteSort).toJS() - const bookmarkFolders = bookmarkFolderSites.toList().sort(siteUtil.siteSort).toJS() + const bookmarks = bookmarkSites.toList().toJS() + const bookmarkFolders = bookmarkFolderSites.toList().toJS() return {bookmarks, bookmarkFolders} } diff --git a/app/browser/windows.js b/app/browser/windows.js index 66c040e290e..e8bd83a6a58 100644 --- a/app/browser/windows.js +++ b/app/browser/windows.js @@ -12,7 +12,6 @@ const LocalShortcuts = require('../localShortcuts') const {getPinnedSiteProps} = require('../common/lib/windowsUtil') const {makeImmutable} = require('../common/state/immutableUtil') const {getPinnedTabsByWindowId} = require('../common/state/tabState') -const {siteSort} = require('../../js/state/siteUtil') const messages = require('../../js/constants/messages') const settings = require('../../js/constants/settings') const siteTags = require('../../js/constants/siteTags') @@ -84,7 +83,7 @@ const updatePinnedTabs = (win) => { const sitesToAdd = pinnedSites.filter((site) => !win.__alreadyPinnedSites.find((pinned) => pinned.equals(site))) - sitesToAdd.sort(siteSort).forEach((site) => { + sitesToAdd.forEach((site) => { win.__alreadyPinnedSites = win.__alreadyPinnedSites.add(site) appActions.createTabRequested({ url: site.get('location'), diff --git a/app/index.js b/app/index.js index bc07a738668..db2fe91c1e4 100644 --- a/app/index.js +++ b/app/index.js @@ -76,6 +76,7 @@ const privacy = require('../js/state/privacy') const async = require('async') const settings = require('../js/constants/settings') const BookmarksExporter = require('./browser/bookmarksExporter') +const siteUtil = require('../js/state/siteUtil') app.commandLine.appendSwitch('enable-features', 'BlockSmallPluginContent,PreferHtmlOverPlugins') @@ -321,7 +322,10 @@ app.on('ready', () => { // For tests we always want to load default app state const loadedPerWindowState = initialState.perWindowState delete initialState.perWindowState - appActions.setState(Immutable.fromJS(initialState)) + // Retore map order after load + let state = Immutable.fromJS(initialState) + state = state.set('sites', state.get('sites').sort(siteUtil.siteSort)) + appActions.setState(state) setImmediate(() => perWindowStateLoaded(loadedPerWindowState)) }) diff --git a/app/renderer/components/bookmarks/bookmarksToolbar.js b/app/renderer/components/bookmarks/bookmarksToolbar.js index 2a271c1d469..99a9a3f2df0 100644 --- a/app/renderer/components/bookmarks/bookmarksToolbar.js +++ b/app/renderer/components/bookmarks/bookmarksToolbar.js @@ -108,7 +108,8 @@ class BookmarksToolbar extends ImmutableComponent { contextMenus.onShowBookmarkFolderMenu(this.bookmarks, bookmark, this.activeFrame, e) } updateBookmarkData (props) { - this.bookmarks = siteUtil.getBookmarks(props.sites).toList().sort(siteUtil.siteSort) + // TODO(darkdh): Remove siteSort when we have #9030 landed + this.bookmarks = siteUtil.getBookmarks(props.sites).sort(siteUtil.siteSort) const noParentItems = this.bookmarks .filter((bookmark) => !bookmark.get('parentFolderId')) @@ -153,9 +154,9 @@ class BookmarksToolbar extends ImmutableComponent { break } } - this.bookmarksForToolbar = noParentItems.take(i).sort(siteUtil.siteSort) + this.bookmarksForToolbar = noParentItems.take(i) // Show at most 100 items in the overflow menu - this.overflowBookmarkItems = noParentItems.skip(i).take(100).sort(siteUtil.siteSort) + this.overflowBookmarkItems = noParentItems.skip(i).take(100) } componentWillMount () { this.updateBookmarkData(this.props) diff --git a/test/unit/app/browser/reducers/sitesReducerTest.js b/test/unit/app/browser/reducers/sitesReducerTest.js index 244abe0ea63..1c74313c68a 100644 --- a/test/unit/app/browser/reducers/sitesReducerTest.js +++ b/test/unit/app/browser/reducers/sitesReducerTest.js @@ -139,6 +139,9 @@ describe('sitesReducerTest', function () { newState = sitesReducer(newState, addAction) assert.equal(Object.keys(newState.get('sites').toJS()).length, 3) + // sites are sorted after #8075 landed + // sites[0] will be order 0, sites[1] will be order 1...etc + // Move the site to the 2nd position newState = sitesReducer(newState, moveAction).toJS() assert.equal(Object.keys(newState.sites).length, 3) @@ -149,8 +152,8 @@ describe('sitesReducerTest', function () { moveAction.prepend = true newState = sitesReducer(Immutable.fromJS(newState), moveAction).toJS() assert.equal(Object.keys(newState.sites).length, 3) - assert.equal(Object.values(newState.sites)[2].location, url) - assert.equal(Object.values(newState.sites)[2].order, 1) + assert.equal(Object.values(newState.sites)[1].location, url) + assert.equal(Object.values(newState.sites)[1].order, 1) }) }) })