grafana / clock-panel

Clock Panel Plugin for Grafana
MIT License
95 stars 62 forks source link

Added Query feature #179

Closed Humbarrt closed 6 months ago

Humbarrt commented 8 months ago

Hi there,

I wanted the clock to be able to utilize the query as input. Since other people were asking for the same feature (#168), I figured I might take a shot at it. My implementation isn't pretty, but it's a start. This is my first time working on a Grafana plugin. Have a great day!

CLAassistant commented 8 months ago

CLA assistant check
All committers have signed the CLA.

Humbarrt commented 6 months ago

Hi, I changed things according to your requests. I added a No Value Text and a Invalid Value Text, so if a user wants to debug their query, they can. In production you might want an empty field, thus I made it changeable. I also added a description under the countdown/countup so that you can have a label for the query result... something like: { 'name': ['Next New Year', 'Last New Year', 'Year After Next Year'], 'event': ['2025-01-01', '2024-01-01', '2026-01-01'] } where the 'name' could be the description and the 'event' could be the countup/countdown time. The description can also be static or not be displayed at all.

In my tests i found that all normal time inputs are working, except the epoch input. But a user could fix that with a grafana data transform to multiply the row by 1000, so I think it's no big deal. In the future you maybe want to consider a custom input format though.

Anyway, I know ComputeTime.tsx isn't super pretty, since it handles both time and description, maybe you have a better idea on how to handle that. Cheers!

Humbarrt commented 6 months ago

Hi, I think I addressed all your requested changes. Please let me know if you find new problems. I will do the tests next. Cheers!

academo commented 6 months ago

Hi, I think I addressed all your requested changes. Please let me know if you find new problems. I will do the tests next. Cheers!

This looks almost ready to do. looking forward for the tests and merging this :D

Humbarrt commented 6 months ago

Hi, while creating the tests I stumbled upon something. The showDate setting renders the current date and not the targetTime. When I do a countdown/countup it would be useful to know what date I am counting down/up to. This would be more intuitive, at least for me. Now, I know this is only how I feel and changing it would alter existing behavior for users. I just wanted to bring it up. Cheers!

academo commented 6 months ago

Hi, while creating the tests I stumbled upon something. The showDate setting renders the current date and not the targetTime. When I do a countdown/countup it would be useful to know what date I am counting down/up to. This would be more intuitive, at least for me. Now, I know this is only how I feel and changing it would alter existing behavior for users. I just wanted to bring it up. Cheers!

It is best not to change any of the existing behaviour even if that behaviour is somewhat counter intuitive because there are many plugin users that rely on it. Clock panel is rather a popular plugin in Grafana and we don't want to break the workflow of the existing users.

Humbarrt commented 6 months ago

Ok, I thought so. Maybe an option on the date display could be the solution?

I've done the tests you asked for. Maybe I'll add some more tomorrow. E.g: Time/Description from different Series Null as input Undefined as input NaN as input different modes: lastNotNull, last, firstNotNull, first, min, minFuture, max and maxPast

Cheers!

Humbarrt commented 6 months ago

Hi, I've done all tests I can think of. If there is anything else I can do, let me know :) Cheers!

tolzhabayev commented 6 months ago

The description settings are confusing to the users as they are only relevant for countdown and countup but are still displayed for any mode. The setting should only be visible when relevant.

tolzhabayev commented 6 months ago

Generally we should add this case to the testing dashboard https://github.com/grafana/clock-panel/tree/main/provisioning/dashboards - maybe together with a testdata datasource to showcase different behaviour.

Humbarrt commented 6 months ago

The description settings are confusing to the users as they are only relevant for countdown and countup but are still displayed for any mode. The setting should only be visible when relevant.

Imo the description should be available in all modes. But you were right, there was a bug that showed query as a description source for the time mode. I fixed that. Now you can have none/input description source for time/countup/countdown if you don't use a query and none/input/query if you use a query for you countup/countdown

Humbarrt commented 6 months ago

Generally we should add this case to the testing dashboard https://github.com/grafana/clock-panel/tree/main/provisioning/dashboards - maybe together with a testdata datasource to showcase different behaviour.

Hi, I added panels to the testing dashboard and fixed a bug that occurred when using the older grafana version. Cheers!

Humbarrt commented 6 months ago

@academo @tolzhabayev thanks for the feedback, glad I could help!