Skip to content

Commit

Permalink
Moves bookmark text calculation into the state
Browse files Browse the repository at this point in the history
Resolves brave#9517

Auditors:

Test Plan:
  • Loading branch information
NejcZdovc committed Aug 15, 2017
1 parent 0a599c6 commit 8ef275c
Show file tree
Hide file tree
Showing 11 changed files with 268 additions and 115 deletions.
17 changes: 17 additions & 0 deletions app/browser/reducers/bookmarkFoldersReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ const {STATE_SITES} = require('../../../js/constants/stateConstants')
// Utils
const {makeImmutable} = require('../../common/state/immutableUtil')
const syncUtil = require('../../../js/state/syncUtil')
const bookmarkToolbarState = require('../../common/state/bookmarkToolbarState')
const bookmarksState = require('../../common/state/bookmarksState')

const bookmarkFoldersReducer = (state, action, immutableAction) => {
action = immutableAction || makeImmutable(action)
Expand Down Expand Up @@ -92,6 +94,21 @@ const bookmarkFoldersReducer = (state, action, immutableAction) => {
state = syncUtil.updateObjectCache(state, folder, STATE_SITES.BOOKMARK_FOLDERS)
}

break
}
case appConstants.APP_ON_BOOKMARK_FOLDER_TEXT_CALC:
{
const values = action.get('values') || Immutable.List()

if (values == null) {
break
}

values.forEach((item) => {
state = bookmarkFoldersState.setTextWidth(state, item.get('name'), item.get('width'))
})

state = bookmarkToolbarState.setToolbar(state, bookmarksState.getBookmarksWithFolders(state, 0))
break
}
}
Expand Down
16 changes: 16 additions & 0 deletions app/browser/reducers/bookmarksReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,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 bookmarkToolbarState = require('../../common/state/bookmarkToolbarState')

const bookmarksReducer = (state, action, immutableAction) => {
action = immutableAction || makeImmutable(action)
Expand Down Expand Up @@ -97,6 +98,21 @@ const bookmarksReducer = (state, action, immutableAction) => {
state = bookmarkUtil.updateActiveTabBookmarked(state)
break
}
case appConstants.APP_ON_BOOKMARK_TEXT_CALC:
{
const values = action.get('values') || Immutable.List()

if (values == null) {
break
}

values.forEach((item) => {
state = bookmarksState.setTextWidth(state, item.get('key'), item.get('width'))
})

state = bookmarkToolbarState.setToolbar(state, bookmarksState.getBookmarksWithFolders(state, 0))
break
}
}

return state
Expand Down
114 changes: 3 additions & 111 deletions app/common/lib/bookmarkUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const Immutable = require('immutable')

// State
const bookmarksState = require('../state/bookmarksState')
const bookmarkFoldersState = require('../state/bookmarkFoldersState')
const tabState = require('../state/tabState')

// Constants
Expand All @@ -17,14 +16,9 @@ const siteTags = require('../../../js/constants/siteTags')

// 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 bookmarkHangerHeading = (editMode, isAdded) => {
if (isAdded) {
return 'bookmarkAdded'
Expand Down Expand Up @@ -67,109 +61,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
Expand Down Expand Up @@ -266,12 +157,13 @@ module.exports = {
showOnlyFavicon,
showFavicon,
getDNDBookmarkData,
getToolbarBookmarks,
getDetailFromFrame,
isLocationBookmarked,
toCreateProperties,
isBookmark,
updateTabBookmarked,
updateActiveTabBookmarked,
getKey
getKey,
showOnlyText,
showTextAndFavicon
}
12 changes: 12 additions & 0 deletions app/common/state/bookmarkFoldersState.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,18 @@ const bookmarkFoldersState = {
destinationKey,
append
)
return state
},

