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/28] 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/28] 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/28] 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/28] 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/28] 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 a33b271776dba09762d28c51e4142c147694d371 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Fri, 26 Nov 2021 16:22:58 +0530 Subject: [PATCH 06/28] feat: grouped cartesian legend --- .../cartesian/cartesian-chart.component.scss | 34 +++++ .../src/shared/components/cartesian/chart.ts | 7 + .../cartesian/d3/legend/cartesian-legend.ts | 134 +++++++++++++++--- .../explore-cartesian-data-source.model.ts | 26 +++- 4 files changed, 178 insertions(+), 23 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..076e8d8f7 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; @@ -153,6 +154,39 @@ } } + &.grouped { + flex-direction: column; + gap: 12px; + + .legend-entries { + flex-direction: row; + display: grid; + grid-template-columns: 200px auto; + gap: 20px; + border: 1px solid $gray-2; + border-radius: 8px; + padding: 8px 20px; + + &.active { + border-color: $blue-5; + } + + .legend-entries-title { + @include body-1-regular($gray-5); + cursor: pointer; + + &.active { + color: $blue-5; + } + } + + .legend-entry-values { + display: flex; + flex-wrap: wrap; + } + } + } + .interval-control { padding: 0 8px; } diff --git a/projects/observability/src/shared/components/cartesian/chart.ts b/projects/observability/src/shared/components/cartesian/chart.ts index 6331dcc27..f4fb5cf58 100644 --- a/projects/observability/src/shared/components/cartesian/chart.ts +++ b/projects/observability/src/shared/components/cartesian/chart.ts @@ -26,6 +26,7 @@ export interface Series { // Override the default color string using a method that takes data point as input getColor?(datum?: TInterval): string; name: string; + assignedGroup?: AssignedGroup; symbol?: SeriesSymbol; type: CartesianSeriesVisualizationType; stacking?: boolean; @@ -33,6 +34,12 @@ export interface Series { getTooltipTitle?(datum: TInterval): string; } +export interface AssignedGroup { + id: number; + specName: string; + groupName: string; +} + export interface Band { upper: Series; lower: 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 b084018c9..1d38bd093 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,7 @@ import { ComponentRef, Injector } from '@angular/core'; import { DynamicComponentService } from '@hypertrace/common'; import { ContainerElement, EnterElement, select, Selection } from 'd3-selection'; +import { isNil } from 'lodash'; import { Observable, of, Subject } from 'rxjs'; import { startWith, switchMap } from 'rxjs/operators'; import { LegendPosition } from '../../../legend/legend.component'; @@ -23,8 +24,11 @@ export class CartesianLegend { public readonly activeSeries$: Observable[]>; private readonly activeSeriesSubject: Subject = new Subject(); private readonly initialSeries: Series<{}>[]; + private readonly groupedSeries: Series<{}>[][]; + private readonly numberOfgroups: number; private isDefault: boolean = true; + private isGrouped: boolean = true; private legendElement?: HTMLDivElement; private activeSeries: Series<{}>[]; private intervalControl?: ComponentRef; @@ -36,6 +40,14 @@ export class CartesianLegend { private readonly intervalData?: CartesianIntervalData, private readonly summaries: Summary[] = [] ) { + // TODO (Sandeep): Create initialization method instead + this.numberOfgroups = + this.series.length === 0 || isNil(this.series[0].assignedGroup) + ? 0 + : this.series[this.series.length - 1].assignedGroup!.id; + this.isGrouped = this.numberOfgroups !== 0; + this.groupedSeries = this.getGroupedSeries(); + this.activeSeries = [...this.series]; this.initialSeries = [...this.series]; this.activeSeries$ = this.activeSeriesSubject.asObservable().pipe( @@ -85,13 +97,43 @@ export class CartesianLegend { } private drawLegendEntries(container: ContainerElement): void { - select(container) - .append('div') - .classed('legend-entries', true) - .selectAll('.legend-entry') - .data(this.series.filter(series => !series.hide)) - .enter() - .each((_, index, elements) => this.drawLegendEntry(elements[index])); + const containerSelection = select(container); + if (!this.isGrouped) { + containerSelection + .append('div') + .classed('legend-entries', true) + .selectAll('.legend-entry') + .data(this.series.filter(series => !series.hide)) + .enter() + .each((_, index, elements) => this.drawLegendEntry(elements[index])); + } else { + containerSelection + .selectAll('.legend-entries') + .data(this.groupedSeries) + .enter() + .append('div') + .attr('class', seriesGroup => `legend-entries group-${seriesGroup[0].assignedGroup!.id}`) + .each((seriesGroup, index, elements) => { + const legendEntriesSelection = select(elements[index]); + + legendEntriesSelection + .selectAll('.legend-entries-title') + .data([seriesGroup]) + .enter() + .append('div') + .classed('legend-entries-title', true) + .text(group => `${group[0].assignedGroup!.specName}:`) + .on('click', () => this.updateActiveSeries(seriesGroup)); + + legendEntriesSelection + .append('div') + .classed('legend-entry-values', true) + .selectAll('.legend-entry') + .data(seriesGroup) + .enter() + .each((_, index, elements) => this.drawLegendEntry(elements[index])); + }); + } } private drawLegendContainer( @@ -109,7 +151,8 @@ export class CartesianLegend { return select(hostElement) .append('div') .classed(CartesianLegend.CSS_CLASS, true) - .classed(`position-${legendPosition}`, true); + .classed(`position-${legendPosition}`, true) + .classed('grouped', this.isGrouped); } private drawLegendEntry(element: EnterElement): Selection, null, undefined> { @@ -119,15 +162,32 @@ export class CartesianLegend { legendEntry .append('span') .classed('legend-text', true) - .text(series => series.name) - .on('click', series => this.updateActiveSeries(series)); + .text(series => (!this.isGrouped ? series.name : series.assignedGroup!.groupName)) + .on('click', series => this.updateActiveSeries([series])); - this.updateLegendTextClasses(); + this.updateLegendClasses(); return legendEntry; } - private updateLegendTextClasses(): void { + private updateLegendClasses(): void { + if (this.isGrouped) { + // Legend entries + select(this.legendElement!) + .selectAll('.legend-entries') + .classed(CartesianLegend.ACTIVE_CSS_CLASS, seriesGroup => + this.isThisLegendSeriesGroupActive(seriesGroup as Series<{}>[]) + ); + + // Legend entry title + select(this.legendElement!) + .selectAll('.legend-entries-title') + .classed(CartesianLegend.ACTIVE_CSS_CLASS, seriesGroup => + this.isThisLegendSeriesGroupActive(seriesGroup as Series<{}>[]) + ); + } + + // Legend entry value text select(this.legendElement!) .selectAll('span.legend-text') .classed(CartesianLegend.DEFAULT_CSS_CLASS, this.isDefault) @@ -187,24 +247,52 @@ export class CartesianLegend { private resetToDefault(): void { this.activeSeries = [...this.initialSeries]; this.isDefault = true; - this.updateLegendTextClasses(); + this.updateLegendClasses(); this.setResetVisibility(this.isDefault); this.activeSeriesSubject.next(); } - private updateActiveSeries(seriesEntry: Series<{}>): void { + private getGroupedSeries(): Series<{}>[][] { + if (!this.isGrouped) { + return []; + } + + const nonHiddenSeries = this.series.filter(series => !series.hide); + const groupedSeries: Series<{}>[][] = []; + const currentGroup: Series<{}>[] = []; + + nonHiddenSeries.forEach(seriesEntry => { + if ( + currentGroup.length === 0 || + currentGroup[currentGroup.length - 1].assignedGroup!.id === seriesEntry.assignedGroup!.id + ) { + currentGroup.push(seriesEntry); + } else { + groupedSeries.push([...currentGroup]); + currentGroup.length = 0; + currentGroup.push(seriesEntry); + } + }); + groupedSeries.push([...currentGroup]); + + return groupedSeries; + } + + private updateActiveSeries(seriesEntries: Series<{}>[]): void { if (this.isDefault) { this.activeSeries = []; - this.activeSeries.push(seriesEntry); + this.activeSeries.push(...seriesEntries); this.isDefault = false; } else { - if (this.isThisLegendEntryActive(seriesEntry)) { - this.activeSeries = this.activeSeries.filter(series => series !== seriesEntry); - } else { - this.activeSeries.push(seriesEntry); - } + seriesEntries.forEach(seriesEntry => { + if (this.isThisLegendEntryActive(seriesEntry)) { + this.activeSeries = this.activeSeries.filter(series => series !== seriesEntry); + } else { + this.activeSeries.push(seriesEntry); + } + }); } - this.updateLegendTextClasses(); + this.updateLegendClasses(); this.setResetVisibility(this.isDefault); this.activeSeriesSubject.next(); } @@ -212,4 +300,8 @@ export class CartesianLegend { private isThisLegendEntryActive(seriesEntry: Series<{}>): boolean { return this.activeSeries.includes(seriesEntry); } + + private isThisLegendSeriesGroupActive(seriesGroup: Series<{}>[]): boolean { + return this.isDefault ? false : seriesGroup.every(series => this.activeSeries.includes(series)); + } } diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts index 2cafb5bea..7c7837b29 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts @@ -30,6 +30,7 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM private readonly metadataService!: MetadataService; protected abstract buildRequestState(interval: TimeDuration | 'AUTO'): ExploreRequestState | undefined; + private readonly allGroups: string[] = []; public getData(): Observable> { return of({ @@ -97,7 +98,7 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM const seriesData = this.gatherSeriesData(request, result); const colors = this.colorService.getColorPalette().forNColors(seriesData.length); - return forkJoinSafeEmpty(seriesData.map((data, index) => this.buildSeries(request, data, colors[index]))); + return forkJoinSafeEmpty(seriesData.map((data, index) => this.buildSeries(request, data, colors[index], index))); } private gatherSeriesData(request: ExploreRequestState, result: ExploreResult): SeriesData[] { @@ -125,7 +126,12 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM return []; } - private buildSeries(request: ExploreRequestState, result: SeriesData, color: string): Observable { + private buildSeries( + request: ExploreRequestState, + result: SeriesData, + color: string, + seriesIndex: number + ): Observable { return forkJoinSafeEmpty({ specDisplayName: this.metadataService.getSpecificationDisplayName(request.context, result.spec), attribute: this.metadataService.getAttribute(request.context, result.spec.name) @@ -139,6 +145,13 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM : request.useGroupName ? result.groupName! : `${obj.specDisplayName}: ${result.groupName}`, + assignedGroup: !isEmpty(result.groupName) + ? { + id: this.getGroupId(result.groupName!, seriesIndex), + specName: obj.specDisplayName, + groupName: result.groupName! + } + : undefined, color: color })) ); @@ -177,6 +190,15 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM private buildGroupedSeriesName(groupKeys: string[]): string { return groupKeys.join(', '); } + + private getGroupId(groupName: string, index: number): number { + if (this.allGroups.length === 0 || !this.allGroups.includes(groupName)) { + this.allGroups.push(groupName); + return 1; + } + + return Math.ceil((index + 1) / this.allGroups.length); + } } export type ExplorerData = MetricTimeseriesInterval | [string, number]; From 805d7e1820fadf66984b6adb635b87fc62380a7e Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Fri, 26 Nov 2021 16:40:56 +0530 Subject: [PATCH 07/28] 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 0244802b81f5f8a588d2841f241a0b5beebb2bf0 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Fri, 26 Nov 2021 18:10:04 +0530 Subject: [PATCH 08/28] fix: code refactoring and lint fixes --- .../cartesian/d3/legend/cartesian-legend.ts | 44 ++++++++++++------- .../explore-cartesian-data-source.model.ts | 1 + 2 files changed, 29 insertions(+), 16 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 ea0508ef8..1e795837c 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,7 +1,7 @@ import { ComponentRef, Injector } from '@angular/core'; import { Color, DynamicComponentService } from '@hypertrace/common'; import { ContainerElement, EnterElement, select, Selection } from 'd3-selection'; -import { isNil } from 'lodash'; +import { isNil } from 'lodash-es'; import { Observable, of, Subject } from 'rxjs'; import { startWith, switchMap } from 'rxjs/operators'; import { LegendPosition } from '../../../legend/legend.component'; @@ -28,7 +28,7 @@ export class CartesianLegend { private readonly numberOfgroups: number; private isDefault: boolean = true; - private isGrouped: boolean = true; + private readonly isGrouped: boolean = true; private legendElement?: HTMLDivElement; private activeSeries: Series<{}>[]; private intervalControl?: ComponentRef; @@ -40,7 +40,6 @@ export class CartesianLegend { private readonly intervalData?: CartesianIntervalData, private readonly summaries: Summary[] = [] ) { - // TODO (Sandeep): Create initialization method instead this.numberOfgroups = this.series.length === 0 || isNil(this.series[0].assignedGroup) ? 0 @@ -113,8 +112,8 @@ export class CartesianLegend { .enter() .append('div') .attr('class', seriesGroup => `legend-entries group-${seriesGroup[0].assignedGroup!.id}`) - .each((seriesGroup, index, elements) => { - const legendEntriesSelection = select(elements[index]); + .each((seriesGroup, id, elems) => { + const legendEntriesSelection = select(elems[id]); legendEntriesSelection .selectAll('.legend-entries-title') @@ -163,7 +162,7 @@ export class CartesianLegend { .append('span') .classed('legend-text', true) .text(series => (!this.isGrouped ? series.name : series.assignedGroup!.groupName)) - .on('click', series => this.updateActiveSeries([series])); + .on('click', series => this.updateActiveSeries(series)); this.updateLegendClassesAndStyle(); @@ -286,19 +285,32 @@ export class CartesianLegend { return groupedSeries; } - private updateActiveSeries(seriesEntries: Series<{}>[]): void { - if (this.isDefault) { - this.activeSeries = []; - this.activeSeries.push(...seriesEntries); - this.isDefault = false; + private updateActiveSeries(series: Series<{}> | Series<{}>[]): void { + if (series instanceof Array) { + if (this.isDefault) { + this.activeSeries = []; + this.activeSeries.push(...series); + this.isDefault = false; + } else { + if (!this.isThisLegendSeriesGroupActive(series)) { + this.activeSeries = this.activeSeries.filter(seriesEntry => !series.includes(seriesEntry)); + this.activeSeries.push(...series); + } else { + this.activeSeries = this.activeSeries.filter(seriesEntry => !series.includes(seriesEntry)); + } + } } else { - seriesEntries.forEach(seriesEntry => { - if (this.isThisLegendEntryActive(seriesEntry)) { - this.activeSeries = this.activeSeries.filter(series => series !== seriesEntry); + if (this.isDefault) { + this.activeSeries = []; + this.activeSeries.push(series); + this.isDefault = false; + } else { + if (this.isThisLegendEntryActive(series)) { + this.activeSeries = this.activeSeries.filter(seriesEntry => series !== seriesEntry); } else { - this.activeSeries.push(seriesEntry); + this.activeSeries.push(series); } - }); + } } this.updateLegendClassesAndStyle(); this.setResetVisibility(this.isDefault); diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts index 7c7837b29..fc43721d6 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts @@ -194,6 +194,7 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM private getGroupId(groupName: string, index: number): number { if (this.allGroups.length === 0 || !this.allGroups.includes(groupName)) { this.allGroups.push(groupName); + return 1; } From 458fa52782fc36336daac86945b450746f83dce3 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Fri, 26 Nov 2021 18:22:11 +0530 Subject: [PATCH 09/28] fix: test cases --- .../explore-cartesian-data-source.model.test.ts | 15 +++++++++++++-- ...ualization-cartesian-data-source.model.test.ts | 15 +++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts index dc492c64f..0178aa3a4 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts @@ -1,3 +1,4 @@ +// import { ColorService, FixedTimeRange, TimeDuration, TimeUnit } from '@hypertrace/common'; import { createModelFactory } from '@hypertrace/dashboards/testing'; import { Model } from '@hypertrace/hyperdash'; @@ -295,7 +296,12 @@ describe('Explore cartesian data source model', () => { timestamp: secondIntervalTime, value: 15 } - ] + ], + assignedGroup: { + id: 1, + groupName: 'first', + specName: 'sum(foo)' + } }, { color: 'second color', @@ -314,7 +320,12 @@ describe('Explore cartesian data source model', () => { timestamp: secondIntervalTime, value: 25 } - ] + ], + assignedGroup: { + id: 1, + groupName: 'second', + specName: 'sum(foo)' + } } ], bands: [] diff --git a/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts index 5a251f1db..6652fd736 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts @@ -1,3 +1,4 @@ +// import { ColorService, FixedTimeRange, TimeDuration, TimeUnit } from '@hypertrace/common'; import { createModelFactory } from '@hypertrace/dashboards/testing'; import { runFakeRxjs } from '@hypertrace/test-utils'; @@ -317,7 +318,12 @@ describe('Explorer Visualization cartesian data source model', () => { timestamp: secondIntervalTime, value: 15 } - ] + ], + assignedGroup: { + id: 1, + groupName: 'first', + specName: 'sum(foo)' + } }, { color: 'second color', @@ -336,7 +342,12 @@ describe('Explorer Visualization cartesian data source model', () => { timestamp: secondIntervalTime, value: 25 } - ] + ], + assignedGroup: { + id: 1, + groupName: 'second', + specName: 'sum(foo)' + } } ], bands: [] From 43c206e32a5ec96ef266872f429d9da2b59affa6 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Fri, 26 Nov 2021 18:27:39 +0530 Subject: [PATCH 10/28] fix: adding tests --- .../cartesian/cartesian-chart.component.test.ts | 16 ++++++++++++++++ 1 file changed, 16 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 3e9b49904..684df1de0 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 @@ -185,6 +185,11 @@ describe('Cartesian Chart component', () => { name: 'test series 1', color: 'blue', type: CartesianSeriesVisualizationType.Column, + assignedGroup: { + id: 1, + groupName: 'first', + specName: 'sum(foo)' + }, stacking: true }, { @@ -192,6 +197,11 @@ describe('Cartesian Chart component', () => { name: 'test series 2', color: 'red', type: CartesianSeriesVisualizationType.Column, + assignedGroup: { + id: 1, + groupName: 'first', + specName: 'sum(foo)' + }, stacking: true } ], @@ -200,11 +210,17 @@ describe('Cartesian Chart component', () => { tick(); expect(chart.queryAll(CartesianLegend.CSS_SELECTOR, { root: true }).length).toBe(1); expect(chart.queryAll('.legend-entry').length).toBe(2); + expect(chart.query('.group-1')).toExist(); expect(chart.query('.reset.hidden')).toExist(); + chart.click(chart.query('.legend-entries-title') as Element); + tick(); + expect(chart.queryAll('.legend-text.active').length).toBe(2); + const legendEntryTexts = chart.queryAll('.legend-text'); chart.click(legendEntryTexts[0]); tick(); + expect(chart.queryAll('.legend-text.active').length).toBe(1); expect(chart.query('.reset.hidden')).not.toExist(); chart.click(chart.query('.reset') as Element); From 0d2b82ed98fa009f71e62942bf6f9c11ca71c0c1 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Fri, 26 Nov 2021 18:40:58 +0530 Subject: [PATCH 11/28] fix: adding test for coverage --- .../cartesian/cartesian-chart.component.test.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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 684df1de0..56ce03d87 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 @@ -213,10 +213,15 @@ describe('Cartesian Chart component', () => { expect(chart.query('.group-1')).toExist(); expect(chart.query('.reset.hidden')).toExist(); - chart.click(chart.query('.legend-entries-title') as Element); + const legendEntriesTitleElement = chart.query('.legend-entries-title') as Element; + chart.click(legendEntriesTitleElement); tick(); expect(chart.queryAll('.legend-text.active').length).toBe(2); + chart.click(legendEntriesTitleElement); + tick(); + expect(chart.queryAll('.legend-text.active').length).toBe(0); + const legendEntryTexts = chart.queryAll('.legend-text'); chart.click(legendEntryTexts[0]); tick(); @@ -226,6 +231,10 @@ describe('Cartesian Chart component', () => { chart.click(chart.query('.reset') as Element); tick(); expect(chart.query('.reset.hidden')).toExist(); + + chart.click(legendEntryTexts[0]); + tick(); + expect(chart.queryAll('.legend-text.active').length).toBe(1); })); test('should render column chart', fakeAsync(() => { From af35f003e1285748007288d0bef1473c4d090813 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Tue, 30 Nov 2021 21:27:06 +0530 Subject: [PATCH 12/28] fix: addressing review comments --- .../src/shared/components/cartesian/chart.ts | 1 - .../cartesian/d3/legend/cartesian-legend.ts | 41 ++++++++++--------- .../explore-cartesian-data-source.model.ts | 7 +--- 3 files changed, 22 insertions(+), 27 deletions(-) diff --git a/projects/observability/src/shared/components/cartesian/chart.ts b/projects/observability/src/shared/components/cartesian/chart.ts index f4fb5cf58..c80f17f05 100644 --- a/projects/observability/src/shared/components/cartesian/chart.ts +++ b/projects/observability/src/shared/components/cartesian/chart.ts @@ -36,7 +36,6 @@ export interface Series { export interface AssignedGroup { id: number; - specName: string; groupName: string; } 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 1e795837c..a91805e97 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 @@ -112,29 +112,30 @@ export class CartesianLegend { .enter() .append('div') .attr('class', seriesGroup => `legend-entries group-${seriesGroup[0].assignedGroup!.id}`) - .each((seriesGroup, id, elems) => { - const legendEntriesSelection = select(elems[id]); - - legendEntriesSelection - .selectAll('.legend-entries-title') - .data([seriesGroup]) - .enter() - .append('div') - .classed('legend-entries-title', true) - .text(group => `${group[0].assignedGroup!.specName}:`) - .on('click', () => this.updateActiveSeries(seriesGroup)); - - legendEntriesSelection - .append('div') - .classed('legend-entry-values', true) - .selectAll('.legend-entry') - .data(seriesGroup) - .enter() - .each((_, index, elements) => this.drawLegendEntry(elements[index])); - }); + .each((seriesGroup, index, elements) => this.drawLegendEntriesTitleAndValues(seriesGroup, elements[index])); } } + private drawLegendEntriesTitleAndValues(seriesGroup: Series<{}>[], element: HTMLDivElement): void { + const legendEntriesSelection = select(element); + legendEntriesSelection + .selectAll('.legend-entries-title') + .data([seriesGroup]) + .enter() + .append('div') + .classed('legend-entries-title', true) + .text(group => `${group[0].name}:`) + .on('click', () => this.updateActiveSeries(seriesGroup)); + + legendEntriesSelection + .append('div') + .classed('legend-entry-values', true) + .selectAll('.legend-entry') + .data(seriesGroup) + .enter() + .each((_, index, elements) => this.drawLegendEntry(elements[index])); + } + private drawLegendContainer( hostElement: Element, position: LegendPosition, diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts index fc43721d6..3684ed05d 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts @@ -140,15 +140,10 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM data: result.data, units: obj.attribute.units !== '' ? obj.attribute.units : undefined, type: request.series.find(series => series.specification === result.spec)!.visualizationOptions.type, - name: isEmpty(result.groupName) - ? obj.specDisplayName - : request.useGroupName - ? result.groupName! - : `${obj.specDisplayName}: ${result.groupName}`, + name: request.useGroupName ? result.groupName! : obj.specDisplayName, assignedGroup: !isEmpty(result.groupName) ? { id: this.getGroupId(result.groupName!, seriesIndex), - specName: obj.specDisplayName, groupName: result.groupName! } : undefined, From 967346e4dbf916c38f620604dbe118ab67e283f0 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Tue, 30 Nov 2021 21:37:56 +0530 Subject: [PATCH 13/28] fix: test cases --- .../cartesian/cartesian-chart.component.test.ts | 6 ++---- .../explore-cartesian-data-source.model.test.ts | 11 ++++------- ...-visualization-cartesian-data-source.model.test.ts | 10 ++++------ 3 files changed, 10 insertions(+), 17 deletions(-) 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 56ce03d87..e0e00c2ac 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 @@ -187,8 +187,7 @@ describe('Cartesian Chart component', () => { type: CartesianSeriesVisualizationType.Column, assignedGroup: { id: 1, - groupName: 'first', - specName: 'sum(foo)' + groupName: 'first' }, stacking: true }, @@ -199,8 +198,7 @@ describe('Cartesian Chart component', () => { type: CartesianSeriesVisualizationType.Column, assignedGroup: { id: 1, - groupName: 'first', - specName: 'sum(foo)' + groupName: 'first' }, stacking: true } diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts index 0178aa3a4..81dfbff48 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts @@ -1,4 +1,3 @@ -// import { ColorService, FixedTimeRange, TimeDuration, TimeUnit } from '@hypertrace/common'; import { createModelFactory } from '@hypertrace/dashboards/testing'; import { Model } from '@hypertrace/hyperdash'; @@ -281,7 +280,7 @@ describe('Explore cartesian data source model', () => { series: [ { color: 'first color', - name: 'sum(foo): first', + name: 'sum(foo)', type: CartesianSeriesVisualizationType.Area, data: [ { @@ -299,13 +298,12 @@ describe('Explore cartesian data source model', () => { ], assignedGroup: { id: 1, - groupName: 'first', - specName: 'sum(foo)' + groupName: 'first' } }, { color: 'second color', - name: 'sum(foo): second', + name: 'sum(foo)', type: CartesianSeriesVisualizationType.Area, data: [ { @@ -323,8 +321,7 @@ describe('Explore cartesian data source model', () => { ], assignedGroup: { id: 1, - groupName: 'second', - specName: 'sum(foo)' + groupName: 'second' } } ], diff --git a/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts index 6652fd736..65b75ee3c 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts @@ -303,7 +303,7 @@ describe('Explorer Visualization cartesian data source model', () => { series: [ { color: 'first color', - name: 'sum(foo): first', + name: 'sum(foo)', type: CartesianSeriesVisualizationType.Area, data: [ { @@ -321,13 +321,12 @@ describe('Explorer Visualization cartesian data source model', () => { ], assignedGroup: { id: 1, - groupName: 'first', - specName: 'sum(foo)' + groupName: 'first' } }, { color: 'second color', - name: 'sum(foo): second', + name: 'sum(foo)', type: CartesianSeriesVisualizationType.Area, data: [ { @@ -345,8 +344,7 @@ describe('Explorer Visualization cartesian data source model', () => { ], assignedGroup: { id: 1, - groupName: 'second', - specName: 'sum(foo)' + groupName: 'second' } } ], From d9be3992407b4436a1c84bc7b826c49942eeb888 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Wed, 1 Dec 2021 18:39:18 +0530 Subject: [PATCH 14/28] 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 72d2af7c8671689640522badf1f00e74f5a9eae6 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Wed, 1 Dec 2021 20:33:01 +0530 Subject: [PATCH 15/28] fix: addressing review comments --- .../src/shared/components/cartesian/chart.ts | 7 +- .../cartesian/d3/chart/cartesian-chart.ts | 8 +- .../cartesian/d3/legend/cartesian-legend.ts | 80 +++++++++---------- .../explore-cartesian-data-source.model.ts | 28 +------ 4 files changed, 44 insertions(+), 79 deletions(-) diff --git a/projects/observability/src/shared/components/cartesian/chart.ts b/projects/observability/src/shared/components/cartesian/chart.ts index c80f17f05..4fbc2933e 100644 --- a/projects/observability/src/shared/components/cartesian/chart.ts +++ b/projects/observability/src/shared/components/cartesian/chart.ts @@ -26,7 +26,7 @@ export interface Series { // Override the default color string using a method that takes data point as input getColor?(datum?: TInterval): string; name: string; - assignedGroup?: AssignedGroup; + groupName?: string; symbol?: SeriesSymbol; type: CartesianSeriesVisualizationType; stacking?: boolean; @@ -34,11 +34,6 @@ export interface Series { getTooltipTitle?(datum: TInterval): string; } -export interface AssignedGroup { - id: number; - groupName: string; -} - export interface Band { upper: Series; lower: Series; 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..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 @@ -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, @@ -376,12 +376,12 @@ 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 { // 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 7a5eca83e..2529fc2d7 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,7 +1,7 @@ import { ComponentRef, Injector } from '@angular/core'; -import { Color, DynamicComponentService } from '@hypertrace/common'; +import { Color, Dictionary, DynamicComponentService } from '@hypertrace/common'; import { ContainerElement, EnterElement, select, Selection } from 'd3-selection'; -import { isNil } from 'lodash-es'; +import { isEmpty } from 'lodash-es'; import { Observable, of, Subject } from 'rxjs'; import { startWith, switchMap } from 'rxjs/operators'; import { LegendPosition } from '../../../legend/legend.component'; @@ -13,7 +13,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'; @@ -21,31 +21,26 @@ export class CartesianLegend { private static readonly INACTIVE_CSS_CLASS: string = 'inactive'; public static readonly CSS_SELECTOR: string = `.${CartesianLegend.CSS_CLASS}`; - public readonly activeSeries$: Observable[]>; + public readonly activeSeries$: Observable[]>; private readonly activeSeriesSubject: Subject = new Subject(); - private readonly initialSeries: Series<{}>[]; - private readonly groupedSeries: Series<{}>[][]; - private readonly numberOfgroups: number; + private readonly initialSeries: Series[]; + private readonly groupedSeries: Dictionary[]>; private readonly isGrouped: boolean = true; 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.numberOfgroups = - this.series.length === 0 || isNil(this.series[0].assignedGroup) - ? 0 - : this.series[this.series.length - 1].assignedGroup!.id; - this.isGrouped = this.numberOfgroups !== 0; this.groupedSeries = this.getGroupedSeries(); + this.isGrouped = Object.keys(this.groupedSeries).length > 0; this.activeSeries = [...this.series]; this.initialSeries = [...this.series]; @@ -108,15 +103,15 @@ export class CartesianLegend { } else { containerSelection .selectAll('.legend-entries') - .data(this.groupedSeries) + .data(Object.values(this.groupedSeries)) .enter() .append('div') - .attr('class', seriesGroup => `legend-entries group-${seriesGroup[0].assignedGroup!.id}`) + .attr('class', (_, index) => `legend-entries group-${index + 1}`) .each((seriesGroup, index, elements) => this.drawLegendEntriesTitleAndValues(seriesGroup, elements[index])); } } - private drawLegendEntriesTitleAndValues(seriesGroup: Series<{}>[], element: HTMLDivElement): void { + private drawLegendEntriesTitleAndValues(seriesGroup: Series[], element: HTMLDivElement): void { const legendEntriesSelection = select(element); legendEntriesSelection .selectAll('.legend-entries-title') @@ -155,14 +150,14 @@ export class CartesianLegend { .classed('grouped', this.isGrouped); } - 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 => (!this.isGrouped ? series.name : series.assignedGroup!.groupName)) + .text(series => (!this.isGrouped ? series.name : series.groupName ?? '')) .on('click', series => this.updateActiveSeries(series)); this.updateLegendClassesAndStyle(); @@ -177,14 +172,14 @@ export class CartesianLegend { select(this.legendElement!) .selectAll('.legend-entries') .classed(CartesianLegend.ACTIVE_CSS_CLASS, seriesGroup => - this.isThisLegendSeriesGroupActive(seriesGroup as Series<{}>[]) + this.isThisLegendSeriesGroupActive(seriesGroup as Series[]) ); // Legend entry title select(this.legendElement!) .selectAll('.legend-entries-title') .classed(CartesianLegend.ACTIVE_CSS_CLASS, seriesGroup => - this.isThisLegendSeriesGroupActive(seriesGroup as Series<{}>[]) + this.isThisLegendSeriesGroupActive(seriesGroup as Series[]) ); } @@ -192,7 +187,7 @@ export class CartesianLegend { 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 @@ -201,15 +196,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) @@ -260,33 +255,30 @@ export class CartesianLegend { this.activeSeriesSubject.next(); } - private getGroupedSeries(): Series<{}>[][] { - if (!this.isGrouped) { - return []; + private getGroupedSeries(): Dictionary[]> { + const nonHiddenSeries = this.series.filter(series => !series.hide); + if (nonHiddenSeries.length === 0 || nonHiddenSeries.some(series => isEmpty(series.groupName ?? ''))) { + return {}; } - const nonHiddenSeries = this.series.filter(series => !series.hide); - const groupedSeries: Series<{}>[][] = []; - const currentGroup: Series<{}>[] = []; + const groupedSeries: Dictionary[]> = {}; + const currentGroup: Series[] = []; + let currentGroupIndex = 1; nonHiddenSeries.forEach(seriesEntry => { - if ( - currentGroup.length === 0 || - currentGroup[currentGroup.length - 1].assignedGroup!.id === seriesEntry.assignedGroup!.id - ) { - currentGroup.push(seriesEntry); - } else { - groupedSeries.push([...currentGroup]); + if (currentGroup.length !== 0 && currentGroup[0].groupName === seriesEntry.groupName) { + groupedSeries[`group-${currentGroupIndex}`] = [...currentGroup]; currentGroup.length = 0; - currentGroup.push(seriesEntry); + currentGroupIndex++; } + currentGroup.push(seriesEntry); }); - groupedSeries.push([...currentGroup]); + groupedSeries[`group-${currentGroupIndex}`] = [...currentGroup]; return groupedSeries; } - private updateActiveSeries(series: Series<{}> | Series<{}>[]): void { + private updateActiveSeries(series: Series | Series[]): void { if (series instanceof Array) { if (!this.isSelectionModeOn) { this.activeSeries = []; @@ -318,11 +310,11 @@ export class CartesianLegend { this.activeSeriesSubject.next(); } - private isThisLegendEntryActive(seriesEntry: Series<{}>): boolean { + private isThisLegendEntryActive(seriesEntry: Series): boolean { return this.activeSeries.includes(seriesEntry); } - private isThisLegendSeriesGroupActive(seriesGroup: Series<{}>[]): boolean { + private isThisLegendSeriesGroupActive(seriesGroup: Series[]): boolean { return !this.isSelectionModeOn ? false : seriesGroup.every(series => this.activeSeries.includes(series)); } } diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts index 3684ed05d..3bc975361 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts @@ -1,6 +1,5 @@ import { ColorService, forkJoinSafeEmpty, RequireBy, TimeDuration } from '@hypertrace/common'; import { ModelInject } from '@hypertrace/hyperdash-angular'; -import { isEmpty } from 'lodash-es'; import { NEVER, Observable, of } from 'rxjs'; import { map, mergeMap } from 'rxjs/operators'; import { Series } from '../../../../components/cartesian/chart'; @@ -30,7 +29,6 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM private readonly metadataService!: MetadataService; protected abstract buildRequestState(interval: TimeDuration | 'AUTO'): ExploreRequestState | undefined; - private readonly allGroups: string[] = []; public getData(): Observable> { return of({ @@ -98,7 +96,7 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM const seriesData = this.gatherSeriesData(request, result); const colors = this.colorService.getColorPalette().forNColors(seriesData.length); - return forkJoinSafeEmpty(seriesData.map((data, index) => this.buildSeries(request, data, colors[index], index))); + return forkJoinSafeEmpty(seriesData.map((data, index) => this.buildSeries(request, data, colors[index]))); } private gatherSeriesData(request: ExploreRequestState, result: ExploreResult): SeriesData[] { @@ -126,12 +124,7 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM return []; } - private buildSeries( - request: ExploreRequestState, - result: SeriesData, - color: string, - seriesIndex: number - ): Observable { + private buildSeries(request: ExploreRequestState, result: SeriesData, color: string): Observable { return forkJoinSafeEmpty({ specDisplayName: this.metadataService.getSpecificationDisplayName(request.context, result.spec), attribute: this.metadataService.getAttribute(request.context, result.spec.name) @@ -141,12 +134,7 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM units: obj.attribute.units !== '' ? obj.attribute.units : undefined, type: request.series.find(series => series.specification === result.spec)!.visualizationOptions.type, name: request.useGroupName ? result.groupName! : obj.specDisplayName, - assignedGroup: !isEmpty(result.groupName) - ? { - id: this.getGroupId(result.groupName!, seriesIndex), - groupName: result.groupName! - } - : undefined, + groupName: result.groupName, color: color })) ); @@ -185,16 +173,6 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM private buildGroupedSeriesName(groupKeys: string[]): string { return groupKeys.join(', '); } - - private getGroupId(groupName: string, index: number): number { - if (this.allGroups.length === 0 || !this.allGroups.includes(groupName)) { - this.allGroups.push(groupName); - - return 1; - } - - return Math.ceil((index + 1) / this.allGroups.length); - } } export type ExplorerData = MetricTimeseriesInterval | [string, number]; From 863d65dfc83838dfe63a692a90bb27e403864713 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Wed, 1 Dec 2021 20:39:41 +0530 Subject: [PATCH 16/28] fix: test cases --- .../explore-cartesian-data-source.model.test.ts | 10 ++-------- ...r-visualization-cartesian-data-source.model.test.ts | 10 ++-------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts index 81dfbff48..20fceaec3 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts @@ -296,10 +296,7 @@ describe('Explore cartesian data source model', () => { value: 15 } ], - assignedGroup: { - id: 1, - groupName: 'first' - } + groupName: 'first' }, { color: 'second color', @@ -319,10 +316,7 @@ describe('Explore cartesian data source model', () => { value: 25 } ], - assignedGroup: { - id: 1, - groupName: 'second' - } + groupName: 'second' } ], bands: [] diff --git a/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts index 65b75ee3c..c14120b68 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts @@ -319,10 +319,7 @@ describe('Explorer Visualization cartesian data source model', () => { value: 15 } ], - assignedGroup: { - id: 1, - groupName: 'first' - } + groupName: 'first' }, { color: 'second color', @@ -342,10 +339,7 @@ describe('Explorer Visualization cartesian data source model', () => { value: 25 } ], - assignedGroup: { - id: 1, - groupName: 'second' - } + groupName: 'second' } ], bands: [] From 902c0d87357c3ce6973418cd461759dc12dccecd Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Wed, 1 Dec 2021 20:44:59 +0530 Subject: [PATCH 17/28] fix: test cases --- .../cartesian/cartesian-chart.component.test.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) 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 e0e00c2ac..584d822e4 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 @@ -185,10 +185,7 @@ describe('Cartesian Chart component', () => { name: 'test series 1', color: 'blue', type: CartesianSeriesVisualizationType.Column, - assignedGroup: { - id: 1, - groupName: 'first' - }, + groupName: 'first', stacking: true }, { @@ -196,10 +193,7 @@ describe('Cartesian Chart component', () => { name: 'test series 2', color: 'red', type: CartesianSeriesVisualizationType.Column, - assignedGroup: { - id: 1, - groupName: 'first' - }, + groupName: 'seond', stacking: true } ], From a9b166b63aa3ff0bab7cb53be089531ea56a096d Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Wed, 1 Dec 2021 22:40:42 +0530 Subject: [PATCH 18/28] 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 19/28] 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 20/28] 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)); From 8ef7d2e6e515fd063335b0230719470dc01721ee Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Thu, 2 Dec 2021 14:29:35 +0530 Subject: [PATCH 21/28] fix: legend text name --- .../shared/components/cartesian/d3/legend/cartesian-legend.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 841b4d326..aea4c03b0 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 @@ -156,7 +156,7 @@ export class CartesianLegend { .append('span') .classed('legend-text', true) .classed(CartesianLegend.SELECTABLE_CSS_CLASS, this.series.length > 1) - .text(series => series.name) + .text(series => (this.isGrouped ? series.groupName ?? '' : series.name)) .on('click', series => (this.series.length > 1 ? this.updateActiveSeries(series) : undefined)); this.updateLegendClassesAndStyle(); From 5f5aafe3649138d9a2f90732bd2d5c5e83c46ecb Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Tue, 14 Dec 2021 15:14:44 -0800 Subject: [PATCH 22/28] fix: addressing review comments --- .../cartesian/cartesian-chart.component.scss | 5 +- .../cartesian/d3/legend/cartesian-legend.ts | 88 ++++++++----------- .../explore-cartesian-data-source.model.ts | 9 +- 3 files changed, 45 insertions(+), 57 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 f71671fea..fedaed806 100644 --- a/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss +++ b/projects/observability/src/shared/components/cartesian/cartesian-chart.component.scss @@ -163,8 +163,7 @@ .legend-entries { flex-direction: row; - display: grid; - grid-template-columns: 200px auto; + display: flex; gap: 20px; border: 1px solid $gray-2; border-radius: 8px; @@ -176,6 +175,7 @@ .legend-entries-title { @include body-1-regular($gray-5); + width: 200px; cursor: pointer; &.active { @@ -184,6 +184,7 @@ } .legend-entry-values { + flex: 1; display: flex; flex-wrap: wrap; } 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 aea4c03b0..2bba42411 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,7 +1,7 @@ import { ComponentRef, Injector } from '@angular/core'; import { Color, Dictionary, DynamicComponentService } from '@hypertrace/common'; import { ContainerElement, EnterElement, select, Selection } from 'd3-selection'; -import { isEmpty } from 'lodash-es'; +import { groupBy, isNil } from 'lodash-es'; import { Observable, Subject } from 'rxjs'; import { startWith } from 'rxjs/operators'; import { LegendPosition } from '../../../legend/legend.component'; @@ -28,6 +28,7 @@ export class CartesianLegend { private readonly groupedSeries: Dictionary[]>; private readonly isGrouped: boolean = true; + private readonly isNonTitledGrouped: boolean = false; private isSelectionModeOn: boolean = false; private legendElement?: HTMLDivElement; private activeSeries: Series[]; @@ -40,8 +41,9 @@ export class CartesianLegend { private readonly intervalData?: CartesianIntervalData, private readonly summaries: Summary[] = [] ) { - this.groupedSeries = this.getGroupedSeries(); - this.isGrouped = Object.keys(this.groupedSeries).length > 0; + this.isGrouped = !isNil(this.series[0].groupName); + this.isNonTitledGrouped = this.series[0].name === this.series[0].groupName; + this.groupedSeries = this.isGrouped ? groupBy(this.series, seriesEntry => seriesEntry.groupName) : {}; this.activeSeries = [...this.series]; this.initialSeries = [...this.series]; @@ -111,14 +113,16 @@ export class CartesianLegend { private drawLegendEntriesTitleAndValues(seriesGroup: Series[], element: HTMLDivElement): void { const legendEntriesSelection = select(element); - legendEntriesSelection - .selectAll('.legend-entries-title') - .data([seriesGroup]) - .enter() - .append('div') - .classed('legend-entries-title', true) - .text(group => `${group[0].name}:`) - .on('click', () => this.updateActiveSeries(seriesGroup)); + if (!this.isNonTitledGrouped) { + legendEntriesSelection + .selectAll('.legend-entries-title') + .data([seriesGroup]) + .enter() + .append('div') + .classed('legend-entries-title', true) + .text(group => `${group[0].groupName}:`) + .on('click', () => this.updateActiveSeriesGroup(seriesGroup)); + } legendEntriesSelection .append('div') @@ -156,7 +160,7 @@ export class CartesianLegend { .append('span') .classed('legend-text', true) .classed(CartesianLegend.SELECTABLE_CSS_CLASS, this.series.length > 1) - .text(series => (this.isGrouped ? series.groupName ?? '' : series.name)) + .text(series => series.name) .on('click', series => (this.series.length > 1 ? this.updateActiveSeries(series) : undefined)); this.updateLegendClassesAndStyle(); @@ -254,51 +258,29 @@ export class CartesianLegend { this.activeSeriesSubject.next(this.activeSeries); } - private getGroupedSeries(): Dictionary[]> { - const nonHiddenSeries = this.series.filter(series => !series.hide); - if (nonHiddenSeries.length === 0 || nonHiddenSeries.some(series => isEmpty(series.groupName ?? ''))) { - return {}; + private updateActiveSeriesGroup(seriesGroup: Series[]): void { + if (!this.isSelectionModeOn) { + this.activeSeries = [...seriesGroup]; + this.isSelectionModeOn = true; + } else if (!this.isThisLegendSeriesGroupActive(seriesGroup)) { + this.activeSeries = this.activeSeries.filter(series => !seriesGroup.includes(series)); + this.activeSeries.push(...seriesGroup); + } else { + this.activeSeries = this.activeSeries.filter(series => !seriesGroup.includes(series)); } - - const groupedSeries: Dictionary[]> = {}; - const currentGroup: Series[] = []; - let currentGroupIndex = 1; - - nonHiddenSeries.forEach(seriesEntry => { - if (currentGroup.length !== 0 && currentGroup[0].groupName === seriesEntry.groupName) { - groupedSeries[`group-${currentGroupIndex}`] = [...currentGroup]; - currentGroup.length = 0; - currentGroupIndex++; - } - currentGroup.push(seriesEntry); - }); - groupedSeries[`group-${currentGroupIndex}`] = [...currentGroup]; - - return groupedSeries; + this.updateLegendClassesAndStyle(); + this.updateResetElementVisibility(!this.isSelectionModeOn); + this.activeSeriesSubject.next(this.activeSeries); } - private updateActiveSeries(series: Series | Series[]): void { - if (series instanceof Array) { - if (!this.isSelectionModeOn) { - this.activeSeries = []; - this.activeSeries.push(...series); - this.isSelectionModeOn = true; - } else if (!this.isThisLegendSeriesGroupActive(series)) { - this.activeSeries = this.activeSeries.filter(seriesEntry => !series.includes(seriesEntry)); - this.activeSeries.push(...series); - } else { - this.activeSeries = this.activeSeries.filter(seriesEntry => !series.includes(seriesEntry)); - } + private updateActiveSeries(series: Series): void { + if (!this.isSelectionModeOn) { + this.activeSeries = [series]; + this.isSelectionModeOn = true; + } else if (this.isThisLegendEntryActive(series)) { + this.activeSeries = this.activeSeries.filter(seriesEntry => series !== seriesEntry); } else { - if (!this.isSelectionModeOn) { - this.activeSeries = []; - this.activeSeries.push(series); - this.isSelectionModeOn = true; - } else if (this.isThisLegendEntryActive(series)) { - this.activeSeries = this.activeSeries.filter(seriesEntry => series !== seriesEntry); - } else { - this.activeSeries.push(series); - } + this.activeSeries.push(series); } this.updateLegendClassesAndStyle(); this.updateResetElementVisibility(!this.isSelectionModeOn); diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts index 3bc975361..a7a73f214 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts @@ -1,5 +1,6 @@ import { ColorService, forkJoinSafeEmpty, RequireBy, TimeDuration } from '@hypertrace/common'; import { ModelInject } from '@hypertrace/hyperdash-angular'; +import { isNil } from 'lodash-es'; import { NEVER, Observable, of } from 'rxjs'; import { map, mergeMap } from 'rxjs/operators'; import { Series } from '../../../../components/cartesian/chart'; @@ -133,8 +134,12 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM data: result.data, units: obj.attribute.units !== '' ? obj.attribute.units : undefined, type: request.series.find(series => series.specification === result.spec)!.visualizationOptions.type, - name: request.useGroupName ? result.groupName! : obj.specDisplayName, - groupName: result.groupName, + name: result.groupName ?? obj.specDisplayName, + groupName: !isNil(result.groupName) + ? request.useGroupName + ? result.groupName + : obj.specDisplayName + : undefined, color: color })) ); From 3572db1b51c10a90b0935dbe70f947977cc4272c Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Tue, 14 Dec 2021 15:20:53 -0800 Subject: [PATCH 23/28] fix: some test cases --- .../explore/explore-cartesian-data-source.model.test.ts | 8 ++++---- ...orer-visualization-cartesian-data-source.model.test.ts | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts index 20fceaec3..e922329db 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts @@ -280,7 +280,7 @@ describe('Explore cartesian data source model', () => { series: [ { color: 'first color', - name: 'sum(foo)', + name: 'first', type: CartesianSeriesVisualizationType.Area, data: [ { @@ -296,11 +296,11 @@ describe('Explore cartesian data source model', () => { value: 15 } ], - groupName: 'first' + groupName: 'sum(foo)' }, { color: 'second color', - name: 'sum(foo)', + name: 'second', type: CartesianSeriesVisualizationType.Area, data: [ { @@ -316,7 +316,7 @@ describe('Explore cartesian data source model', () => { value: 25 } ], - groupName: 'second' + groupName: 'sum(foo)' } ], bands: [] diff --git a/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts index c14120b68..42093e552 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts @@ -303,7 +303,7 @@ describe('Explorer Visualization cartesian data source model', () => { series: [ { color: 'first color', - name: 'sum(foo)', + name: 'first', type: CartesianSeriesVisualizationType.Area, data: [ { @@ -319,11 +319,11 @@ describe('Explorer Visualization cartesian data source model', () => { value: 15 } ], - groupName: 'first' + groupName: 'sum(foo)' }, { color: 'second color', - name: 'sum(foo)', + name: 'second', type: CartesianSeriesVisualizationType.Area, data: [ { @@ -339,7 +339,7 @@ describe('Explorer Visualization cartesian data source model', () => { value: 25 } ], - groupName: 'second' + groupName: 'sum(foo)' } ], bands: [] From a46983304414a64fddde953cbe866cc0a68e10a4 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Tue, 14 Dec 2021 15:24:09 -0800 Subject: [PATCH 24/28] fix: bug --- .../shared/components/cartesian/d3/legend/cartesian-legend.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 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 2bba42411..77fde1608 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 @@ -41,8 +41,8 @@ export class CartesianLegend { private readonly intervalData?: CartesianIntervalData, private readonly summaries: Summary[] = [] ) { - this.isGrouped = !isNil(this.series[0].groupName); - this.isNonTitledGrouped = this.series[0].name === this.series[0].groupName; + this.isGrouped = !isNil(this.series[0]?.groupName); + this.isNonTitledGrouped = this.series.length > 0 && this.series[0].name === this.series[0].groupName; this.groupedSeries = this.isGrouped ? groupBy(this.series, seriesEntry => seriesEntry.groupName) : {}; this.activeSeries = [...this.series]; From 3387b08d2633faadc40f3c40bd19987eb481fdca Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Tue, 14 Dec 2021 15:27:34 -0800 Subject: [PATCH 25/28] fix: test cases --- .../cartesian/cartesian-chart.component.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) 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 584d822e4..172396538 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 @@ -9,6 +9,7 @@ import { Axis, AxisLocation, AxisType, CartesianSeriesVisualizationType, ScaleTy import { CartesianAxis } from './d3/axis/cartesian-axis'; import { CartesianNoDataMessage } from './d3/cartesian-no-data-message'; import { CartesianLegend } from './d3/legend/cartesian-legend'; +// describe('Cartesian Chart component', () => { // NOTE: tests need to query from root because angular abstraction does not support SVG @@ -182,18 +183,18 @@ describe('Cartesian Chart component', () => { series: [ { data: [[1, 2]], - name: 'test series 1', + name: 'first', color: 'blue', type: CartesianSeriesVisualizationType.Column, - groupName: 'first', + groupName: 'test series', stacking: true }, { data: [[1, 6]], - name: 'test series 2', + name: 'second', color: 'red', type: CartesianSeriesVisualizationType.Column, - groupName: 'seond', + groupName: 'test series', stacking: true } ], From 37efef0bc2cbb7b12a14388ef091b3f634563498 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Tue, 14 Dec 2021 15:29:21 -0800 Subject: [PATCH 26/28] fix: remove unwanted code --- .../components/cartesian/cartesian-chart.component.test.ts | 1 - 1 file changed, 1 deletion(-) 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 172396538..bb1146bab 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 @@ -9,7 +9,6 @@ import { Axis, AxisLocation, AxisType, CartesianSeriesVisualizationType, ScaleTy import { CartesianAxis } from './d3/axis/cartesian-axis'; import { CartesianNoDataMessage } from './d3/cartesian-no-data-message'; import { CartesianLegend } from './d3/legend/cartesian-legend'; -// describe('Cartesian Chart component', () => { // NOTE: tests need to query from root because angular abstraction does not support SVG From f24362207adf068018a15e978ac5ce53c9a449fc Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Fri, 17 Dec 2021 14:04:11 -0800 Subject: [PATCH 27/28] fix: addressing review comments --- .../cartesian/d3/legend/cartesian-legend.ts | 27 +++++++++---------- .../explore-cartesian-data-source.model.ts | 11 +++----- 2 files changed, 16 insertions(+), 22 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 77fde1608..5c0552862 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,7 +1,7 @@ import { ComponentRef, Injector } from '@angular/core'; import { Color, Dictionary, DynamicComponentService } from '@hypertrace/common'; import { ContainerElement, EnterElement, select, Selection } from 'd3-selection'; -import { groupBy, isNil } from 'lodash-es'; +import { groupBy } from 'lodash-es'; import { Observable, Subject } from 'rxjs'; import { startWith } from 'rxjs/operators'; import { LegendPosition } from '../../../legend/legend.component'; @@ -28,7 +28,6 @@ export class CartesianLegend { private readonly groupedSeries: Dictionary[]>; private readonly isGrouped: boolean = true; - private readonly isNonTitledGrouped: boolean = false; private isSelectionModeOn: boolean = false; private legendElement?: HTMLDivElement; private activeSeries: Series[]; @@ -41,8 +40,8 @@ export class CartesianLegend { private readonly intervalData?: CartesianIntervalData, private readonly summaries: Summary[] = [] ) { - this.isGrouped = !isNil(this.series[0]?.groupName); - this.isNonTitledGrouped = this.series.length > 0 && this.series[0].name === this.series[0].groupName; + this.isGrouped = + this.series.length > 0 && this.series.every(seriesEntry => seriesEntry.groupName !== seriesEntry.name); this.groupedSeries = this.isGrouped ? groupBy(this.series, seriesEntry => seriesEntry.groupName) : {}; this.activeSeries = [...this.series]; @@ -106,23 +105,21 @@ export class CartesianLegend { .data(Object.values(this.groupedSeries)) .enter() .append('div') - .attr('class', (_, index) => `legend-entries group-${index + 1}`) + .classed('legend-entries', true) .each((seriesGroup, index, elements) => this.drawLegendEntriesTitleAndValues(seriesGroup, elements[index])); } } private drawLegendEntriesTitleAndValues(seriesGroup: Series[], element: HTMLDivElement): void { const legendEntriesSelection = select(element); - if (!this.isNonTitledGrouped) { - legendEntriesSelection - .selectAll('.legend-entries-title') - .data([seriesGroup]) - .enter() - .append('div') - .classed('legend-entries-title', true) - .text(group => `${group[0].groupName}:`) - .on('click', () => this.updateActiveSeriesGroup(seriesGroup)); - } + legendEntriesSelection + .selectAll('.legend-entries-title') + .data([seriesGroup]) + .enter() + .append('div') + .classed('legend-entries-title', true) + .text(group => `${group[0].groupName}:`) + .on('click', () => this.updateActiveSeriesGroup(seriesGroup)); legendEntriesSelection .append('div') diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts index a7a73f214..27fa2e81e 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts @@ -1,6 +1,6 @@ import { ColorService, forkJoinSafeEmpty, RequireBy, TimeDuration } from '@hypertrace/common'; import { ModelInject } from '@hypertrace/hyperdash-angular'; -import { isNil } from 'lodash-es'; +import { isEmpty } from 'lodash-es'; import { NEVER, Observable, of } from 'rxjs'; import { map, mergeMap } from 'rxjs/operators'; import { Series } from '../../../../components/cartesian/chart'; @@ -134,12 +134,9 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM data: result.data, units: obj.attribute.units !== '' ? obj.attribute.units : undefined, type: request.series.find(series => series.specification === result.spec)!.visualizationOptions.type, - name: result.groupName ?? obj.specDisplayName, - groupName: !isNil(result.groupName) - ? request.useGroupName - ? result.groupName - : obj.specDisplayName - : undefined, + name: !isEmpty(result.groupName) ? result.groupName! : obj.specDisplayName, + groupName: + !isEmpty(result.groupName) && (request.useGroupName ?? false) ? result.groupName! : obj.specDisplayName, color: color })) ); From 3d585ba6881d0b69e99ebaa294c19a467ad925b6 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Fri, 17 Dec 2021 14:16:55 -0800 Subject: [PATCH 28/28] fix: test cases --- .../components/cartesian/cartesian-chart.component.test.ts | 1 - .../explore/explore-cartesian-data-source.model.test.ts | 6 ++++-- ...lorer-visualization-cartesian-data-source.model.test.ts | 7 ++++--- 3 files changed, 8 insertions(+), 6 deletions(-) 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 bb1146bab..2b67efac3 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 @@ -202,7 +202,6 @@ describe('Cartesian Chart component', () => { tick(); expect(chart.queryAll(CartesianLegend.CSS_SELECTOR, { root: true }).length).toBe(1); expect(chart.queryAll('.legend-entry').length).toBe(2); - expect(chart.query('.group-1')).toExist(); expect(chart.query('.reset.hidden')).toExist(); const legendEntriesTitleElement = chart.query('.legend-entries-title') as Element; diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts index e922329db..b901b1ba7 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts @@ -134,7 +134,8 @@ describe('Explore cartesian data source model', () => { timestamp: secondIntervalTime, value: 15 } - ] + ], + groupName: 'sum(foo)' } ], bands: [] @@ -198,7 +199,8 @@ describe('Explore cartesian data source model', () => { data: [ ['first', 10], ['second', 15] - ] + ], + groupName: 'sum(foo)' } ], bands: [] diff --git a/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts index 42093e552..836deda98 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts @@ -1,4 +1,3 @@ -// import { ColorService, FixedTimeRange, TimeDuration, TimeUnit } from '@hypertrace/common'; import { createModelFactory } from '@hypertrace/dashboards/testing'; import { runFakeRxjs } from '@hypertrace/test-utils'; @@ -154,7 +153,8 @@ describe('Explorer Visualization cartesian data source model', () => { timestamp: secondIntervalTime, value: 15 } - ] + ], + groupName: 'sum(foo)' } ], bands: [] @@ -219,7 +219,8 @@ describe('Explorer Visualization cartesian data source model', () => { data: [ ['first', 10], ['second', 15] - ] + ], + groupName: 'sum(foo)' } ], bands: []