fix(queryHook): restore behaviour of queryHook #4202
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There has been many complaints from user on our [debouncing guide](https://www.algolia.com/doc/guides/building-search-ui/going-further/improve-performance/js/#debouncing) no longer working as expected. This issue was introduced when the SearchBox component was converted into a Preact, and the following condition removed: c073a9a#diff-530222e0c4597f2110dc6ba173a306b0L98 **Before SearchBox refactor**: - when input was focused, the query exposed by the connector didn't update the input value - when input was not focused, the query exposed by the connector did update the input value **After SearchBox refactor**: - regardless on whether input was focused, the query exposed by the connector updates the input value This PR restores the logic as found before the refactor. fixes #4141
Haroenv
approved these changes
Nov 20, 2019
tkrugg
added a commit
to algolia/instantsearch-e2e-tests
that referenced
this pull request
Nov 21, 2019
In React and Preact the CLEAR selenium instruction does not fire any event, and therefore does not update the component's internal state (see this job for example: https://app.saucelabs.com/tests/4518f96fdfd447e998786d0ff463658a#54) It is equivalent to `input.value = ''`. There are multiple sources reporting a similar behaviour (see SeleniumHQ/selenium#6741) Since this change: algolia/instantsearch#4202 ```jsx <input value={state.query} /> ``` instead of ```jsx <input value={props.query} /> ``` Since binding the input value to a `state.query` (rather than a `props.query`), it appears the click we use to focus on the searchbox re-syncs `input.value` with `state.query`.
There was a problem hiding this comment.
Nice to see this fixed!
I originally omitted the focused condition simply because I though that was some legacy code that didn't have effect anymore... And it was merged like that. All that to say that we should definitely document in the code why we need to check for the focused state; this solution is not trivial to grasp.
tkrugg
added a commit
to algolia/instantsearch-e2e-tests
that referenced
this pull request
Nov 26, 2019
* fix(setSearchBoxValue): reset searchbox before editing In React and Preact the CLEAR selenium instruction does not fire any event, and therefore does not update the component's internal state (see this job for example: https://app.saucelabs.com/tests/4518f96fdfd447e998786d0ff463658a#54) It is equivalent to `input.value = ''`. There are multiple sources reporting a similar behaviour (see SeleniumHQ/selenium#6741) Since this change: algolia/instantsearch#4202 ```jsx <input value={state.query} /> ``` instead of ```jsx <input value={props.query} /> ``` Since binding the input value to a `state.query` (rather than a `props.query`), it appears the click we use to focus on the searchbox re-syncs `input.value` with `state.query`.
francoischalifour
approved these changes
Nov 26, 2019
tkrugg
added a commit
that referenced
this pull request
Nov 26, 2019
* fix(queryHook): restore behaviour of queryHook There has been many complaints from user on our [debouncing guide](https://www.algolia.com/doc/guides/building-search-ui/going-further/improve-performance/js/#debouncing) no longer working as expected. This issue was introduced when the SearchBox component was converted into a Preact, and the following condition removed: c073a9a#diff-530222e0c4597f2110dc6ba173a306b0L98 **Before SearchBox refactor**: - when input was focused, the query exposed by the connector didn't update the input value - when input was not focused, the query exposed by the connector did update the input value **After SearchBox refactor**: - regardless on whether input was focused, the query exposed by the connector updates the input value This PR restores the logic as found before the refactor. fixes #4141 * chore: update snapshots * fix: reset works again * add a comment explaining the bahviour of the widget on focus * add variable names to comment * chore: update e2e tests * add comment explaining test puropose
Haroenv
added a commit
that referenced
this pull request
Nov 28, 2019
## [4.0.1](v4.0.0...v4.0.1) (2019-11-28) ### Bug Fixes * widget name in documentation link for index ([#4172](#4172)) ([fe7e588](fe7e588)) * **helper:** rely on stable version of algoliasearch-helper ([#4200](#4200)) ([ff11731](ff11731)) * **infiniteHits:** correct widget options types ([#4222](#4222)) ([bb1b327](bb1b327)) * **queryHook:** restore behaviour of queryHook ([#4202](#4202)) ([7bf96cb](7bf96cb)), closes [/github.com/algolia/instantsearch.js/commit/c073a9acb51fff3c15278fcd563e47fec55c8365#diff-530222e0c4597f2110dc6ba173a306b0L98](https://github.com//github.com/algolia/instantsearch.js/commit/c073a9acb51fff3c15278fcd563e47fec55c8365/issues/diff-530222e0c4597f2110dc6ba173a306b0L98) ### Features * **transformers:** add tests ([#4153](#4153)) ([5a28415](5a28415))
Haroenv
pushed a commit
that referenced
this pull request
Nov 23, 2022
* fix(setSearchBoxValue): reset searchbox before editing In React and Preact the CLEAR selenium instruction does not fire any event, and therefore does not update the component's internal state (see this job for example: https://app.saucelabs.com/tests/4518f96fdfd447e998786d0ff463658a#54) It is equivalent to `input.value = ''`. There are multiple sources reporting a similar behaviour (see SeleniumHQ/selenium#6741) Since this change: #4202 ```jsx <input value={state.query} /> ``` instead of ```jsx <input value={props.query} /> ``` Since binding the input value to a `state.query` (rather than a `props.query`), it appears the click we use to focus on the searchbox re-syncs `input.value` with `state.query`.
Haroenv
pushed a commit
that referenced
this pull request
Nov 24, 2022
…ntsearch-e2e-tests#15) * fix(setSearchBoxValue): reset searchbox before editing In React and Preact the CLEAR selenium instruction does not fire any event, and therefore does not update the component's internal state (see this job for example: https://app.saucelabs.com/tests/4518f96fdfd447e998786d0ff463658a#54) It is equivalent to `input.value = ''`. There are multiple sources reporting a similar behaviour (see SeleniumHQ/selenium#6741) Since this change: #4202 ```jsx <input value={state.query} /> ``` instead of ```jsx <input value={props.query} /> ``` Since binding the input value to a `state.query` (rather than a `props.query`), it appears the click we use to focus on the searchbox re-syncs `input.value` with `state.query`.
Haroenv
pushed a commit
that referenced
this pull request
Nov 24, 2022
…ntsearch-e2e-tests#15) * fix(setSearchBoxValue): reset searchbox before editing In React and Preact the CLEAR selenium instruction does not fire any event, and therefore does not update the component's internal state (see this job for example: https://app.saucelabs.com/tests/4518f96fdfd447e998786d0ff463658a#54) It is equivalent to `input.value = ''`. There are multiple sources reporting a similar behaviour (see SeleniumHQ/selenium#6741) Since this change: #4202 ```jsx <input value={state.query} /> ``` instead of ```jsx <input value={props.query} /> ``` Since binding the input value to a `state.query` (rather than a `props.query`), it appears the click we use to focus on the searchbox re-syncs `input.value` with `state.query`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
There has been many complaints from user on our debouncing guide no longer working as expected.
This issue was introduced when the SearchBox component was converted into a Preact, and the following condition removed:
c073a9a#diff-530222e0c4597f2110dc6ba173a306b0L98
Before SearchBox refactor:
After SearchBox refactor:
This PR restores the logic as found before the refactor.
Result
fixes #4141
How to test this:
yarn startto start the storybookFast 3G.Before
After
