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

Adds state as param to currentWindow.js #10605

Merged
merged 1 commit into from
Sep 17, 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/renderer/components/bookmarks/bookmarksToolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class BookmarksToolbar extends React.Component {
// used in renderer
props.showOnlyFavicon = bookmarkUtil.showOnlyFavicon()
props.showFavicon = bookmarkUtil.showFavicon()
props.shouldAllowWindowDrag = windowState.shouldAllowWindowDrag(state, currentWindow, activeFrame, isFocused()) &&
props.shouldAllowWindowDrag = windowState.shouldAllowWindowDrag(state, currentWindow, activeFrame, isFocused(state)) &&
!isWindows
props.visibleBookmarks = bookmarks.visibleBookmarks
props.hiddenBookmarks = bookmarks.hiddenBookmarks
Expand Down
3 changes: 2 additions & 1 deletion app/renderer/components/frame/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class Frame extends React.Component {
}

onPropsChanged (prevProps = {}) {
if (this.props.isActive && !prevProps.isActive && isFocused()) {
if (this.props.isActive && !prevProps.isActive && this.props.isFocused) {
windowActions.setFocusedFrame(this.props.location, this.props.tabId)
}
}
Expand Down Expand Up @@ -885,6 +885,7 @@ class Frame extends React.Component {
props.location = location
props.tabId = tabId
props.showMessageBox = tabMessageBoxState.hasMessageBoxDetail(state, tabId)
props.isFocused = isFocused(state)

// used in other functions
props.frameKey = ownProps.frameKey
Expand Down
7 changes: 4 additions & 3 deletions app/renderer/components/main/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -530,11 +530,12 @@ class Main extends React.Component {
const activeOrigin = !activeFrame.isEmpty() ? urlUtil.getOrigin(activeFrame.get('location')) : null
const widevinePanelDetail = currentWindow.get('widevinePanelDetail', Immutable.Map())
const loginRequiredDetails = basicAuthState.getLoginRequiredDetail(state, activeTabId)
const focused = isFocused(state)

const props = {}
// used in renderer
props.isFullScreen = activeFrame.get('isFullScreen', false)
props.isMaximized = isMaximized() || isFullScreen()
props.isMaximized = isMaximized(state) || isFullScreen(state)
props.captionButtonsVisible = isWindows
props.showContextMenu = currentWindow.has('contextMenuDetail')
props.showPopupWindow = currentWindow.has('popupWindowDetail')
Expand All @@ -554,10 +555,10 @@ class Main extends React.Component {
props.showNoScript = currentWindow.getIn(['ui', 'noScriptInfo', 'isVisible']) &&
urlUtil.getOrigin(activeFrame.get('location'))
props.showReleaseNotes = currentWindow.getIn(['ui', 'releaseNotes', 'isVisible'])
props.showCheckDefault = isFocused() && defaultBrowserState.shouldDisplayDialog(state)
props.showCheckDefault = focused && defaultBrowserState.shouldDisplayDialog(state)
props.showUpdate = updateState.isUpdateVisible(state)
props.showBookmarksToolbar = getSetting(settings.SHOW_BOOKMARKS_TOOLBAR)
props.shouldAllowWindowDrag = windowState.shouldAllowWindowDrag(state, currentWindow, activeFrame, isFocused())
props.shouldAllowWindowDrag = windowState.shouldAllowWindowDrag(state, currentWindow, activeFrame, focused)
props.isSinglePage = nonPinnedFrames.size <= tabsPerPage
props.showTabPages = nonPinnedFrames.size > tabsPerPage
props.showNotificationBar = activeOrigin && state.get('notifications').filter((item) =>
Expand Down
27 changes: 14 additions & 13 deletions app/renderer/components/navigation/buttons/windowCaptionButtons.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const windowActions = require('../../../../../js/actions/windowActions')

// Utils
const locale = require('../../../../../js/l10n')
const {getCurrentWindowId, isMaximized, isFullScreen} = require('../../../currentWindow')
const {getCurrentWindowId} = require('../../../currentWindow')
const cx = require('../../../../../js/lib/classSet')

class WindowCaptionButtons extends ImmutableComponent {
Expand All @@ -26,26 +26,27 @@ class WindowCaptionButtons extends ImmutableComponent {
}

get maximizeTitle () {
return this.props.windowMaximized
return this.props.windowMaximizedFullScreen
? 'windowCaptionButtonRestore'
: 'windowCaptionButtonMaximize'
}

onMinimizeClick (e) {
onMinimizeClick () {
windowActions.shouldMinimize(getCurrentWindowId())
}

onMaximizeClick (e) {
if (isFullScreen()) {
onMaximizeClick () {
const currentWindowId = getCurrentWindowId()
if (this.props.windowFullScreen) {
// If full screen, toggle full screen status and restore window (make smaller)
windowActions.shouldExitFullScreen(getCurrentWindowId())
if (isMaximized()) windowActions.shouldUnmaximize(getCurrentWindowId())
windowActions.shouldExitFullScreen(currentWindowId)
if (this.props.windowMaximized) windowActions.shouldUnmaximize(currentWindowId)
return false
}
return (!isMaximized()) ? windowActions.shouldMaximize(getCurrentWindowId()) : windowActions.shouldUnmaximize(getCurrentWindowId())
return (!this.props.windowMaximized) ? windowActions.shouldMaximize(currentWindowId) : windowActions.shouldUnmaximize(currentWindowId)
}

onCloseClick (e) {
onCloseClick () {
appActions.closeWindow(getCurrentWindowId())
}

Expand All @@ -60,7 +61,7 @@ class WindowCaptionButtons extends ImmutableComponent {
const props = { tabIndex: -1 }

return <div className={cx({
fullscreen: this.props.windowMaximized,
fullscreen: this.props.windowMaximizedFullScreen,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maximized and fullscreen are only the same thing on macos. We should specify one or the other

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I preserved the same functionality, I didn't change anything. So it's working the same way it was before. The only thing that I changed is naming, because we had/have both things in it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only referring to the naming. We have maximized and fullscreen in one property? That's a bit confusing imo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is how we have it now 😃

props.isMaximized = isMaximized() || isFullScreen()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you propose? split it and use it separately?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the existing logic seems wrong to me because fullscreen and maximized should not be treated the same way

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current logic is working (no reports about any problems), so I would suggest that if it's not working we should open new issue for it. If we just want to refactor it we should open refactor issue. I think that this PR is not a right place to fix this, because we didn't introduced this naming in this PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NejcZdovc that seems fair but pls open an issue to track it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue created #10989

windowCaptionButtons: true,
verticallyCenter: this.props.verticallyCenter
})}>
Expand All @@ -69,7 +70,7 @@ class WindowCaptionButtons extends ImmutableComponent {
{...props}
className={cx({
normalizeButton: true,
fullscreen: this.props.windowMaximized,
fullscreen: this.props.windowMaximizedFullScreen,
captionButton: true,
minimize: true
})}
Expand All @@ -81,7 +82,7 @@ class WindowCaptionButtons extends ImmutableComponent {
{...props}
className={cx({
normalizeButton: true,
fullscreen: this.props.windowMaximized,
fullscreen: this.props.windowMaximizedFullScreen,
captionButton: true,
maximize: true
})}
Expand All @@ -97,7 +98,7 @@ class WindowCaptionButtons extends ImmutableComponent {
{...props}
className={cx({
normalizeButton: true,
fullscreen: this.props.windowMaximized,
fullscreen: this.props.windowMaximizedFullScreen,
captionButton: true,
close: true
})}
Expand Down
23 changes: 18 additions & 5 deletions app/renderer/components/navigation/navigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,21 +116,25 @@ class Navigator extends React.Component {
})
.filter((browserAction) => browserAction)
.toList()
const fullScreen = isFullScreen(state)
const maximized = isMaximized(state)

const props = {}
// used in renderer
props.totalBlocks = activeFrame ? frameStateUtil.getTotalBlocks(activeFrame) : false
props.shieldsDown = !braverySettings.shieldsUp
props.shieldEnabled = braveShieldsEnabled(activeFrame)
props.menuBarVisible = menuBarState.isMenuBarVisible(currentWindow)
props.isMaximized = isMaximized() || isFullScreen()
props.isMaximizedFullScreen = maximized || fullScreen
props.isMaximized = maximized
props.isFullScreen = fullScreen
props.isCaptionButton = isWindows && !props.menuBarVisible
props.activeTabShowingMessageBox = activeTabShowingMessageBox
props.extensionBrowserActions = extensionBrowserActions
props.showBrowserActions = !activeTabShowingMessageBox &&
extensionBrowserActions &&
extensionBrowserActions.size > 0
props.shouldAllowWindowDrag = windowState.shouldAllowWindowDrag(state, currentWindow, activeFrame, isFocused())
props.shouldAllowWindowDrag = windowState.shouldAllowWindowDrag(state, currentWindow, activeFrame, isFocused(state))
props.isCounterEnabled = getSetting(settings.BLOCKED_COUNT_BADGE) &&
props.totalBlocks &&
props.shieldEnabled
Expand All @@ -154,7 +158,11 @@ class Navigator extends React.Component {
this.props.menuBarVisible
? <div className='menubarContainer'>
<MenuBar />
<WindowCaptionButtons windowMaximized={this.props.isMaximized} />
<WindowCaptionButtons
windowMaximizedFullScreen={this.props.isMaximizedFullScreen}
windowMaximized={this.props.isMaximized}
windowFullScreen={this.props.isFullScreen}
/>
</div>
: null
}
Expand All @@ -165,7 +173,7 @@ class Navigator extends React.Component {
>
<div className={cx({
backforward: true,
fullscreen: isFullScreen()
fullscreen: this.props.isFullScreen
})}>
<BackButton />
<ForwardButton />
Expand Down Expand Up @@ -228,7 +236,12 @@ class Navigator extends React.Component {
</div>
{
this.props.isCaptionButton
? <WindowCaptionButtons windowMaximized={this.props.isMaximized} verticallyCenter='true' />
? <WindowCaptionButtons
windowMaximizedFullScreen={this.props.isMaximizedFullScreen}
windowMaximized={this.props.isMaximized}
windowFullScreen={this.props.isFullScreen}
verticallyCenter='true'
/>
: null
}
</div>
Expand Down
2 changes: 1 addition & 1 deletion app/renderer/components/tabs/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ class Tabs extends React.Component {
props.partOfFullPageSet = currentTabs.size === tabsPerTabPage
props.onNextPage = currentTabs.size >= tabsPerTabPage && totalPages > pageIndex + 1
props.onPreviousPage = pageIndex > 0
props.shouldAllowWindowDrag = windowState.shouldAllowWindowDrag(state, currentWindow, activeFrame, isFocused())
props.shouldAllowWindowDrag = windowState.shouldAllowWindowDrag(state, currentWindow, activeFrame, isFocused(state))

// used in other functions
props.fixTabWidth = currentWindow.getIn(['ui', 'tabs', 'fixTabWidth'])
Expand Down
22 changes: 12 additions & 10 deletions app/renderer/currentWindow.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
const appStoreRenderer = require('../../js/stores/appStoreRenderer')
const windowState = require('../common/state/windowState')
/* 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 windowState = require('../common/state/windowState')
let currentWindowId = -1
let currentWindow = null

const isMaximized = () => {
const win = windowState.getByWindowId(appStoreRenderer.state, currentWindowId)
return win && win.get('state') === 'maximized'
const isMaximized = (state) => {
const win = windowState.getByWindowId(state, currentWindowId)
return (win && win.get('state') === 'maximized') || false
}

const isFullScreen = () => {
const win = windowState.getByWindowId(appStoreRenderer.state, currentWindowId)
return win && win.get('state') === 'fullscreen'
const isFullScreen = (state) => {
const win = windowState.getByWindowId(state, currentWindowId)
return (win && win.get('state') === 'fullscreen') || false
}

const isFocused = () => {
const win = windowState.getByWindowId(appStoreRenderer.state, currentWindowId)
const isFocused = (state) => {
const win = windowState.getByWindowId(state, currentWindowId)
return (win && win.get('focused')) || false
}

Expand Down
85 changes: 85 additions & 0 deletions test/unit/app/renderer/currentWindowTest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/* 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/. */

/* global describe, it, before */
const Immutable = require('immutable')
const assert = require('assert')
const currentWindow = require('../../../../app/renderer/currentWindow')

describe('currentWindow unit test', function () {
const windowId = 1
const state = Immutable.fromJS({
windows: [
{
windowId: windowId
}
]
})

before(function () {
currentWindow.setWindowId(windowId)
})

describe('isMaximized', function () {
it('null case', function () {
const result = currentWindow.isMaximized(state, 2)
assert.equal(result, false)
})

it('false case', function () {
const newState = state.setIn(['windows', 0, 'state'], 'test')
const result = currentWindow.isMaximized(newState, windowId)
assert.equal(result, false)
})

it('true case', function () {
const newState = state.setIn(['windows', 0, 'state'], 'maximized')
const result = currentWindow.isMaximized(newState, windowId)
assert.equal(result, true)
})
})

describe('isFullScreen', function () {
it('null case', function () {
const result = currentWindow.isFullScreen(state, 2)
assert.equal(result, false)
})

it('false case', function () {
const newState = state.setIn(['windows', 0, 'state'], 'test')
const result = currentWindow.isFullScreen(newState, windowId)
assert.equal(result, false)
})

it('true case', function () {
const newState = state.setIn(['windows', 0, 'state'], 'fullscreen')
const result = currentWindow.isFullScreen(newState, windowId)
assert.equal(result, true)
})
})

describe('isFocused', function () {
it('null case', function () {
const result = currentWindow.isFocused(state, 2)
assert.equal(result, false)
})

it('not provided', function () {
const result = currentWindow.isFocused(state, windowId)
assert.equal(result, false)
})

it('false case', function () {
const newState = state.setIn(['windows', 0, 'focused'], false)
const result = currentWindow.isFocused(newState, windowId)
assert.equal(result, false)
})

it('true case', function () {
const newState = state.setIn(['windows', 0, 'focused'], true)
const result = currentWindow.isFocused(newState, windowId)
assert.equal(result, true)
})
})
})