Skip to content

Commit

Permalink
Saves computed styles in the state
Browse files Browse the repository at this point in the history
Resolves brave#9149

Auditors:

Test Plan:
  • Loading branch information
NejcZdovc committed Jun 13, 2017
1 parent 7ce06ee commit a4213bc
Show file tree
Hide file tree
Showing 14 changed files with 132 additions and 24 deletions.
3 changes: 3 additions & 0 deletions app/browser/reducers/windowsReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ const windowsReducer = (state, action, immutableAction) => {
case windowConstants.WINDOW_SHOULD_OPEN_DEV_TOOLS:
windows.openDevTools(action.get('windowId'))
break
case windowConstants.WINDOW_COMPUTED_STYLES_CHANGED:
state = windowState.setComputedStyles(state, action.get('windowId'), action.get('values'))
break
}
return state
}
Expand Down
8 changes: 5 additions & 3 deletions app/browser/windows.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const debounce = require('../../js/lib/debounce')
const {getSetting} = require('../../js/settings')
const locale = require('../locale')
const LocalShortcuts = require('../localShortcuts')
const {getPinnedSiteProps} = require('../common/lib/windowsUtil')
const {getPinnedSiteProps, updateComputedStyles} = require('../common/lib/windowsUtil')
const {makeImmutable} = require('../common/state/immutableUtil')
const {getPinnedTabsByWindowId} = require('../common/state/tabState')
const messages = require('../../js/constants/messages')
Expand Down Expand Up @@ -55,9 +55,11 @@ const getWindowValue = (windowId) => {
}

