Skip to content

Conversation

@bgentry
Copy link
Contributor

@bgentry bgentry commented May 4, 2025

This adds complex faceted search filters to the job list UI to filter by kind, ID, priority, or queue. The UI supports essentially all keyboard and mouse/touch input, including navigation between filter items.

Not every combination of search here will be efficient at the database level, but this isn't a typical runtime query—it's a debugging tool for operators. Some risk here should be acceptable, and can be mitigated by setting appropriate query timeouts on the pool given to the UI's River client.

autocomplete.mp4

Fixes #242.

@bgentry bgentry requested a review from brandur May 4, 2025 03:51
@brandur
Copy link
Collaborator

brandur commented May 5, 2025

Awesome man! Very cool stuff. Love how conditions get sent to the var bar so you can link to the page afterwards.

Not going to really try to review the code here, but I pulled it locally, and just a few comments/suggestions:

  1. Should the state nav bar on the left possibly also be integrated into the search bar as another filter? My worry would be that you put in a bunch of work applying custom filters, then realize that you're on the wrong state tab, so you have to click the right state tab and then you lose all your filters.
image
  1. When selecting a filter from the possibilities list, I found myself wanting to hit "tab" to complete it and move into it to select criteria (similar to how Vim's selection modals work, but also others), but if you do that, you tab right out of everything, and then need to use the mouse to select back in again. It turns out that you have to use "enter" instead of "tab" for this to work. I wonder if both buttons could do it? (And reserve deselect for "escape" or something?)
image
  1. Similarly, I find that once I'm entering criteria, it'd be nice to be able to hit "enter" to be able complete the criteria (especially since the point in (2) just trained me to use "enter" instead of "tab"), but this doesn't work as it deselects and loses focus. In an inversion of (2), you have to know to use "tab" in this case and not "enter". Could "enter" work as well in addition to "tab"?
image

Even if the answer to (2) and (3) is "no", IMO we should be consistent in either the use of "tab" or "enter" to carry about both actions, not one in one case and the other in the other.

@bgentry
Copy link
Contributor Author

bgentry commented May 6, 2025

  1. Yeah I can apply that behavior to preserve filters when navigating between states. Done ✅
  2. Tab currently jumps between filter elements, so I don't think I can switch it to navigate the autocomplete dropdown without breaking that. You can use up/down arrow keys to navigate the list of course.
  3. Pressing enter does finalize either an autocomplete choice or a given filter element. However there's a bug where after finalizing a single filter item Enter doesn't bump you to adding a new filter, which was how it's supposed to work—I've fixed that. ✅

Another aspect to highlight about this search input: it attempts to behave as a single input in most ways, such as being able to use left/right arrows to continuously move the cursor between filters. Ideally it should be easy to use only the keyboard or to avoid needing to switch between input modes repeatedly.

Copy link
Collaborator

@brandur brandur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks for taking a look.

@bgentry bgentry merged commit 71032b9 into master May 8, 2025
13 checks passed
@bgentry bgentry deleted the bg-filter-jobs branch May 8, 2025 00:39
bgentry added a commit that referenced this pull request May 8, 2025
Issue #347 details how #344 broke some key parts of the frontend due to
missing fields in the job list payloads (errors, logs, and metadata were
removed from the job payload for the list API). The root issue is that
the TypeScript types were not adjusted to reflect the new split Job
type, so several parts of the code made reference to things they
could no longer access (and didn't actually need anyway). In particular
this was true for deserializing job payloads where the same logic was
applied to all job responses.

To fix this, update the types to have a separate `JobMinimal` and `Job`
type just as the API backend does. Update all code that uses them to
ensure the correct one is utilized. It's only list view stuff that needs
`JobMinimal` for now.

Fixes #347.
bgentry added a commit that referenced this pull request May 8, 2025
Issue #347 details how #344 broke some key parts of the frontend due to
missing fields in the job list payloads (errors, logs, and metadata were
removed from the job payload for the list API). The root issue is that
the TypeScript types were not adjusted to reflect the new split Job
type, so several parts of the code made reference to things they
could no longer access (and didn't actually need anyway). In particular
this was true for deserializing job payloads where the same logic was
applied to all job responses.

To fix this, update the types to have a separate `JobMinimal` and `Job`
type just as the API backend does. Update all code that uses them to
ensure the correct one is utilized. It's only list view stuff that needs
`JobMinimal` for now.

Fixes #347.
bgentry added a commit that referenced this pull request May 9, 2025
Issue #347 details how #344 broke some key parts of the frontend due to
missing fields in the job list payloads (errors, logs, and metadata were
removed from the job payload for the list API). The root issue is that
the TypeScript types were not adjusted to reflect the new split Job
type, so several parts of the code made reference to things they
could no longer access (and didn't actually need anyway). In particular
this was true for deserializing job payloads where the same logic was
applied to all job responses.

To fix this, update the types to have a separate `JobMinimal` and `Job`
type just as the API backend does. Update all code that uses them to
ensure the correct one is utilized. It's only list view stuff that needs
`JobMinimal` for now.

Fixes #347.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Filter jobs by kind

4 participants