New format filter #12593
New format filter #12593
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the filterOutExhibitions parameter with a negated format filter (!exhibitions) to exclude exhibitions from event listings. The implementation introduces a utility function to identify negated filter values and ensures they are hidden from the UI while still being applied to API requests.
- Introduces
isNegatedValue()utility to detect negated filter values (prefixed with '!') - Updates all filter UI components to exclude negated values from display and active filter counts
- Modifies event pages to use
format: ['!exhibitions']instead offilterOutExhibitions: 'true'
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| content/webapp/utils/filters.ts | New utility function to check if a filter value is negated |
| content/webapp/services/wellcome/common/filters.ts | Filters out negated values from filter options before display |
| content/webapp/views/components/SearchFilters/index.tsx | Excludes negated values from active filter count for checkboxes |
| content/webapp/views/components/SearchFilters/ResetActiveFilters.tsx | Excludes negated values from the reset active filters UI |
| content/webapp/utils/search.ts | Excludes negated values from active filter labels for accessibility |
| content/webapp/pages/events/index.tsx | Replaces filterOutExhibitions with format filter !exhibitions for future events |
| content/webapp/pages/events/past.tsx | Replaces filterOutExhibitions with format filter !exhibitions for past events |
| content/webapp/pages/search/events.tsx | Replaces filterOutExhibitions with format filter !exhibitions for event search |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rcantin-w
left a comment
rcantin-w
left a comment
There was a problem hiding this comment.
It works well!
I only wonder if there'd be a way to filter out the negated value at the top of the chain so we don't have to take them into consideration in all the other files, and potentially eventually add them to other types of filters.
|
|
||
| const allPossibleParams = { ...params, ...setParams }; | ||
| const queriedParams = { ...restOfQuery, ...setParams }; | ||
| const allPossibleParams = { ...params, timespan, format }; |
There was a problem hiding this comment.
does this not need the same logic as queriedParams?
| return allOptions.sort(optionOrder).filter(option => !isNegatedValue(option.value)); | ||
| } | ||
|
|
||
| return allOptions; | ||
| return allOptions.filter(option => !isNegatedValue(option.value)); |
There was a problem hiding this comment.
np; could this be redefined once above and then reused so if we change the logic we don't have to change it in both places?
What does this change?
For wellcomecollection/content-api#331
Relies on wellcomecollection/content-api#338
uses the format parameter with negated values to filter out exhibitions rather than the bespoke filterOutExhibitions param
How to test
They should show they same number of results
How can we measure success?
The filters work, which will allow us to remove the filterOutExhibitions param from the content api
Have we considered potential risks?
None really