Change availability filter to a toggle #44182
Conversation
| availabilityFilter.mode === 'requestable' || | ||
| availabilityFilter.mode === 'all' |
There was a problem hiding this comment.
It would be nice if these were enums
There was a problem hiding this comment.
(it's a union at least)
But it seems that we should get rid of the now unused none option.
There was a problem hiding this comment.
we'll have to keep none around for a little bit to ensure users who had it selected will still work correctly
| @@ -140,7 +140,7 @@ export function FilterPanel({ | |||
| kindsFromParams={kinds || []} | |||
| /> | |||
| {ClusterDropdown} | |||
| {availabilityFilter && ( | |||
| {availabilityFilter && availabilityFilter.canRequestAll && ( | |||
There was a problem hiding this comment.
What if canRequestAll is false? Previously we could choose between accessible and requestable.
I think we should still allow this.
There was a problem hiding this comment.
Ok sure. I originally programmed this as "if the feature is off, just dont show it at all", but i'll udpate to still show it and use requestable
| return ( | ||
| availabilityFilter.mode === option || availabilityFilter.mode === 'all' | ||
| ); | ||
| onChange('all'); |
There was a problem hiding this comment.
Hmm, shouldn't we set all or accessible depending on canRequestAll?
| availabilityFilter.mode === 'requestable' || | ||
| availabilityFilter.mode === 'all' | ||
| } | ||
| onToggle={handleToggle} |
There was a problem hiding this comment.
The popover closes as soon as the user clicks the toggle, this is unexpected.
I think clicking on the toggle should update the unified view, but not close the popover.
There was a problem hiding this comment.
The popup doesn't close for me. Am i using it weird?
5a2b8062aaa9984db71529f6dcb5150a.mp4
There was a problem hiding this comment.
I tested it in Connect:
https://github.com/user-attachments/assets/a82b2cb8-7d69-408c-98c2-39a1d2a898ff
This happens because when params change, we remount the component so it can re-fetch the list.
I don't have a good idea how to fix it, if you don't have either, we can keep it as is :(
There was a problem hiding this comment.
I can't think of anything quick. We'll keep as is and see if anyone complains
| availabilityFilter.mode === 'requestable' || | ||
| availabilityFilter.mode === 'all' |
There was a problem hiding this comment.
(it's a union at least)
But it seems that we should get rid of the now unused none option.
This PR will remove the radio/checkbox options for the availability filter and instead use a toggle to "show requestable resources or not" The default option for new users is still "toggled on" to show requestable resources.
avatus
force-pushed
the
avatus/fix_filter
branch
from
b5972ab to
8c5e740
Compare
| <ChevronDown ml={2} size="small" color="text.slightlyMuted" /> | ||
| {availabilityFilter.canRequestAll === true && | ||
| availabilityFilter.mode !== 'none' && <FiltersExistIndicator />} | ||
| {availabilityFilter.mode === 'accessible' && ( |
There was a problem hiding this comment.
I think it doesn't work correctly for show_resources: 'accessible'. It shows the indicator when the toggle is not checked (so it's in a default state):
There was a problem hiding this comment.
The default state is "on" so the indicator should show when accessible is the value
| availabilityFilter.mode === 'requestable' || | ||
| availabilityFilter.mode === 'all' | ||
| } | ||
| onToggle={handleToggle} |
There was a problem hiding this comment.
I tested it in Connect:
https://github.com/user-attachments/assets/a82b2cb8-7d69-408c-98c2-39a1d2a898ff
This happens because when params change, we remount the component so it can re-fetch the list.
I don't have a good idea how to fix it, if you don't have either, we can keep it as is :(
This PR will remove the radio/checkbox options for the availability filter and instead use a toggle to "show requestable resources or not"
This takes into account if users have the
requestableoption toggled in their user prefs still so everything should work for them in the new versionThe default option for new users is still to show requestable resources, aka "toggled on".
changelog: the availability filter is now a toggle to show (or hide) requestable resources