Merge pull request #1591 from carbon-design-system/fix-timeseries-ise…

…ditable fix(timeseries): the iseditable state was allowing interaction and not showing sampledata
carbon-design-system · Sep 13, 2020 · 1e7514f · 1e7514f
2 parents 911ac70 + 5f42782
commit 1e7514f
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 33 deletions.
diff --git a/src/components/BarChartCard/_bar-chart-card.scss b/src/components/BarChartCard/_bar-chart-card.scss
@@ -14,8 +14,11 @@
   }
 
   &--editable {
-    .bx--cc--tooltip {
-      display: 'none';
+    .#{$prefix}--cc--tooltip {
+      display: none;
+    }
+    .#{$prefix}--cc--ruler line.ruler-line {
+      display: none;
     }
   }
 

diff --git a/src/components/TimeSeriesCard/TimeSeriesCard.jsx b/src/components/TimeSeriesCard/TimeSeriesCard.jsx
@@ -116,6 +116,7 @@ const TimeSeriesCardPropTypes = {
  * @returns {object} with a labels array and a datasets array
  */
 export const formatChartData = (timeDataSourceId = 'timestamp', series, values) => {
+  // Generate a set of unique timestamps for the values
   const timestamps = [...new Set(values.map(val => val[timeDataSourceId]))];
   const data = [];
 
@@ -131,16 +132,19 @@ export const formatChartData = (timeDataSourceId = 'timestamp', series, values)
             // only allow valid timestamp matches
             return !isNil(dataItem[timeDataSourceId]) && dataItem[timeDataSourceId] === timestamp;
           })
-          .forEach(dataItem =>
-            data.push({
-              date:
-                dataItem[timeDataSourceId] instanceof Date
-                  ? dataItem[timeDataSourceId]
-                  : new Date(dataItem[timeDataSourceId]),
-              value: dataItem[dataSourceId],
-              group: label,
-            })
-          );
+          .forEach(dataItem => {
+            // Check to see if the data Item actually exists in this timestamp before adding to data (to support sparse data in the values)
+            if (dataItem[dataSourceId]) {
+              data.push({
+                date:
+                  dataItem[timeDataSourceId] instanceof Date
+                    ? dataItem[timeDataSourceId]
+                    : new Date(dataItem[timeDataSourceId]),
+                value: dataItem[dataSourceId],
+                group: label,
+              });
+            }
+          });
       }
     });
   });
