Skip to content

Commit

Permalink
fix(chart): set y domain appropriately (RedHatInsights#61)
Browse files Browse the repository at this point in the history
  • Loading branch information
priley86 committed Jul 29, 2019
1 parent c568794 commit b521303
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 11 deletions.
3 changes: 2 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@
"ignoreUrls": true
}
],
"no-case-declarations": 0,
"no-console": 0,
"no-debugger": 1,
"no-lonely-if": 1,
"no-plusplus": 0,
"no-case-declarations": 0,
"no-restricted-properties": [0, {"object": "Math", "property": "pow"}],
"prettier/prettier": [
"error",
{
Expand Down
44 changes: 41 additions & 3 deletions src/common/__tests__/__snapshots__/graphHelpers.test.js.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,38 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`GraphHelpers getChartDomain should return expected y domain for given inputs 1`] = `
Object {
"empty array": Array [
0,
10,
],
"maxY = 0": Array [
0,
10,
],
"maxY = 100": Array [
0,
1000,
],
"maxY = 1000": Array [
0,
10000,
],
"maxY = 49": Array [
0,
50,
],
"maxY = 50": Array [
0,
100,
],
"maxY = 9": Array [
0,
10,
],
}
`;

exports[`GraphHelpers should convert graph data and generate tooltips when usage is populated: usage populated 1`] = `
Object {
"chartData": Array [
Expand Down Expand Up @@ -37,6 +70,10 @@ Object {
0,
31,
],
"y": Array [
0,
10,
],
},
}
`;
Expand Down Expand Up @@ -72,7 +109,7 @@ Object {
],
"y": Array [
0,
100,
10,
],
},
}
Expand Down Expand Up @@ -109,7 +146,7 @@ Object {
],
"y": Array [
0,
100,
10,
],
},
}
Expand Down Expand Up @@ -146,7 +183,7 @@ Object {
],
"y": Array [
0,
100,
10,
],
},
}
Expand All @@ -156,6 +193,7 @@ exports[`GraphHelpers should have specific functions: graphHelpers 1`] = `
Object {
"chartDateFormat": "MMM D",
"convertGraphUsageData": [Function],
"getChartDomain": [Function],
"getGraphHeight": [Function],
"getTooltipDimensions": [Function],
"getTooltipFontSize": [Function],
Expand Down
13 changes: 13 additions & 0 deletions src/common/__tests__/graphHelpers.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
graphHelpers,
convertGraphUsageData,
getChartDomain,
getGraphHeight,
getTooltipDimensions,
getTooltipFontSize
Expand Down Expand Up @@ -94,6 +95,18 @@ describe('GraphHelpers', () => {
).toMatchSnapshot('throws error');
});

it('getChartDomain should return expected y domain for given inputs', () => {
expect({
'empty array': getChartDomain({ empty: true }).y,
'maxY = 0': getChartDomain({ maxY: 0 }).y,
'maxY = 9': getChartDomain({ maxY: 9 }).y,
'maxY = 49': getChartDomain({ maxY: 49 }).y,
'maxY = 50': getChartDomain({ maxY: 50 }).y,
'maxY = 100': getChartDomain({ maxY: 100 }).y,
'maxY = 1000': getChartDomain({ maxY: 1000 }).y
}).toMatchSnapshot();
});

it('should match graph heights at all breakpoints', () => {
const { breakpoints } = helpers;

Expand Down
26 changes: 19 additions & 7 deletions src/common/graphHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,26 @@ const fillMissingValues = ({ startDate, endDate, values, label, previousLabel })
};

/**
* Returns chart domain setting for given inputs
* Returns chart domain setting for given inputs.
*
* the y axis max should be rounded to the nearest 10 if less than 50,
* otherwise round to the nearest power of 10
*
* the y axis returns large enough number that zeroed bars dont show
*
* @param empty {boolean} Chart data is empty
* @param maxY {boolean} The max y-value
* @returns {Object}
*/
const getChartDomain = ({ empty }) => {
// todo: daily vs weekly
const getChartDomain = ({ empty, maxY = 0 }) => {
const chartDomain = { x: [0, 31] };
if (empty) {
chartDomain.y = [0, 100];

if (empty || maxY < 50) {
chartDomain.y = [0, Math.ceil((maxY + 1) / 10) * 10];
} else {
chartDomain.y = [0, Math.pow(10, maxY.toString().length)];
}

return chartDomain;
};

Expand All @@ -127,9 +135,9 @@ const convertGraphUsageData = ({ data, startDate, endDate, label, previousLabel
let chartDomain = {};

try {
chartDomain = getChartDomain({});

const values = {};
let maxY = 0;

for (let i = 0; i < data.length; i++) {
const formattedDate = moment
.utc(data[i][rhelApiTypes.RHSM_API_RESPONSE_PRODUCTS_DATA_DATE])
Expand All @@ -139,7 +147,9 @@ const convertGraphUsageData = ({ data, startDate, endDate, label, previousLabel
x: formattedDate,
y: data[i][rhelApiTypes.RHSM_API_RESPONSE_PRODUCTS_DATA_SOCKETS]
};
maxY = values[formattedDate].y > maxY ? values[formattedDate].y : maxY;
}
chartDomain = getChartDomain({ maxY });

if (data.length) {
chartData = fillMissingValues({ startDate, endDate, values, label, previousLabel });
Expand Down Expand Up @@ -193,6 +203,7 @@ const getTooltipFontSize = (breakpoints, currentBreakpoint) => {
const graphHelpers = {
chartDateFormat,
convertGraphUsageData,
getChartDomain,
getGraphHeight,
getTooltipDimensions,
getTooltipFontSize,
Expand All @@ -204,6 +215,7 @@ export {
graphHelpers,
chartDateFormat,
convertGraphUsageData,
getChartDomain,
getGraphHeight,
getTooltipDimensions,
getTooltipFontSize,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,10 @@ exports[`RhelGraphCard Component should render multiple states: fulfilled 1`] =
0,
31,
],
"y": Array [
0,
10,
],
}
}
domainPadding={
Expand Down

0 comments on commit b521303

Please sign in to comment.