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: sensors with % units get chart including 0-100% in their domain #739

Merged
merged 5 commits into from Jun 25, 2023

Conversation

Flix6x
Copy link
Contributor

@Flix6x Flix6x commented Jun 21, 2023

Having percentages within [0, 100] is such a common use case that we should always include this domain in the scale. This makes it easier to read off individual charts, and also to compare across charts (for example, when showing simulation results of different scenarios).

Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x added this to the 0.15.0 milestone Jun 21, 2023
@Flix6x Flix6x self-assigned this Jun 21, 2023
Copy link
Contributor

@victorgarcia98 victorgarcia98 left a comment

Choose a reason for hiding this comment

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

Nice!

It's good that you did union to support other valid percentages (e.g. negative or >100%).

Trying it out locally I find that the it doesn't show the whole range [0,100], just [0, max(data)]. Optionally, I would use the interval [0,105], to leave some space above.

When 100% is in the data:
image

When 100% not in the data:

image

@Flix6x
Copy link
Contributor Author

Flix6x commented Jun 22, 2023

Looks like you tried the sensor page. Which is good that you did. I had only implemented this for the asset page. I'll amend.

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Copy link
Contributor

@victorgarcia98 victorgarcia98 left a comment

Choose a reason for hiding this comment

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

Thanks!

@Flix6x Flix6x merged commit b80de0a into main Jun 25, 2023
6 of 7 checks passed
@Flix6x Flix6x deleted the feature/default-domain-for-percentage-sensors branch June 25, 2023 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants