Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Improved comprehensive monitor list filtering #3312

Merged

Conversation

chakflying
Copy link
Collaborator

@chakflying chakflying commented Jun 26, 2023

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

Fixes #1134.
Fixes #1585.

Design inspired by the GitHub mobile app. The left most button shows the number of active filters, and clears all active filters. Each category of filter is a multi-select, which when only one is selected, the filter state will be shown in the button, otherwise the generic "Status", "Active" is shown. Works independent of the search text.

This PR is a partial duplicate of #1971, but does not handle the filtering of the events list.

Also fixes bugs:

  • monitor.active returns 0 instead of false when it has no parent
  • "Pending" monitors counted as "Up". Debatable but doesn't make sense for the statistics listed in the filters.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • User interface (UI)
  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas
    (including JSDoc for methods)
  • My changes generate no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

image

light dark
image image
image image
image image

@CommanderStorm
Copy link
Collaborator

  • could you add Resolves #1134 to the description
  • From just looking at the screenshots (this is not a code review, i can do that later today) the design looks a bit asymertical (red arrows). Could we make the search bar stretch to the full width in the mobile view? (I am not a designer, feel free to overrule)
    image

@louislam
Copy link
Owner

Nice work. I prefer your solution rather than #1971.

@louislam louislam added this to the 1.23.0 milestone Jun 26, 2023
@chakflying
Copy link
Collaborator Author

image image

The "Add Monitor" button lines up with the edge of the monitor list, while the Filters lines up with the status pills of the monitors.

The space left of the search box is reserved for #1886.

@CommanderStorm
Copy link
Collaborator

@louislam
Copy link
Owner

Added a missing translate key and split Active into another key activeFilter.
Thanks for the big feature.

@louislam
Copy link
Owner

Merging this should also close the PRs:

* [Option to display only monitors that are down #3045](https://github.com/louislam/uptime-kuma/pull/3045)

* [Add filtering monitor list and status history by its status (UP|DOWN) #1971](https://github.com/louislam/uptime-kuma/pull/1971)

Thanks for finding them, they will be closed.

@derekoharrow
Copy link

Just updated to 1.23 and trying the new filters. I love them, but...

I have a monitor that is down, and this is under a nested group ("the lounge" under "Everything->Media")

If I have the view unfiltered, the default, then everything shows as expected.

If I filter for "down" monitors, then it seems to list everything multiple times. In this case, under "Everything", under "Everything / Media" and under "Everything / Media / thelounge".

Also, when filtered, instead of showing the monitor name on its own, it shows all the groups above it - e.g. instead of "the lounge" it says "Everything / Media / thelounge" which makes it very difficult to read.

@ajnadox
Copy link

ajnadox commented Aug 18, 2023

Just tested this out and its nice but the feature should be on Status pages since thats whats showing on another screen we have running.

@derekoharrow
Copy link

Just tested this out and its nice but the feature should be on Status pages since thats whats showing on another screen we have running.

Agreed - would love the Status screens to only show problems and not everything...

@CommanderStorm
Copy link
Collaborator

@ajnadox @derekoharrow
Filtering of status pages is tracked in #1512
If you have productive comments (UI-Draft, …) please leave them there.

Some maintainers use 👍🏻 on issues to prioritise work, as always: Pull Requests welcome.

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.

Ability to sort list or filter list by Status Monitor filter online and offline
5 participants