Skip to content

Add Any and No value filter#260

Merged
JeroenDeDauw merged 7 commits intomasterfrom
any-value
Apr 3, 2025
Merged

Add Any and No value filter#260
JeroenDeDauw merged 7 commits intomasterfrom
any-value

Conversation

@malberts
Copy link
Contributor

@malberts malberts commented Mar 31, 2025

For #119
For #117

Screencast_20250331_140434.webm

TODO:

  • Add JS tests
  • Hide And/Or toggle when Any/No checkbox is selected

@malberts malberts changed the title Handle Any and No values Add Any and No value filter Mar 31, 2025
@codecov-commenter
Copy link

codecov-commenter commented Mar 31, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.48%. Comparing base (5df4c2c) to head (dd2b237).

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #260      +/-   ##
============================================
+ Coverage     72.85%   73.48%   +0.63%     
- Complexity      388      398      +10     
============================================
  Files            47       47              
  Lines          1260     1290      +30     
============================================
+ Hits            918      948      +30     
  Misses          342      342              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@malberts malberts changed the title Add Any and No value filter WIP: Add Any and No value filter Mar 31, 2025
@alistair3149
Copy link
Member

I have updated the implementation to use a dropdown menu instead, and merge the AND/OR toggle into the dropdown:

Recording.2025-04-02.151712.mp4

@alistair3149 alistair3149 marked this pull request as ready for review April 2, 2025 19:17
@alistair3149 alistair3149 changed the title WIP: Add Any and No value filter Add Any and No value filter Apr 2, 2025
@JeroenDeDauw
Copy link
Member

Does the current behavior of hiding list facets when there are no values to list always make sense?

What if I select no value for "field of work" and then select no value for "occupation", and there are no items without "occupation" that have a value for "field of work"? Would the "field of work" facet get hidden? If so, that seems a particularly bad outcome since it is still applying a filter.

We could show the list facet always if it has a filter. If we are not already doing that.

But that might not go far enough. We are always showing the range facets, so dropping the list facets seems inconsistent. If there are no values to list, what about showing no matching items instead of the list?

@alistair3149
Copy link
Member

Does the current behavior of hiding list facets when there are no values to list always make sense?

What if I select no value for "field of work" and then select no value for "occupation", and there are no items without "occupation" that have a value for "field of work"? Would the "field of work" facet get hidden? If so, that seems a particularly bad outcome since it is still applying a filter.

We could show the list facet always if it has a filter. If we are not already doing that.

But that might not go far enough. We are always showing the range facets, so dropping the list facets seems inconsistent. If there are no values to list, what about showing no matching items instead of the list?

We should always show facets that has an active filter. If we hide the facet, then there are no good way to dismiss the filter other than modifying the query string (which is not feasible).

There are multiple things that we can do to improve the experience:

  • Sort the facets with active filters above the rest, and use the config as a fallback
  • Add a "clear all" or "dismiss" button for each facet with an active filter
  • Instead of hiding the whole facet when there are no value count, we can add an empty state stating that there are no available/matching items like you suggested

@malberts
Copy link
Contributor Author

malberts commented Apr 3, 2025

Does the current behavior of hiding list facets when there are no values to list always make sense?

The current idea was to hide a facet with no values when nothing was selected in that specific facet. And if values were selected in the facet, but ended up with no results due to switching from OR to AND, at least show the facet and the selected values with 0 count (TODO as per #180 and #249).

We are always showing the range facets

This is to some extent unintentional. Hiding this would only work if we check that all the matching results do not have that date field. Since we didn't do much with "no value" before (besides making the query work), the UI wasn't catering for it. With the list facet it was different, since we already got the allowed values from Elasticsearch, and therefore knew when nothing existed.

Additionally, with a list facet you are not supposed to select something that can lead to no results. With a range facet you can enter a range that might lead to no results. We cannot really prevent that, unless we query the min/max for that field and add validation.

In other words: range facets can also be hidden, but requires additional ES queries and logic.

what about showing no matching items instead of the list?

The current behavior is similar to e-commerce facets, and zbMath, where empty facets are not shown. But specifically: only if nothing was selected. In the example of selecting no value in both facets leading to no results, the TODO behavior should be to show both.

IMO adding the message would be a non-standard solution to the current problem of hiding facets inconsistently. The appeal of hiding facets is to narrow the UI and show the user exactly what can be filtered on. So if that's not entirely our intention, and we instead want to repurpose the facets to also act as "data quality indicators" (by explicitly saying when there is nothing), then a message might be necessary.

@JeroenDeDauw JeroenDeDauw merged commit 6827582 into master Apr 3, 2025
14 checks passed
@JeroenDeDauw JeroenDeDauw deleted the any-value branch April 3, 2025 12:55
@JeroenDeDauw
Copy link
Member

TODO tracked by #261

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants