Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow chart labels to be provided together datasets #2157

Merged
merged 12 commits into from
Apr 21, 2022
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ describe('ChartJSService', () => {

chartJSService.renderChart({
targetElement: canvasElement,
type: 'bar',
type: type,
data: [1, 2, 3],
labels: ['one', 'two', 'three'],
customOptions: {
Expand All @@ -170,18 +170,106 @@ describe('ChartJSService', () => {
});
});

describe('when no data labels are provided', () => {
describe('when labels are provided', () => {
describe("via 'labels' argument", () => {
it('should apply them to the graph', () => {
const data = [1, 2, 3];
const labels = ['one', 'two', 'three'];
expect(labels.length).toBeGreaterThan(0);
expect(data.length).toBeGreaterThan(0);

chartJSService.renderChart({
targetElement: canvasElement,
type: 'bar',
data,
labels,
});

const chartLabels = chartJSService['chart'].data.labels;
expect(chartLabels.length).toEqual(labels.length);
expect(chartLabels).toEqual(labels);
});

describe('and the dataset also contains labels', () => {
it('should apply argument labels', () => {
const data = [
{
data: [
{ x: 'en', y: 1 },
{ x: 'to', y: 2 },
{ x: 'tre', y: 3 },
],
},
] as any;
const labels = ['one', 'two', 'three'];
expect(labels.length).toBeGreaterThan(0);
expect(data.length).toBeGreaterThan(0);

chartJSService.renderChart({
targetElement: canvasElement,
type: 'column',
data,
labels,
});

const chartLabels = chartJSService['chart'].data.labels;
expect(chartLabels.length).toEqual(labels.length);
expect(chartLabels).toEqual(labels);
});
});
});
});

describe('when no labels are provided', () => {
it('should have a blank label for each data point', () => {
const data = [1, 2, 3];
const expectedLabels = ['', '', ''];
expect(data.length).toBeGreaterThan(0);
expect(expectedLabels.length).toEqual(data.length);

chartJSService.renderChart({
targetElement: canvasElement,
type: 'column',
data: [1, 2, 3],
data,
});

const chartLabels = chartJSService['chart'].data.labels;
expect(chartLabels.length).toEqual(3);
chartLabels.forEach((label) => {
expect(label).toEqual('');
expect(chartLabels.length).toEqual(data.length);
expect(chartLabels).toEqual(expectedLabels);
});

describe('but the dataset contains labels for the index axis', () => {
it('should use the dataset labels', () => {
const data = [
{ x: 'en', y: 1 },
{ x: 'to', y: 2 },
{ x: 'tre', y: 3 },
] as any;
const dataset = [
{
data,
},
];
expect(data.length).toBeGreaterThan(0);
expect(data.every(({ x }) => typeof x === 'string')).toBeTrue();

chartJSService.renderChart({
targetElement: canvasElement,
type: 'column',
data: dataset,
});

/* This assertion relies on the assumption that:
1. if chart.data.labels is empty &
2. datasets contains string along the index axis
: then the strings in datasets will be used as labels.

This assumption comes from the internal logic of chartjs.
*/
const chartData = chartJSService['chart'].data;
expect(chartJSService['chart'].options.indexAxis).toEqual('x');
expect(chartData.labels.length).toEqual(0);
expect(chartData.datasets).toEqual(dataset);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
ChartDataLabelOptions,
ChartDataset,
ChartHighlightedElements,
ChartLabel,
ChartLocale,
ChartType,
isNumberArray,
Expand All @@ -32,7 +33,7 @@ export class ChartJSService {
targetElement: ElementRef<HTMLCanvasElement>;
type: ChartType;
data: ChartDataset[] | number[];
labels?: string[] | string[][];
labels?: ChartLabel[];
customOptions?: ChartOptions;
annotations?: AnnotationOptions[];
dataLabelOptions?: ChartDataLabelOptions;
Expand Down Expand Up @@ -76,7 +77,9 @@ export class ChartJSService {
this.chart.data.datasets = datasets;
}

public updateLabels(labels: string[] | string[][]) {
public updateLabels(labels: ChartLabel[]) {
/* As labels provided via the 'labels' input property always has
highest priority - we can just set the property directly */
this.chart.data.labels = labels;
}

Expand Down Expand Up @@ -133,7 +136,7 @@ export class ChartJSService {

private destructivelyUpdateType(type: ChartType, customOptions?: ChartOptions) {
const datasets = this.chart.data.datasets as ChartDataset[];
const labels = this.chart.data.labels;
const labels = this.chart.data.labels as ChartLabel[]; // chart.js stores labels as unknown[]; cast it to ChartLabel[]
const annotations = this.getExistingChartAnnotations();

this.chartType = type;
Expand Down Expand Up @@ -272,35 +275,57 @@ export class ChartJSService {
);
}

private getLabelsToApply(args: {
datasets: ChartDataset[];
type: ChartType;
indexAxis: 'x' | 'y';
labels?: ChartLabel[];
}) {
const { datasets, labels, type, indexAxis } = args;

const datasetHasLabels = ({ data }: ChartDataset) =>
!!data?.some(
(datapoint) => typeof datapoint === 'object' && typeof datapoint[indexAxis] === 'string'
);

/*
Labels can be provided by the user two ways:
1. As a seperate ChartLabel[] via the 'labels' input prop - this has highest priority
2. Together with the dataset via the 'data' input prop - here each datapoint contains
a label for the indexAxis.
For example: { x: 'label1', y: 1} in the case where the index axis is 'x'.

If no labels are provided then default labels are used.
*/
const labelsAreGivenAsSeperateArray = labels !== undefined;
const labelsAreGivenTogetherWithDataset = datasets.some(datasetHasLabels);

if (labelsAreGivenAsSeperateArray) {
return labels;
} else if (labelsAreGivenTogetherWithDataset) {
return null;
} else {
/*
Chart.js requires labels along the index axis to render anything therefore
all other types than stock uses empty labels as default. The stock type
displays day & month as default.
*/
return type === 'stock'
? this.getDefaultStockLabels(datasets, this.locale)
: this.createBlankLabels(datasets);
}
}

private createConfigurationObject(
type: ChartType,
datasets: ChartDataset[],
options: ChartOptions,
labels?: unknown[]
labels?: ChartLabel[]
): ChartConfiguration {
const typeConfig = this.chartConfigService.getTypeConfig(type);

const labelsToApply = (() => {
if (type === 'stock') {
/*
The time scale will auto generate labels based on data.
It should be possible to pass an empty array to get the default
behaviour of chart.js when using stock chart.

TODO: Simplify to always pass labels, if given, to chart.js.
Would be a breaking change. See issue:
https://github.com/kirbydesign/designsystem/issues/2085
*/
const shouldUseTimescaleLabels =
labels?.length === 0 && options?.scales?.x?.type === 'time';
if (shouldUseTimescaleLabels) return labels;
return this.getDefaultStockLabels(datasets, this.locale);
} else if (labels?.length > 0) {
return labels;
} else {
return this.createBlankLabels(datasets);
}
})();
const indexAxis = typeConfig?.options?.indexAxis ?? 'x';
const labelsToApply = this.getLabelsToApply({ labels, datasets, type, indexAxis });

return mergeDeepAll(typeConfig, {
data: {
Expand Down
7 changes: 4 additions & 3 deletions libs/designsystem/src/lib/components/chart/chart.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
ChartDataLabelOptions,
ChartDataset,
ChartHighlightedElements,
ChartLabel,
ChartType,
} from './chart.types';

Expand All @@ -34,12 +35,12 @@ export class ChartComponent implements AfterViewInit, OnChanges {
@Input() type: ChartType = 'column';
@Input() data: ChartDataset[] | number[];

private _labels: string[] | string[][];
private _labels: ChartLabel[];

get labels(): string[] | string[][] {
get labels(): ChartLabel[] {
return this._labels;
}
@Input() set labels(value: string[] | string[][]) {
@Input() set labels(value: ChartLabel[]) {
this._labels = value;
}
@Input() set dataLabels(value: string[] | string[][]) {
Expand Down
1 change: 1 addition & 0 deletions libs/designsystem/src/lib/components/chart/chart.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export type ChartTypeConfig = { type: ChartJSType; options?: ChartOptions };
export type ChartTypesConfig = {
[key in ChartType]: ChartTypeConfig;
};
export type ChartLabel = string | string[]; // String[] allows for multi-line labels

export type ChartDataLabelOptions = {
showMin?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ChartDataLabelOptions,
ChartDataset,
ChartHighlightedElements,
ChartLabel,
ChartOptions,
ChartType,
} from '@kirbydesign/designsystem';
Expand All @@ -24,7 +25,7 @@ import {
export class MockChartComponent {
@Input() type: ChartType;
@Input() data: ChartDataset[] | number[];
@Input() labels: string[] | string[][];
@Input() labels: ChartLabel[];
@Input() dataLabels: string[] | string[][];
@Input() customOptions: ChartOptions;
@Input() dataLabelOptions: ChartDataLabelOptions;
Expand Down