From f28283c81aed899b1d3d76b30111e2480f2310bf 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/frame/frame.js | 8 +- app/renderer/components/main/findbar.js | 214 +++++++++++++----------- app/renderer/components/main/main.js | 27 +-- docs/windowActions.md | 8 +- js/actions/windowActions.js | 10 +- js/stores/windowStore.js | 2 +- 6 files changed, 134 insertions(+), 135 deletions(-) diff --git a/app/renderer/components/frame/frame.js b/app/renderer/components/frame/frame.js index 1c1e49f4b8f..2337e466c47 100644 --- a/app/renderer/components/frame/frame.js +++ b/app/renderer/components/frame/frame.js @@ -832,6 +832,7 @@ class Frame extends React.Component { if (this.frame.isEmpty()) { return } + if (e.result !== undefined && (e.result.matches !== undefined || e.result.activeMatchOrdinal !== undefined)) { if (e.result.matches === 0) { windowActions.setFindDetail(this.props.frameKey, Immutable.fromJS({ @@ -840,9 +841,10 @@ class Frame extends React.Component { })) return } + windowActions.setFindDetail(this.props.frameKey, Immutable.fromJS({ - numberOfMatches: e.result.matches || this.props.findDetailNumberOfMatches, - activeMatchOrdinal: e.result.activeMatchOrdinal || this.props.findDetailActiveMatchOrdinal + numberOfMatches: e.result.matches || -1, + activeMatchOrdinal: e.result.activeMatchOrdinal || -1 })) } }) @@ -933,10 +935,8 @@ class Frame extends React.Component { props.isSecure = frame.getIn(['security', 'isSecure']) props.findbarShown = frame.get('findbarShown') props.findDetailCaseSensitivity = frame.getIn(['findDetail', 'caseSensitivity']) || undefined - props.findDetailNumberOfMatches = frame.getIn(['findDetail', 'numberOfMatches']) || 0 props.findDetailSearchString = frame.getIn(['findDetail', 'searchString']) props.findDetailInternalFindStatePresent = frame.getIn(['findDetail', 'internalFindStatePresent']) - props.findDetailActiveMatchOrdinal = frame.getIn(['findDetail', 'activeMatchOrdinal']) props.isPrivate = frame.get('isPrivate') props.activeShortcut = frame.get('activeShortcut') props.shortcutDetailsUsername = frame.getIn(['activeShortcutDetails', 'username']) diff --git a/app/renderer/components/main/findbar.js b/app/renderer/components/main/findbar.js index 95e5ee1f02b..65b23e0c34e 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,34 @@ 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) + this.onFind = this.onFind.bind(this) + this.onFindHide = this.onFindHide.bind(this) } 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 +74,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,29 +89,37 @@ 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) { - this.searchInput.value = (nextProps.findDetail && nextProps.findDetail.get('searchString')) || '' + if (nextProps.activeFrameKey !== this.props.activeFrameKey) { + this.searchInput.value = nextProps.searchString + } + } + + componentWillUnmount () { + if (this.props.isPrivate) { + windowActions.setFindDetail(this.props.activeFrameKey, Immutable.fromJS({ + searchString: '', + caseSensitivity: this.props.isCaseSensitive + })) } } 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')) { + + if (this.props.searchString !== prevProps.searchString || this.props.isCaseSensitive !== prevProps.isCaseSensitive) { // Redo search if details have changed this.onFindFirst() } @@ -123,7 +129,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 +147,86 @@ 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') + onFindHide () { + frameStateUtil.onFindBarHide(this.props.activeFrameKey) } - get activeMatchOrdinal () { - if (!this.props.findDetail || this.props.findDetail.get('activeMatchOrdinal') === undefined) { - return -1 + 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('activeMatchOrdinal') || -1 - } - - get isCaseSensitive () { - if (!this.props.findDetail) { - return false - } - return this.props.findDetail.get('caseSensitivity') - } - - get searchString () { - if (!this.props.findDetail) { - return '' - } - 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 > 0 && this.props.activeMatchOrdinal > 0 && 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 === 0 && 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 = props.backgroundColor && 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']) + props.isPrivate = activeFrame.get('isPrivate', false) + + 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 +238,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..63cf9f32f25 100644 --- a/app/renderer/components/main/main.js +++ b/app/renderer/components/main/main.js @@ -86,8 +86,6 @@ class Main extends ImmutableComponent { this.onHideReleaseNotes = this.onHideReleaseNotes.bind(this) this.onHideCheckDefaultBrowserDialog = this.onHideCheckDefaultBrowserDialog.bind(this) this.onTabContextMenu = this.onTabContextMenu.bind(this) - this.onFind = this.onFind.bind(this) - this.onFindHide = this.onFindHide.bind(this) this.checkForTitleMode = debounce(this.checkForTitleMode.bind(this), 20) this.resetAltMenuProcessing() } @@ -625,21 +623,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 +848,7 @@ class Main extends ImmutableComponent { { activeFrame && activeFrame.get('findbarShown') && !activeFrame.get('isFullScreen') - ? + ? : null }
diff --git a/docs/windowActions.md b/docs/windowActions.md index a9c017039f2..00ec999942c 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -98,15 +98,15 @@ Shows/hides the find-in-page bar. -### setFindbarSelected(frameProps, selected) +### setFindbarSelected(frameKey, selected) -Highlight text in the findbar +Highlight text in the find bar **Parameters** -**frameProps**: `Object`, The frame properties to modify +**frameKey**: `Object`, The frame key to modify -**selected**: `boolean`, Whether to select the findbar search text +**selected**: `boolean`, Whether to select the find bar search text 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