Step Selector: Add smoothed column in data table when enabled #5839
Conversation
|
|
||
| expect(scalarCardComponent.componentInstance.dataHeaders).toContain( | ||
| ColumnHeaders.SMOOTHED | ||
| ); |
There was a problem hiding this comment.
Can you also add tests for the value (0.1)?
There was a problem hiding this comment.
Added test to ensure we use the y value.
| }); | ||
| })); | ||
|
|
||
| it('adds smoothed column header when smoothed is enabled', fakeAsync(() => { |
There was a problem hiding this comment.
Should this moved into "getTimeSelectionTableData" section?
There was a problem hiding this comment.
Good call. Moved.
JamesHollyer
left a comment
JamesHollyer
left a comment
There was a problem hiding this comment.
Thanks Jie-Wei. Let me know if you see anything else you want to add/change.
| }); | ||
| })); | ||
|
|
||
| it('adds smoothed column header when smoothed is enabled', fakeAsync(() => { |
There was a problem hiding this comment.
Good call. Moved.
|
|
||
| expect(scalarCardComponent.componentInstance.dataHeaders).toContain( | ||
| ColumnHeaders.SMOOTHED | ||
| ); |
There was a problem hiding this comment.
Added test to ensure we use the y value.
| expect( | ||
| scalarCardComponent.componentInstance.getTimeSelectionTableData()[0] | ||
| .SMOOTHED | ||
| ).toBe(scalarCardComponent.componentInstance.dataSeries[1].points[1].y); |
There was a problem hiding this comment.
To make it clear and simpler in testing, I think it's a good practice to make the input be a simple number or string instead of variable. (constant variable is fine)
Could you replace the variable here with a number (or string)?
There was a problem hiding this comment.
Yea it was just a weird number so I thought this might make more sense. I am cool using it though. Changed.
There was a problem hiding this comment.
If the number is not intuitive, please add comment above to explain. I guess this is caused by values of 1, 20, 30. Yet I cannot recall how is the smooth calculated.
Motivation for features / changes This change adds the "Smoothed" column to the data table which we expect users to want. It also helps align the data table more closely with the tooltip. Technical description of changes Since this column is more dynamic and should only appear when smoothing is enabled I moved the dataHeaders construction to the container where I made it dependent on the smoothingEnabled$ Observable.
…flow#5839) Motivation for features / changes This change adds the "Smoothed" column to the data table which we expect users to want. It also helps align the data table more closely with the tooltip. Technical description of changes Since this column is more dynamic and should only appear when smoothing is enabled I moved the dataHeaders construction to the container where I made it dependent on the smoothingEnabled$ Observable.
…flow#5839) Motivation for features / changes This change adds the "Smoothed" column to the data table which we expect users to want. It also helps align the data table more closely with the tooltip. Technical description of changes Since this column is more dynamic and should only appear when smoothing is enabled I moved the dataHeaders construction to the container where I made it dependent on the smoothingEnabled$ Observable.
2 participants
Motivation for features / changes
This change adds the "Smoothed" column to the data table which we expect users to want. It also helps align the data table more closely with the tooltip.
Technical description of changes
Since this column is more dynamic and should only appear when smoothing is enabled I moved the dataHeaders construction to the container where I made it dependent on the smoothingEnabled$ Observable.
Screenshots of UI changes