Skip to content

feat: toggle able legend for cartesian chart #1270

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.

Sign up for GitHub

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

Merged
itssharmasandeep merged 14 commits into from
Dec 2, 2021
Merged

feat: toggle able legend for cartesian chart #1270

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a3be401
feat: toggle able legend for cartesian chart
itssharmasandeep Nov 18, 2021
1fe78fe
fix: add test cases
itssharmasandeep Nov 18, 2021
1fbd6c9
Merge remote-tracking branch 'origin/main' into cartesian-toggle-legend
itssharmasandeep Nov 19, 2021
b53fe40
fix: addressing review comments
itssharmasandeep Nov 19, 2021
747556e
fix: addressing review comments
itssharmasandeep Nov 22, 2021
09513b3
fix: addressing review comments
itssharmasandeep Nov 22, 2021
1781dee
Merge remote-tracking branch 'origin/main' into cartesian-toggle-legend
itssharmasandeep Nov 25, 2021
805d7e1
fix: some style changes
itssharmasandeep Nov 26, 2021
c9df216
Merge remote-tracking branch 'origin/main' into cartesian-toggle-legend
itssharmasandeep Nov 30, 2021
9f5f84b
Merge remote-tracking branch 'origin/main' into cartesian-toggle-legend
itssharmasandeep Nov 30, 2021
d9be399
fix: addressing review comments
itssharmasandeep Dec 1, 2021
a9b166b
fix: suggested changes
itssharmasandeep Dec 1, 2021
c78844b
fix: lint fixes
itssharmasandeep Dec 1, 2021
c2f177b
fix: addressing review comments
itssharmasandeep Dec 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
width: 100%;
position: absolute;
min-height: 48px;
padding-bottom: 20px;

&.position-none {
display: none;
Expand Down Expand Up @@ -118,13 +119,41 @@
}

.legend-text {
fill: $gray-5;
font-size: 14px;
padding-left: 2px;
cursor: pointer;

&.default {
color: $gray-9;
}

&.active {
color: $blue-4;
}

&.inactive {
color: $gray-5;
}
}
}
}

.reset {
@include font-title($blue-4);
cursor: pointer;
position: absolute;
bottom: 0;
right: 0;

&.hidden {
display: none;
}

&:hover {
color: $blue-6;
}
}

.interval-control {
padding: 0 8px;
}
Expand Down
41 changes: 41 additions & 0 deletions projects/observability/src/shared/components/cartesian/cartesian-chart.component.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,47 @@ describe('Cartesian Chart component', () => {
expect(chart.queryAll(CartesianLegend.CSS_SELECTOR, { root: true }).length).toBe(1);
}));

test('should have correct active series', fakeAsync(() => {
const chart = createHost(`<ht-cartesian-chart [series]="series" [legend]="legend"></ht-cartesian-chart>`, {
hostProps: {
series: [],
legend: undefined
}
});
chart.setHostInput({
series: [
{
data: [[1, 2]],
name: 'test series 1',
color: 'blue',
type: CartesianSeriesVisualizationType.Column,
stacking: true
},
{
data: [[1, 6]],
name: 'test series 2',
color: 'red',
type: CartesianSeriesVisualizationType.Column,
stacking: true
}
],
legend: LegendPosition.Bottom
});
tick();
expect(chart.queryAll(CartesianLegend.CSS_SELECTOR, { root: true }).length).toBe(1);
expect(chart.queryAll('.legend-entry').length).toBe(2);
expect(chart.query('.reset.hidden')).toExist();

const legendEntryTexts = chart.queryAll('.legend-text');
chart.click(legendEntryTexts[0]);
tick();
expect(chart.query('.reset.hidden')).not.toExist();

chart.click(chart.query('.reset') as Element);
tick();
expect(chart.query('.reset.hidden')).toExist();
}));

