-
Notifications
You must be signed in to change notification settings - Fork 332
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
feat(recent-searches): support search and templating #353
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 4ecb9ee:
|
packages/autocomplete-plugin-recent-searches/src/createStore.ts
Outdated
Show resolved
Hide resolved
packages/autocomplete-plugin-recent-searches/src/usecases/localStorage/createLocalStorage.ts
Outdated
Show resolved
Hide resolved
...mplete-plugin-recent-searches/src/usecases/localStorage/createSearchableLocalStorageStore.ts
Outdated
Show resolved
Hide resolved
After syncing with @Haroenv we decided on a few changes that'll make the API more flexible—see 1d7e6bf. Exporting
|
packages/autocomplete-plugin-recent-searches/src/createRecentSearchesPlugin.ts
Show resolved
Hide resolved
* | ||
* @default 5 | ||
*/ | ||
limit?: number; |
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.
the other api is using hitsPerPage, but this doesn't have pagination per se. Could it make sense to have hitsPerPage
already even if pagination isn't yet implemented?
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.
Can you elaborate?
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.
I saw a bit lower in this file hitsPerPage being around searchParameters, but here we are using limit for the argument. If we call this hitsPerPage
, the API is one word smaller I think
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.
hitsPerPage
is a concept in Algolia, and the Recent Searches plugin is not tied to Algolia, so I'm not sure about that.
This makes me think that the data
properties that we have should probably contain the word Algolia
. I'll change that.
packages/autocomplete-plugin-recent-searches/src/usecases/localStorage/search.ts
Show resolved
Hide resolved
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.
some great improvements here! let's make sure this is marked as a breaking change in the alpha changelog
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.
The changes look good to me 👍
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.
good for this pr, you can make the algolia changes in a subsequent one
This PR is a new architecture of our Plugins API and of the Recent Searches Plugin that allows search and templating in the Recent Searches plugins. It also unlocks different Autocomplete use cases (same-page experience and redirect-page experience), as well as hooking other storages to Recent Searches.
Features
Storage abstraction
The storage is now abstracted and the Local Storage becomes one implementation that we support.
In the long run, we want to supported other storages to be able to sync Recent Searches on different devices (desktop, laptop, mobiles, etc.).
There's no conditional in the plugin itself, because the storage drives which items are returned. We therefore share a single plugin implementation for all plugins, only the storage differs.
Searching in Recent Searches
This new storage abstraction has allowed to export two distinct plugins:
createLocalStorageRecentSearchesPlugin
: the plugin that we've known so far. It displays Recent Searches on empty query, and nothing when there's a query.createSearchableLocalStorageRecentSearchesPlugin
: the plugin that also display Recent Searches when there's a query, and searches in them. The search is currently only astartsWith
check and doesn't support highlighting.Again, the only difference between this two plugins are the storage implementation.
Shared limit between Recent Searches and Query Suggestions
The way results are shared between Recent Searches and Query Suggestions got more clever. They're now supporting "grouping limit". I created a Data Plugin API called
getQuerySuggestionsHitsPerPage
that allows to retrieve a number of "RS + QS"hitsPerPage
value.This feature avoid jumping UI and provides a seamless RS + QS integration:
Templating
Since the logic about conditional displaying in now only on the storage, we can share templating options to all kinds of Recent Searches plugin.
We now support a
getTemplates
API that allows to override the template displayed by the plugin. ThisgetTemplates
API is used internally to provide the default template. The proponRemove
is passed so that our logic remains internal and doesn't leak (reminder: internally, it callsstore.remove
andrefresh
).Here's a preview using Preact:
This new API covers the second Recent Searches experience we wanted to unlock: redirect-page results. You can now provide a link to wrap your item in.
Future API changes
subscribed.onSelect
should not be called when the current source is selected