Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
fix(chart): Fixes an issue where series visibility was not retained o…
Browse files Browse the repository at this point in the history
…ver new data set.

When new seriesdata was set into the chart, the visibility settings for each series were not carried
over, as the chart is completely destroyed and reinstantiated every time new data is set.
To mitigate this issue, whenever a new data is set, we now look in the old series dataset for a
series with the same name and carry over the `visible` setting from the previous source.

Fixes #1412
  • Loading branch information
tomheller committed Aug 5, 2020
1 parent 9d5f935 commit 23815c6
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 5 deletions.
139 changes: 138 additions & 1 deletion libs/barista-components/chart/src/chart.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ import {
DT_CHART_COLOR_PALETTES,
DT_CHART_COLOR_PALETTE_ORDERED,
} from '@dynatrace/barista-components/theming';
import { createComponent } from '@dynatrace/testing/browser';
import {
createComponent,
dispatchMouseEvent,
} from '@dynatrace/testing/browser';
import {
AxisOptions,
ChartOptions,
Expand Down Expand Up @@ -425,6 +428,140 @@ describe('DtChart', () => {
}
}));
});

describe('series visibility', () => {
it('should retain the visibilty of a series when new data is set via an observable', () => {
const fixture = createComponent(DynamicSeries);
const fixtureNative = fixture.debugElement.nativeElement;
fixture.detectChanges();

fixture.componentInstance.series.next([
{
name: 'Actions/min',
id: 'someid',
data: [
[1523972199774, 0],
[1523972201622, 10],
],
},
{
name: 'Requests/min',
id: 'another id',
data: [
[1523972199774, 0],
[1523972201622, 10],
],
},
]);
fixture.detectChanges();

const legendItem = fixtureNative.querySelector('.highcharts-legend-item');

// Expect both series to be there.
expect(fixtureNative.querySelectorAll('.highcharts-series')).toHaveLength(
2,
);
dispatchMouseEvent(legendItem, 'click');
fixture.detectChanges();

// Expect series 0 to have visibility hidden set
let series0 = fixtureNative.querySelector('.highcharts-series-0');
expect(series0.getAttribute('visibility')).toBe('hidden');

// set the next data
fixture.componentInstance.series.next([
{
name: 'Actions/min',
id: 'someid',
data: [
[1523972199775, 0],
[1523972201623, 10],
],
},
{
name: 'Requests/min',
id: 'another id',
data: [
[1523972199775, 0],
[1523972201623, 10],
],
},
]);
fixture.detectChanges();

// Expect series 0 to still be hidden.
series0 = fixtureNative.querySelector('.highcharts-series-0');
expect(series0.getAttribute('visibility')).toBe('hidden');
});

it('should retain the visibilty of a series when new data is set as array', () => {
const fixture = createComponent(SeriesMulti);
const fixtureNative = fixture.debugElement.nativeElement;
fixture.detectChanges();

fixture.componentInstance.series = [
{
name: 'Actions/min',
type: 'line',
id: 'someid',
data: [
[1523972199774, 0],
[1523972201622, 10],
],
},
{
name: 'Requests/min',
type: 'line',
id: 'another id',
data: [
[1523972199774, 0],
[1523972201622, 10],
],
},
];
fixture.detectChanges();

const legendItem = fixtureNative.querySelector('.highcharts-legend-item');

// Expect both series to be there.
expect(fixtureNative.querySelectorAll('.highcharts-series')).toHaveLength(
2,
);
dispatchMouseEvent(legendItem, 'click');
fixture.detectChanges();

// Expect series 0 to have visibility hidden set
let series0 = fixtureNative.querySelector('.highcharts-series-0');
expect(series0.getAttribute('visibility')).toBe('hidden');

// set the next data
fixture.componentInstance.series = [
{
name: 'Actions/min',
type: 'line',
id: 'someid',
data: [
[1523972199775, 0],
[1523972201623, 10],
],
},
{
name: 'Requests/min',
type: 'line',
id: 'another id',
data: [
[1523972199775, 0],
[1523972201623, 10],
],
},
];
fixture.detectChanges();

// Expect series 0 to still be hidden.
series0 = fixtureNative.querySelector('.highcharts-series-0');
expect(series0.getAttribute('visibility')).toBe('hidden');
});
});
});

/** Test component that contains an DtChart with static data */
Expand Down
11 changes: 7 additions & 4 deletions libs/barista-components/chart/src/chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ import {
import { DtChartRange } from './range/range';
import { DtChartTimestamp } from './timestamp/timestamp';
import { DtChartTooltip } from './tooltip/chart-tooltip';
import { getPlotBackgroundInfo } from './utils';
import { getPlotBackgroundInfo, retainSeriesVisibility } from './utils';
import { DtChartOptions, DtChartSeries } from './chart.interface';

const HIGHCHARTS_PLOT_BACKGROUND = '.highcharts-plot-background';
Expand Down Expand Up @@ -231,11 +231,15 @@ export class DtChart
}
if (series instanceof Observable) {
this._dataSub = series.subscribe((s: DtChartSeries[]) => {
this._currentSeries = s;
this._currentSeries = s.map(
retainSeriesVisibility(this._chartObject?.series),
);
this._update();
});
} else {
this._currentSeries = series;
this._currentSeries = !series
? series
: series.map(retainSeriesVisibility(this._chartObject?.series));
}
this._series = series;
this._changeDetectorRef.markForCheck();
Expand Down Expand Up @@ -594,7 +598,6 @@ export class DtChart
this._chartObject = this._ngZone.runOutsideAngular(() =>
chart(this._container.nativeElement, this.highchartsOptions),
);

this._chartObject.series.forEach((series, index) => {
addHighchartsEvent(series, 'hide', () => {
if (this._currentSeries) {
Expand Down
19 changes: 19 additions & 0 deletions libs/barista-components/chart/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { Observable, OperatorFunction, fromEvent, merge } from 'rxjs';
import { filter, map } from 'rxjs/operators';

import { _readKeyCode } from '@dynatrace/barista-components/core';
import { DtChartSeries } from './chart.interface';

/** @internal Highcharts plot background dimensions */
export interface PlotBackgroundInfo {
Expand Down Expand Up @@ -281,3 +282,21 @@ export function getPlotBackgroundInfo(
top: parseInt(plotBackground.getAttribute('y')!, 10) || 0,
};
}

/**
* Retains the visibility setting of a series, if the series was already
* set to not visible via highcharts legend clicks or settings.
*/
export function retainSeriesVisibility(
oldSeries: Highcharts.Series[] | undefined,
): (singleSeries: DtChartSeries) => DtChartSeries {
return (singleSeries: DtChartSeries) => {
const previeousSingleSeries = oldSeries?.find(
(s) => s.name === singleSeries.name,
);
if (previeousSingleSeries) {
singleSeries.visible = previeousSingleSeries.visible;
}
return singleSeries;
};
}

0 comments on commit 23815c6

Please sign in to comment.