test('should render column chart', fakeAsync(() => {
const chart = createHost(`<ht-cartesian-chart [series]="series"></ht-cartesian-chart>`, {
hostProps: {
Expand Down
48 changes: 40 additions & 8 deletions projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Injector, Renderer2 } from '@angular/core';
import { TimeRange } from '@hypertrace/common';
import { ContainerElement, mouse, select } from 'd3-selection';
import { Subscription } from 'rxjs';
import { LegendPosition } from '../../../legend/legend.component';
import { ChartTooltipRef } from '../../../utils/chart-tooltip/chart-tooltip-popover';
import { D3UtilService } from '../../../utils/d3/d3-util.service';
Expand Down Expand Up @@ -35,6 +36,7 @@ import { CartesianScaleBuilder } from '../scale/cartesian-scale-builder';
// tslint:disable:max-file-line-count
export class DefaultCartesianChart<TData> implements CartesianChart<TData> {
public static DATA_SERIES_CLASS: string = 'data-series';
public static CHART_VISUALIZATION_CLASS: string = 'chart-visualization';

protected readonly margin: number = 16;
protected readonly axisHeight: number = 16;
Expand All @@ -45,7 +47,7 @@ export class DefaultCartesianChart<TData> implements CartesianChart<TData> {
protected chartBackgroundSvgElement?: SVGSVGElement;
protected dataElement?: ContainerElement;
protected mouseEventContainer?: SVGSVGElement;
protected legend?: CartesianLegend;
protected legend?: CartesianLegend<TData>;
protected tooltip?: ChartTooltipRef<TData>;
protected allSeriesData: CartesianData<TData, Series<TData>>[] = [];
protected allCartesianData: CartesianData<TData, Series<TData> | Band<TData>>[] = [];
Expand All @@ -65,6 +67,9 @@ export class DefaultCartesianChart<TData> implements CartesianChart<TData> {
onEvent: ChartEventListener<TData>;
}[] = [];

private activeSeriesSubscription?: Subscription;
private activeSeries: Series<TData>[] = [];

public constructor(
protected readonly hostElement: Element,
protected readonly injector: Injector,
Expand All @@ -80,6 +85,10 @@ export class DefaultCartesianChart<TData> implements CartesianChart<TData> {
this.tooltip && this.tooltip.destroy();
this.legend && this.legend.destroy();

if (this.activeSeriesSubscription) {
this.activeSeriesSubscription.unsubscribe();
}

return this;
}

Expand All @@ -104,6 +113,7 @@ export class DefaultCartesianChart<TData> implements CartesianChart<TData> {
public withSeries(...series: Series<TData>[]): this {
this.series.length = 0;
this.series.push(...series);
this.activeSeries = [...series];

this.seriesSummaries.length = 0;
this.seriesSummaries.push(
Expand Down Expand Up @@ -273,6 +283,10 @@ export class DefaultCartesianChart<TData> implements CartesianChart<TData> {

private updateData(): void {
this.drawLegend();
this.drawVisualizations();
}

private drawVisualizations(): void {
this.buildVisualizations();
this.drawChartBackground();
this.drawAxes();
Expand All @@ -283,6 +297,16 @@ export class DefaultCartesianChart<TData> implements CartesianChart<TData> {
this.setupEventListeners();
}

private redrawVisualization(): void {
const chartViz = select(this.chartContainerElement!).selectAll(
`.${DefaultCartesianChart.CHART_VISUALIZATION_CLASS}`
);
if (chartViz.nodes().length > 0) {
chartViz.remove();
this.drawVisualizations();
}
}

private moveDataOnTopOfAxes(): void {
if (!this.dataElement) {
return;
Expand Down Expand Up @@ -338,19 +362,26 @@ export class DefaultCartesianChart<TData> implements CartesianChart<TData> {
return;
}

new CartesianNoDataMessage(this.chartBackgroundSvgElement, this.series).updateMessage();
new CartesianNoDataMessage(this.chartBackgroundSvgElement, this.activeSeries).updateMessage();
}

private drawLegend(): void {
if (this.chartContainerElement) {
if (this.legendPosition !== undefined && this.legendPosition !== LegendPosition.None) {
this.legend = new CartesianLegend(this.series, this.injector, this.intervalData, this.seriesSummaries).draw(
this.chartContainerElement,
this.legendPosition
);
this.legend = new CartesianLegend<TData>(
this.activeSeries,
this.injector,
this.intervalData,
this.seriesSummaries
).draw(this.chartContainerElement, this.legendPosition);
this.activeSeriesSubscription?.unsubscribe();
this.activeSeriesSubscription = this.legend.activeSeries$.subscribe(activeSeries => {
this.activeSeries = activeSeries;
this.redrawVisualization();
});
} else {
// The legend also contains the interval selector, so even without a legend we need to create an element for that
this.legend = new CartesianLegend([], this.injector, this.intervalData, this.seriesSummaries).draw(
this.legend = new CartesianLegend<TData>([], this.injector, this.intervalData, this.seriesSummaries).draw(
this.chartContainerElement,
LegendPosition.None
);
Expand All @@ -370,6 +401,7 @@ export class DefaultCartesianChart<TData> implements CartesianChart<TData> {

this.chartBackgroundSvgElement = select(this.chartContainerElement)
.append('svg')
.classed(DefaultCartesianChart.CHART_VISUALIZATION_CLASS, true)
.style('position', 'absolute')
.attr('width', `${chartBox.width}px`)
.attr('height', `${chartBox.height}px`)
Expand Down Expand Up @@ -430,7 +462,7 @@ export class DefaultCartesianChart<TData> implements CartesianChart<TData> {

private buildVisualizations(): void {
this.allSeriesData = [
...this.series.map(series => this.getChartSeriesVisualization(series)),
...this.activeSeries.map(series => this.getChartSeriesVisualization(series)),
...this.bands.flatMap(band => [
// Need to add bands as series to get tooltips
this.getChartSeriesVisualization(band.upper),
Expand Down
101 changes: 92 additions & 9 deletions projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { ComponentRef, Injector } from '@angular/core';
import { DynamicComponentService } from '@hypertrace/common';
import { Color, DynamicComponentService } from '@hypertrace/common';
import { ContainerElement, EnterElement, select, Selection } from 'd3-selection';
import { Observable, Subject } from 'rxjs';
import { distinctUntilChanged, startWith } from 'rxjs/operators';
import { LegendPosition } from '../../../legend/legend.component';
import { Series, Summary } from '../../chart';
import {
Expand All @@ -10,20 +12,34 @@ import {
} from './cartesian-interval-control.component';
import { CartesianSummaryComponent, SUMMARIES_DATA } from './cartesian-summary.component';

export class CartesianLegend {
export class CartesianLegend<TData> {
private static readonly CSS_CLASS: string = 'legend';
private static readonly RESET_CSS_CLASS: string = 'reset';
private static readonly DEFAULT_CSS_CLASS: string = 'default';
private static readonly ACTIVE_CSS_CLASS: string = 'active';
private static readonly INACTIVE_CSS_CLASS: string = 'inactive';
public static readonly CSS_SELECTOR: string = `.${CartesianLegend.CSS_CLASS}`;

public readonly activeSeries$: Observable<Series<TData>[]>;
private readonly activeSeriesSubject: Subject<Series<TData>[]> = new Subject();
private readonly initialSeries: Series<TData>[];

private isSelectionModeOn: boolean = false;
private legendElement?: HTMLDivElement;
private activeSeries: Series<TData>[];
private intervalControl?: ComponentRef<unknown>;
private summaryControl?: ComponentRef<unknown>;

public constructor(
private readonly series: Series<{}>[],
private readonly series: Series<TData>[],
private readonly injector: Injector,
private readonly intervalData?: CartesianIntervalData,
private readonly summaries: Summary[] = []
) {}
) {
this.activeSeries = [...this.series];
this.initialSeries = [...this.series];
this.activeSeries$ = this.activeSeriesSubject.asObservable().pipe(distinctUntilChanged(), startWith(this.series));
Copy link
Contributor

@aaron-steinfeld aaron-steinfeld Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This distinctUntilChanged doesn't solve our problem - see my previous comment about isEqual https://github.com/hypertrace/hypertrace-ui/pull/1270/files/d9be3992407b4436a1c84bc7b826c49942eeb888#r760277561

Copy link
Contributor Author

@itssharmasandeep itssharmasandeep Dec 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not working.
The problem I found it, distinctUntilChanged is not behaving the way we want. I tried to do this, to check where is the problem.

this.activeSeries$ = this.activeSeriesSubject.asObservable().pipe(
      tap(console.log),
      distinctUntilChanged((prev, curr) => {
        console.log(prev, curr);
        return prev.length === curr.length;
      })
    );

The problem I found was, prev is always the main series (the initial one).

I think, we can take this later as you mentioned, there are only two case where we would need this, only one series (this has already been taken care of where we disable the click behaviour), second when everything is selected and and then we do a reset. I think this is fine, It is also not very common to do, so we can skip

Copy link
Contributor

@aaron-steinfeld aaron-steinfeld Dec 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd expect that on the first call, but assuming the first call returns true, it shouldn't be in the future. Something else is up. This is a bug, so we should fix it, but can be done as a followup.

}

public draw(hostElement: Element, position: LegendPosition): this {
this.legendElement = this.drawLegendContainer(hostElement, position, this.intervalData !== undefined).node()!;
Expand All @@ -33,6 +49,7 @@ export class CartesianLegend {
}

this.drawLegendEntries(this.legendElement);
this.drawReset(this.legendElement);

if (this.intervalData) {
this.intervalControl = this.drawIntervalControl(this.legendElement, this.intervalData);
Expand All @@ -50,6 +67,20 @@ export class CartesianLegend {
this.summaryControl && this.summaryControl.destroy();
}

private drawReset(container: ContainerElement): void {
select(container)
.append('span')
.classed(CartesianLegend.RESET_CSS_CLASS, true)
.text('Reset')
.on('click', () => this.disableSelectionMode());

this.updateResetElementVisibility(!this.isSelectionModeOn);
}

private updateResetElementVisibility(isHidden: boolean): void {
select(this.legendElement!).select(`span.${CartesianLegend.RESET_CSS_CLASS}`).classed('hidden', isHidden);
}

private drawLegendEntries(container: ContainerElement): void {
select(container)
.append('div')
Expand Down Expand Up @@ -78,20 +109,46 @@ export class CartesianLegend {
.classed(`position-${legendPosition}`, true);
}

private drawLegendEntry(element: EnterElement): Selection<HTMLDivElement, Series<{}>, null, undefined> {
const legendEntry = select<EnterElement, Series<{}>>(element).append('div').classed('legend-entry', true);
private drawLegendEntry(element: EnterElement): Selection<HTMLDivElement, Series<TData>, null, undefined> {
const legendEntry = select<EnterElement, Series<TData>>(element).append('div').classed('legend-entry', true);

this.appendLegendSymbol(legendEntry);

legendEntry
.append('span')
.classed('legend-text', true)
.text(series => series.name);
.text(series => series.name)
.on('click', series => (this.series.length > 1 ? this.updateActiveSeries(series) : undefined));

this.updateLegendClassesAndStyle();

return legendEntry;
}

private appendLegendSymbol(selection: Selection<HTMLDivElement, Series<{}>, null, undefined>): void {
private updateLegendClassesAndStyle(): void {
const legendElementSelection = select(this.legendElement!);

// Legend entry symbol
legendElementSelection
.selectAll('.legend-symbol circle')
.style('fill', series =>
!this.isThisLegendEntryActive(series as Series<TData>) ? Color.Gray3 : (series as Series<TData>).color
);

// Legend entry value text
legendElementSelection
.selectAll('span.legend-text')
.classed(CartesianLegend.DEFAULT_CSS_CLASS, !this.isSelectionModeOn)
.classed(
CartesianLegend.ACTIVE_CSS_CLASS,
series => this.isSelectionModeOn && this.isThisLegendEntryActive(series as Series<TData>)
)
.classed(
CartesianLegend.INACTIVE_CSS_CLASS,
series => this.isSelectionModeOn && !this.isThisLegendEntryActive(series as Series<TData>)
);
}

private appendLegendSymbol(selection: Selection<HTMLDivElement, Series<TData>, null, undefined>): void {
selection
.append('svg')
.classed('legend-symbol', true)
Expand Down Expand Up @@ -133,4 +190,30 @@ export class CartesianLegend {
})
);
}

private disableSelectionMode(): void {
this.activeSeries = [...this.initialSeries];
this.isSelectionModeOn = false;
this.updateLegendClassesAndStyle();
this.updateResetElementVisibility(!this.isSelectionModeOn);
this.activeSeriesSubject.next(this.activeSeries);
}

private updateActiveSeries(seriesEntry: Series<TData>): void {
if (!this.isSelectionModeOn) {
this.activeSeries = [seriesEntry];
this.isSelectionModeOn = true;
} else if (this.isThisLegendEntryActive(seriesEntry)) {
this.activeSeries = this.activeSeries.filter(series => series !== seriesEntry);
} else {
this.activeSeries.push(seriesEntry);
}
this.updateLegendClassesAndStyle();
this.updateResetElementVisibility(!this.isSelectionModeOn);
this.activeSeriesSubject.next(this.activeSeries);
}

private isThisLegendEntryActive(seriesEntry: Series<TData>): boolean {
return this.activeSeries.includes(seriesEntry);
}
}