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

fix(ui): updated table threshold to set background colors for thresholds correctly #16172

Merged
merged 4 commits into from Dec 9, 2019

Conversation

asalem1
Copy link
Contributor

@asalem1 asalem1 commented Dec 9, 2019

Closes #15904

Problem

Setting table thresholds would not reflect changes to styling in the table ui

Solution

Added in check to colorization of the table thresholds to set the background colors correctly. Also updated a table method to correctly determine whether a value is a number or not

  • CHANGELOG.md updated with a link to the PR (not the Issue)

@asalem1 asalem1 requested a review from a team December 9, 2019 19:00
@ghost ghost requested review from hoorayimhelping and TCL735 and removed request for a team December 9, 2019 19:00
@asalem1 asalem1 changed the title Table threshold fix(ui): updated table threshold to set background colors for thresholds correctly Dec 9, 2019
}

private get isNaN(): boolean {
return isNaN(Number(this.props.data))
Copy link
Contributor

@TCL735 TCL735 Dec 9, 2019

Choose a reason for hiding this comment

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

Passing in null has now changed. Previously, using Number.parseFloat(null) would return NaN and ultimately the expression is NaN

Now, Number(null) returns 0, and ultimately the expression is not NaN

This looks like an improvement, because we do want to handle null values. Just wanted to make sure it was intentional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TCL735 it was intentional in terms of my understanding of how the UI should handle UI values, but it was not explicit in terms of what the criteria should have been. @sjwang90 what're your thoughts on the way we should handle null values?

@asalem1 asalem1 merged commit 9991a27 into master Dec 9, 2019
@mark-rushakoff mark-rushakoff deleted the table-threshold branch April 16, 2020 20:30
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.

Table thresholds are not functional
2 participants