Skip to content

Commit

Permalink
fix(chartArea): issues/79 streamline multiple data facets (#122)
Browse files Browse the repository at this point in the history
* chartArea, allow multiple data facets as prep for hypervisor
* rhelGraphCard, update towards chartArea props update
* tests, chartArea, i18n and rhelGraphCard tweak
  • Loading branch information
cdcabrera committed Nov 5, 2019
1 parent 479f05d commit 2fbc4e0
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 131 deletions.
40 changes: 27 additions & 13 deletions src/components/chartArea/__tests__/chartArea.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ describe('ChartArea Component', () => {
yAxisLabel: '2 y axis label'
}
],
dataLegendLabel: 'Arma virumque cano'
legendLabel: 'Arma virumque cano',
isStacked: true
}
]
};
Expand Down Expand Up @@ -58,8 +59,13 @@ describe('ChartArea Component', () => {
xAxisLabel: '2 x axis label'
}
],
dataLegendLabel: 'Arma virumque cano',
thresholdLegendLabel: 'Arma virumque cano'
legendLabel: 'Arma virumque cano',
isStacked: true
},
{
data: [],
legendLabel: 'Arma virumque cano',
isThreshold: true
}
]
};
Expand Down Expand Up @@ -93,8 +99,8 @@ describe('ChartArea Component', () => {
xAxisLabel: '2 x axis label'
}
],
dataLegendLabel: 'Arma virumque cano',
thresholdLegendLabel: 'Arma virumque cano'
legendLabel: 'Arma virumque cano',
isStacked: true
}
]
};
Expand Down Expand Up @@ -125,7 +131,11 @@ describe('ChartArea Component', () => {
xAxisLabel: '2 x axis label'
}
],
threshold: [
legendLabel: 'Arma virumque cano',
isStacked: true
},
{
data: [
{
x: 1,
y: 10,
Expand All @@ -139,8 +149,8 @@ describe('ChartArea Component', () => {
xAxisLabel: '2 x axis label'
}
],
dataLegendLabel: 'Arma virumque cano',
thresholdLegendLabel: 'Arma virumque cano'
legendLabel: 'Arma virumque cano',
isThreshold: true
}
]
};
Expand All @@ -161,17 +171,21 @@ describe('ChartArea Component', () => {
xAxisLabel: '1 x axis label'
}
],
threshold: [
interpolation: 'natural',
legendLabel: 'Arma virumque cano',
isStacked: true
},
{
data: [
{
x: 1,
y: 10,
tooltip: '10 dolor sit',
xAxisLabel: '1 x axis label'
}
],
dataInterpolation: 'natural',
dataLegendLabel: 'Arma virumque cano',
thresholdLegendLabel: 'Arma virumque cano'
legendLabel: 'Arma virumque cano',
isThreshold: true
}
]
};
Expand Down Expand Up @@ -232,7 +246,7 @@ describe('ChartArea Component', () => {
yAxisLabel: '1 hello world y-axis label'
}
],
legendData: { name: 'Hello world' },
legendLabel: 'Hello world',
xAxisLabelUseDataSet: true,
yAxisLabelUseDataSet: true
}
Expand Down
146 changes: 70 additions & 76 deletions src/components/chartArea/chartArea.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,10 @@ class ChartArea extends React.Component {
let dataSetMaxY = 0;

dataSets.forEach(dataSet => {
dataSetMaxX = dataSet.data.length > dataSetMaxX ? dataSet.data.length : dataSetMaxX;
if (dataSet.data) {
dataSetMaxX = dataSet.data.length > dataSetMaxX ? dataSet.data.length : dataSetMaxX;

dataSet.data.forEach(value => {
dataSetMaxY = value.y > dataSetMaxY ? value.y : dataSetMaxY;
});

if (dataSet.threshold) {
dataSet.threshold.forEach(value => {
dataSet.data.forEach(value => {
dataSetMaxY = value.y > dataSetMaxY ? value.y : dataSetMaxY;
});
}
Expand Down Expand Up @@ -172,24 +168,18 @@ class ChartArea extends React.Component {
const legendData = [];

dataSets.forEach(dataSet => {
if (dataSet.dataLegendLabel) {
const legendDataSettings = { symbol: {}, name: dataSet.dataLegendLabel };
if (dataSet.legendLabel) {
const legendDataSettings = { symbol: {}, name: dataSet.legendLabel };

if (dataSet.dataColor) {
legendDataSettings.symbol.fill = dataSet.dataColor;
if (dataSet.isThreshold) {
legendDataSettings.symbol = { type: 'threshold' };
}

legendData.push(legendDataSettings);
}

if (dataSet.thresholdLegendLabel) {
const legendThresholdSettings = { symbol: { type: 'threshold' }, name: dataSet.thresholdLegendLabel };

if (dataSet.thresholdColor) {
legendThresholdSettings.symbol.fill = dataSet.thresholdColor;
if (dataSet.color) {
legendDataSettings.symbol.fill = dataSet.color;
}

legendData.push(legendThresholdSettings);
legendData.push(legendDataSettings);
}
});

Expand All @@ -201,9 +191,52 @@ class ChartArea extends React.Component {
};
}

renderChart({ stacked = false }) {
const { dataSets } = this.props;
const charts = [];
const chartsStacked = [];

const thresholdChart = dataSet => (
<ChartThreshold
animate={dataSet.animate || false}
interpolation={dataSet.interpolation || 'step'}
key={helpers.generateId()}
data={dataSet.data}
// FixMe: PFCharts inconsistent implementation around themeColor and style, see ChartArea. Appears enforced, see PFCharts. Leads to multiple checks and implementations.
themeColor={dataSet.color}
style={dataSet.style || {}}
/>
);

const areaChart = dataSet => (
<PfChartArea
animate={dataSet.animate || false}
interpolation={dataSet.interpolation || 'catmullRom'}
key={helpers.generateId()}
data={dataSet.data}
// FixMe: PFCharts inconsistent implementation around themeColor and style, see ChartThreshold themeColor and style
style={{ data: { fill: dataSet.color }, ...dataSet.style }}
/>
);

dataSets.forEach(dataSet => {
if (dataSet.data && dataSet.data.length) {
const updatedDataSet = (dataSet.isThreshold && thresholdChart(dataSet)) || areaChart(dataSet);

if (dataSet.isStacked) {
chartsStacked.push(updatedDataSet);
} else {
charts.push(updatedDataSet);
}
}
});

return (stacked && chartsStacked) || charts;
}

render() {
const { chartWidth } = this.state;
const { dataSets, padding } = this.props;
const { padding } = this.props;

const { isXAxisTicks, isYAxisTicks, xAxisProps, yAxisProps } = this.getChartTicks();
const { chartDomain, maxY } = this.getChartDomain({ isXAxisTicks, isYAxisTicks });
Expand Down Expand Up @@ -231,48 +264,18 @@ class ChartArea extends React.Component {
);
}

/**
* FixMe: PFCharts or Victory, unable to return null or empty content.
* General practice of returning "null" shouldn't necessarily melt the
* graph. To avoid issues we return an empty array
*/
return (
<div ref={this.containerRef}>
<Chart animate={{ duration: 0 }} width={chartWidth} {...chartProps}>
<ChartAxis {...xAxisProps} animate={false} />
<ChartAxis {...yAxisProps} animate={false} />
{(dataSets &&
dataSets.length &&
dataSets.map(
dataSet =>
(dataSet.threshold && dataSet.threshold.length && (
<ChartThreshold
animate={dataSet.thresholdAnimate || false}
interpolation={dataSet.thresholdInterpolation || 'step'}
key={helpers.generateId()}
data={dataSet.threshold}
// FixMe: PFCharts inconsistent implementation around themeColor and style, see ChartArea. Appears enforced, see PFCharts. Leads to multiple annoyance checks and implementations.
themeColor={dataSet.thresholdColor}
style={dataSet.thresholdStyle || {}}
/>
)) ||
null
)) ||
null}
<ChartStack>
{(dataSets &&
dataSets.length &&
dataSets.map(
dataSet =>
(dataSet.data && dataSet.data.length && (
<PfChartArea
animate={dataSet.dataAnimate || false}
interpolation={dataSet.dataInterpolation || 'catmullRom'}
key={helpers.generateId()}
data={dataSet.data}
// FixMe: PFCharts inconsistent implementation around themeColor and style, see ChartThreshold themeColor and style
style={{ data: { fill: dataSet.dataColor }, ...dataSet.dataStyle }}
/>
)) ||
null
)) ||
null}
</ChartStack>
{this.renderChart({})}
<ChartStack>{this.renderChart({ stacked: true })}</ChartStack>
</Chart>
</div>
);
Expand All @@ -284,29 +287,20 @@ ChartArea.propTypes = {
PropTypes.shape({
data: PropTypes.arrayOf(
PropTypes.shape({
x: PropTypes.number,
y: PropTypes.number,
x: PropTypes.number.isRequired,
y: PropTypes.number.isRequired,
tooltip: PropTypes.string,
xAxisLabel: PropTypes.string,
yAxisLabel: PropTypes.string
})
),
dataAnimate: PropTypes.oneOfType([PropTypes.bool, PropTypes.object]),
dataColor: PropTypes.string,
dataInterpolation: PropTypes.string,
dataLegendLabel: PropTypes.string,
dataStyle: PropTypes.object,
threshold: PropTypes.arrayOf(
PropTypes.shape({
x: PropTypes.number,
y: PropTypes.number
})
),
thresholdAnimate: PropTypes.oneOfType([PropTypes.bool, PropTypes.object]),
thresholdColor: PropTypes.string,
thresholdInterpolation: PropTypes.string,
thresholdLegendLabel: PropTypes.string,
thresholdStyle: PropTypes.object,
animate: PropTypes.oneOfType([PropTypes.bool, PropTypes.object]),
color: PropTypes.string,
interpolation: PropTypes.string,
legendLabel: PropTypes.string,
style: PropTypes.object,
isStacked: PropTypes.bool,
isThreshold: PropTypes.bool,
xAxisLabelUseDataSet: PropTypes.bool,
yAxisLabelUseDataSet: PropTypes.bool
})
Expand Down
6 changes: 3 additions & 3 deletions src/components/i18n/__tests__/__snapshots__/i18n.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ msgstr \\"\\"
msgid \\"curiosity-graph.legendSocketsLabel\\"
msgstr \\"\\"
#: src/components/rhelGraphCard/rhelGraphCard.js:87
#: src/components/rhelGraphCard/rhelGraphCard.js:90
msgid \\"curiosity-graph.legendSocketsThresholdLabel\\"
msgstr \\"\\"
Expand Down Expand Up @@ -48,7 +48,7 @@ msgstr \\"\\"
msgid \\"curiosity-graph.tooltipSocketsThreshold\\"
msgstr \\"\\"
#: src/components/rhelGraphCard/rhelGraphCard.js:102
#: src/components/rhelGraphCard/rhelGraphCard.js:106
msgctxt \\"CPU socket usage\\"
msgid \\"curiosity-graph.heading\\"
msgstr \\"\\"
Expand All @@ -68,8 +68,8 @@ msgctxt \\"Quarterly\\"
msgid \\"curiosity-graph.dropdownQuarterly\\"
msgstr \\"\\"
#: src/components/rhelGraphCard/rhelGraphCard.js:105
#: src/components/rhelGraphCard/rhelGraphCard.js:109
#: src/components/rhelGraphCard/rhelGraphCard.js:113
msgctxt \\"Select date range\\"
msgid \\"curiosity-graph.dropdownPlaceholder\\"
msgstr \\"\\"
Expand Down
Loading

0 comments on commit 2fbc4e0

Please sign in to comment.