Rework Dropdown filtering #463
Conversation
🦋 Changeset detectedLatest commit: 1c20438 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
69cf44d to
05b58e5
Compare
| }\n\n// When overriding, return the options you want visible. The rest will be hidden. If you fetch on\n// filter, this is the place to do it.`, | ||
| }, | ||
| }, | ||
| }, |
9b3c34a to
eeb5019
Compare
| // eslint-disable-next-line no-empty | ||
| } catch {} | ||
|
|
||
| if (options) { |
There was a problem hiding this comment.
Wrapped everything below in this condition. No other changes.
eeb5019 to
1c20438
Compare
Same, yeah.
|
| }); | ||
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/require-await |
There was a problem hiding this comment.
nit: Rather than needing to eslint-disable, maybe you could have the function return an immediately-resolving Promise? I realize it's effectively the same behavior, but maybe it's slightly more readable? (and I'm eslint-disable-phobic)
There was a problem hiding this comment.
Haha. It wouldn't be self documenting, would it? I could see someone coming along and wondering why we're doing it, removing await Promise.resolve(...), then discovering the lint error.
There was a problem hiding this comment.
@danwenzel: I've got some other work that this depends on. So I'm going to merge. Feel free to chime in after I merge.
🚀 Description
@ynotdraw were chatting about the asynchronous filtering case, which we hadn't been forced to think through until now. We realized that Dropdown doesn't handle that case and has other issues.
Dropdown currently dispatches a "filter" event on "input". The idea is that consumers can listen for that event and then decide to render or not render certain options.
One problem with that approach is multiselect Dropdown has a tag for each selected option, and Dropdown relies on those selected options being in the DOM. If consumers remove options from the DOM based on the "filter" event, then the corresponding tags for those options will also be removed.
Making matter worse is that Dropdown also does its own filtering, which happens synchronously and doesn't take into account consumers wanting to filter a different way or on the server.
One solution, here, to both problems is to provide a
filter()method that consumers can override. It's not very web component-y. But it seems to be the only path forward—though I'm totally open to alternatives.filter()returns a promise that Dropdown awaits before showing and hiding options.📋 Checklist
🔬 How to Test
<input>.📸 Images/Videos of Functionality
N/A