From a3be401f80e59cd96b433cae1494eba7511d3c43 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Thu, 18 Nov 2021 20:00:01 +0530 Subject: [PATCH 01/10] feat: toggle able legend for cartesian chart --- .../cartesian/cartesian-chart.component.scss | 30 ++++++- .../cartesian/d3/chart/cartesian-chart.ts | 33 ++++++- .../cartesian/d3/legend/cartesian-legend.ts | 85 ++++++++++++++++++- 3 files changed, 142 insertions(+), 6 deletions(-) diff --git a/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss b/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss index 709b50dc6..9fc751f1b 100644 --- a/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss +++ b/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss @@ -118,13 +118,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; } diff --git a/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts b/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts index 79b787686..0ae28b03c 100644 --- a/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts +++ b/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts @@ -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'; @@ -35,6 +36,7 @@ import { CartesianScaleBuilder } from '../scale/cartesian-scale-builder'; // tslint:disable:max-file-line-count export class DefaultCartesianChart implements CartesianChart { 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; @@ -65,6 +67,8 @@ export class DefaultCartesianChart implements CartesianChart { onEvent: ChartEventListener; }[] = []; + private activeSeriesSubscription?: Subscription; + public constructor( protected readonly hostElement: Element, protected readonly injector: Injector, @@ -80,6 +84,10 @@ export class DefaultCartesianChart implements CartesianChart { this.tooltip && this.tooltip.destroy(); this.legend && this.legend.destroy(); + if (this.activeSeriesSubscription) { + this.activeSeriesSubscription.unsubscribe(); + } + return this; } @@ -271,8 +279,10 @@ export class DefaultCartesianChart implements CartesianChart { } } - private updateData(): void { - this.drawLegend(); + private updateData(withLegend: boolean = true): void { + if (withLegend) { + this.drawLegend(); + } this.buildVisualizations(); this.drawChartBackground(); this.drawAxes(); @@ -283,6 +293,16 @@ export class DefaultCartesianChart implements CartesianChart { this.setupEventListeners(); } + private redrawVisualization(): void { + const chartViz = select(this.chartContainerElement!).selectAll( + `.${DefaultCartesianChart.CHART_VISUALIZATION_CLASS}` + ); + if (chartViz.nodes().length > 0) { + chartViz.remove(); + this.updateData(false); + } + } + private moveDataOnTopOfAxes(): void { if (!this.dataElement) { return; @@ -348,6 +368,14 @@ export class DefaultCartesianChart implements CartesianChart { this.chartContainerElement, this.legendPosition ); + if (this.activeSeriesSubscription) { + this.activeSeriesSubscription.unsubscribe(); + } + this.activeSeriesSubscription = this.legend.activeSeries$.subscribe(activeSeries => { + this.series.length = 0; + this.series.push(...(activeSeries as Series[])); + 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( @@ -370,6 +398,7 @@ export class DefaultCartesianChart implements CartesianChart { 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`) diff --git a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts index f93945369..b21f1689f 100644 --- a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts +++ b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts @@ -1,6 +1,8 @@ import { ComponentRef, Injector } from '@angular/core'; import { DynamicComponentService } from '@hypertrace/common'; import { ContainerElement, EnterElement, select, Selection } from 'd3-selection'; +import { Observable, of, Subject } from 'rxjs'; +import { startWith, switchMap } from 'rxjs/operators'; import { LegendPosition } from '../../../legend/legend.component'; import { Series, Summary } from '../../chart'; import { @@ -12,9 +14,19 @@ import { CartesianSummaryComponent, SUMMARIES_DATA } from './cartesian-summary.c export class CartesianLegend { 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[]>; + private readonly activeSeriesSubject: Subject = new Subject(); + private readonly initialSeries: Series<{}>[]; + + private isDefault: boolean = true; private legendElement?: HTMLDivElement; + private activeSeries: Series<{}>[]; private intervalControl?: ComponentRef; private summaryControl?: ComponentRef; @@ -23,7 +35,14 @@ export class CartesianLegend { 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( + startWith(undefined), + switchMap(() => of(this.activeSeries)) + ); + } public draw(hostElement: Element, position: LegendPosition): this { this.legendElement = this.drawLegendContainer(hostElement, position, this.intervalData !== undefined).node()!; @@ -33,6 +52,7 @@ export class CartesianLegend { } this.drawLegendEntries(this.legendElement); + this.drawReset(this.legendElement); if (this.intervalData) { this.intervalControl = this.drawIntervalControl(this.legendElement, this.intervalData); @@ -50,6 +70,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.resetToDefault()); + + this.toggleClearAll(this.isDefault); + } + + private toggleClearAll(isHidden: boolean): void { + select(this.legendElement!).select(`span.${CartesianLegend.RESET_CSS_CLASS}`).classed('hidden', isHidden); + } + private drawLegendEntries(container: ContainerElement): void { select(container) .append('div') @@ -82,15 +116,31 @@ export class CartesianLegend { const legendEntry = select>(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.updateActiveSeries(series)); + + this.updateLegendTextClasses(); return legendEntry; } + private updateLegendTextClasses(): void { + select(this.legendElement!) + .selectAll('span.legend-text') + .classed(CartesianLegend.DEFAULT_CSS_CLASS, this.isDefault) + .classed( + CartesianLegend.ACTIVE_CSS_CLASS, + series => !this.isDefault && this.isThisLegendEntryActive(series as Series<{}>) + ) + .classed( + CartesianLegend.INACTIVE_CSS_CLASS, + series => !this.isDefault && !this.isThisLegendEntryActive(series as Series<{}>) + ); + } + private appendLegendSymbol(selection: Selection, null, undefined>): void { selection .append('svg') @@ -133,4 +183,33 @@ export class CartesianLegend { }) ); } + + private resetToDefault(): void { + this.activeSeries = [...this.initialSeries]; + this.isDefault = true; + this.updateLegendTextClasses(); + this.toggleClearAll(this.isDefault); + this.activeSeriesSubject.next(); + } + + private updateActiveSeries(seriesEntry: Series<{}>): void { + if (this.isDefault) { + this.activeSeries = []; + this.activeSeries.push(seriesEntry); + this.isDefault = false; + } else { + if (this.isThisLegendEntryActive(seriesEntry)) { + this.activeSeries = this.activeSeries.filter(series => series !== seriesEntry); + } else { + this.activeSeries.push(seriesEntry); + } + } + this.updateLegendTextClasses(); + this.toggleClearAll(this.isDefault); + this.activeSeriesSubject.next(); + } + + private isThisLegendEntryActive(seriesEntry: Series<{}>): boolean { + return this.activeSeries.findIndex(series => series === seriesEntry) >= 0; + } } From 1fe78fe05483dc81c8219feaff41564a3ef990bd Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Thu, 18 Nov 2021 23:55:31 +0530 Subject: [PATCH 02/10] fix: add test cases --- .../cartesian-chart.component.test.ts | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/projects/observability/src/shared/components/cartesian/cartesian-chart.component.test.ts b/projects/observability/src/shared/components/cartesian/cartesian-chart.component.test.ts index 92c0c91ab..3e9b49904 100644 --- a/projects/observability/src/shared/components/cartesian/cartesian-chart.component.test.ts +++ b/projects/observability/src/shared/components/cartesian/cartesian-chart.component.test.ts @@ -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(``, { + 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(``, { hostProps: { From b53fe409115382a52fe6bd7f605f024f1f6e2bc5 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Fri, 19 Nov 2021 14:53:52 +0530 Subject: [PATCH 03/10] fix: addressing review comments --- .../components/cartesian/d3/legend/cartesian-legend.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts index b21f1689f..9df422f50 100644 --- a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts +++ b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts @@ -77,10 +77,10 @@ export class CartesianLegend { .text('Reset') .on('click', () => this.resetToDefault()); - this.toggleClearAll(this.isDefault); + this.toggleReset(this.isDefault); } - private toggleClearAll(isHidden: boolean): void { + private toggleReset(isHidden: boolean): void { select(this.legendElement!).select(`span.${CartesianLegend.RESET_CSS_CLASS}`).classed('hidden', isHidden); } @@ -188,7 +188,7 @@ export class CartesianLegend { this.activeSeries = [...this.initialSeries]; this.isDefault = true; this.updateLegendTextClasses(); - this.toggleClearAll(this.isDefault); + this.toggleReset(this.isDefault); this.activeSeriesSubject.next(); } @@ -205,7 +205,7 @@ export class CartesianLegend { } } this.updateLegendTextClasses(); - this.toggleClearAll(this.isDefault); + this.toggleReset(this.isDefault); this.activeSeriesSubject.next(); } From 747556eee0538d413f69adc9726316904a0a5392 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Mon, 22 Nov 2021 16:07:38 +0530 Subject: [PATCH 04/10] fix: addressing review comments --- .../components/cartesian/d3/chart/cartesian-chart.ts | 4 +--- .../components/cartesian/d3/legend/cartesian-legend.ts | 10 +++++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts b/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts index 0ae28b03c..4a34945fd 100644 --- a/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts +++ b/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts @@ -368,9 +368,7 @@ export class DefaultCartesianChart implements CartesianChart { this.chartContainerElement, this.legendPosition ); - if (this.activeSeriesSubscription) { - this.activeSeriesSubscription.unsubscribe(); - } + this.activeSeriesSubscription?.unsubscribe(); this.activeSeriesSubscription = this.legend.activeSeries$.subscribe(activeSeries => { this.series.length = 0; this.series.push(...(activeSeries as Series[])); diff --git a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts index 9df422f50..b084018c9 100644 --- a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts +++ b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts @@ -77,10 +77,10 @@ export class CartesianLegend { .text('Reset') .on('click', () => this.resetToDefault()); - this.toggleReset(this.isDefault); + this.setResetVisibility(this.isDefault); } - private toggleReset(isHidden: boolean): void { + private setResetVisibility(isHidden: boolean): void { select(this.legendElement!).select(`span.${CartesianLegend.RESET_CSS_CLASS}`).classed('hidden', isHidden); } @@ -188,7 +188,7 @@ export class CartesianLegend { this.activeSeries = [...this.initialSeries]; this.isDefault = true; this.updateLegendTextClasses(); - this.toggleReset(this.isDefault); + this.setResetVisibility(this.isDefault); this.activeSeriesSubject.next(); } @@ -205,11 +205,11 @@ export class CartesianLegend { } } this.updateLegendTextClasses(); - this.toggleReset(this.isDefault); + this.setResetVisibility(this.isDefault); this.activeSeriesSubject.next(); } private isThisLegendEntryActive(seriesEntry: Series<{}>): boolean { - return this.activeSeries.findIndex(series => series === seriesEntry) >= 0; + return this.activeSeries.includes(seriesEntry); } } From 09513b38d3ca32bc394c60cc3273866758c04501 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Mon, 22 Nov 2021 19:23:24 +0530 Subject: [PATCH 05/10] fix: addressing review comments --- .../cartesian/d3/chart/cartesian-chart.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts b/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts index 4a34945fd..52ad52bec 100644 --- a/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts +++ b/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts @@ -68,6 +68,7 @@ export class DefaultCartesianChart implements CartesianChart { }[] = []; private activeSeriesSubscription?: Subscription; + private activeSeries: Series[] = []; public constructor( protected readonly hostElement: Element, @@ -112,6 +113,7 @@ export class DefaultCartesianChart implements CartesianChart { public withSeries(...series: Series[]): this { this.series.length = 0; this.series.push(...series); + this.activeSeries = [...series]; this.seriesSummaries.length = 0; this.seriesSummaries.push( @@ -358,20 +360,21 @@ export class DefaultCartesianChart implements CartesianChart { 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( + this.activeSeries, + this.injector, + this.intervalData, + this.seriesSummaries + ).draw(this.chartContainerElement, this.legendPosition); this.activeSeriesSubscription?.unsubscribe(); this.activeSeriesSubscription = this.legend.activeSeries$.subscribe(activeSeries => { - this.series.length = 0; - this.series.push(...(activeSeries as Series[])); + this.activeSeries = activeSeries as Series[]; this.redrawVisualization(); }); } else { @@ -457,7 +460,7 @@ export class DefaultCartesianChart implements CartesianChart { 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), From 805d7e1820fadf66984b6adb635b87fc62380a7e Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Fri, 26 Nov 2021 16:40:56 +0530 Subject: [PATCH 06/10] fix: some style changes --- .../cartesian/cartesian-chart.component.scss | 1 + .../cartesian/d3/legend/cartesian-legend.ts | 22 ++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss b/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss index 9fc751f1b..7bf1ceae5 100644 --- a/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss +++ b/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss @@ -55,6 +55,7 @@ width: 100%; position: absolute; min-height: 48px; + padding-bottom: 20px; &.position-none { display: none; diff --git a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts index b084018c9..a915316f9 100644 --- a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts +++ b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts @@ -1,5 +1,5 @@ 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, of, Subject } from 'rxjs'; import { startWith, switchMap } from 'rxjs/operators'; @@ -122,13 +122,23 @@ export class CartesianLegend { .text(series => series.name) .on('click', series => this.updateActiveSeries(series)); - this.updateLegendTextClasses(); + this.updateLegendClassesAndStyle(); return legendEntry; } - private updateLegendTextClasses(): void { - select(this.legendElement!) + private updateLegendClassesAndStyle(): void { + const legendElementSelection = select(this.legendElement!); + + // Legend entry symbol + legendElementSelection + .selectAll('.legend-symbol circle') + .style('fill', series => + !this.isThisLegendEntryActive(series as Series<{}>) ? Color.Gray3 : (series as Series<{}>).color + ); + + // Legend entry value text + legendElementSelection .selectAll('span.legend-text') .classed(CartesianLegend.DEFAULT_CSS_CLASS, this.isDefault) .classed( @@ -187,7 +197,7 @@ export class CartesianLegend { private resetToDefault(): void { this.activeSeries = [...this.initialSeries]; this.isDefault = true; - this.updateLegendTextClasses(); + this.updateLegendClassesAndStyle(); this.setResetVisibility(this.isDefault); this.activeSeriesSubject.next(); } @@ -204,7 +214,7 @@ export class CartesianLegend { this.activeSeries.push(seriesEntry); } } - this.updateLegendTextClasses(); + this.updateLegendClassesAndStyle(); this.setResetVisibility(this.isDefault); this.activeSeriesSubject.next(); } From d9be3992407b4436a1c84bc7b826c49942eeb888 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Wed, 1 Dec 2021 18:39:18 +0530 Subject: [PATCH 07/10] fix: addressing review comments --- .../cartesian/d3/chart/cartesian-chart.ts | 12 ++++++---- .../cartesian/d3/legend/cartesian-legend.ts | 24 +++++++++---------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts b/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts index 52ad52bec..4607be66b 100644 --- a/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts +++ b/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts @@ -281,10 +281,12 @@ export class DefaultCartesianChart implements CartesianChart { } } - private updateData(withLegend: boolean = true): void { - if (withLegend) { - this.drawLegend(); - } + private updateData(): void { + this.drawLegend(); + this.drawVisualizations(); + } + + private drawVisualizations(): void { this.buildVisualizations(); this.drawChartBackground(); this.drawAxes(); @@ -301,7 +303,7 @@ export class DefaultCartesianChart implements CartesianChart { ); if (chartViz.nodes().length > 0) { chartViz.remove(); - this.updateData(false); + this.drawVisualizations(); } } diff --git a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts index a915316f9..60a489612 100644 --- a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts +++ b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts @@ -24,7 +24,7 @@ export class CartesianLegend { private readonly activeSeriesSubject: Subject = new Subject(); private readonly initialSeries: Series<{}>[]; - private isDefault: boolean = true; + private isSelectionModeOn: boolean = false; private legendElement?: HTMLDivElement; private activeSeries: Series<{}>[]; private intervalControl?: ComponentRef; @@ -75,9 +75,9 @@ export class CartesianLegend { .append('span') .classed(CartesianLegend.RESET_CSS_CLASS, true) .text('Reset') - .on('click', () => this.resetToDefault()); + .on('click', () => this.makeSelectionModeOff()); - this.setResetVisibility(this.isDefault); + this.setResetVisibility(!this.isSelectionModeOn); } private setResetVisibility(isHidden: boolean): void { @@ -140,14 +140,14 @@ export class CartesianLegend { // Legend entry value text legendElementSelection .selectAll('span.legend-text') - .classed(CartesianLegend.DEFAULT_CSS_CLASS, this.isDefault) + .classed(CartesianLegend.DEFAULT_CSS_CLASS, !this.isSelectionModeOn) .classed( CartesianLegend.ACTIVE_CSS_CLASS, - series => !this.isDefault && this.isThisLegendEntryActive(series as Series<{}>) + series => this.isSelectionModeOn && this.isThisLegendEntryActive(series as Series<{}>) ) .classed( CartesianLegend.INACTIVE_CSS_CLASS, - series => !this.isDefault && !this.isThisLegendEntryActive(series as Series<{}>) + series => this.isSelectionModeOn && !this.isThisLegendEntryActive(series as Series<{}>) ); } @@ -194,19 +194,19 @@ export class CartesianLegend { ); } - private resetToDefault(): void { + private makeSelectionModeOff(): void { this.activeSeries = [...this.initialSeries]; - this.isDefault = true; + this.isSelectionModeOn = false; this.updateLegendClassesAndStyle(); - this.setResetVisibility(this.isDefault); + this.setResetVisibility(!this.isSelectionModeOn); this.activeSeriesSubject.next(); } private updateActiveSeries(seriesEntry: Series<{}>): void { - if (this.isDefault) { + if (!this.isSelectionModeOn) { this.activeSeries = []; this.activeSeries.push(seriesEntry); - this.isDefault = false; + this.isSelectionModeOn = true; } else { if (this.isThisLegendEntryActive(seriesEntry)) { this.activeSeries = this.activeSeries.filter(series => series !== seriesEntry); @@ -215,7 +215,7 @@ export class CartesianLegend { } } this.updateLegendClassesAndStyle(); - this.setResetVisibility(this.isDefault); + this.setResetVisibility(!this.isSelectionModeOn); this.activeSeriesSubject.next(); } From a9b166b63aa3ff0bab7cb53be089531ea56a096d Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Wed, 1 Dec 2021 22:40:42 +0530 Subject: [PATCH 08/10] fix: suggested changes --- .../cartesian/d3/chart/cartesian-chart.ts | 6 +- .../cartesian/d3/legend/cartesian-legend.ts | 68 +++++++++---------- 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts b/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts index 4607be66b..9bdbb3f23 100644 --- a/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts +++ b/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts @@ -47,7 +47,7 @@ export class DefaultCartesianChart implements CartesianChart { protected chartBackgroundSvgElement?: SVGSVGElement; protected dataElement?: ContainerElement; protected mouseEventContainer?: SVGSVGElement; - protected legend?: CartesianLegend; + protected legend?: CartesianLegend; protected tooltip?: ChartTooltipRef; protected allSeriesData: CartesianData>[] = []; protected allCartesianData: CartesianData | Band>[] = []; @@ -368,7 +368,7 @@ export class DefaultCartesianChart implements CartesianChart { private drawLegend(): void { if (this.chartContainerElement) { if (this.legendPosition !== undefined && this.legendPosition !== LegendPosition.None) { - this.legend = new CartesianLegend( + this.legend = new CartesianLegend( this.activeSeries, this.injector, this.intervalData, @@ -381,7 +381,7 @@ export class DefaultCartesianChart implements CartesianChart { }); } 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([], this.injector, this.intervalData, this.seriesSummaries).draw( this.chartContainerElement, LegendPosition.None ); diff --git a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts index 60a489612..08eaa1db6 100644 --- a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts +++ b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts @@ -1,8 +1,8 @@ import { ComponentRef, Injector } from '@angular/core'; import { Color, DynamicComponentService } from '@hypertrace/common'; import { ContainerElement, EnterElement, select, Selection } from 'd3-selection'; -import { Observable, of, Subject } from 'rxjs'; -import { startWith, switchMap } from 'rxjs/operators'; +import { Observable, Subject } from 'rxjs'; +import { distinctUntilChanged, startWith } from 'rxjs/operators'; import { LegendPosition } from '../../../legend/legend.component'; import { Series, Summary } from '../../chart'; import { @@ -12,7 +12,7 @@ import { } from './cartesian-interval-control.component'; import { CartesianSummaryComponent, SUMMARIES_DATA } from './cartesian-summary.component'; -export class CartesianLegend { +export class CartesianLegend { private static readonly CSS_CLASS: string = 'legend'; private static readonly RESET_CSS_CLASS: string = 'reset'; private static readonly DEFAULT_CSS_CLASS: string = 'default'; @@ -20,28 +20,25 @@ export class CartesianLegend { private static readonly INACTIVE_CSS_CLASS: string = 'inactive'; public static readonly CSS_SELECTOR: string = `.${CartesianLegend.CSS_CLASS}`; - public readonly activeSeries$: Observable[]>; - private readonly activeSeriesSubject: Subject = new Subject(); - private readonly initialSeries: Series<{}>[]; + public readonly activeSeries$: Observable[]>; + private readonly activeSeriesSubject: Subject[]> = new Subject(); + private readonly initialSeries: Series[]; private isSelectionModeOn: boolean = false; private legendElement?: HTMLDivElement; - private activeSeries: Series<{}>[]; + private activeSeries: Series[]; private intervalControl?: ComponentRef; private summaryControl?: ComponentRef; public constructor( - private readonly series: Series<{}>[], + private readonly series: Series[], 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( - startWith(undefined), - switchMap(() => of(this.activeSeries)) - ); + this.activeSeries$ = this.activeSeriesSubject.asObservable().pipe(distinctUntilChanged(), startWith(this.series)); } public draw(hostElement: Element, position: LegendPosition): this { @@ -75,12 +72,12 @@ export class CartesianLegend { .append('span') .classed(CartesianLegend.RESET_CSS_CLASS, true) .text('Reset') - .on('click', () => this.makeSelectionModeOff()); + .on('click', () => this.disableSelectionMode()); - this.setResetVisibility(!this.isSelectionModeOn); + this.updateResetElementVisibility(!this.isSelectionModeOn); } - private setResetVisibility(isHidden: boolean): void { + private updateResetElementVisibility(isHidden: boolean): void { select(this.legendElement!).select(`span.${CartesianLegend.RESET_CSS_CLASS}`).classed('hidden', isHidden); } @@ -112,15 +109,15 @@ export class CartesianLegend { .classed(`position-${legendPosition}`, true); } - private drawLegendEntry(element: EnterElement): Selection, null, undefined> { - const legendEntry = select>(element).append('div').classed('legend-entry', true); + private drawLegendEntry(element: EnterElement): Selection, null, undefined> { + const legendEntry = select>(element).append('div').classed('legend-entry', true); this.appendLegendSymbol(legendEntry); legendEntry .append('span') .classed('legend-text', true) .text(series => series.name) - .on('click', series => this.updateActiveSeries(series)); + .on('click', series => (this.series.length > 1 ? this.updateActiveSeries(series) : null)); this.updateLegendClassesAndStyle(); @@ -128,13 +125,13 @@ export class CartesianLegend { } private updateLegendClassesAndStyle(): void { - const legendElementSelection = select(this.legendElement!); + const legendElementSelection = select>(this.legendElement!); // Legend entry symbol legendElementSelection .selectAll('.legend-symbol circle') .style('fill', series => - !this.isThisLegendEntryActive(series as Series<{}>) ? Color.Gray3 : (series as Series<{}>).color + !this.isThisLegendEntryActive(series as Series) ? Color.Gray3 : (series as Series).color ); // Legend entry value text @@ -143,15 +140,15 @@ export class CartesianLegend { .classed(CartesianLegend.DEFAULT_CSS_CLASS, !this.isSelectionModeOn) .classed( CartesianLegend.ACTIVE_CSS_CLASS, - series => this.isSelectionModeOn && this.isThisLegendEntryActive(series as Series<{}>) + series => this.isSelectionModeOn && this.isThisLegendEntryActive(series as Series) ) .classed( CartesianLegend.INACTIVE_CSS_CLASS, - series => this.isSelectionModeOn && !this.isThisLegendEntryActive(series as Series<{}>) + series => this.isSelectionModeOn && !this.isThisLegendEntryActive(series as Series) ); } - private appendLegendSymbol(selection: Selection, null, undefined>): void { + private appendLegendSymbol(selection: Selection, null, undefined>): void { selection .append('svg') .classed('legend-symbol', true) @@ -194,32 +191,29 @@ export class CartesianLegend { ); } - private makeSelectionModeOff(): void { + private disableSelectionMode(): void { this.activeSeries = [...this.initialSeries]; this.isSelectionModeOn = false; this.updateLegendClassesAndStyle(); - this.setResetVisibility(!this.isSelectionModeOn); - this.activeSeriesSubject.next(); + this.updateResetElementVisibility(!this.isSelectionModeOn); + this.activeSeriesSubject.next(this.activeSeries); } - private updateActiveSeries(seriesEntry: Series<{}>): void { + private updateActiveSeries(seriesEntry: Series): void { if (!this.isSelectionModeOn) { - this.activeSeries = []; - this.activeSeries.push(seriesEntry); + this.activeSeries = [seriesEntry]; this.isSelectionModeOn = true; + } else if (this.isThisLegendEntryActive(seriesEntry)) { + this.activeSeries = this.activeSeries.filter(series => series !== seriesEntry); } else { - if (this.isThisLegendEntryActive(seriesEntry)) { - this.activeSeries = this.activeSeries.filter(series => series !== seriesEntry); - } else { - this.activeSeries.push(seriesEntry); - } + this.activeSeries.push(seriesEntry); } this.updateLegendClassesAndStyle(); - this.setResetVisibility(!this.isSelectionModeOn); - this.activeSeriesSubject.next(); + this.updateResetElementVisibility(!this.isSelectionModeOn); + this.activeSeriesSubject.next(this.activeSeries); } - private isThisLegendEntryActive(seriesEntry: Series<{}>): boolean { + private isThisLegendEntryActive(seriesEntry: Series): boolean { return this.activeSeries.includes(seriesEntry); } } From c78844b18b313bb7913331aef73203b089721eec Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Wed, 1 Dec 2021 22:42:24 +0530 Subject: [PATCH 09/10] fix: lint fixes --- .../shared/components/cartesian/d3/chart/cartesian-chart.ts | 2 +- .../shared/components/cartesian/d3/legend/cartesian-legend.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts b/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts index 9bdbb3f23..2ba8c615b 100644 --- a/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts +++ b/projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts @@ -376,7 +376,7 @@ export class DefaultCartesianChart implements CartesianChart { ).draw(this.chartContainerElement, this.legendPosition); this.activeSeriesSubscription?.unsubscribe(); this.activeSeriesSubscription = this.legend.activeSeries$.subscribe(activeSeries => { - this.activeSeries = activeSeries as Series[]; + this.activeSeries = activeSeries; this.redrawVisualization(); }); } else { diff --git a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts index 08eaa1db6..8c323bcdf 100644 --- a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts +++ b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts @@ -117,7 +117,7 @@ export class CartesianLegend { .append('span') .classed('legend-text', true) .text(series => series.name) - .on('click', series => (this.series.length > 1 ? this.updateActiveSeries(series) : null)); + .on('click', series => (this.series.length > 1 ? this.updateActiveSeries(series) : undefined)); this.updateLegendClassesAndStyle(); @@ -125,7 +125,7 @@ export class CartesianLegend { } private updateLegendClassesAndStyle(): void { - const legendElementSelection = select>(this.legendElement!); + const legendElementSelection = select(this.legendElement!); // Legend entry symbol legendElementSelection From c2f177bc94107a0ab08f70c041bde655df0cf34b Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Thu, 2 Dec 2021 14:09:32 +0530 Subject: [PATCH 10/10] fix: addressing review comments --- .../components/cartesian/cartesian-chart.component.scss | 5 ++++- .../components/cartesian/d3/legend/cartesian-legend.ts | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss b/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss index 7bf1ceae5..202eb55c4 100644 --- a/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss +++ b/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss @@ -121,7 +121,10 @@ .legend-text { font-size: 14px; padding-left: 2px; - cursor: pointer; + + &.selectable { + cursor: pointer; + } &.default { color: $gray-9; diff --git a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts index 8c323bcdf..2321c7ad7 100644 --- a/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts +++ b/projects/observability/src/shared/components/cartesian/d3/legend/cartesian-legend.ts @@ -2,7 +2,7 @@ import { ComponentRef, Injector } from '@angular/core'; 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 { startWith } from 'rxjs/operators'; import { LegendPosition } from '../../../legend/legend.component'; import { Series, Summary } from '../../chart'; import { @@ -15,6 +15,7 @@ import { CartesianSummaryComponent, SUMMARIES_DATA } from './cartesian-summary.c export class CartesianLegend { private static readonly CSS_CLASS: string = 'legend'; private static readonly RESET_CSS_CLASS: string = 'reset'; + private static readonly SELECTABLE_CSS_CLASS: string = 'selectable'; private static readonly DEFAULT_CSS_CLASS: string = 'default'; private static readonly ACTIVE_CSS_CLASS: string = 'active'; private static readonly INACTIVE_CSS_CLASS: string = 'inactive'; @@ -38,7 +39,7 @@ export class CartesianLegend { ) { this.activeSeries = [...this.series]; this.initialSeries = [...this.series]; - this.activeSeries$ = this.activeSeriesSubject.asObservable().pipe(distinctUntilChanged(), startWith(this.series)); + this.activeSeries$ = this.activeSeriesSubject.asObservable().pipe(startWith(this.series)); } public draw(hostElement: Element, position: LegendPosition): this { @@ -116,6 +117,7 @@ export class CartesianLegend { legendEntry .append('span') .classed('legend-text', true) + .classed(CartesianLegend.SELECTABLE_CSS_CLASS, this.series.length > 1) .text(series => series.name) .on('click', series => (this.series.length > 1 ? this.updateActiveSeries(series) : undefined));