@@ -208,13 +212,16 @@ export const handleTooltip = (
 /**
  * Formats and maps the colors to their corresponding datasets in the carbon charts tabular data format
  * @param {Array} series an array of dataset group classifications
+ * @param {Boolean} isEditable is the graph currently editable
  * @returns {Object} colors - formatted
  */
-export const formatColors = series => {
+export const formatColors = (series, isEditable) => {
   const colors = { identifier: 'group', scale: {} };
   if (Array.isArray(series)) {
-    series.forEach(dataset => {
-      colors.scale[dataset.label] = dataset.color;
+    series.forEach((dataset, index) => {
+      colors.scale[dataset.label] = !isEditable
+        ? dataset.color
+        : DISABLED_COLORS[index % DISABLED_COLORS.length];
     });
   } else {
     colors.scale[series.label] = series.color;
@@ -321,17 +328,8 @@ const TimeSeriesCard = ({
     [interval, locale, showTimeInGMT]
   );
 
-  const lines = useMemo(
-    () =>
-      series.map((line, index) => ({
-        ...line,
-        color: !isEditable ? line.color : DISABLED_COLORS[index % DISABLED_COLORS.length],
-      })),
-    [isEditable, series]
-  );
-
   // Set the colors for each dataset
-  const colors = formatColors(series);
+  const colors = formatColors(series, isEditable);
 
   const handleStrokeColor = (datasetLabel, label, data, originalStrokeColor) => {
     if (!isNil(data)) {
@@ -365,17 +363,17 @@ const TimeSeriesCard = ({
   useDeepCompareEffect(
     () => {
       if (chartRef && chartRef.chart) {
-        const chartData = formatChartData(timeDataSourceId, lines, valueSort);
+        const chartData = formatChartData(timeDataSourceId, series, valueSort);
         chartRef.chart.model.setData(chartData);
       }
     },
-    [valueSort, lines, timeDataSourceId]
+    [valueSort, series, timeDataSourceId]
   );
 
   /** This caches the chart value */
-  const chartData = useMemo(() => formatChartData(timeDataSourceId, lines, valueSort), [
+  const chartData = useMemo(() => formatChartData(timeDataSourceId, series, valueSort), [
     timeDataSourceId,
-    lines,
+    series,
     valueSort,
   ]);
 
@@ -488,12 +486,12 @@ const TimeSeriesCard = ({
                     ...(chartType !== TIME_SERIES_TYPES.BAR
                       ? { yMaxAdjuster: yMaxValue => yMaxValue * 1.3 }
                       : {}),
-                    stacked: chartType === TIME_SERIES_TYPES.BAR && lines.length > 1,
+                    stacked: chartType === TIME_SERIES_TYPES.BAR && series.length > 1,
                     includeZero: includeZeroOnYaxis,
                     scaleType: 'linear',
                   },
                 },
-                legend: { position: 'bottom', clickable: !isEditable, enabled: lines.length > 1 },
+                legend: { position: 'bottom', clickable: !isEditable, enabled: series.length > 1 },
                 containerResizable: true,
                 tooltip: {
                   valueFormatter: tooltipValue =>

diff --git a/src/components/TimeSeriesCard/TimeSeriesCard.story.jsx b/src/components/TimeSeriesCard/TimeSeriesCard.story.jsx
@@ -896,7 +896,6 @@ storiesOf('Watson IoT/TimeSeriesCard', module)
           interval={select('interval', ['hour', 'day', 'week', 'month', 'year'], 'hour')}
           breakpoint="lg"
           showTimeInGMT={boolean('showTimeInGMT', false)}
-          values={[]}
           size={size}
           onCardAction={action('onCardAction')}
         />

diff --git a/src/components/TimeSeriesCard/TimeSeriesCard.test.jsx b/src/components/TimeSeriesCard/TimeSeriesCard.test.jsx
@@ -1,10 +1,16 @@
 import React from 'react';
 import { mount } from 'enzyme';
 import { render, screen } from '@testing-library/react';
+import omit from 'lodash/omit';
 
 import Table from '../Table/Table';
 import { getIntervalChartData } from '../../utils/sample';
-import { CARD_SIZES, COLORS, TIME_SERIES_TYPES } from '../../constants/LayoutConstants';
+import {
+  CARD_SIZES,
+  COLORS,
+  TIME_SERIES_TYPES,
+  DISABLED_COLORS,
+} from '../../constants/LayoutConstants';
 import { barChartData } from '../../utils/barChartDataSample';
 
 import TimeSeriesCard, { handleTooltip, formatChartData, formatColors } from './TimeSeriesCard';
@@ -269,6 +275,27 @@ describe('TimeSeriesCard', () => {
       },
     ]);
   });
+  it('formatChartData can handle sparse values from the backend', () => {
+    const series = [
+      {
+        label: 'particles',
+        dataSourceId: 'particles',
+      },
+      {
+        label: 'emissions',
+        dataSourceId: 'emissions',
+      },
+    ];
+    const formattedChartData = formatChartData(
+      'timestamp',
+      series,
+      barChartData.timestamps.slice(0, 2).map(
+        (dataPoint, index) => (index % 2 === 0 ? omit(dataPoint, 'emissions') : dataPoint) // make the data sparse (this skips the emissions datapoint in a few points)
+      )
+    );
+    expect(formattedChartData).toHaveLength(3);
+    expect(formattedChartData.every(dataPoint => dataPoint.value)).toEqual(true); // every value should be valid
+  });
   it('formatChartData returns empty array if no data matches dataFilter', () => {
     const series = [
       {
@@ -307,6 +334,22 @@ describe('TimeSeriesCard', () => {
       scale: { Amsterdam: 'blue', 'New York': 'yellow' },
     });
   });
+  it('formatColors returns valid colors if the time series card isEditable is true', () => {
+    const series = [
+      {
+        label: 'Amsterdam',
+        dataSourceId: 'particles',
+        color: 'blue',
+      },
+      {
+        label: 'New York',
+        dataSourceId: 'particles',
+        color: 'yellow',
+      },
+    ];
+    const selectedColors = Object.values(formatColors(series, true).scale);
+    expect(DISABLED_COLORS).toEqual(expect.arrayContaining(selectedColors)); // all of the selectedColors should come from the DISABLED_COLORS map
+  });
   it('formatColors returns correct format if series is object', () => {
     const series = {
       label: 'Amsterdam',

diff --git a/src/components/TimeSeriesCard/_time-series-card.scss b/src/components/TimeSeriesCard/_time-series-card.scss
@@ -60,4 +60,7 @@
   .#{$prefix}--cc--tooltip {
     display: none;
   }
+  .#{$prefix}--cc--ruler line.ruler-line {
+    display: none;
+  }
 }