Add basic search filters (experimental) #1080
Conversation
github-actions
bot
added
the
status:conflicts
This comment was marked as resolved.
This comment was marked as resolved.
owi92
added
area:search
github-actions
bot
removed
the
status:conflicts
There was a problem hiding this comment.
Thanks for adding filters!
I only tested this, without having looked at the code much. I have a few design suggestions below. But unfortunately, one big thing still needs to change: the filtering needs to happen in the backend, not the frontend.
Filtering in the frontend means that the "hits" count is wrong. But more importantly, it can break in other ways. The backend currently returns a fixed number of results. If you filter by date, and it happens that none of the returned results lie in the date range, but some events (that were not returned by the endpoint) would be in the date range, then that's bad. It also doesn't work well with pagination/infinite scrolling.
So yes, the API needs to be extended to allow specifying the filter parameter. And then the backend need to pass the filter settings to Meili so that Meili performs the actual filter. Filtering the kind of item (video, page, ...) can be done in our backend because we send one Meili request per kind. So we can just not send the ones that are not requested. That surely requires a small bit of refactoring but should work.
Now onto my other suggestions:
- I would probably suggest adding all filter settings as URL parameters. Otherwise if you would click an item, and then navigate back, all your filter settings are gone.
- I would separate the different kinds of filters into different sections, so that the date filter is not in the same line as the kind filters.
- I would add a heading (at least visually, don't know if it needs to be
<hX>to the whole filter section ("Filter") and also to each subsection. See the sketch below. - On narrow screens, when the filter is above the results, we probably need to make it collapsible, especially as we add more filter options. But doesn't necessarily need to happen now.
- I find the "kind" filter buttons quite large. They are our normal buttons, but I would rather go with a smaller "label-ly" style. Also see sketch.
- For UX it would be nice to have a few quick buttons for date ranges ("last week", ...).
- The button to select the time frame could use a tooltip. Or... thinking about it, in my design it would even fit well to show the actual date range without hiding it in a button. Then you can see at a glance what the current filter is.
So yeah here is some rough sketch. It would need more polish for sure, but sth like that I had in mind. Let me know what you think, this is just a suggestion, so we can arrive at a good result together.
|
Thanks, Ole. Some unsorted comments from the user perspective:
|
|
PS: "Pages" are also difficult to combine with future "source filters" (metadata/slides/transcripts). |
|
Thank you for your feedback. Both of you make valid points, so I will crawl back into my cave and come up with something nicer to accomodate your suggestions. Regarding the relevance of searching pages, I definitely agree that it makes filtering unreasonably complicated. |
I also think this can be useful. Most of the time the page will list exactly one series which, in the future, means we will only show that series in the results. And I think we all agree that series should appear in the search results right?
That's technically very hard to do. At least in the general case. I just took a look at your old video portal. And that would indeed be possible: sort each of the results in one of a fixed number of buckets, and show how many are in each bucket. But with some filters we want to add, this model does not apply.
Mh that is a good point. I also checked your old video portal regarding that. And I agree that from a UX perspective this is good. But... this is not trivial to do, especially not in a way that works for everyone (semesters are different for every university after all). We will come up with an implementation plan internally.
So are "series" and as mentioned above, I think it's clear we want to be able to find those. So yes, with that difficulity we have to deal. |
Nope. If it makes things easier with respect to some other features mentioned here, I'm not that keen. |
Yes. I would appreciate series appearing in the search results and some day adding a filter for series (and maybe also playlists). |
owi92
changed the title
There was a problem hiding this comment.
I didn't have a close look on all frontend components, but all of this looks like a good approach. No rework necessary. I will have to do a closer frontend review later, but I left a few comments already that should be fixed.
For the bystanders Olaf and David: we decided to make this PR just about code architecture stuff. Merging it will not actually result in any user-visible change. Before we do that we will reconsider the search page design a bit, including filters. And show you ideas before implementing it. So for now you can probably ignore this pull request.
backend/src/api/model/search/mod.rs
Outdated
| // We can either use score details or adding dummy searchable fields to the | ||
| // realm index. See this discussion for more info: | ||
| // https://github.com/orgs/meilisearch/discussions/489#discussioncomment-6160361 | ||
| let events = event_results.hits.into_iter() | ||
| .map(|result| (NodeValue::from(result.result), result.ranking_score)); | ||
| let realms = realm_results.hits.into_iter() | ||
| .map(|result| (NodeValue::from(result.result), result.ranking_score)); | ||
| let mut merged = realms.chain(events).collect::<Vec<_>>(); | ||
| let mut merged: Vec<(NodeValue, Option<f64>)> = Vec::new(); | ||
| // Probably also not the best way of doing this. |
There was a problem hiding this comment.
Yeah so again, if the user specified only searching for events, we shouldn't send a realm search request to Meili and just ignore the results. I realize that this might be harder to implement though, but yeah it would be nice to just send the search requests that are necessary. And while you're refactoring that anyway, keep in mind that series and playlists might be added in the future, so maybe opt for a design that can be easily extended.
owi92
changed the title
This adds a slight delay before starting the search while typing.
This adds filters to show either videos or pages exclusively. When we add more item types in the future, we might want to consider to allow to combine filters, i.e. showing both series and videos but not pages, or any other combination.
While there are nice libraries for this, they are either quite large, unmaintained or don't work out of the box, so I built a simple component for this myself, though that is neither super fancy nor pretty.
For now, the filters are hidden behind the `experimentalFeatures` flag. They are to be extended and or redone in the future, but the aim of this commit is to lay the foundation for that.
Previously the numbers of total hits were bunched together regardless of the selected filter. Now it only shows the number of the filtered results.
This PR slightly changes the breadcrumb formatting, so that needed to get adjusted accordingly in the respective UI test.
This adds a first iteration of basic filters for item type ("all", "videos", "pages") and time frame.
There are some open questions regarding how to deal with pages while the time filter is active, since pages do not have a timestamp. This means they are not affected by the filter.
Lukas and I decided to split to reworking of the search into multiple PRs with incremental improvements and features added. This is now only laying the foundation for these work packages, and will not change anything visible for now.
Filters are included but hidden behind the
tobiraExperimentalFeaturesflag (this is mainly information for developers).Part of #1063