Skip to content

Commit

Permalink
Make it possible to use time scale as a custom option for chart (#2082)
Browse files Browse the repository at this point in the history
* Reintroduce TimeScale for ChartJS

With stock charts we need Time Cartesian Axis for
comparison of securities.

When using time as X scale type we need a way providing
zero labels (an empty array), since these are autogenerated
by TimeScale. In chart-js.service the
method labelsToApply needs a way of returning []

* ✨ Allow an empty array to disable labels

* 🔥 Remove log statements

* ✨ Make it possible to bypass default labels for stock

Co-authored-by: Mads Buchmann Frederiksen <mbf@prochimp.dk>
  • Loading branch information
2 people authored and RasmusKjeldgaard committed Mar 17, 2022
1 parent 7fb3319 commit 67472ff
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('ChartJSService', () => {

const mockChartConfigService = MockProvider(ChartConfigService, {
getTypeConfig: (chartType: ChartType) => deepCopy(TEST_CHART_TYPES_CONFIG[chartType]),
getInteractionFunctionsExtensions: () => ({ onHover: () => console.log('testing') }),
getInteractionFunctionsExtensions: () => ({ onHover: () => null }),
getAnnotationDefaults: (type: string) => TEST_CHART_ANNOTATIONS_CONFIG[type],
chartTypeToChartJSType: (type: ChartType) => TEST_CHART_TYPES_CONFIG[type].type as ChartJSType,
});
Expand Down Expand Up @@ -201,7 +201,7 @@ describe('ChartJSService', () => {
],
};
chartJSService.renderChart(stockChartConfig);
console.log(chartJSService['chart'].options.locale);

expect(chartJSService['chart'].data.labels.length).toEqual(3);
chartJSService['chart'].data.labels.forEach((label) => {
expect(label).not.toBe('');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,25 @@ export class ChartJSService {
const typeConfig = this.chartConfigService.getTypeConfig(type);

const labelsToApply = (() => {
if (labels?.length > 0) return labels;
else if (type === 'stock') return this.getDefaultStockLabels(datasets, this.locale);
else return this.createBlankLabels(datasets); // ChartJS requires labels
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);
}
})();

return mergeDeepAll(typeConfig, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,18 @@ import {
LineController,
LineElement,
PointElement,
TimeScale,
Tooltip,
} from 'chart.js';
import 'chartjs-adapter-date-fns';
import annotationPlugin from 'chartjs-plugin-annotation';
import ChartDataLabels from 'chartjs-plugin-datalabels';

import { mergeDeepAll } from '../../../helpers/merge-deep';
import MarkerPlugin from '../chart-js/chartjs-plugin-marker/chartjs-plugin-marker';
import { CHART_GLOBAL_DEFAULTS } from '../configs/global-defaults.config';

const CHART_SCALES = [CategoryScale, LinearScale];
const CHART_SCALES = [CategoryScale, LinearScale, TimeScale];
const CHART_ELEMENTS = [BarElement, LineElement, PointElement];
const CHART_CONTROLLERS = [BarController, LineController];
const CHART_PLUGINS = [annotationPlugin, Filler, ChartDataLabels, Tooltip, MarkerPlugin];
Expand Down

0 comments on commit 67472ff

Please sign in to comment.