From d73dd6edb2dd11518781f6c2c640322adea92434 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Thu, 3 Nov 2016 21:23:14 -0400 Subject: [PATCH] Make findbar not a partially controlled component This gets rid of the warning that React gives about a controlled component. It also fixes the problem with when state updates because a background tab changes, it should not re-select text Fix #5363 --- js/components/findbar.js | 13 ++++++------ test/components/frameTest.js | 41 +++++++++++++++++++++++++++++------- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/js/components/findbar.js b/js/components/findbar.js index 27cbcb5df4e..6c261e2a036 100644 --- a/js/components/findbar.js +++ b/js/components/findbar.js @@ -79,16 +79,20 @@ class FindBar extends ImmutableComponent { } componentDidMount () { + this.searchInput.value = this.searchString this.focus() + this.select() windowActions.setFindbarSelected(false) } componentWillUpdate (nextProps) { - this.didFrameChange = nextProps.frameKey !== this.props.frameKey + if (nextProps.frameKey !== this.props.frameKey) { + this.searchInput.value = nextProps.findDetail && nextProps.findDetail.get('searchString') || '' + } } componentDidUpdate (prevProps) { - if (this.props.selected) { + if (this.props.selected && !prevProps.selected) { this.focus() // Findbar might already be focused, so make sure select happens even if no // onFocus event happens. @@ -203,10 +207,6 @@ class FindBar extends ImmutableComponent { } } - const inputValue = this.didFrameChange - ? this.searchString || undefined - : undefined - return
@@ -215,7 +215,6 @@ class FindBar extends ImmutableComponent { spellCheck='false' onContextMenu={this.onContextMenu} ref={(node) => { this.searchInput = node }} - value={inputValue} onFocus={this.onInputFocus} onKeyDown={this.onKeyDown} onKeyUp={this.onKeyUp} /> diff --git a/test/components/frameTest.js b/test/components/frameTest.js index 749ab7e28eb..752d46f5360 100644 --- a/test/components/frameTest.js +++ b/test/components/frameTest.js @@ -12,7 +12,7 @@ describe('findbar', function () { yield setup(this.app.client) const url = Brave.server.url('find_in_page.html') yield this.app.client - .tabByUrl(Brave.newTabUrl) + .tabByIndex(0) .url(url) .waitForUrl(url) .windowParentByUrl(url) @@ -163,6 +163,30 @@ describe('findbar', function () { assert.equal(match, '2 of 2') }) + it('typing while another frame is loading', function * () { + const url2 = Brave.server.url('find_in_page2.html') + yield this.app.client + .showFindbar() + .waitForElementFocus(findBarInput) + .ipcSend(messages.SHORTCUT_NEW_FRAME, url2, { openInForeground: false }) + .setValue(findBarInput, 'test') + .waitUntil(function () { + return this.getValue(findBarInput).then((val) => val === 'test') + }) + .showFindbar() + .waitForElementFocus(findBarInput) + .keys('x') + .waitUntil(function () { + return this.getValue(findBarInput).then((val) => val === 'x') + }) + .keys('y') + .keys('z') + .waitUntil(function () { + return this.getValue(findBarInput).then((val) => val === 'xyz') + }) + .ipcSend(messages.SHORTCUT_CLOSE_FRAME, 2) + }) + it('findbar input remembered but no active ordinals after navigation until RETURN key', function * () { const url2 = Brave.server.url('find_in_page2.html') yield this.app.client @@ -177,7 +201,7 @@ describe('findbar', function () { yield this.app.client .waitForVisible(findBarMatches) - .tabByUrl(Brave.newTabUrl) + .tabByIndex(0) .url(url2) .waitForUrl(url2) .windowParentByUrl(url2) @@ -199,17 +223,18 @@ describe('findbar', function () { it('remembers findbar input when switching frames', function * () { const url = Brave.server.url('find_in_page.html') yield this.app.client + .tabByIndex(0) + .url(url) + .waitForUrl(url) + .windowParentByUrl(url) .showFindbar() .waitForElementFocus(findBarInput) .setValue(findBarInput, 'test') - yield this.app.client .ipcSend(messages.SHORTCUT_NEW_FRAME, url) + .waitForTabCount(2) + .tabByIndex(1) + .waitForUrl(url) .windowParentByUrl(url) - .waitUntil(function () { - return this.getAttribute('webview[data-frame-key="2"]', 'src').then((src) => src === url) - }) - .waitForElementFocus('webview[data-frame-key="2"]') - yield this.app.client .showFindbar(true, 2) .waitForElementFocus(findBarInput) .setValue(findBarInput, 'abc')