From a4213bc823c1d66c4a48ec23cb783b7955cf1bc9 Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Tue, 30 May 2017 16:01:52 -0700 Subject: [PATCH] Saves computed styles in the state Resolves #9149 Auditors: Test Plan: --- app/browser/reducers/windowsReducer.js | 3 ++ app/browser/windows.js | 8 +++-- app/common/constants/computedStyles.js | 35 ++++++++++++++++++ app/common/lib/windowsUtil.js | 36 ++++++++++++++++++- app/common/state/windowState.js | 12 +++++++ .../components/download/downloadsBar.js | 9 +++++ app/renderer/components/frame/frame.js | 8 ++--- .../navigation/urlBarSuggestions.js | 7 ++-- docs/state.md | 1 + js/actions/appActions.js | 1 - js/actions/windowActions.js | 8 +++++ js/constants/windowConstants.js | 3 +- js/state/downloadUtil.js | 10 +++--- .../components/download/downloadBarTest.js | 15 +++++--- 14 files changed, 132 insertions(+), 24 deletions(-) create mode 100644 app/common/constants/computedStyles.js diff --git a/app/browser/reducers/windowsReducer.js b/app/browser/reducers/windowsReducer.js index e0591042b58..03f87026722 100644 --- a/app/browser/reducers/windowsReducer.js +++ b/app/browser/reducers/windowsReducer.js @@ -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 } diff --git a/app/browser/windows.js b/app/browser/windows.js index b99d5182829..264f492afaf 100644 --- a/app/browser/windows.js +++ b/app/browser/windows.js @@ -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') @@ -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) } } diff --git a/app/common/constants/computedStyles.js b/app/common/constants/computedStyles.js new file mode 100644 index 00000000000..c1ec4324776 --- /dev/null +++ b/app/common/constants/computedStyles.js @@ -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 diff --git a/app/common/lib/windowsUtil.js b/app/common/lib/windowsUtil.js index c15f82e9bde..d62a4eb4fae 100644 --- a/app/common/lib/windowsUtil.js +++ b/app/common/lib/windowsUtil.js @@ -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 +} diff --git a/app/common/state/windowState.js b/app/common/state/windowState.js index e0d265b9bdb..406db7ef9ad 100644 --- a/app/common/state/windowState.js +++ b/app/common/state/windowState.js @@ -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') diff --git a/app/renderer/components/download/downloadsBar.js b/app/renderer/components/download/downloadsBar.js index f05b5983767..8c5344ad8e3 100644 --- a/app/renderer/components/download/downloadsBar.js +++ b/app/renderer/components/download/downloadsBar.js @@ -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 { @@ -34,6 +35,14 @@ class DownloadsBar extends React.Component { webviewActions.setWebviewFocused() } + componentDidMount () { + windowsUtils.updateComputedStyles() + } + + componentWillUnmount () { + windowsUtils.updateComputedStyles() + } + onShowDownloads () { appActions.createTabRequested({ activateIfOpen: true, diff --git a/app/renderer/components/frame/frame.js b/app/renderer/components/frame/frame.js index 1c1e49f4b8f..28c79840bd5 100644 --- a/app/renderer/components/frame/frame.js +++ b/app/renderer/components/frame/frame.js @@ -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') @@ -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) @@ -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) } diff --git a/app/renderer/components/navigation/urlBarSuggestions.js b/app/renderer/components/navigation/urlBarSuggestions.js index 182fa6f46df..206665747b3 100644 --- a/app/renderer/components/navigation/urlBarSuggestions.js +++ b/app/renderer/components/navigation/urlBarSuggestions.js @@ -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') @@ -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 = {} diff --git a/docs/state.md b/docs/state.md index eb37f131b64..e5fc1574e83 100644 --- a/docs/state.md +++ b/docs/state.md @@ -356,6 +356,7 @@ AppStore }, windows: [{ // persistent properties + computedStyles: object, // all computed styles defined in computedStyles.js focused: boolean, height: number, left: number, diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 4cfd9693e74..76870c2de17 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -1429,7 +1429,6 @@ const appActions = { actionType: appConstants.APP_UPDATE_LOG_OPENED }) } - } module.exports = appActions diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index a6806fda057..e476e8857e8 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -1099,6 +1099,14 @@ const windowActions = { partition, tabId }) + }, + + computedStylesChanged: function (windowId, values) { + dispatch({ + actionType: windowConstants.WINDOW_COMPUTED_STYLES_CHANGED, + windowId, + values + }) } } diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index aae5ad7a779..706c4ff9c4e 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -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) diff --git a/js/state/downloadUtil.js b/js/state/downloadUtil.js index c248e80e6eb..cb1eba3bd36 100644 --- a/js/state/downloadUtil.js +++ b/js/state/downloadUtil.js @@ -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] @@ -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) diff --git a/test/unit/app/renderer/components/download/downloadBarTest.js b/test/unit/app/renderer/components/download/downloadBarTest.js index 1f780accf8f..94e0b2bb970 100644 --- a/test/unit/app/renderer/components/download/downloadBarTest.js +++ b/test/unit/app/renderer/components/download/downloadBarTest.js @@ -44,7 +44,8 @@ const appStoreRenderer = Immutable.fromJS({ receivedBytes: 1, state: downloadStates.IN_PROGRESS } - } + }, + windows: [] }) describe('downloadsBar component', function () { @@ -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 () { @@ -70,6 +73,7 @@ describe('downloadsBar component', function () { describe('multiple downloads with space', function () { before(function () { + appStore.state = appStoreRenderer this.result = mount() }) @@ -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() }) + it('renders no downloads', function () { assert.equal(this.result.find(DownloadItem).length, 0) })