feat(menu): implement search functionality #57
Conversation
alexlafroscia
force-pushed
the
menu-search
branch
from
449d3ed to
2f505ca
Compare
|
Thanks for this @alexlafroscia. 👍 on the inclusion of If it hasn't been reviewed by then, I'll give this PR a proper review later today |
|
This is great, thanks! |
| async function type(selector, value) { | ||
| let finalKeyEventProcessing; | ||
|
|
||
| for (const char of value) { | ||
| finalKeyEventProcessing = triggerKeyEvent( | ||
| selector, | ||
| 'keydown', | ||
| char.toUpperCase() | ||
| ); | ||
| } | ||
|
|
||
| await finalKeyEventProcessing; | ||
| } | ||
|
|
There was a problem hiding this comment.
Just wanted to throw an explanation in here, since this is a little weird:
We need a custom way to trigger a sequence of key events because Ember's fillIn and typeIn helpers reject sending input to an element that isn't an input or contenteditable.
We only want to await the final key event, otherwise we will wait the 350ms after each event for the search term to be cleared, which is not what we want.
I tried to implement the Ember Concurrency task using rawTimeout (which the tests will not wait for) but then ran into a problem where it's quite for the test to wait for the search term to clear when we do want it to, since in some tests we search for multiple items in a row.
TL;DR: I tried to clean this up but the other approaches I could think of didn't actually work in practice!
💯 has my support too |
This implements the currently-missing ability to activate menu items by pressing the first (or first few) characters of the menu item's text.
Pros:
Cons:
timeout(350)that clears the search term. This could be resolved by using a different timeout length in a test build, but I don't like to introduce differences in behavior between tests and the "real" implementation unless it's really necessary, and it just doesn't feel that necessary to me in this case.ember-concurrency, and I know that @achambers is (rightfully) hesitant to add additional dependencies. I believe this one is justifiable for a handful of reasons, but am happy to re-build this without it if these reasons aren't enough:ember-concurrencydue to how common (and useful) it isember-concurrencygoing un-maintained in the future due to it's prevalence in the communityember-concurrencyis great at! If you look at the logic for the search functionality in the "official" implementations, it is way more complicated than the task-based implementation in this PR.