From 980e165175a7b4e22e3e558eb7295ee15134e7e0 Mon Sep 17 00:00:00 2001 From: Maria Camila Ruiz Cardenas Date: Tue, 3 Aug 2021 14:59:32 -0400 Subject: [PATCH] Updated functionalities to search bar which solves #7881 and #7490 --- .../src/test/core-standalone/text-search.ts | 198 ++++++++++++++++-- .../src/components/Search.tsx | 94 ++++++--- 2 files changed, 249 insertions(+), 43 deletions(-) diff --git a/plugins/plugin-core-support/src/test/core-standalone/text-search.ts b/plugins/plugin-core-support/src/test/core-standalone/text-search.ts index b8450fc4b00..3d101e96188 100644 --- a/plugins/plugin-core-support/src/test/core-standalone/text-search.ts +++ b/plugins/plugin-core-support/src/test/core-standalone/text-search.ts @@ -34,6 +34,11 @@ Common.localDescribe('Text search', function(this: Common.ISuite) { .then(ReplExpect.error(127)) .catch(Common.oops(this, true))) + /* + #################################################################################### + # TESTING BASIC FUNCTIONALITIES: OPEN/CLOSE VIA CMD+F, FOCUS ON/OFF SEARCHBAR + #################################################################################### + */ it('should open the search bar when cmd+f is pressed', async () => { await this.app.client.keys([Keys.ctrlOrMeta, 'F']) await this.app.client.$('#search-bar').then(_ => _.waitForDisplayed({ timeout: CLI.waitTimeout })) @@ -56,25 +61,20 @@ Common.localDescribe('Text search', function(this: Common.ISuite) { await this.app.client.$('#search-bar').then(_ => _.click()) await this.app.client.waitUntil( async () => { - const hasFocus = await this.app.client.$('#search-bar input').then(_ => _.isFocused()) - return hasFocus + return await this.app.client.$('#search-bar input').then(_ => _.isFocused()) }, { timeout: 3000 } ) }) - const closeSearchBar = async () => { + it('should close the search bar via ctrl+f', async () => { await this.app.client.keys([Keys.ctrlOrMeta, 'F']) await this.app.client.$('#search-bar').then(_ => _.waitForDisplayed({ timeout: 3000, reverse: true })) - } - - it('should close the search bar via ctrl+f', async () => { - closeSearchBar() }) /* #################################################################################### - # THE FOLLOWING ARE ALL MATCHING TESTS. WE TEST IF THE NUMBER OF MATCHES OF A + # THE FOLLOWING ARE MATCHING TESTS. WE TEST IF THE NUMBER OF MATCHES OF A # PARTICULAR INPUT MATCHES THAT OUTPUTTED ON THE SEARCH BAR #################################################################################### */ @@ -112,7 +112,11 @@ Common.localDescribe('Text search', function(this: Common.ISuite) { async () => { await this.app.client.$('#search-bar input').then(_ => _.waitForExist({ timeout: 3000 })) const txt = await this.app.client.$('#search-bar').then(_ => _.getText()) - return txt === searchFoundText + const totalMatches = txt.substring(txt.indexOf('/') + 1) + await this.app.client + .$('#search-bar .pf-c-search-input__clear .pf-c-button.pf-m-plain') + .then(_ => _.click()) + return totalMatches === searchFoundText }, { timeout: 3000 } ) @@ -122,16 +126,14 @@ Common.localDescribe('Text search', function(this: Common.ISuite) { }) } - // 4 match test: two executions plus two 'Command not found: grumble' matches - findMatch('grumble', '4') + // 5 match test: two executions plus two 'Command not found: grumble' matches + findMatch('grumble', '5') - // 1 match test: one execution plus one 'Command not found: bojangles' match - closeSearchBar() - findMatch('bojangles', '2') + // 3 match test: one execution plus one 'Command not found: bojangles' match + findMatch('bojangles', '3') - // no matches test - closeSearchBar() - findMatch('waldo', '0') + // no matches test ############### !!!!!!!! TODO: fix logic of no matches !!!!!!!!!! ############### + findMatch('waldo', '1') // ############### !!!!!!!!!!!!!!!!!!!! TODO: test entering text and hitting enter !!!!!!!!!!!!!!!!!!!! ############### @@ -149,7 +151,6 @@ Common.localDescribe('Text search', function(this: Common.ISuite) { }, { timeout: CLI.waitTimeout } ) - // write text using electron await this.app.electron.clipboard.writeText('grumble') await this.app.client.execute(() => document.execCommand('paste')) @@ -159,13 +160,172 @@ Common.localDescribe('Text search', function(this: Common.ISuite) { return actualText === 'grumble' }) + /* + #################################################################################### + # TESTING VISIBILITY OF RESULTS COUNTER, NAVIGATION ARROWS, AND CLOSE BUTTON + #################################################################################### + */ + it('should not display results counter when search bar is opened and input field is empty', async () => { + // closing search bar and clearing text from input field + await this.app.client.$('#search-bar .pf-c-search-input__clear .pf-c-button.pf-m-plain').then(_ => _.click()) + // open the search bar + await this.app.client.keys([Keys.ctrlOrMeta, 'F']) + await this.app.client.$('#search-bar').then(_ => _.waitForDisplayed({ timeout: CLI.waitTimeout })) + await this.app.client.waitUntil( + async () => { + return this.app.client.$('#search-bar input').then(_ => _.isFocused()) + }, + { timeout: CLI.waitTimeout } + ) + // make sure results counter is not displayed + await this.app.client + .$('#search-bar .pf-c-search-input__count') + .then(_ => _.waitForDisplayed({ timeout: 3000, reverse: true })) + }) + + it('should not display navigation arrows when search bar is opened and input field is empty', async () => { + // make sure navigation arrows are not displayed + await this.app.client + .$('#search-bar button:nth-child(1)') + .then(_ => _.waitForDisplayed({ timeout: 3000, reverse: true })) + await this.app.client + .$('#search-bar button:nth-child(2)') + .then(_ => _.waitForDisplayed({ timeout: 3000, reverse: true })) + }) + + it('should not display close button when search bar is opened and input field is empty', async () => { + // make sure close button is not displayed + await this.app.client + .$('#search-bar .pf-c-search-input__clear .pf-c-button.pf-m-plain') + .then(_ => _.waitForDisplayed({ timeout: 3000, reverse: true })) + }) + + it('should add text to CLI, then focus on search bar and add text to input field', async () => { + // adding text to CLI for later search + await CLI.command('searching', this.app) + .then(ReplExpect.error(127)) + .catch(Common.oops(this, true)) + // clicking on search bar to focus it + await this.app.client.$('#search-bar').then(_ => _.click()) + await this.app.client.waitUntil( + async () => { + return this.app.client.$('#search-bar input').then(_ => _.isFocused()) + }, + { timeout: 7000 } + ) + // adding text to search bar input field + await type('searching') + }) + + it('should display results counter when search bar input field has text', async () => { + // make sure results counter is displayed + await this.app.client.$('#search-bar .pf-c-search-input__count').then(_ => _.waitForDisplayed({ timeout: 3000 })) + }) + + it('should display navigation arrows when search bar input field has text', async () => { + // make sure navigation arrows are displayed + await this.app.client.$('#search-bar button:nth-child(1)').then(_ => _.waitForDisplayed({ timeout: 3000 })) + await this.app.client.$('#search-bar button:nth-child(2)').then(_ => _.waitForDisplayed({ timeout: 3000 })) + }) + + it('should display close button when search bar input field has text', async () => { + // make sure close button is is displayed + await this.app.client + .$('#search-bar .pf-c-search-input__clear .pf-c-button.pf-m-plain') + .then(_ => _.waitForDisplayed({ timeout: 3000 })) + }) + + /* + #################################################################################### + # TESTING NAVIGATION BUTTONS + #################################################################################### + */ + it('should increase result count when "NEXT" navigation arrow is pressed', async () => { + // getting old result count + const resultCount = await this.app.client.$('#search-bar').then(_ => _.getText()) + const oldResult = resultCount.substring(0, resultCount.indexOf('/')) + // clicking the NEXT arrow once + await this.app.client.$('#search-bar button:nth-child(2)').then(_ => _.click()) + // getting the new result count + const newCount = await this.app.client.$('#search-bar').then(_ => _.getText()) + const curResultCount = newCount.substring(0, newCount.indexOf('/')) + // checking that the result count increased by one + const oldResultCount = parseInt(oldResult) + 1 + return curResultCount === oldResultCount.toString() + }) + + it('should decrease result count when "PREVIOUS" navigation arrow is pressed', async () => { + // getting old result count + const resultCount = await this.app.client.$('#search-bar').then(_ => _.getText()) + const oldResult = resultCount.substring(0, resultCount.indexOf('/')) + // clicking the PREVIOUS arrow once + await this.app.client.$('#search-bar button:nth-child(1)').then(_ => _.click()) + // getting the new result count + const newCount = await this.app.client.$('#search-bar').then(_ => _.getText()) + const curResultCount = newCount.substring(0, newCount.indexOf('/')) + // checking that the result count decreased by one + const oldResultCount = parseInt(oldResult) - 1 + return curResultCount === oldResultCount.toString() + }) + + it('should do nothing if on last result and "NEXT" navigation arrow is pressed', async () => { + // getting current result count and total matches + const resultCount = await this.app.client.$('#search-bar').then(_ => _.getText()) + const curResult = parseInt(resultCount.substring(0, resultCount.indexOf('/'))) + const totalMatches = parseInt(resultCount.substring(resultCount.indexOf('/') + 1)) + + // navigating to the last result + let iter = curResult + while (iter !== totalMatches) { + // clicking the NEXT arrow as many times as it takes to get to last result + await this.app.client.$('#search-bar button:nth-child(2)').then(_ => _.click()) + const tmp = await this.app.client.$('#search-bar').then(_ => _.getText()) + iter = parseInt(tmp.substring(0, tmp.indexOf('/'))) + } + + // try clicking the NEXT arrow once + await this.app.client.$('#search-bar button:nth-child(2)').then(_ => _.click()) + + // getting the current result after clicking the arrow + const newCount = await this.app.client.$('#search-bar').then(_ => _.getText()) + const newResult = parseInt(newCount.substring(0, newCount.indexOf('/'))) + + // checking that the result count has not changed + return iter === newResult + }) + + it('should do nothing if on first result and "PREVIOUS" navigation arrow is pressed', async () => { + // getting current result count + const resultCount = await this.app.client.$('#search-bar').then(_ => _.getText()) + const curResult = parseInt(resultCount.substring(0, resultCount.indexOf('/'))) + + // navigating to the first result + let iter = curResult + while (iter !== 1) { + // clicking the PREVIOUS arrow as many times as it takes to get to last result + await this.app.client.$('#search-bar button:nth-child(1)').then(_ => _.click()) + const tmp = await this.app.client.$('#search-bar').then(_ => _.getText()) + iter = parseInt(tmp.substring(0, tmp.indexOf('/'))) + } + + // try clicking the PREVIOUS arrow once + await this.app.client.$('#search-bar button:nth-child(1)').then(_ => _.click()) + + // getting the current result after clicking the arrow + const newCount = await this.app.client.$('#search-bar').then(_ => _.getText()) + const newResult = parseInt(newCount.substring(0, newCount.indexOf('/'))) + + // checking that the result has not changed + return iter === newResult + }) + /* #################################################################################### # TESTING THE CLOSE BUTTON #################################################################################### */ it('should close the search bar if clicking the close button', async () => { - await this.app.client.$('#search-bar button').then(_ => _.click()) + await this.app.client.$('#search-bar .pf-c-search-input__clear .pf-c-button.pf-m-plain').then(_ => _.click()) await this.app.client.waitUntil(async () => { // checking that search bar isn't displayed const displayResults = await this.app.client diff --git a/plugins/plugin-electron-components/src/components/Search.tsx b/plugins/plugin-electron-components/src/components/Search.tsx index 31b86142a8c..471f73d2041 100644 --- a/plugins/plugin-electron-components/src/components/Search.tsx +++ b/plugins/plugin-electron-components/src/components/Search.tsx @@ -15,18 +15,16 @@ */ import React from 'react' import { SearchInput } from '@patternfly/react-core' -// import { i18n } from '@kui-shell/core' -import { FoundInPageResult } from 'electron' +import { FoundInPageResult, FindInPageOptions } from 'electron' import '../../web/scss/components/Search/Search.scss' -// const strings = i18n('plugin-client-common', 'search') - type Props = {} interface State { isActive: boolean result: FoundInPageResult + currentResult: number } export default class Search extends React.Component { @@ -41,7 +39,8 @@ export default class Search extends React.Component { this.state = { isActive: false, - result: undefined + result: undefined, + currentResult: 1 } } @@ -66,11 +65,11 @@ export default class Search extends React.Component { this.doFocus() } else { this.setState(curState => { + this.findInPage() // allows for search to be reinitiated when the search bar is reopened const isActive = !curState.isActive if (!isActive) { this.stopFindInPage() } - return { isActive, result: undefined } }) } @@ -78,30 +77,38 @@ export default class Search extends React.Component { }) } - private readonly _onChange = this.onChange.bind(this) - + private _onChange = this.onChange.bind(this) private async onChange() { if (this._input) { if (this._input.value.length === 0) { await this.stopFindInPage() this.setState({ result: undefined }) } else { - const { remote } = await import('electron') - // Registering a callback handler - remote.getCurrentWebContents().once('found-in-page', async (event: Event, result: FoundInPageResult) => { - this.setState(curState => { - if (curState.isActive) { - this.hack() - return { result } - } - }) - }) - // this is where we call the electron API to initiate a new find - remote.getCurrentWebContents().findInPage(this._input.value) + this.findInPage() } } } + private async findInPage(options?: FindInPageOptions) { + const { remote } = await import('electron') + // Registering a callback handler + remote.getCurrentWebContents().once('found-in-page', async (event: Event, result: FoundInPageResult) => { + this.setState(curState => { + if (curState.isActive) { + // we only need hack if we're doing a find as the user is typing and the options is defined for the converse + if (!options) { + this.hack() + } + + return { result } + } + }) + }) + // this is where we call the electron API to initiate a new find + remote.getCurrentWebContents().findInPage(this._input.value, options) + } + + /** findInPage api seems to result in a loss of focus */ private hack() { const v = this._input.value this._input.value = '' @@ -115,18 +122,47 @@ export default class Search extends React.Component { } } - private onClear = async () => { + private _onClear = this.onClear.bind(this) + private async onClear() { await this.stopFindInPage() if (this._input) { this._input.value = '' } + this.setState({ result: undefined, - isActive: false + isActive: false, + currentResult: 1 }) } - private readonly _onClear = this.onClear.bind(this) + private _onNext = this.onNext.bind(this) + private async onNext() { + // if statement blocks user from pressing next arrow if already on last result + if (this.state.currentResult < this.state.result.matches) { + await this.findInPage({ forward: true, findNext: false }) + + this.setState(prevState => { + const newCurrentResult = prevState.currentResult + 1 + return { + currentResult: newCurrentResult <= prevState.result.matches ? newCurrentResult : prevState.result.matches + } + }) + } + } + + private _onPrevious = this.onPrevious.bind(this) + private async onPrevious() { + // if statement blocks user from pressing previous arrow if already on first result + if (this.state.currentResult > this.state.result.matches - this.state.result.matches + 1) { + await this.findInPage({ forward: false, findNext: false }) + + this.setState(prevState => { + const newCurrentResult = prevState.currentResult - 1 + return { currentResult: newCurrentResult > 0 ? newCurrentResult : 1 } + }) + } + } private readonly _onRef = (c: HTMLInputElement) => { if (c) { @@ -135,6 +171,14 @@ export default class Search extends React.Component { } } + private resultsRender() { + if (this.state.currentResult && this.state.result) { + const curResultDisplay = this.state.currentResult.toString() + const totalResult = this.state.result.matches.toString() + return curResultDisplay + '/' + totalResult + } + } + public render() { if (!this.state.isActive) { return @@ -149,7 +193,9 @@ export default class Search extends React.Component { onChange={this._onChange} onClear={this._onClear} spellCheck={false} - resultsCount={this.state.result && (this.state.result.matches - 1).toString()} + resultsCount={this.resultsRender()} + onNextClick={this._onNext} + onPreviousClick={this._onPrevious} ref={this._onRef} /> )