From 5c7db9cb230d59252a19e886cdcdf020e9f006d4 Mon Sep 17 00:00:00 2001 From: Jon Kuperman Date: Thu, 18 Aug 2016 16:05:02 -0700 Subject: [PATCH 1/3] Improved UI/UX of find bar --- js/actions/webviewActions.js | 25 ++++++++++ js/components/button.js | 2 + js/components/findbar.js | 93 +++++++++++++++++++++++------------- js/components/frame.js | 39 ++------------- js/components/main.js | 27 ++++++++++- less/findbar.less | 71 +++++++++++++++++++++++---- less/variables.less | 2 + 7 files changed, 181 insertions(+), 78 deletions(-) diff --git a/js/actions/webviewActions.js b/js/actions/webviewActions.js index b5617c4dab4..6f01560694c 100644 --- a/js/actions/webviewActions.js +++ b/js/actions/webviewActions.js @@ -81,6 +81,31 @@ const webviewActions = { webview.executeJavaScript('document.webkitRequestFullscreen()') } } + }, + + findInPage (searchString, caseSensitivity, forward, webview) { + webview = webview || getWebview() + if (!webview) { + return + } + + if (searchString) { + webview.findInPage(searchString, { + matchCase: caseSensitivity, + forward: forward !== undefined ? forward : true, + findNext: forward !== undefined + }) + } else { + webview.stopFindInPage('clearSelection') + } + }, + + stopFindInPage (webview) { + webview = webview || getWebview() + if (!webview) { + return + } + webview.stopFindInPage('keepSelection') } } diff --git a/js/components/button.js b/js/components/button.js index e756fbf9b0c..bd9c2e7974e 100644 --- a/js/components/button.js +++ b/js/components/button.js @@ -11,6 +11,7 @@ class Button extends ImmutableComponent { if (this.props.iconClass) { return } - return
-
- { this.searchInput = node }} - onKeyDown={this.onKeyDown} - onChange={this.onChange} - value={this.searchString} /> - {findMatchText} -
-
+ + } } diff --git a/js/components/frame.js b/js/components/frame.js index 401620f7dee..450812d26f9 100644 --- a/js/components/frame.js +++ b/js/components/frame.js @@ -5,6 +5,7 @@ const React = require('react') const urlParse = require('url').parse const windowActions = require('../actions/windowActions') +const webviewActions = require('../actions/webviewActions') const appActions = require('../actions/appActions') const ImmutableComponent = require('./immutableComponent') const Immutable = require('immutable') @@ -22,7 +23,6 @@ const debounce = require('../lib/debounce.js') const getSetting = require('../settings').getSetting const config = require('../constants/config') const settings = require('../constants/settings') -const FindBar = require('./findbar.js') const { aboutUrls, isSourceAboutUrl, isTargetAboutUrl, getTargetAboutUrl, getBaseUrl, isNavigatableAboutPage } = require('../lib/appUrlUtil') const { isFrameError } = require('../lib/errorUtil') const locale = require('../l10n') @@ -44,8 +44,6 @@ class Frame extends ImmutableComponent { constructor () { super() this.onUpdateWheelZoom = debounce(this.onUpdateWheelZoom.bind(this), 20) - this.onFind = this.onFind.bind(this) - this.onFindHide = this.onFindHide.bind(this) this.onFocus = this.onFocus.bind(this) // Maps notification message to its callback this.notificationCallbacks = {} @@ -832,7 +830,7 @@ class Frame extends ImmutableComponent { this.webview.addEventListener('did-navigate', (e) => { if (this.props.findbarShown) { - this.onFindHide() + windowActions.setFindbarShown(this.frame, false) } for (let message in this.notificationCallbacks) { @@ -993,15 +991,10 @@ class Frame extends ImmutableComponent { } const searchString = this.props.findDetail && this.props.findDetail.get('searchString') if (searchString) { - this.onFind(searchString, this.props.findDetail && this.props.findDetail.get('caseSensitivity') || undefined, forward) + webviewActions.findInPage(searchString, this.props.findDetail && this.props.findDetail.get('caseSensitivity') || undefined, forward, this.webview) } } - onFindHide () { - windowActions.setFindbarShown(this.frame, false) - this.webview.stopFindInPage('keepSelection') - } - onUpdateWheelZoom () { if (this.wheelDeltaY > 0) { this.zoomIn() @@ -1021,22 +1014,6 @@ class Frame extends ImmutableComponent { } } - onFind (searchString, caseSensitivity, forward) { - if (searchString) { - this.webview.findInPage(searchString, { - matchCase: caseSensitivity, - forward: forward !== undefined ? forward : true, - findNext: forward !== undefined - }) - } else { - this.onClearMatch() - } - } - - onClearMatch () { - this.webview.stopFindInPage('clearSelection') - } - get webRTCPolicy () { return this.webview ? this.webview.getWebRTCIPHandlingPolicy() : WEBRTC_DEFAULT } @@ -1062,16 +1039,6 @@ class Frame extends ImmutableComponent { ? : null } - { - this.props.findbarShown && !this.props.isFullScreen - ? - : null - }
{ this.webviewContainer = node }} className={cx({ webviewContainer: true, diff --git a/js/components/main.js b/js/components/main.js index 704b8c2568b..c6850e607fd 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -20,6 +20,7 @@ const NavigationBar = require('./navigationBar') const Frame = require('./frame') const TabPages = require('./tabPages') const TabsToolbar = require('./tabsToolbar') +const FindBar = require('./findbar.js') const UpdateBar = require('./updateBar') const NotificationBar = require('./notificationBar') const DownloadsBar = require('./downloadsBar') @@ -86,6 +87,8 @@ class Main extends ImmutableComponent { this.onBraveMenu = this.onBraveMenu.bind(this) this.onHamburgerMenu = this.onHamburgerMenu.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) } registerWindowLevelShortcuts () { @@ -627,6 +630,15 @@ class Main extends ImmutableComponent { windowActions.setUrlBarActive(false) } + onFindHide () { + const activeFrame = FrameStateUtil.getActiveFrame(this.props.windowState) + windowActions.setFindbarShown(activeFrame, false) + } + + onFind (searchString, caseSensitivity, forward) { + webviewActions.findInPage(searchString, caseSensitivity, forward) + } + onTabContextMenu (e) { const activeFrame = FrameStateUtil.getActiveFrame(this.props.windowState) contextMenus.onTabsToolbarContextMenu(activeFrame, undefined, undefined, e) @@ -901,6 +913,20 @@ class Main extends ImmutableComponent { activeFrameKey={activeFrame && activeFrame.get('key') || undefined} onMenu={this.onHamburgerMenu} /> + + { + activeFrame && activeFrame.get('findbarShown') && !activeFrame.get('isFullScreen') + ? + : null + }
Date: Thu, 25 Aug 2016 17:23:03 -0400 Subject: [PATCH 2/3] Fix syntax and clear highlight on findbar close Auditors: @jkup --- js/actions/webviewActions.js | 4 ++-- js/components/main.js | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/js/actions/webviewActions.js b/js/actions/webviewActions.js index 6f01560694c..1e23694da7b 100644 --- a/js/actions/webviewActions.js +++ b/js/actions/webviewActions.js @@ -83,7 +83,7 @@ const webviewActions = { } }, - findInPage (searchString, caseSensitivity, forward, webview) { + findInPage: function (searchString, caseSensitivity, forward, webview) { webview = webview || getWebview() if (!webview) { return @@ -100,7 +100,7 @@ const webviewActions = { } }, - stopFindInPage (webview) { + stopFindInPage: function (webview) { webview = webview || getWebview() if (!webview) { return diff --git a/js/components/main.js b/js/components/main.js index c6850e607fd..37d9b5f9bfd 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -633,6 +633,7 @@ class Main extends ImmutableComponent { onFindHide () { const activeFrame = FrameStateUtil.getActiveFrame(this.props.windowState) windowActions.setFindbarShown(activeFrame, false) + webviewActions.stopFindInPage() } onFind (searchString, caseSensitivity, forward) { From 313d743012e2aa73408f15f02ef888fdf9e3f102 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Fri, 26 Aug 2016 23:19:22 -0400 Subject: [PATCH 3/3] Track presence of internal find state Auditors: @jkup The trick to this was to keep track in our app state when electron has find state defined. When it is not defined always do the findFirst call --- docs/state.md | 3 ++- js/actions/webviewActions.js | 6 +++--- js/components/findbar.js | 6 +++--- js/components/frame.js | 2 +- js/components/main.js | 12 ++++++++++-- 5 files changed, 19 insertions(+), 10 deletions(-) diff --git a/docs/state.md b/docs/state.md index 03131061666..668afcdff03 100644 --- a/docs/state.md +++ b/docs/state.md @@ -277,7 +277,8 @@ WindowStore searchString: string, // the string being searched caseSensitivity: boolean, // whether we are doing a case sensitive search numberOfMatches: number, // Total number of matches on the page - activeMatchOrdinal: number // The current ordinal of the match + activeMatchOrdinal: number, // The current ordinal of the match + internalFindStatePresent: boolean // true if a find-first (ie findNext: false) call has been made } unloaded: boolean, // true if the tab is unloaded diff --git a/js/actions/webviewActions.js b/js/actions/webviewActions.js index 1e23694da7b..0ae2afa74df 100644 --- a/js/actions/webviewActions.js +++ b/js/actions/webviewActions.js @@ -83,7 +83,7 @@ const webviewActions = { } }, - findInPage: function (searchString, caseSensitivity, forward, webview) { + findInPage: function (searchString, caseSensitivity, forward, findNext, webview) { webview = webview || getWebview() if (!webview) { return @@ -92,8 +92,8 @@ const webviewActions = { if (searchString) { webview.findInPage(searchString, { matchCase: caseSensitivity, - forward: forward !== undefined ? forward : true, - findNext: forward !== undefined + forward, + findNext }) } else { webview.stopFindInPage('clearSelection') diff --git a/js/components/findbar.js b/js/components/findbar.js index 8d1218335ce..b6048b58a9c 100644 --- a/js/components/findbar.js +++ b/js/components/findbar.js @@ -43,15 +43,15 @@ class FindBar extends ImmutableComponent { } onFindFirst () { - this.props.onFind(this.searchString, this.isCaseSensitive) + this.props.onFind(this.searchString, this.isCaseSensitive, true, false) } onFindNext () { - this.props.onFind(this.searchString, this.isCaseSensitive, true) + this.props.onFind(this.searchString, this.isCaseSensitive, true, this.props.findDetail.get('internalFindStatePresent')) } onFindPrev () { - this.props.onFind(this.searchString, this.isCaseSensitive, false) + this.props.onFind(this.searchString, this.isCaseSensitive, false, this.props.findDetail.get('internalFindStatePresent')) } /** diff --git a/js/components/frame.js b/js/components/frame.js index 450812d26f9..aebd63170e0 100644 --- a/js/components/frame.js +++ b/js/components/frame.js @@ -991,7 +991,7 @@ class Frame extends ImmutableComponent { } const searchString = this.props.findDetail && this.props.findDetail.get('searchString') if (searchString) { - webviewActions.findInPage(searchString, this.props.findDetail && this.props.findDetail.get('caseSensitivity') || undefined, forward, this.webview) + webviewActions.findInPage(searchString, this.props.findDetail && this.props.findDetail.get('caseSensitivity') || undefined, forward, this.props.findDetail.get('internalFindStatePresent'), this.webview) } } diff --git a/js/components/main.js b/js/components/main.js index 37d9b5f9bfd..746a8a6aad3 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -634,10 +634,18 @@ class Main extends ImmutableComponent { const activeFrame = FrameStateUtil.getActiveFrame(this.props.windowState) windowActions.setFindbarShown(activeFrame, false) webviewActions.stopFindInPage() + windowActions.setFindDetail(this.frame, Immutable.fromJS({ + internalFindStatePresent: false + })) } - onFind (searchString, caseSensitivity, forward) { - webviewActions.findInPage(searchString, caseSensitivity, forward) + onFind (searchString, caseSensitivity, forward, findNext) { + webviewActions.findInPage(searchString, caseSensitivity, forward, findNext) + if (!findNext) { + windowActions.setFindDetail(this.frame, Immutable.fromJS({ + internalFindStatePresent: true + })) + } } onTabContextMenu (e) {