Added switch based mapping for filters in searchbar #448
Conversation
arkid15r
requested review from
aramattamara,
harsh3dev,
abhayymishraa and
Rajgupta36
| case /^contributors([><]=?|=)(\d+)$/.test(filter): | ||
| return filter.replace(/^contributors([><]=?|=)(\d+)$/, 'idx_contributors_count$1$2') | ||
| case /^stars([><]=?|=)(\d+)$/.test(filter): | ||
| return filter.replace(/^stars([><]=?|=)(\d+)$/, 'idx_stars_count$1$2') | ||
| case /^forks([><]=?|=)(\d+)$/.test(filter): | ||
| return filter.replace(/^forks([><]=?|=)(\d+)$/, 'idx_forks_count$1$2') | ||
| case /^is:([a-zA-Z]+)$/.test(filter): | ||
| return filter.replace(/^is:([a-zA-Z]+)$/, 'idx_type:$1') | ||
| default: | ||
| return filter |
There was a problem hiding this comment.
Can we extend it to datetime fields (created/updated) too?
There was a problem hiding this comment.
@arkid15r We can do that, how the user will input the dates?
Is it like 1sep24 or 01092024 or ?
As the index contains the that flied in UNIX timestamp
There was a problem hiding this comment.
@ShashaankS we can always convert the dates into desirable format.
| export const getParamsForFilters = (indexName: string, filter: string) => { | ||
| filter = filter.trim() | ||
| switch (indexName) { | ||
| case 'projects': |
There was a problem hiding this comment.
What about contribute page (issues), will it be addressed in a separate PR?
There was a problem hiding this comment.
I have gone through the issues index, its most of the attributes are seachable,
only attribute we can use was comments<10 or is:open is:close
Even there is no attribute for open and close
There was a problem hiding this comment.
I think we can also add languages and topics based filtering in contribute page
There was a problem hiding this comment.
1.is: documentation is working but for this is:documentation no result same as in case of project. can we trim space and add one space automatically
2.also when I write dual query in search bar is: documentation, stars<10 or is: documentation stars<10 or is: documentation,star>10 the result is something like second param override first one can we add multiple query support together
3.suppose this is the query stars<10@ it throws error on page but i think it should return no project found something. look at filters
i think there are some regex error pls fix that
| let filters = '' | ||
| let searchQuery = query | ||
|
|
||
| const filterRegex = /(\w+)([:><=!]+)([^ ]+)/g |
There was a problem hiding this comment.
can we put this logic in another function
| @@ -17,17 +18,37 @@ export const fetchAlgoliaData = async <T>( | |||
| } | |||
| try { | |||
| const params = getParamsForIndexName(indexName) | |||
|
|
|||
| let filters = '' | |||
| let searchQuery = query | |||
There was a problem hiding this comment.
Could you please define the types for this? Otherwise, we may unintentionally mutate the variable from a string to another type, which could lead to unexpected behavior.
There was a problem hiding this comment.
@arkid15r @ShashaankS I think Raj has already suggested some changes.
I think there should be more parameters across pages to use.
Also please include tests for this PR.
I have a question: Currently there is no way users can know what are all the available options for filters. So should that be tackled in the new issue?
Thanks.
This was also my concern. We could do something like a sample of possible options bellow the search bar or a link with a modal to look those up. Anything like that should help. Without it it's super unclear how to use search this way if person never had experience with this type of search before. |
There was a problem hiding this comment.
@arkid15r @ShashaankS Should we proceed as is and add possible options later on?
I still need to review it on the frontend, just wanted to make sure it's not still work in progress.
Resolves #305
There is a provision for other indexes also but github like filters may be not meaningful.
there are some configs to bet set on algolia app:
