Issue 857 - Chart loading skeleton

packages/2019-housing/src/components/HomeAppreciation/HomeAppreciationVisualization.js

@@ -33,8 +33,8 @@ const HomeAppreciationVisualization = ({ data }) => {
   const lineChartData = data.annualHomeAppreciation.value.results.flatMap(
     yearData => [
       {
-        series: "raw_appreciation_med",
-        value: yearData.raw_appreciation_med,
+        series: "raw_appreciation_med", // make this match the dataSeriesLabels
+        value: yearData.raw_appreciation_med, // make this match the dataSeriesLabels
         sale_year: yearData.sale_year
       },
       {
@@ -91,12 +91,14 @@ const HomeAppreciationVisualization = ({ data }) => {
         xLabel="Home Ownership Rate"
         yLabel="Race"
         dataValueFormatter={x => civicFormat.percentage(x)}
+        protect
       />
       <strong style={{ color: "crimson" }}>
         LineChart Visualization TODO:
         <ul>
           <li>
             Make the confidence interval lines dashed & all lines the same color
+            (see note on lineChartData)
           </li>
         </ul>
       </strong>
@@ -112,6 +114,7 @@ const HomeAppreciationVisualization = ({ data }) => {
         yLabel="Appreciation ($)"
         xNumberFormatter={x => civicFormat.year(x)}
         yNumberFormatter={y => civicFormat.dollars(y)}
+        protect
       />
       <strong style={{ color: "crimson" }}>
         Map Visualization TODO:

packages/2019-housing/src/components/HomeOwnershipRates/HomeOwnershipRatesVisualization.js

@@ -33,6 +33,7 @@ const HomeOwnershipRatesVisualization = ({ data }) => {
         yLabel="Home Ownership Rate"
         xNumberFormatter={x => civicFormat.year(x)}
         yNumberFormatter={y => civicFormat.decimalToPercent(y)}
+        protect
       />
     )
   );

packages/2019-housing/src/components/HousingDisplacement/HousingDisplacementVisualization.js

@@ -82,6 +82,7 @@ const HousingDisplacementVisualization = ({ isLoading, data }) => {
             xLabel={xLabel}
             yLabel={yLabel}
             xNumberFormatter={tick => tick.toString()}
+            protect
           />
         </div>
         <div style={{ width: "50%" }}>
@@ -94,6 +95,7 @@ const HousingDisplacementVisualization = ({ isLoading, data }) => {
             xLabel={xLabel}
             yLabel={yLabel}
             xNumberFormatter={tick => tick.toString()}
+            protect
           />
         </div>
       </div>

packages/2019-template/src/components/DemoCard/DemoCardVisualization.js

@@ -76,19 +76,18 @@ export default function DemoCardVisualization({ isLoading, data }) {
           Late Gentrification
         </Button>
       </div>
-      {!isLoading && data[dataType] && (
-        <LineChart
-          data={data[dataType]}
-          dataKey="year"
-          dataValue="ridership"
-          dataSeries="series"
-          title={title}
-          xLabel="Year"
-          yLabel="Ridership"
-          xNumberFormatter={civicFormat.year}
-          subtitle={`Average daily ridership ${dataTypeDescription}`}
-        />
-      )}
+      <LineChart
+        loading={isLoading}
+        data={data[dataType]}
+        dataKey="year"
+        dataValue="ridership"
+        dataSeries="series"
+        title={title}
+        xLabel="Year"
+        yLabel="Ridership"
+        xNumberFormatter={civicFormat.year}
+        subtitle={`Average daily ridership ${dataTypeDescription}`}
+      />
     </Fragment>
   );
 }

packages/component-library/src/BarChart/BarChart.js

@@ -34,7 +34,8 @@ const BarChart = ({
   error,
   theme
 }) => {
-  const chartDomain = domain || getDefaultDomain(data, dataKey, dataValue);
+  const safeData = data && data.length ? data : [{}];
+  const chartDomain = domain || getDefaultDomain(safeData, dataKey, dataValue);
 
   return (
     <ThemeProvider theme={theme}>
@@ -44,7 +45,7 @@ const BarChart = ({
         loading={loading}
         error={error}
       >
-        <DataChecker dataAccessors={{ dataKey, dataValue }} data={data}>
+        <DataChecker dataAccessors={{ dataKey, dataValue }} data={safeData}>
           <VictoryChart
             padding={{ left: 90, right: 50, bottom: 50, top: 50 }}
             domainPadding={{ x: [40, 40], y: [0, 0] }}
@@ -95,7 +96,7 @@ const BarChart = ({
                   theme={theme}
                 />
               }
-              data={data.map(d => ({
+              data={safeData.map(d => ({
                 dataKey: d[dataKey],
                 dataValue: d[dataValue],
                 label: `${xLabel}: ${xNumberFormatter(

packages/component-library/src/ChartContainer/ChartContainer.js

@@ -1,21 +1,18 @@
 import PropTypes from "prop-types";
 /** @jsx jsx */
 import { jsx, css } from "@emotion/core";
-
+import { BrandColors } from "../index";
 import ChartTitle from "../ChartTitle";
-
-const chartLoading = css`
-  text-align: center;
-  background: #eee;
-  height: 100%;
-`;
+import Logo from "../Logo/Logo";
 
 const chartError = css`
   text-align: center;
   background: #fdd;
   height: 100%;
 `;
 
+const defaultVictoryAspectRatio = 650 / 350;
+
 /**
   ChartContainer renders titles, subtitles, and provides some default styling for charts.
   It is designed to render a VictoryChart as children.
@@ -29,7 +26,8 @@ const ChartContainer = ({
   loading,
   subtitle,
   children,
-  className
+  className,
+  aspectRatio
 }) => {
   const figureWrapper = css`
     margin: 0;
@@ -40,21 +38,42 @@ const ChartContainer = ({
     width: 100%;
     ${className};
   `;
+  const fullHeight = css`
+    position: relative;
+    padding-top: ${100 / aspectRatio}%;
+    height: 100%;
+  `;
+
+  const skeletonStyle = css`
+    margin: 0 auto;
+    width: 100%;
+    max-width: 900px;
+    height: calc(100vw / ${aspectRatio});
+    max-height: ${900 / aspectRatio}px;
+    background-color: ${BrandColors.subdued.hex};
+    display: grid;
+    justify-items: center;
+    align-items: center;
+  `;
 
   let content = (
-    <figure css={figureWrapper}>
-      <ChartTitle title={title} subtitle={subtitle} />
-      <div css={wrapperStyle}>{children}</div>
-    </figure>
+    <div css={skeletonStyle}>
+      <Logo type="squareLogoAnimated" />
+    </div>
   );
 
-  if (loading) {
-    content = <div css={chartLoading}>Loading...</div>;
+  if (!loading) {
+    content = <div css={wrapperStyle}>{children}</div>;
   } else if (error) {
-    content = <div css={chartError}>{error}</div>;
+    content = <div css={[wrapperStyle, fullHeight, chartError]}>{error}</div>;
   }
 
-  return content;
+  return (
+    <figure css={figureWrapper}>
+      <ChartTitle title={title} subtitle={subtitle} />
+      {content}
+    </figure>
+  );
 };
 
 ChartContainer.propTypes = {
@@ -63,9 +82,12 @@ ChartContainer.propTypes = {
   loading: PropTypes.bool,
   children: PropTypes.node,
   subtitle: PropTypes.string,
-  className: PropTypes.string
+  className: PropTypes.string,
+  aspectRatio: PropTypes.number
 };
 
-ChartContainer.defaultProps = {};
+ChartContainer.defaultProps = {
+  aspectRatio: defaultVictoryAspectRatio
+};
 
 export default ChartContainer;

packages/component-library/src/HorizontalBarChart/HorizontalBarChart.js

@@ -21,6 +21,7 @@ import {
 } from "../utils/chartHelpers";
 import groupByKey from "../utils/groupByKey";
 import DataChecker from "../utils/DataChecker";
+import protectData from "../utils/protectData";
 import { VictoryTheme } from "../_Themes/index";
 
 const HorizontalBarChart = ({
@@ -43,26 +44,35 @@ const HorizontalBarChart = ({
   dataSeriesKey,
   dataSeriesLabel,
   legendComponent,
-  theme
+  theme,
+  protect
 }) => {
+  const safeData =
+    // eslint-disable-next-line no-nested-ternary
+    data && data.length
+      ? protect
+        ? protectData(data, { dataLabel, dataSeriesKey })
+        : data
+      : [{}];
+
   const groupedDataIfStacked = () => {
     if (stacked) {
       if (hundredPercentData) {
         return transformDatato100(
-          groupByKey(data, dataSeriesKey, dataLabel),
+          groupByKey(safeData, dataSeriesKey, dataLabel),
           dataLabel,
           dataValue
         );
       }
-      return groupByKey(data, dataSeriesKey, dataLabel);
+      return groupByKey(safeData, dataSeriesKey, dataLabel);
     }
-    return data;
+    return safeData;
   };
   const groupedData = groupedDataIfStacked();
   const barData =
     sortOrder && sortOrder.length
-      ? data
-      : data.map((d, index) => {
+      ? safeData
+      : safeData.map((d, index) => {
           return { ...d, defaultSort: index + 1 };
         });
   const sortOrderKey =
@@ -73,13 +83,13 @@ const HorizontalBarChart = ({
   const barHeight = theme.bar.style.data.width;
   const spaceHeight = theme.bar.style.data.padding * 2;
   const additionalHeight = padding.bottom + padding.top;
-  const minValue = Math.min(0, ...data.map(d => d[dataValue]));
+  const minValue = Math.min(0, ...safeData.map(d => d[dataValue]));
 
-  const bars = stacked ? groupedData.length : data.length;
+  const bars = stacked ? groupedData.length : safeData.length;
   const spaces = bars - 1;
   const dataHeight = bars * barHeight + spaces * spaceHeight;
   const dataSeriesLabels = dataSeriesKey
-    ? dataSeriesLabel || getDefaultDataSeriesLabels(data, dataSeriesKey)
+    ? dataSeriesLabel || getDefaultDataSeriesLabels(safeData, dataSeriesKey)
     : null;
   const legendData =
     dataSeriesLabels && dataSeriesLabels.length
@@ -100,8 +110,9 @@ const HorizontalBarChart = ({
       subtitle={subtitle}
       loading={loading}
       error={error}
+      aspectRatio={650 / (dataHeight + additionalHeight)}
     >
-      <DataChecker dataAccessors={{ dataValue }} data={data}>
+      <DataChecker dataAccessors={{ dataValue }} data={safeData}>
         {legendData &&
           (legendComponent ? (
             legendComponent(legendData)
@@ -287,7 +298,8 @@ HorizontalBarChart.propTypes = {
   dataSeriesKey: PropTypes.string,
   dataSeriesLabel: PropTypes.shape({}),
   legendComponent: PropTypes.func,
-  theme: PropTypes.shape({})
+  theme: PropTypes.shape({}),
+  protect: PropTypes.bool
 };
 
 HorizontalBarChart.defaultProps = {
@@ -308,7 +320,8 @@ HorizontalBarChart.defaultProps = {
   dataSeriesKey: null,
   dataSeriesLabel: {},
   legendComponent: null,
-  theme: VictoryTheme
+  theme: VictoryTheme,
+  protect: false
 };
 
 export default HorizontalBarChart;

packages/component-library/src/HorizontalBarChart/HorizontalBarChart.test.js

@@ -18,6 +18,21 @@ const updatedSimpleData = [
   { x: 500, y: "bat" }
 ];
 
+const simpleColorData = [
+  { x: 100, y: "black" },
+  { x: 200, y: "white" },
+  { x: 300, y: "red" },
+  { x: 400, y: "rat" }
+];
+
+const updatedSimpleColorData = [
+  { x: 100, y: "black" },
+  { x: 200, y: "white" },
+  { x: 300, y: "red" },
+  { x: 400, y: "rat" },
+  { x: 500, y: "bat" }
+];
+
 const simpleDataDomain = { x: [0, 400], y: [0, 4] };
 
 const sampleUnstructuredData = [
@@ -148,6 +163,7 @@ describe("HorizontalBarChart", () => {
     const wrapper = shallow(<HorizontalBarChart data={simpleData} />);
     const dataProp = wrapper
       .find("VictoryBar")
+
       .first()
       .prop("data");
     expect(dataProp).to.have.length(4);
@@ -166,4 +182,45 @@ describe("HorizontalBarChart", () => {
     expect(xProp).to.eql("sortOrder");
     expect(yProp).to.eql("dataValue");
   });
+
+  // Tests for https://github.com/FormidableLabs/victory/issues/928 workaround
+
+  it("renders a HorizontalBarChart correctly with sample color data **invisible whitespace warning, see protectData util**", () => {
+    const c = "‌‌​"; // U+200B (zero-width space character)
+    const wrapper = shallow(
+      <HorizontalBarChart data={simpleColorData} protect />
+    );
+    expect(wrapper.find({ title: "Horizontal Bar Chart" }).length).to.eql(1);
+    expect(wrapper.find({ title: "Horizontal Bar Chart" }).props().data).to.eql(
+      // invisible whitespace character injected in label before :
+      [
+        { sortOrder: 1, dataValue: 100, label: `black${c}: 100` },
+        { sortOrder: 2, dataValue: 200, label: `white${c}: 200` },
+        { sortOrder: 3, dataValue: 300, label: `red${c}: 300` },
+        { sortOrder: 4, dataValue: 400, label: `rat${c}: 400` }
+      ]
+    );
+  });
+
+  it("renders an updated HorizontalBarChart when passed new data **invisible whitespace warning, see protectData util**", () => {
+    const c = "‌‌​"; // U+200B (zero-width space character)
+    const wrapper = shallow(
+      <HorizontalBarChart data={simpleColorData} protect />
+    );
+    expect(wrapper.find({ title: "Horizontal Bar Chart" }).length).to.eql(1);
+    wrapper.setProps({ data: updatedSimpleColorData });
+
+    expect(wrapper.find({ title: "Horizontal Bar Chart" }).props().data).to.eql(
+      // invisible whitespace character injected in label before :
+      [
+        { sortOrder: 1, dataValue: 100, label: `black${c}: 100` },
+        { sortOrder: 2, dataValue: 200, label: `white${c}: 200` },
+        { sortOrder: 3, dataValue: 300, label: `red${c}: 300` },
+        { sortOrder: 4, dataValue: 400, label: `rat${c}: 400` },
+        { sortOrder: 5, dataValue: 500, label: `bat${c}: 500` }
+      ]
+    );
+  });
+
+  // End tests for https://github.com/FormidableLabs/victory/issues/928 workaround
 });