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

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

Merged
merged 4 commits into from
Dec 9, 2019
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bbedef1
fix(ui): updated set threshold function to account for table
asalem1 Dec 6, 2019
876ced1
fix(ui): added styling for tables, need to solidify timestamp logic
asalem1 Dec 9, 2019
f195523
fix(ui): added threshold styling for tables
asalem1 Dec 9, 2019
87c1806
chore(ui): updated changelog
asalem1 Dec 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
1. [16101](https://github.com/influxdata/influxdb/pull/16101): Gracefully handle invalid user-supplied JSON
1. [16105](https://github.com/influxdata/influxdb/pull/16105): Fix crash when loading queries built using Query Builder
1. [16112](https://github.com/influxdata/influxdb/pull/16112): Create cell view properties on dashboard creation
1. [16172](https://github.com/influxdata/influxdb/pull/16172): Fixed table ui threshold colorization issue where setting thresholds would not change table UI

### UI Improvements


## v2.0.0-alpha.20 [2019-11-20]

### Features
Expand All @@ -45,6 +45,7 @@
1. [15628](https://github.com/influxdata/influxdb/pull/15628): Horizontal scrollbar no longer covering data

### UI Improvements

1. [15809](https://github.com/influxdata/influxdb/pull/15809): Redesign cards and animations on getting started page
1. [15787](https://github.com/influxdata/influxdb/pull/15787): Allow the users to filter with labels in telegraph input search

Expand Down
13 changes: 8 additions & 5 deletions ui/src/shared/components/tables/TableCell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class TableCell extends PureComponent<Props> {
'table-graph-cell__fixed-corner': this.isFixedCorner,
'table-graph-cell__highlight-row': this.isHighlightedRow,
'table-graph-cell__highlight-column': this.isHighlightedColumn,
'table-graph-cell__numerical': this.isNumerical,
'table-graph-cell__numerical': !this.isNaN,
'table-graph-cell__field-name': this.isFieldName,
'table-graph-cell__sort-asc':
this.isFieldName && this.isSorted && this.isAscending,
Expand Down Expand Up @@ -137,8 +137,12 @@ class TableCell extends PureComponent<Props> {
return this.isFirstRow && this.isFirstCol
}

private get isNumerical(): boolean {
return !isNaN(Number.parseFloat(this.props.data))
private get isTimestamp(): boolean {
return this.props.dataType === 'dateTime:RFC3339'
}

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

@TCL735 TCL735 Dec 9, 2019 •
edited
Loading

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?

}

private get isFixed(): boolean {
Expand All @@ -165,13 +169,12 @@ class TableCell extends PureComponent<Props> {
const {style, properties, data} = this.props
const {colors} = properties

if (this.isFixed || this.isTimeData || this.isNumerical) {
if (this.isFixed || this.isTimeData || this.isTimestamp || this.isNaN) {
return style
}

const thresholdData = {colors, lastValue: data, cellType: 'table'}
const {bgColor, textColor} = generateThresholdsListHexs(thresholdData)

return {
...style,
backgroundColor: bgColor,
Expand Down
7 changes: 7 additions & 0 deletions ui/src/shared/constants/colorOperations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ export const generateThresholdsListHexs = ({
)

if (shouldColorizeText && colors.length === 1 && baseColor) {
if (cellType === 'table') {
return {textColor: '#000', bgColor: baseColor.hex}
}
return {bgColor: null, textColor: baseColor.hex}
}

Expand All @@ -85,6 +88,10 @@ export const generateThresholdsListHexs = ({
lastValueNumber
)

if (cellType === 'table') {
return {textColor: '#000', bgColor: nearestCrossedThreshold.hex}
}

return {bgColor: null, textColor: nearestCrossedThreshold.hex}
}

Expand Down
1 change: 1 addition & 0 deletions ui/src/timeMachine/reducers/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ export const timeMachineReducer = (

switch (state.view.properties.type) {
case 'gauge':
case 'table':
case 'single-stat':
case 'scatter':
case 'check':
Expand Down