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: show monitor descriptions on status page #4612

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hmnd
Copy link

@hmnd hmnd commented Mar 23, 2024

Support showing monitor description on status page, with configurable visibility at the monitor and status page levels.

⚠️⚠️⚠️ 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 #3008

Type of change

Please delete any options that are not relevant.

  • 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 generates no new warnings

Screenshots (if any)

Show description toggle for monitors
Show description toggle for monitors

Show descriptions toggle for status pages
Show descriptions toggle for status pages

Description displayed for a monitor on a status page
Description displayed for a monitor on a status page

- configure description visibility per monitor
- configure description visiblitiy per status page
@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Mar 23, 2024

The next time around please have a discussion how to implement a new feature beforehand (see our contribution guide). This paragraph was added to safe us all time ^^

To the feature:
I am not a fan of introducing another field where you added it. I think adding it in the settings on the status page itself would be better.
=> via this alternate way no "enable descriptions" toggle needs to be added nor does a "show descriptions on status page" toggle need to be added.

image

=> new relation monitor <- status_page_description -> status_page

@hmnd
Copy link
Author

hmnd commented Mar 23, 2024

@CommanderStorm sorry about that! 😬

Using settings sounds good, though I wonder if still keeping the status page level toggle might still make sense? From my ux perspective, the option to show the descriptions wouldn't be discoverable enough if it's hidden on he status page -> monitor settings level. Personally, if I wanted to show one monitor's description on a status page, I would want to show it for all the monitors.

I would propose removing the show_description column I added to monitors, but keeping the show_descriptions field added to status pages + adding it as a setting per monitor on the status page, with default = true.

This way, backwards-compatibility is still retained, while having an easily accessible way to enable showing all descriptions, just like the setting that already exists for tags. Then, if a user wants to hide the description for a monitor, they would logically go to its settings and toggle it off.

@CommanderStorm
Copy link
Collaborator

Personally, if I wanted to show one monitor's description on a status page, I would want to show it for all the monitors.

This is another point where #4456 might be helpfull.
I don't know how many monitors our users share on multiple status page (and how many status pages our users have in general).

I see this case as not so common => keeping this as a status page level setting makes sense to me.

but keeping the show_descriptions field added to status pages

I don't really see a reason for this if this is a status page level setting.
Either there is a description for a monitor or there is not.. => no need for a status page override..

@CommanderStorm CommanderStorm marked this pull request as draft March 25, 2024 16:27
@Killa4
Copy link

Killa4 commented Mar 27, 2024

Honestly, we were looking for this override today. We had to do some hacky stuff with CSS and Tags to achieve what we wanted. A override would be great.

@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Mar 27, 2024

You are not looking for the proposed override, since we currently don't support descriptions on status pages.

Could you give further context why you need to disable descriptions for a status page instead of not configuring the descriptions for said status page?
(What was the problem you were facing in the first place?)

@chakflying chakflying added the area:status-page Everything related to the status page label Mar 29, 2024
@twl9
Copy link

twl9 commented Mar 31, 2024

You are not looking for the proposed override, since we currently don't support descriptions on status pages.

Could you give further context why you need to disable descriptions for a status page instead of not configuring the descriptions for said status page? (What was the problem you were facing in the first place?)

I can. We now use the (single) description field for a technical description of all checks. The way we implemented exactly what @hmnd has built is via CCS with tags as workaround. This way we actually have 2 descriptions per monitor: a technical for the admin and an short explanation on user-level for the visitor.

Nevertheless I would keep it simple and vote for this change.

@CommanderStorm CommanderStorm added the pr:please address review comments this PR needs a bit more work to be mergable label May 19, 2024
@gtirloni
Copy link

This would be really useful and help avoid using the hack with tags described in #3008

The monitor's description should live in the monitor, not in the status page. The status page can decide to show each monitor's description or not but it makes no sense to have separate descriptions.

What might be useful is to have annotations in case there are certain outages but it's beyond the point here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:status-page Everything related to the status page needs:resolve-merge-conflict pr:please address review comments this PR needs a bit more work to be mergable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Monitor Descriptions to the Status Page
6 participants