feat(filtering): filter by query

[QZera]

Closes 5288652

Product-y Notes

For @gsaslis and @maninak

Dimming the rest of the UI while the filter input is focused

Tried this, got the best results with opacity around 80% but even then it was a bit hit and miss between dark and light themes. Might be able to make it work with more tinkering but elected not to spend any more time on it for the time being.

Ctrl/Cmd+F as the shortcut for the input

I didn't really like the idea of hijacking an existing browser shortcut. After some research I found /to be a pretty common shortcut for search so I went with that.

Input width

Given that we want filtering qualifiers in the future (e.g. creator:zac), queries might get long. I'm thinking we might want to have the input span the remaining available header width. Thoughts?

Hiding the keyboard shortcut advertisement during input focus

I'm on the fence about this. If you feel we should keep it visible while the input is focused, just let me know and I'll change it.

Tech-y/Code-y Notes

For @maninak

To dos

I've left a few Todos which I want to do (get it?), either before this gets merged or after in another PR depending on time. Either way I didn't want them to hold up the review.

[Accessibility] Using the output element and <input type="search" />

output element

I couldn't really find any information or cases of it being used to indicate search results. So instead, I set aria-live="polite" in each columns' ul element.

<input type="search" />

This is implemented differently across browsers which would compromise our UX. In addition, I couldn't find information indicating that type="search" has any accessibility impact.

Instead, I used type="text" with an aria-label and wrapped it in a form with role="search" as suggested by MDN

[netlify]

✅ Deploy Preview for chipper-wisp-c7553e ready!

Name	Link
🔨 Latest commit	c12b82b
🔍 Latest deploy log	https://app.netlify.com/sites/chipper-wisp-c7553e/deploys/668390684c9c3300081fab4a
😎 Deploy Preview	https://deploy-preview-80--chipper-wisp-c7553e.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[maninak]

Well done, this already feels great to use and have in the app. 👏
Also props for all the attention to detail. I was especially positively surprised to see that filtering for the full id of a patch will still match it even though only the first characters of it are shown on the card and more amazingly filtering for the latter chars of an id, the ones not shown will still match!! 💯

I'll try to review the code perhaps tomorrow, but until then a few UI/UX points:

1. submitting the form, e.g. with the enter key, should have its submit event prevented so as to not reload the page (@submit.prevent). It should instead remove the focus from the input and move it to the first meaningful result of the "form submission" for tab indexing to flow smoothly.
2. filtering should search across all tasks we have in mem, even the ones hidden (e.g. because they are older or otherwise). In general, the same URL should always (unless impossible or hugely impractical) result in the same resulting view given the same data powering the UI.
3. every input field, of any type should have a label to go with it. Always. Affects a11y in multiple important ways.
4. input font-size should be same as other controls (another case of issues when escaping the UI lib and going for bespoke custom components)
5. the input shouldn't be using both border and outline which results in showing both sometimes. Also, consider tailwind's ring class.
6. input outline on focus should be thinner, same as when task kind dropdown is being click&held or tabbed into (again, see previous comment about custom cmp trade-offs)
7. the tooltip for X to clear the filterbox seems odd mentioning "filters", because reading it I'm thinking if this will affecto more stuff than just the input. It would prob be better as either singular "Clear filter" or "Clear input" or just "Clear".
8. when focused the X inside the input should be as far away from the right edge as the filter icon is from its left edge
9. the keyboard shortcut indicator shouldn't be selectable e.g. when pressing ctrl+a (user-select: none)
10. I think when "all" kind is used the input's placeholder should not use the word "tasks" just "filter", since :tasks" is not a term we have defined with the user.
11. if/when we have "kind:issue" etc modifiers we can consider removing the task kind selector altogether
12. hovering over the kbd shorcut indicator should have a tooltip explaining what it is/does/means e.g. "Press XYZ for ABC"
13. input should flex its width trying to take more horizontal space if value is too big for the field to show without scrolling
14. pressing esc should unfocus (blur) the input (counteracts the press-key-to-focus functionality)
15. I think the the aria-live="polite" isn't related to using <output>. I think for this one aria-live="assertive" would be better, but TBH id' just not use it at all unless I test it against an actual screen reader. Beyond that, the output elem isn't very common but it does offer meta info connecting the form with what is shown below it, so we should replace the generic div containing the columns with that element regardless. It doesn't hurt and only adds.
16. the input type affects many things on a meta-data level and since that's what we're using it for, again like above, we should use type="search". Just as an example the icon on some mobile keyboards changes depending on the input type.
17. using / for focusing the filterbox is very aggressive for an app where we're highly likely to have mutations and other inputs later where the user may be typing this character and would instead have the focus jump to the filterbox. That's exactly why in my specs I said it should only apply for this route and (should be removed when the route changes away from it, e.g. when seeing app options or task details). Also overloading native functionality like Ctrl + F is fine as long as we're providing a superset of the native functionality, which is the case for us here. In that sense when a user presses this kbd shortcut and types a term they will still achieve the their intended goals only with overhauled UX.
18. the URL query param would be better as filter vs the current generic query, e.g. ?filter=e2e
19. issue on narrower viewports
    

Hiding the kbd shortcut indicator for focusing the filterbox when it's already focused makes sense, since it no longer applies (in fact pressing that button will not do what it advertises any more).

For any follow-up discussions on individual topics I would suggest we move them on zulip if we can't have threaded comments here on GitHub (it's always confusing me when those are available and when not).

[maninak]

The more I play around with it the more I feel like fuzzy search is needed, but at the same time I feel like there needs to be "strict" matching mode resulting in the current behaviour.

For example (in this current demo seeded with radicle-explorer tasks) as a user I want to find all tasks that relate to the terms "test" and "e2e". If I type "e2e test" I get many results, but for "test e2e" I get nothing. Fuzzy would solve that (buy definition ignoring word order), but fuzzy does much more than that, e.g. accounting for misspellings.

Here I'd propose the following algo that is also optimized for speed. I'd advise that search function is memoized (ideally after splitting and sorting the terms).

filter algo:

1. take input and split by space
2. sort terms by longest first and those with same lenght also lexicographically (A before Z) using String.localeCompare() to account for special chars in non-english languages.
3. find tasks matching the first term
4. from the results of the previous filtering further refine those that match the second term
5. repeat step (4) until filtering for all terms has been applied

Ideally (as part of this PR or a future one) we should not split terms inside double quotes so test e2e "patch button" would become ['patch button', 'test', 'e2e'] after completion of step (2) and also memoize the filtering function meaning steps (3) to (5) (inclusive range).

[QZera]

In response to #80 (review)

• 1. Done, good catch
• 2. I think we discussed not to do that in our catchup, let's discuss further on Zulip
• 3. That's why I've used the aria-label for this input, as demonstrated in this example from MDN
• 4. Updated the font-size
• 5-6. Used the ring-2 class as suggested
• 7. Went with "Clear"
• 8. Done
• 9. Done
• 10. Done
• 11. Nothing to be done for now
• 12. Done
• 13. Agreed. Will have a look at this on the "second review round"
• 14. Done
• 15. Done (also removed the aria-live I had added to the columns)
• 16. This requires more effort than it seems, will have a look at this on the "second review round"
• 17. Done. However, I'm still not a fan of this
• 18. Done
• 19. Tried to do a quick fix by adding overflow scroll, but had issues with the kind select overflowing vertically, will have a look at this on the "second review round"

[maninak]

In the interest of time before our deadline, as agreed, I'm deferring the code review for later.

I also consider feedback items (3), (16) and (11) resolved too (though I'm curious to eventually know why this approach was preferred).

As far as I can tell with a quick look everything else was addressed.

From the UX perspective this is a joy to use, well done! 👏