From addc530c45c6d08f24206e832099a15a78721cbc Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Tue, 13 Jun 2017 17:59:24 +0200 Subject: [PATCH] Converts FindBar into redux component Resolves #9338 Auditors: @bsclifton @bridiver Test Plan: --- app/renderer/components/main/findbar.js | 196 +++++++++++++----------- app/renderer/components/main/main.js | 25 +-- js/actions/windowActions.js | 10 +- js/stores/windowStore.js | 2 +- 4 files changed, 112 insertions(+), 121 deletions(-) diff --git a/app/renderer/components/main/findbar.js b/app/renderer/components/main/findbar.js index 95e5ee1f02b..1289618a15b 100644 --- a/app/renderer/components/main/findbar.js +++ b/app/renderer/components/main/findbar.js @@ -6,28 +6,30 @@ const React = require('react') const Immutable = require('immutable') // Components -const ImmutableComponent = require('../immutableComponent') +const ReduxComponent = require('../reduxComponent') const BrowserButton = require('../common/browserButton') const SwitchControl = require('../common/switchControl') // Constants const keyCodes = require('../../../common/constants/keyCodes') +const settings = require('../../../../js/constants/settings') // Actions const windowActions = require('../../../../js/actions/windowActions') - -// Stores -const windowStore = require('../../../../js/stores/windowStore') +const webviewActions = require('../../../../js/actions/webviewActions') // Utils const contextMenus = require('../../../../js/contextMenus') const {getTextColorForBackground} = require('../../../../js/lib/color') +const frameStateUtil = require('../../../../js/state/frameStateUtil') +const {getSetting} = require('../../../../js/settings') +// Styles const globalStyles = require('../styles/global') -class FindBar extends ImmutableComponent { - constructor () { - super() +class FindBar extends React.Component { + constructor (props) { + super(props) this.onBlur = this.onBlur.bind(this) this.onInputFocus = this.onInputFocus.bind(this) this.onClear = this.onClear.bind(this) @@ -37,37 +39,32 @@ class FindBar extends ImmutableComponent { this.onFindPrev = this.onFindPrev.bind(this) this.onFindNext = this.onFindNext.bind(this) this.onCaseSensitivityChange = this.onCaseSensitivityChange.bind(this) - this.didFrameChange = true - } - - get frame () { - return windowStore.getFrame(this.props.frameKey) } onInput (e) { - windowActions.setFindDetail(this.props.frameKey, Immutable.fromJS({ + windowActions.setFindDetail(this.props.activeFrameKey, Immutable.fromJS({ searchString: e.target.value, - caseSensitivity: this.isCaseSensitive + caseSensitivity: this.props.isCaseSensitive })) } onCaseSensitivityChange (e) { - windowActions.setFindDetail(this.props.frameKey, Immutable.fromJS({ - searchString: this.searchString, - caseSensitivity: !this.isCaseSensitive + windowActions.setFindDetail(this.props.activeFrameKey, Immutable.fromJS({ + searchString: this.props.searchString, + caseSensitivity: !this.props.isCaseSensitive })) } onFindFirst () { - this.props.onFind(this.searchString, this.isCaseSensitive, true, false) + this.onFind(this.props.searchString, this.props.isCaseSensitive, true, false) } onFindNext () { - this.props.onFind(this.searchString, this.isCaseSensitive, true, this.props.findDetail.get('internalFindStatePresent')) + this.onFind(this.props.searchString, this.props.isCaseSensitive, true, this.props.internalFindStatePresent) } onFindPrev () { - this.props.onFind(this.searchString, this.isCaseSensitive, false, this.props.findDetail.get('internalFindStatePresent')) + this.onFind(this.props.searchString, this.props.isCaseSensitive, false, this.props.internalFindStatePresent) } onContextMenu (e) { @@ -75,8 +72,7 @@ class FindBar extends ImmutableComponent { // a word so the word replacement is kind of a surprise. This is because // our context menus are modal at the moment. If we fix that we can // remove this timeout. - setTimeout(() => - contextMenus.onFindBarContextMenu(e), 10) + setTimeout(() => contextMenus.onFindBarContextMenu(e), 10) } /** @@ -91,26 +87,27 @@ class FindBar extends ImmutableComponent { } componentDidMount () { - this.searchInput.value = this.searchString + this.searchInput.value = this.props.searchString this.focus() this.select() - windowActions.setFindbarSelected(this.frame, false) + windowActions.setFindbarSelected(this.props.activeFrameKey, false) } componentWillUpdate (nextProps) { - if (nextProps.frameKey !== this.props.frameKey) { + if (nextProps.frameKey !== this.props.activeFrameKey) { this.searchInput.value = (nextProps.findDetail && nextProps.findDetail.get('searchString')) || '' } } componentDidUpdate (prevProps) { - if (this.props.selected && !prevProps.selected) { + if (this.props.isSelected && !prevProps.isSelected) { this.focus() // Findbar might already be focused, so make sure select happens even if no // onFocus event happens. this.select() - windowActions.setFindbarSelected(this.frame, false) + windowActions.setFindbarSelected(this.props.activeFrameKey, false) } + if (!this.props.findDetail || !prevProps.findDetail || this.props.findDetail.get('searchString') !== prevProps.findDetail.get('searchString') || this.props.findDetail.get('caseSensitivity') !== prevProps.findDetail.get('caseSensitivity')) { @@ -123,7 +120,7 @@ class FindBar extends ImmutableComponent { switch (e.keyCode) { case keyCodes.ESC: e.preventDefault() - this.props.onFindHide() + this.onFindHide() break case keyCodes.ENTER: e.preventDefault() @@ -141,81 +138,85 @@ class FindBar extends ImmutableComponent { } onBlur (e) { - windowActions.setFindbarSelected(this.frame, false) + windowActions.setFindbarSelected(this.props.activeFrameKey, false) } onClear () { this.searchInput.value = '' - windowActions.setFindDetail(this.props.frameKey, Immutable.fromJS({ + windowActions.setFindDetail(this.props.activeFrameKey, Immutable.fromJS({ searchString: '', - caseSensitivity: this.isCaseSensitive + caseSensitivity: this.props.isCaseSensitive })) this.focus() } - get numberOfMatches () { - if (!this.props.findDetail || this.props.findDetail.get('numberOfMatches') === undefined) { - return -1 - } - return this.props.findDetail.get('numberOfMatches') - } - - get activeMatchOrdinal () { - if (!this.props.findDetail || this.props.findDetail.get('activeMatchOrdinal') === undefined) { - return -1 - } - return this.props.findDetail.get('activeMatchOrdinal') || -1 - } - - get isCaseSensitive () { - if (!this.props.findDetail) { - return false - } - return this.props.findDetail.get('caseSensitivity') + onFindHide () { + frameStateUtil.onFindBarHide(this.props.activeFrameKey) } - get searchString () { - if (!this.props.findDetail) { - return '' + onFind (searchString, caseSensitivity, forward, findNext) { + webviewActions.findInPage(searchString, caseSensitivity, forward, findNext) + if (!findNext) { + windowActions.setFindDetail(this.props.activeFrameKey, Immutable.fromJS({ + internalFindStatePresent: true + })) } - return this.props.findDetail.get('searchString') } - get backgroundColor () { - return this.props.paintTabs && (this.props.themeColor || this.props.computedThemeColor) - } - - get textColor () { - return getTextColorForBackground(this.backgroundColor) - } - - render () { - let findMatchText - if (this.numberOfMatches !== -1 && this.activeMatchOrdinal !== -1 && this.searchString) { + get findTextMatch () { + if (this.props.numberOfMatches !== -1 && this.props.activeMatchOrdinal !== -1 && this.props.searchString) { const l10nArgs = { - activeMatchOrdinal: this.activeMatchOrdinal, - numberOfMatches: this.numberOfMatches + activeMatchOrdinal: this.props.activeMatchOrdinal, + numberOfMatches: this.props.numberOfMatches } - findMatchText =
- } else if (this.numberOfMatches !== -1 && this.searchString) { + } else if (this.props.numberOfMatches !== -1 && this.props.searchString) { const l10nArgs = { - activeMatchOrdinal: this.activeMatchOrdinal, - numberOfMatches: this.numberOfMatches + activeMatchOrdinal: this.props.activeMatchOrdinal, + numberOfMatches: this.props.numberOfMatches } - findMatchText =
} - const backgroundColor = this.backgroundColor + return null + } + + mergeProps (state, ownProps) { + const currentWindow = state.get('currentWindow') + const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map() + const activeFrameKey = activeFrame.get('key') + + const props = {} + // used in renderer + props.backgroundColor = getSetting(settings.PAINT_TABS) && + (activeFrame.get('themeColor') || activeFrame.get('computedThemeColor')) + props.textColor = getTextColorForBackground(props.backgroundColor) + props.numberOfMatches = activeFrame.getIn(['findDetail', 'numberOfMatches'], -1) + props.activeMatchOrdinal = activeFrame.getIn(['findDetail', 'activeMatchOrdinal'], -1) + props.searchString = activeFrame.getIn(['findDetail', 'searchString'], '') + props.isCaseSensitive = activeFrame.getIn(['findDetail', 'caseSensitivity'], false) + + // used in other functions + props.activeFrameKey = activeFrameKey + props.isSelected = activeFrame.get('findbarSelected', false) + props.internalFindStatePresent = activeFrame.getIn(['findDetail', 'internalFindStatePresent']) + + return props + } + + render () { let findBarStyle = {} let findBarTextStyle = {} - if (backgroundColor) { + if (this.props.backgroundColor) { findBarStyle = { - background: backgroundColor, + background: this.props.backgroundColor, color: this.textColor } findBarTextStyle = { @@ -227,42 +228,55 @@ class FindBar extends ImmutableComponent {
- { this.searchInput = node }} + type='text' spellCheck='false' onContextMenu={this.onContextMenu} - ref={(node) => { this.searchInput = node }} onFocus={this.onInputFocus} onKeyDown={this.onKeyDown} - onInput={this.onInput} /> - + onInput={this.onInput} + /> +
- {findMatchText} - {this.findTextMatch} + - -
- + + onClick={this.onFindHide} + >+
} } -module.exports = FindBar +module.exports = ReduxComponent.connect(FindBar) diff --git a/app/renderer/components/main/main.js b/app/renderer/components/main/main.js index 350c4accb90..b9c544290df 100644 --- a/app/renderer/components/main/main.js +++ b/app/renderer/components/main/main.js @@ -625,21 +625,6 @@ class Main extends ImmutableComponent { windowActions.setUrlBarActive(false) } - onFindHide () { - const activeFrame = frameStateUtil.getActiveFrame(this.props.windowState) - frameStateUtil.onFindBarHide(activeFrame.get('key')) - } - - onFind (searchString, caseSensitivity, forward, findNext) { - const activeFrame = frameStateUtil.getActiveFrame(this.props.windowState) - webviewActions.findInPage(searchString, caseSensitivity, forward, findNext) - if (!findNext) { - windowActions.setFindDetail(activeFrame.get('key'), Immutable.fromJS({ - internalFindStatePresent: true - })) - } - } - onTabContextMenu (e) { const activeFrame = frameStateUtil.getActiveFrame(this.props.windowState) contextMenus.onTabsToolbarContextMenu(activeFrame.get('title'), activeFrame.get('location'), undefined, undefined, e) @@ -865,15 +850,7 @@ class Main extends ImmutableComponent { { activeFrame && activeFrame.get('findbarShown') && !activeFrame.get('isFullScreen') - ? + ? : null }
diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index a6806fda057..0eea26cdfce 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -110,14 +110,14 @@ const windowActions = { }, /** - * Highlight text in the findbar - * @param {Object} frameProps - The frame properties to modify - * @param {boolean} selected - Whether to select the findbar search text + * Highlight text in the find bar + * @param {Object} frameKey - The frame key to modify + * @param {boolean} selected - Whether to select the find bar search text */ - setFindbarSelected: function (frameProps, selected) { + setFindbarSelected: function (frameKey, selected) { dispatch({ actionType: windowConstants.WINDOW_SET_FINDBAR_SELECTED, - frameProps, + frameKey, selected }) }, diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index d4a24f5fcf8..87d4ac5420d 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -298,7 +298,7 @@ const doAction = (action) => { }) break case windowConstants.WINDOW_SET_FINDBAR_SELECTED: - windowState = windowState.mergeIn(['frames', frameStateUtil.getFrameIndex(windowState, action.frameProps.get('key'))], { + windowState = windowState.mergeIn(['frames', frameStateUtil.getFrameIndex(windowState, action.frameKey)], { findbarSelected: action.selected }) break