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.

Sign up for GitHub

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

Jump to bottom

BarGauge: Fixed an issue where the cell color was lit even though there was no data. #39574

Merged
merged 2 commits into from
Sep 23, 2021
Merged

BarGauge: Fixed an issue where the cell color was lit even though there was no data. #39574

merged 2 commits into from
Sep 23, 2021

Conversation

ashharrison90
Copy link
Contributor

What this PR does / why we need it:

  • unfortunately, positionValue > NaN resolves to false
  • add an explicit check for if the value is NaN to mark the cell as off

Which issue(s) this PR fixes:

Fixes #39548

Special notes for your reviewer:

@ashharrison90 ashharrison90 added this to the 8.2.0 milestone Sep 23, 2021
@ashharrison90 ashharrison90 self-assigned this Sep 23, 2021
@ashharrison90 ashharrison90 requested review from a team, hugohaggmark and thisisobate and removed request for a team September 23, 2021 11:20
hugohaggmark
hugohaggmark approved these changes Sep 23, 2021
View reviewed changes
Copy link
Contributor

@hugohaggmark hugohaggmark left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

hugohaggmark
hugohaggmark approved these changes Sep 23, 2021
View reviewed changes
Copy link
Contributor

@hugohaggmark hugohaggmark left a comment

Choose a reason for hiding this comment

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

I did almost add a comment during my last review, that we might extract this to a testable external function, but then I didn't because it was such a minor change.

And then you did it anyway, impressive ⭐ 🤩 🤯

torkelo
torkelo approved these changes Sep 23, 2021
View reviewed changes
Copy link
Member

@torkelo torkelo left a comment

Choose a reason for hiding this comment

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

Awesome fix! 💯

@ashharrison90 ashharrison90 merged commit 6948dbe into main Sep 23, 2021
@ashharrison90 ashharrison90 deleted the ash/39548 branch September 23, 2021 12:25
grafanabot pushed a commit that referenced this pull request Sep 23, 2021
@ashharrison90 @grafanabot
BarGauge: Set cell color to off if numeric value is NaN (#39574)
079b45b 
* BarGauge: Set cell color to off if numeric value is NaN

* BarGauge: Pull getCellColor out into a pure function and add unit tests

(cherry picked from commit 6948dbe)
@grafanabot grafanabot mentioned this pull request Sep 23, 2021
Merged
@torkelo
Copy link
Member

torkelo commented Sep 23, 2021 •
edited
Loading

@ashharrison90 don't forget to add add to changelog label and change PR title to something that explains the fix from the users point of view (not the technical reasons)

@torkelo
Copy link
Member

torkelo commented Sep 23, 2021

Example BarGauge: Fixes cell color being lit when there was no data or NaN value

ashharrison90 added a commit that referenced this pull request Sep 23, 2021
@grafanabot @ashharrison90
BarGauge: Set cell color to off if numeric value is NaN (#39574) (#39580
1fc22f6 
)

* BarGauge: Set cell color to off if numeric value is NaN

* BarGauge: Pull getCellColor out into a pure function and add unit tests

(cherry picked from commit 6948dbe)

Co-authored-by: Ashley Harrison <[email protected]>
@ashharrison90 ashharrison90 changed the title BarGauge: Set cell color to off if numeric value is NaN BarGauge: Fixes cell color being lit when there was no data or NaN value Sep 23, 2021
@achatterjee-grafana achatterjee-grafana changed the title BarGauge: Fixes cell color being lit when there was no data or NaN value BarGauge: Fixed an issue where the cell color was lit even though there was no data. Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@torkelo torkelo torkelo approved these changes

@hugohaggmark hugohaggmark hugohaggmark approved these changes

@thisisobate thisisobate Awaiting requested review from thisisobate

Assignees

@ashharrison90 ashharrison90

Labels
add to changelog area/frontend area/panel/bargauge old backport v8.2.x Mark PR for automatic backport to v8.2.x type/bug
Projects
None yet
Milestone
8.2.0-beta2
Development

Successfully merging this pull request may close these issues.

LCD Bar Gauge shows as full when no data
3 participants
@ashharrison90 @torkelo @hugohaggmark