const updateWindow = (windowId) => {
let windowValue = getWindowValue(windowId)
if (windowValue) {
const win = BrowserWindow.fromId(windowId)
if (win) {
const windowValue = getWindowValue(windowId)
appActions.windowUpdated(windowValue)
updateComputedStyles(win)
}
}

Expand Down
35 changes: 35 additions & 0 deletions app/common/constants/computedStyles.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/* 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 computedStyles = {
'bookmark-item': [
'max-width',
'padding',
'margin',
'font-size'
],
'bookmark-item-chevron': [
'margin'
],
'bookmarks-toolbar': [
'padding'
],
'default': [
'font-family'
],
'download-bar': [
'buttons',
'height',
'padding'
],
'download-item': [
'margin',
'width'
],
'navbar': [
'height'
]
}

module.exports = computedStyles
36 changes: 35 additions & 1 deletion app/common/lib/windowsUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,44 @@

const Immutable = require('immutable')

module.exports.getPinnedSiteProps = site => {
// Constants
const computedStyles = require('../constants/computedStyles')

// Actions
const windowActions = require('../../../js/actions/windowActions')

// Utils
const {getCurrentWindowId} = require('../../renderer/currentWindow')

const getPinnedSiteProps = site => {
return Immutable.fromJS({
location: site.get('location'),
order: site.get('order'),
partitionNumber: site.get('partitionNumber') || 0
})
}

const getComputedStyleData = (root) => {
const result = {}

Object.keys(computedStyles).forEach(group => {
computedStyles[group].forEach(prop => {
const value = `--${group}-${prop}`
result[value] = root.getPropertyValue(value).trim()
})
})

return result
}

const updateComputedStyles = (win) => {
const root = win.getComputedStyle(document.querySelector(':root'))
const computedValues = getComputedStyleData(root)
windowActions.computedStylesChanged(getCurrentWindowId(), computedValues)
}

module.exports = {
getPinnedSiteProps,
getComputedStyleData,
updateComputedStyles
}
12 changes: 12 additions & 0 deletions app/common/state/windowState.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,18 @@ const api = {
return state.set('windows', windows.delete(index).insert(index, windowValue))
},

setComputedStyles: (state, windowId, computedStyles) => {
const windowIndex = api.getWindowIndexByWindowId(state, windowId)
return state.mergeDeepIn(['windows', windowIndex], {
computedStyles: computedStyles
})
},

getComputedProperty: (state, property) => {
const currentWindowIndex = api.getWindowIndexByWindowId(state, api.getActiveWindowId(state))
return state.getIn(['windows', currentWindowIndex, 'computedStyles', property])
},

getWindows: (state) => {
state = validateState(state)
return state.get('windows')
Expand Down
9 changes: 9 additions & 0 deletions app/renderer/components/download/downloadsBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const appActions = require('../../../../js/actions/appActions')
// Utils
const contextMenus = require('../../../../js/contextMenus')
const downloadUtil = require('../../../../js/state/downloadUtil')
const windowsUtils = require('../../../common/lib/windowsUtil')
const cx = require('../../../../js/lib/classSet')

class DownloadsBar extends React.Component {
Expand All @@ -34,6 +35,14 @@ class DownloadsBar extends React.Component {
webviewActions.setWebviewFocused()
}

componentDidMount () {
windowsUtils.updateComputedStyles()
}

componentWillUnmount () {
windowsUtils.updateComputedStyles()
}

onShowDownloads () {
appActions.createTabRequested({
activateIfOpen: true,
Expand Down
8 changes: 3 additions & 5 deletions app/renderer/components/frame/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const appStoreRenderer = require('../../../../js/stores/appStoreRenderer')
const siteSettings = require('../../../../js/state/siteSettings')
const siteSettingsState = require('../../../common/state/siteSettingsState')
const tabState = require('../../../common/state/tabState')
const windowState = require('../../../common/state/windowState')

// Utils
const frameStateUtil = require('../../../../js/state/frameStateUtil')
Expand Down Expand Up @@ -505,11 +506,7 @@ class Frame extends React.Component {
e.stopPropagation()
})
this.webview.addEventListener('update-target-url', (e) => {
if (!this.root) {
this.root = window.getComputedStyle(document.querySelector(':root'))
this.downloadsBarHeight = Number.parseInt(this.root.getPropertyValue('--downloads-bar-height'), 10)
}
let nearBottom = e.y > (window.innerHeight - 150 - this.downloadsBarHeight)
let nearBottom = e.y > (window.innerHeight - 150 - this.props.downloadsBarHeight)
let mouseOnLeft = e.x < (window.innerWidth / 2)
let showOnRight = nearBottom && mouseOnLeft
windowActions.setLinkHoverPreview(e.url, showOnRight)
Expand Down Expand Up @@ -955,6 +952,7 @@ class Frame extends React.Component {
props.allSiteSettings = allSiteSettings // TODO (nejc) can be improved even more
props.tabUrl = tab && tab.get('url')
props.partitionNumber = frame.get('partitionNumber')
props.downloadsBarHeight = Number.parseInt(windowState.getComputedProperty(state, '--download-bar-padding'), 10)

return Object.assign({}, ownProps, props)
}
Expand Down
7 changes: 4 additions & 3 deletions app/renderer/components/navigation/urlBarSuggestions.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ const Immutable = require('immutable')
const ReduxComponent = require('../reduxComponent')
const UrlBarSuggestionItem = require('./urlBarSuggestionItem')

// State
const windowState = require('../../../common/state/windowState')

// Actions
const appActions = require('../../../../js/actions/appActions')

Expand Down Expand Up @@ -73,9 +76,7 @@ class UrlBarSuggestions extends React.Component {
const currentWindow = state.get('currentWindow')
const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map()
const urlBar = activeFrame.getIn(['navbar', 'urlbar'], Immutable.Map())
const documentHeight = Number.parseInt(
window.getComputedStyle(document.querySelector(':root')).getPropertyValue('--navbar-height'), 10
)
const documentHeight = Number.parseInt(windowState.getComputedProperty(state, '--navbar-height'), 10)
const menuHeight = ownProps.menubarVisible ? 30 : 0

const props = {}
Expand Down
1 change: 1 addition & 0 deletions docs/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ AppStore
},
windows: [{
// persistent properties
computedStyles: object, // all computed styles defined in computedStyles.js
focused: boolean,
height: number,
left: number,
Expand Down
1 change: 0 additions & 1 deletion js/actions/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1429,7 +1429,6 @@ const appActions = {
actionType: appConstants.APP_UPDATE_LOG_OPENED
})
}

}

module.exports = appActions
8 changes: 8 additions & 0 deletions js/actions/windowActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,14 @@ const windowActions = {
partition,
tabId
})
},

computedStylesChanged: function (windowId, values) {
dispatch({
actionType: windowConstants.WINDOW_COMPUTED_STYLES_CHANGED,
windowId,
values
})
}
}

Expand Down
3 changes: 2 additions & 1 deletion js/constants/windowConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ const windowConstants = {
WINDOW_SHOULD_OPEN_DEV_TOOLS: _,
WINDOW_SET_ALL_AUDIO_MUTED: _,
WINDOW_ON_GO_BACK_LONG: _,
WINDOW_ON_GO_FORWARD_LONG: _
WINDOW_ON_GO_FORWARD_LONG: _,
WINDOW_COMPUTED_STYLES_CHANGED: _
}

module.exports = mapValuesByKeys(windowConstants)
10 changes: 5 additions & 5 deletions js/state/downloadUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */
const Immutable = require('immutable')
const downloadStates = require('../constants/downloadStates')
const windowState = require('../../app/common/state/windowState')

const pendingStates = [downloadStates.IN_PROGRESS, downloadStates.PAUSED]
const stopStates = [downloadStates.CANCELLED, downloadStates.INTERRUPTED, downloadStates.COMPLETED]
Expand Down Expand Up @@ -66,11 +67,10 @@ const getDownloadItems = (state) => {
}

const downloadsSize = state.get('downloads').size
const root = window.getComputedStyle(document.querySelector(':root'))
const downloadItemWidth = Number.parseInt(root.getPropertyValue('--download-item-width'), 10)
const downloadItemMargin = Number.parseInt(root.getPropertyValue('--download-item-margin'), 10)
const downloadBarPadding = Number.parseInt(root.getPropertyValue('--download-bar-padding'), 10)
const downloadBarButtons = Number.parseInt(root.getPropertyValue('--download-bar-buttons'), 10)
const downloadItemWidth = Number.parseInt(windowState.getComputedProperty(state, '--download-item-width'), 10)
const downloadItemMargin = Number.parseInt(windowState.getComputedProperty(state, '--download-item-margin'), 10)
const downloadBarPadding = Number.parseInt(windowState.getComputedProperty(state, '--download-bar-padding'), 10)
const downloadBarButtons = Number.parseInt(windowState.getComputedProperty(state, '--download-bar-buttons'), 10)
const numItems = Math.floor(
(window.innerWidth - (downloadBarPadding * 2) - downloadBarButtons) /
(downloadItemWidth + downloadItemMargin)
Expand Down
15 changes: 10 additions & 5 deletions test/unit/app/renderer/components/download/downloadBarTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ const appStoreRenderer = Immutable.fromJS({
receivedBytes: 1,
state: downloadStates.IN_PROGRESS
}
}
},
windows: []
})

describe('downloadsBar component', function () {
Expand All @@ -57,10 +58,12 @@ describe('downloadsBar component', function () {
useCleanCache: true
})
mockery.registerMock('electron', fakeElectron)
mockery.registerMock('../../app/common/state/windowState', {
getComputedProperty: () => 104
})
DownloadItem = require('../../../../../../app/renderer/components/download/downloadItem')
DownloadsBar = require('../../../../../../app/renderer/components/download/downloadsBar')
appStore = require('../../../../../../js/stores/appStoreRenderer')
appStore.state = appStoreRenderer
})

after(function () {
Expand All @@ -70,6 +73,7 @@ describe('downloadsBar component', function () {

describe('multiple downloads with space', function () {
before(function () {
appStore.state = appStoreRenderer
this.result = mount(<DownloadsBar />)
})

Expand Down Expand Up @@ -103,12 +107,13 @@ describe('downloadsBar component', function () {
})
})

// skipped until #9176 is merged
describe.skip('very narrow downloads bar with items', function () {
describe('very narrow downloads bar with items', function () {
before(function () {
mockery.registerMock('../../getComputedStyle', () => 10)
window.innerWidth = 10
appStore.state = appStoreRenderer
this.result = mount(<DownloadsBar />)
})

it('renders no downloads', function () {
assert.equal(this.result.find(DownloadItem).length, 0)
})
Expand Down

0 comments on commit a4213bc

Please sign in to comment.