improvement(skeleton): honour loading config to show skeleton chart e…

…ven when data is passed in fix carbon-design-system#677
natashadecoste · Jul 7, 2020 · 19053b6 · 19053b6
1 parent d536c5b
commit 19053b6
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 29 deletions.
diff --git a/packages/core/src/axis-chart.ts b/packages/core/src/axis-chart.ts
@@ -5,15 +5,15 @@ import {
 	LegendOrientations,
 	LegendPositions,
 	ChartConfig,
-	AxisChartOptions,
+	AxisChartOptions
 } from "./interfaces/index";
 import {
 	LayoutComponent,
 	Legend,
 	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<AxisChartOptions>) {
@@ -35,30 +35,36 @@ export class AxisChart extends Chart {
 			components: [new Title(this.model, this.services)],
 			growth: {
 				x: LayoutGrowth.PREFERRED,
-				y: LayoutGrowth.FIXED,
-			},
+				y: LayoutGrowth.FIXED
+			}
 		};
 
 		const legendComponent = {
 			id: "legend",
 			components: [new Legend(this.model, this.services)],
 			growth: {
 				x: LayoutGrowth.PREFERRED,
-				y: LayoutGrowth.FIXED,
-			},
+				y: LayoutGrowth.FIXED
+			}
 		};
 
 		const graphFrameComponent = {
 			id: "graph-frame",
 			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
@@ -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
@@ -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();
 		}