setTextWidth: (state, name, width) => {
const folders = bookmarkFoldersState.getFoldersByParentId(state, 0)

const folder = folders.find(folder => folder.get('title') === name)

if (folder != null) {
state = state.setIn([STATE_SITES.BOOKMARK_FOLDERS, folder.get('key'), 'width'], width)
}

return state
}
}
Expand Down
107 changes: 107 additions & 0 deletions app/common/state/bookmarkToolbarState.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/* 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')

// Utils
const bookmarkUtil = require('../lib/bookmarkUtil')
const bookmarkFoldersUtil = require('../lib/bookmarkFoldersUtil')
const {makeImmutable, isMap} = require('./immutableUtil')

// Styles
const globalStyles = require('../../renderer/components/styles/global')
const {iconSize} = require('../../../js/constants/config')

const validateState = function (state) {
state = makeImmutable(state)
assert.ok(isMap(state), 'state must be an Immutable.Map')
assert.ok(isMap(state.get('bookmarkToolbar')), 'state must contain an Immutable.Map of bookmarkToolbar')
return state
}

const bookmarkToolbarState = {
// TODO trigger this function when BOOKMARKS_TOOLBAR_MODE changes
setToolbar: (state, bookmarks) => {
validateState(state)
let widthAccountedFor = 0

const onlyText = bookmarkUtil.showOnlyText()
const textAndFavicon = bookmarkUtil.showTextAndFavicon()
const onlyFavicon = bookmarkUtil.showOnlyFavicon()

const maxWidth = parseInt(globalStyles.spacing.bookmarksItemMaxWidth, 10)
const padding = parseInt(globalStyles.spacing.bookmarksItemPadding, 10) * 2
const itemMargin = parseInt(globalStyles.spacing.bookmarksItemMargin, 10) * 2

// No margin for show only fav icons
const margin = onlyFavicon ? 0 : itemMargin

// Toolbar padding is only on the left
const toolbarPadding = parseInt(globalStyles.spacing.bookmarksToolbarPadding)

const overflowButtonWidth = parseInt(globalStyles.spacing.bookmarksToolbarOverflowButtonWidth, 10)
const maximumBookmarksToolbarWidth = state.getIn(['defaultWindowParams', '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 + parseInt(globalStyles.spacing.itemsItemMargin, 10)
} else if (onlyFavicon) {
iconWidth = iconSize
}

let extraWidth

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 state
.setIn(['bookmarkToolbar', 'toolbar'], bookmarks.take(i).map((item) => item.get('key')).toList())
.setIn(['bookmarkToolbar', 'other'], bookmarks.skip(i).take(100).map((item) => item.get('key')).toList())
},

getToolbar: (state) => {
validateState(state)

return state.getIn(['bookmarkToolbar', 'toolbar'], Immutable.List())
},

getOther: (state) => {
validateState(state)

return state.getIn(['bookmarkToolbar', 'toolbar'], Immutable.List())
}
}

module.exports = bookmarkToolbarState
10 changes: 10 additions & 0 deletions app/common/state/bookmarksState.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,16 @@ const bookmarksState = {

const cache = bookmarkOrderCache.getBookmarksByParentId(state, folderKey)
return cache.map((item) => bookmarksState.getBookmark(state, item.get('key')))
},

setTextWidth: (state, bookmarkKey, width) => {
let bookmark = bookmarksState.getBookmark(state, bookmarkKey)

if (bookmark.isEmpty()) {
return state
}

return state.setIn([STATE_SITES.BOOKMARKS, bookmarkKey, 'width'], width)
}
}

Expand Down
6 changes: 3 additions & 3 deletions app/renderer/components/bookmarks/bookmarksToolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -147,16 +148,15 @@ 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 props = {}
// used in renderer
props.showOnlyFavicon = bookmarkUtil.showOnlyFavicon()
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)
props.hiddenBookmarks = bookmarkToolbarState.getOther(state)

// used in other functions
props.activeFrameKey = activeFrame.get('key')
Expand Down
4 changes: 4 additions & 0 deletions docs/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ AppStore
title: string
}
},
bookmarkToolbar: {
toolbar: Array<string>, // bookmark and folder keys that we want to display
other: Array<string> // bookmark and folder keys that we display in more menu (limited to 100)
},
cache: {
bookmarLocation: {
[location]: Array<string> // array of bookmark keys
Expand Down
Loading

0 comments on commit 8ef275c

Please sign in to comment.