Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Reduce redundant siteSort to sort once and used everywhere #8075

Merged
merged 1 commit into from
Jun 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/browser/bookmarksExporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function createBookmarkArray (sites, parentFolderId, first = true, depth = 1) {

if (first) payload.push(`${indentFirst}<DL><p>`)

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}<DT><A HREF="${site.get('location')}">${title}</A>`)
Expand Down
4 changes: 2 additions & 2 deletions app/browser/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,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'
Expand Down Expand Up @@ -393,7 +393,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)
Expand Down
3 changes: 3 additions & 0 deletions app/browser/reducers/sitesReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,12 @@ const sitesReducer = (state, action, immutableAction) => {
state = syncUtil.updateSiteCache(state, action.destinationDetail || action.siteDetail)
}
}
state = state.set('sites', state.get('sites').sort(siteUtil.siteSort))
state = updateActiveTabBookmarked(state)
break
case appConstants.APP_REMOVE_SITE:
state = siteUtil.removeSite(state, action.siteDetail, action.tag, true)
state = state.set('sites', state.get('sites').sort(siteUtil.siteSort))
if (syncEnabled()) {
state = syncUtil.updateSiteCache(state, action.siteDetail)
}
Expand All @@ -88,6 +90,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)
Expand Down
9 changes: 4 additions & 5 deletions app/browser/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,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')
Expand Down Expand Up @@ -137,8 +136,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)) {
Expand All @@ -148,8 +147,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}
}

Expand Down
3 changes: 1 addition & 2 deletions app/browser/windows.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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'),
Expand Down
6 changes: 5 additions & 1 deletion app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -329,7 +330,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))
})

Expand Down
7 changes: 4 additions & 3 deletions app/renderer/components/bookmarks/bookmarksToolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
Expand Down Expand Up @@ -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)
Expand Down