Allow to remove undefined filter

[danrot]

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Fixed tickets	---
Related issues/PRs	introduced in #5035
License	MIT
Documentation PR	---

What's in this PR?

This PR also saves the user settings, when only undefined values are removed. But the list should still not reload.

Why?

Because otherwise the list will set the stored user setting as default again, when the last item is undefined and removed.

[niklasnatter]

Found a new strange bug while testing this. When adding a text filter, confirming some value, deleting the value and confirming again, the displayed valueNode is not updated. Not sure if this is caused by this PR. Removing undefined filters works find now at least 🙂

Something similar happens when assigning some values to the types dropdown of the media list, confirming, unselecting all options and confirming again.

[niklasnatter]

Also found another bug with the types dropdown of the media list. When adding a types filter, selecting all options, confirming, changing the selected options and confirming again, the list contains a lot of duplicated entires. It looks like the old list items are not resetted.. 🙈

[danrot]

Can confirm the first bug you've described, but it was already like that before this PR, therefore I am going to fix that separately. The second bug you have described I can't reproduce... Tried the following steps:

1. Go to the Media overview
2. Add the types filter
3. Select Audio, Document, Image and Video
4. Confirm (The select button shows "All selected" and the chip lists these 4 options
5. Click on the chip, remove Video again and confirm (The chip shows just 3 values, and there are still only 4 values in the dropdown)

[danrot]

And changed my mind about the first issue, it's so less code, that I am going to include it in this PR 🙈

[niklasnatter]

Sorry, that wasnt a very good explanation of the second bug. Here is another try:

1. Go to the Media overview
2. Ensure that there is at least one media displayed
3. Add the types filter
4. Select Audio, Document, Image and Video and confirm
5. Unselect Video and confirm
6. Select Video again and confirm
7. Have a look at the list and discover that the some medias are displayed multiple times
8. Have a look at the javascript console and see a lot of Encountered two children with the same key warnings

[niklasnatter]

Do you remember why you implemented this previously? I suspect there is a similar problem in the DropdownFieldFilterType and maybe in the DateTimeFieldFilterType

[danrot]

I think that is just a leftover from before I was using a Promise here... I'll check the other types as well.

[danrot]

Fixed all the other FieldTypes as well.

And I still can't reproduce your second issue. I don't get multiple media by following your instructions, no matter if I confirm between unselecting and reselecting Video or doing it in one step.

[niklasnatter]

I am sorry, but the DropdownFieldFilterType still behaves strange when deselecting all options:

[niklasnatter]

About the second issue. I cant rule out that this is a local problem, but i really doubt it: https://imgur.com/xXnbBCz

[chirimoya]

@danrot @nnatter looking at the screenshots I'm asking myself if the multi-select really makes sense at this point? Maybe only Single-Selection as "DropDown", and the Multi-Selection is always "open"?

[danrot]

@nnatter I think the DropdownFieldFilterType should return undefined if nothing is selected, then it should work... Gonna fix that.

About the second issue, all I can tell, is that this is definitely not happening to me:

@chirimoya A single selection does still not make a lot of sense to me... Can you think any use case, in which you want the user to completely forbid to filter for two options? That doesn't make any sense to me, stuff like types, status and so on should be easily filterable by two options as well IMO.

Using something else for the MultiSelection might make sense, could e.g. imagine a list of checkboxes. However, this could get quite long in e.g. the case of countries... But on the other hand it probably does not matter a lot if you scroll in the dropdown or in the ArrowMenu 🤔

[chirimoya]

@danrot list of checkboxes +1

[danrot]

@chirimoya The only thing that might be a little bit cumbersome is the positioning of the OK button... But let's discuss that somewhere else, this PR is actually about something else 🙂

[niklasnatter]

Looks like there are 3 open topics now, so i will group my answers by topic:

1. Deselecting all options of the dropdown filter works fine now - will merge this PR 🙂

2. I deleted all package-lock.json files and node_modules directories in my project to get a fresh clean build. Guess what? The media type bug is gone now - so this was a local problem 🙈 I am sorry for the inconveniences!

3. As @danrot already mentioned, I am not sure if there is use casese for a single select. But from an UX perspective, I think it would be better to display the select option directly in the popover of the filter-chip instead of forcing the user to open a second popover. Unfortunately, we cannot use an existing component for implementing this as far as i know. But it shouldnt be that hard either.

I would expect the popover of the filter-chip to grow till it reaches a certain maximum height. After that point, U think it would be okay to just make the whole popover scrollable. Only making the options scrollable and always displaying the ok button at the bottom could be confusing because the user might not see that there are other options.