-
Notifications
You must be signed in to change notification settings - Fork 552
feat(autocomplete): support global onSelect and redirect to search page #6777
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit cf22472:
|
|
I'll update the API reference, and will add tests after #6772 is merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, couple of minor comments.
| const indexUiState = | ||
| instantSearchInstance.getUiState()[targetIndex!.getIndexId()]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a possibility this can cause an indexing error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
targetIndex is at least the mainIndex (the parent of the isolated index of an autocomplete), so it should not cause an error. It just is typed as potentially undefined because the renderState needs to be declared before we have access to instantSearchInstance.
| const isSearchPage = useMemo( | ||
| () => | ||
| typeof indexRenderState.hits !== 'undefined' || | ||
| typeof indexRenderState.infiniteHits !== 'undefined', | ||
| [indexRenderState] | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it not possible to also get this info from the targetIndex in react?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now this is defined in the JS widget, and retrieving the index instance in React is not as easy (useIndexContext() is not publicly exported for instance).
Could be worth having it returned by the connector in a future update though.
Summary
This PR adds two top-level parameters to autocomplete:
getSearchPageURLto provide an URL to navigate to when autocomplete is not part of the search pageonSelectto provide a globalonSelect, making the per-index one optional, and defaulted to a method that handles redirection to the search page if necessaryThe order is the following:
getURL()→ navigate to urlgetSearchPageURL()exists → navigate to urlgetQuery()FX-3538