Skip to content

Conversation

Jamesbarford
Copy link
Contributor

Adds the ability to filter jobs in a collector by status;

filter jobs ui

Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Good idea! I would perhaps use CSS to make the button look differently when it's enabled/disabled, as opposed to adding a checkbox, but that's not super important.

@@ -8,6 +9,15 @@ const props = defineProps<{
function statusClass(c: CollectorConfig): string {
return c.isActive ? "active" : "inactive";
}

const FILTERS = ["InProgress", "Queued", "Success", "Failure"];
const ACTIVE_FILTERS = ref(new Set(FILTERS));
Copy link
Member

Choose a reason for hiding this comment

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

Sets don't fully work reactively with ref, because the watching is not "deep" (https://github.com/orgs/vuejs/discussions/10834).

I would suggest representing the state in a structurally more straightforward way, as we do elsewhere in the frontend:

const filter: Ref<{[index: keyof BenchmarkJobStatus]: boolean}> = ref({
  InProgress: true,
  Queued: true,
  Success: true,
  Failed: false,
});

plus something like this:

function toggleJobStatusFilter(status: BenchmarkJobStatus) {
  filter.value[status] = !filter.value[status];
}

You can then index into filter[status] to see if the status value is enabled or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The set did work, I have changed it to how you have suggested. Though I have filtered out both Failed and Success

Copy link
Member

Choose a reason for hiding this comment

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

Right, it did indeed track the inner modifications of the Set, but it's not ideal to track such inner state in Vue, the reactivity tracking works best when observing simple values and objects, and the set wasn't really needed here, so that's why I suggested removing it.

Btw I would prefer to show Failed jobs by default, we should make these highly visible and not hidden by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

e1025b8 👍

Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Thanks!

@Kobzol Kobzol merged commit 00984b2 into rust-lang:master Sep 3, 2025
11 checks passed
@Jamesbarford Jamesbarford deleted the feat/job-filters branch September 3, 2025 07:14
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.

2 participants