Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

feat(voice): add additionalQueryParameters #2366

Merged
merged 7 commits into from
Nov 20, 2019
Merged

Conversation

marieglr
Copy link
Contributor

@marieglr marieglr commented Apr 26, 2019

Two new props:

  • additionalQueryParameters: () => Partial<SearchParameters> (only applied when voice search is mounted)
  • language: string (iso 639-1), the parameter sent to Algolia will be always the short version

New connector: connectVoiceSearch

known limitation: additional query parameters will stay applied as long as the Voice Search widget is mounted, meaning they can cause stale values if you switch input method without unmounting (limitation of the architecture of React InstantSearch)

The connector is written in JS instead of TS, because it's mostly copy-pasted from searchBox, and that would be needless extra work otherwise #hackathon

fixes #2874

@marieglr marieglr requested a review from a team April 26, 2019 09:57
@algobot
Copy link
Contributor

algobot commented Apr 26, 2019

Deploy preview for react-instantsearch ready!

Built with commit 4d8488d

https://deploy-preview-2366--react-instantsearch.netlify.com

@marieglr marieglr force-pushed the poc/voice-parameters branch from 4d8488d to 5d4904a Compare September 6, 2019 09:29
@marieglr marieglr changed the base branch from feat/voice-search to master September 6, 2019 09:33
@marieglr marieglr force-pushed the poc/voice-parameters branch from 5d4904a to a296742 Compare September 6, 2019 09:39
@marieglr marieglr changed the base branch from master to next September 6, 2019 09:45
@marieglr marieglr force-pushed the poc/voice-parameters branch from a296742 to e79488a Compare September 6, 2019 09:56
Two new props:
- `additionalQueryParameters: () => Partial<SearchParameters>` (only applied when voice search is mounted)
- `language: string` (iso 639-1), the parameter sent to Algolia will be always the short version

known limitation: additional query parameters will stay applied as long as the Voice Search widget is mounted, meaning they can cause stale values if you switch input method without unmounting (limitation of the architecture of React InstantSearch)

This does not have additional tests yet.

The connector is written in JS instead of TS, because it's mostly copy-pasted from searchBox, and that would be needless extra work otherwise #hackathon

Co-Authored-By: Haroenv <haroen@algolia.com>
@Haroenv Haroenv force-pushed the poc/voice-parameters branch from e79488a to 8983f7a Compare September 24, 2019 14:09
@Haroenv Haroenv requested a review from samouss September 24, 2019 16:11
@Haroenv Haroenv requested review from a team and removed request for samouss October 11, 2019 08:13
@ghost ghost requested review from francoischalifour and tkrugg and removed request for a team October 11, 2019 08:13
@tkrugg tkrugg requested review from yannickcr and removed request for tkrugg and francoischalifour October 28, 2019 09:56
@eunjae-lee eunjae-lee requested a review from Haroenv November 19, 2019 14:03
@eunjae-lee
Copy link
Contributor

@Haroenv could you review the recent commits?
ab3bd42 c31b590
Is this what you meant by #2366 (comment) ?

Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks ready to me now @eunjae-lee, just maybe tests for multi-index (as far as I can tell it will just work)

@eunjae-lee
Copy link
Contributor

looks ready to me now @eunjae-lee, just maybe tests for multi-index (as far as I can tell it will just work)

added some tests for multi index 349b03e

Copy link
Contributor

@yannickcr yannickcr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it also be possible to do the connector in TypeScript?

@Haroenv
Copy link
Contributor

Haroenv commented Nov 20, 2019

Would it also be possible to do the connector in TypeScript?

I wrote this in the original description, but since it's mostly the same as search box, and the patterns for connectors aren't yet clear in React InstantSearch, I didn't do this in this PR

@Haroenv Haroenv requested a review from yannickcr November 20, 2019 15:40
@Haroenv Haroenv merged commit 81832aa into next Nov 20, 2019
@Haroenv Haroenv deleted the poc/voice-parameters branch November 20, 2019 16:57
@Haroenv Haroenv restored the poc/voice-parameters branch November 20, 2019 16:57
Haroenv pushed a commit that referenced this pull request Nov 20, 2019
* feat(voice): add additionalQueryParameters

Two new props:
- `additionalQueryParameters: () => Partial<SearchParameters>` (only applied when voice search is mounted)
- `language: string` (iso 639-1), the parameter sent to Algolia will be always the short version

known limitation: additional query parameters will stay applied as long as the Voice Search widget is mounted, meaning they can cause stale values if you switch input method without unmounting (limitation of the architecture of React InstantSearch)

This does not have additional tests yet.

The connector is written in JS instead of TS, because it's mostly copy-pasted from searchBox, and that would be needless extra work otherwise #hackathon

Co-Authored-By: Haroenv <haroen@algolia.com>

* chore(size): update bundle size (this change is +270b)

* feat(voice): handle language and additionalQueryParameters separately

* feat(voice): add tests

* feat(voice): add tests for multi index

* feat(voice): add test for additionalVoiceParameters

* chore: remove unused argument
@Haroenv Haroenv deleted the poc/voice-parameters branch November 20, 2019 16:59
@Haroenv
Copy link
Contributor

Haroenv commented Nov 20, 2019

accidentally merged into next instead of master, but fixed in 3a45b2c

Haroenv added a commit that referenced this pull request Dec 17, 2019
# [6.1.0](v6.0.0...v6.1.0) (2019-12-17)

### Bug Fixes

* **connectNumericMenu:** support numeric refinement 0 ([#2882](#2882)) ([30bd9fd](30bd9fd))
* **deps:** update dependency next to v9.1.1 ([9d49d33](9d49d33))
* **helper:** rely on stable version of algoliasearch-helper ([#2871](#2871)) ([e3531a1](e3531a1))

### Features

* **insights:** show an error when 'clickAnalytics: true' is missing. ([#2877](#2877)) ([621656a](621656a))
* **voice:** add additionalQueryParameters ([#2366](#2366)) ([3a45b2c](3a45b2c))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants