From 19053b6772222c081d56bedce8e8cea5ab6878dd Mon Sep 17 00:00:00 2001 From: Natasha DeCoste Date: Tue, 7 Jul 2020 13:07:52 -0400 Subject: [PATCH] improvement(skeleton): honour loading config to show skeleton chart even when data is passed in fix #677 --- packages/core/src/axis-chart.ts | 50 +++++++++++-------- packages/core/src/components/axes/axis.ts | 6 ++- .../core/src/components/graphs/skeleton.ts | 7 +-- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/packages/core/src/axis-chart.ts b/packages/core/src/axis-chart.ts index 908f5e15a8..6a77676678 100644 --- a/packages/core/src/axis-chart.ts +++ b/packages/core/src/axis-chart.ts @@ -5,7 +5,7 @@ import { LegendOrientations, LegendPositions, ChartConfig, - AxisChartOptions, + AxisChartOptions } from "./interfaces/index"; import { LayoutComponent, @@ -13,7 +13,7 @@ import { Title, Tooltip, TooltipBar, - Spacer, + Spacer } from "./components/index"; import { Tools } from "./tools"; @@ -22,7 +22,7 @@ import { CartesianScales, Curves } from "./services/index"; export class AxisChart extends Chart { services: any = Object.assign(this.services, { cartesianScales: CartesianScales, - curves: Curves, + curves: Curves }); constructor(holder: Element, chartConfigs: ChartConfig) { @@ -35,8 +35,8 @@ export class AxisChart extends Chart { components: [new Title(this.model, this.services)], growth: { x: LayoutGrowth.PREFERRED, - y: LayoutGrowth.FIXED, - }, + y: LayoutGrowth.FIXED + } }; const legendComponent = { @@ -44,8 +44,8 @@ export class AxisChart extends Chart { components: [new Legend(this.model, this.services)], growth: { x: LayoutGrowth.PREFERRED, - y: LayoutGrowth.FIXED, - }, + y: LayoutGrowth.FIXED + } }; const graphFrameComponent = { @@ -53,12 +53,18 @@ export class AxisChart extends Chart { components: graphFrameComponents, growth: { x: LayoutGrowth.STRETCH, - y: LayoutGrowth.FIXED, - }, + y: LayoutGrowth.FIXED + } }; + // if the chart is loading but has data, don't enable legend until loading is false + const isDataLoading = Tools.getProperty( + this.model.getOptions(), + "data", + "loading" + ); const isLegendEnabled = - this.model.getOptions().legend.enabled !== false; + this.model.getOptions().legend.enabled !== false && !isDataLoading; // Decide the position of the legend in reference to the chart let fullFrameComponentDirection = LayoutDirection.COLUMN; @@ -92,8 +98,8 @@ export class AxisChart extends Chart { components: [new Spacer(this.model, this.services)], growth: { x: LayoutGrowth.PREFERRED, - y: LayoutGrowth.FIXED, - }, + y: LayoutGrowth.FIXED + } }; const fullFrameComponent = { @@ -104,18 +110,18 @@ export class AxisChart extends Chart { this.services, [ ...(isLegendEnabled ? [legendComponent] : []), - legendSpacerComponent, - graphFrameComponent, + ...(isLegendEnabled ? [legendSpacerComponent] : []), + graphFrameComponent ], { - direction: fullFrameComponentDirection, + direction: fullFrameComponentDirection } - ), + ) ], growth: { x: LayoutGrowth.STRETCH, - y: LayoutGrowth.FIXED, - }, + y: LayoutGrowth.FIXED + } }; // Add chart title if it exists @@ -128,8 +134,8 @@ export class AxisChart extends Chart { components: [new Spacer(this.model, this.services)], growth: { x: LayoutGrowth.PREFERRED, - y: LayoutGrowth.FIXED, - }, + y: LayoutGrowth.FIXED + } }; topLevelLayoutComponents.push(titleSpacerComponent); @@ -142,9 +148,9 @@ export class AxisChart extends Chart { this.services, topLevelLayoutComponents, { - direction: LayoutDirection.COLUMN, + direction: LayoutDirection.COLUMN } - ), + ) ]; } } diff --git a/packages/core/src/components/axes/axis.ts b/packages/core/src/components/axes/axis.ts index 4e8c309768..48492e84dd 100644 --- a/packages/core/src/components/axes/axis.ts +++ b/packages/core/src/components/axes/axis.ts @@ -38,6 +38,7 @@ export class Axis extends Component { const options = this.model.getOptions(); const axisOptions = Tools.getProperty(options, "axes", axisPosition); const axisScaleType = Tools.getProperty(axisOptions, "scaleType"); + const isDataLoading = Tools.getProperty(options, "data", "loading"); const numberOfTicksProvided = Tools.getProperty( axisOptions, "ticks", @@ -269,11 +270,12 @@ export class Axis extends Component { // Position the axis title // check that data exists, if they don't, doesn't show the title axis + const isDataEmpty = this.model.isDataEmpty(); if (axisOptions.title) { const axisTitleRef = DOMUtils.appendOrSelect( container, `text.axis-title` - ).html(this.model.isDataEmpty() ? "" : axisOptions.title); + ).html(isDataEmpty || isDataLoading ? "" : axisOptions.title); switch (axisPosition) { case AxisPositions.LEFT: @@ -429,7 +431,7 @@ export class Axis extends Component { // we don't need to show axes on empty state and on skeleton state // because the Skeleton component draws them - if (this.model.isDataEmpty()) { + if (isDataEmpty || isDataLoading) { container.attr("opacity", 0); } diff --git a/packages/core/src/components/graphs/skeleton.ts b/packages/core/src/components/graphs/skeleton.ts index d41c146d10..582e8e437c 100644 --- a/packages/core/src/components/graphs/skeleton.ts +++ b/packages/core/src/components/graphs/skeleton.ts @@ -30,12 +30,9 @@ export class Skeleton extends Component { "loading" ); - if (isDataEmpty) { + // display a skeleton if there is no chart data or the loading flag is set to true + if (isDataEmpty || isDataLoading) { this.renderSkeleton(isDataLoading); - } else if (!isDataEmpty && isDataLoading) { - throw new Error( - `Something went wrong. You can't provided non-empty data and data loading together.` - ); } else { this.removeSkeleton(); }