Skip to content

feat: adding a new gauge widget #337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Nov 9, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@
"@angular/platform-browser": "^10.0.4",
"@angular/platform-browser-dynamic": "^10.0.4",
"@angular/router": "^10.0.4",
"@apollo/client": "^3.2.5",
"@hypertrace/hyperdash": "^1.1.2",
"@hypertrace/hyperdash-angular": "^2.1.0",
"@types/d3-hierarchy": "^2.0.0",
"@types/d3-transition": "1.1.5",
"@apollo/client": "^3.2.5",
"apollo-angular": "^2.0.4",
"core-js": "^3.5.0",
"d3-array": "^2.8.0",
Expand All @@ -60,6 +60,7 @@
"graphql": "^15.3.0",
"graphql-tag": "^2.11.0",
"lodash-es": "^4.17.15",
"resize-observer-polyfill": "^1.5.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have our own implementation of this which we use on all other visualizations. I can't remember the exact reason why we didn't use something like resize observer, but I do remember looking at it. Example usage from cartesian:

<div #chartContainer class="fill-container" (htLayoutChange)="this.redraw()"></div>

Copy link
Contributor Author

@anandtiwary anandtiwary Nov 6, 2020

Choose a reason for hiding this comment

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

htLayoutChange is triggered for the entire window/dashboard. We can use it, but i strongly feel we should use the resizeObserver from inside this component. It would get triggered even if window/layout size doesn't change. Like if we have gauge and another component inside a flex container, and the other component is removed with NgIf, layout change wouldn't get triggered.

Copy link
Contributor

Choose a reason for hiding this comment

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

htLayoutChange uses the window as its root, but is hierarchical - each new layout change directive creates a new scope. Even if we eventually switch to a lib (don't think it's needed personally), that should be wrapped in this service for simplified use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now we have wrapped it up into an rxjs observable easily. Do we still need a service ? If yes, what would we gain?

Copy link
Contributor

Choose a reason for hiding this comment

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

So trying to page back to why we implemented it ourselves, I think other than the feature not having widespread support, the issue is that the way we often have implemented layouts (particularly with d3 stuff), a resize wouldn't trigger the way we'd want. For example, we may have done a measurement to set the svg's width, so when other parts of the dom are hidden, this component hasn't actually changed size - it's already explicitly sized. That also would applied to a parent div which got its size based on its child content.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried to use LayoutChangeService and replaced it in the design I have above. The first time It tries to render (Default, without layout change), the root element gets a wrong width. This was not happening with the resize observer we used before.

Unclear what you mean - how would adding a (non-structural) directive change the width? Can you post a separate PR where you try the layout change directive? This is the method we use on every other svg, so if there are issues, we should raise them.

Is this the problem you were mentioning above?

The problem I'm referencing above is that a responsive svg scales evenly for all elements, text included. That's not behavior we want with our visualizations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#339 here you go . The problem is the dom first has a larger width. So it rendering origin is off. Then during the next trigger, it gets the correct width. Wondering why it is happening. With Resize observable, this doesn't happen.

Copy link
Contributor

Choose a reason for hiding this comment

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

My guess is that the same issue that appears in both - you're building the measuring observable in the constructor, which is too early to measure as the dom hasn't been laid out yet. In this form, you're debouncing the resize observable which means the first draw doesn't actually happen at that moment, and is likely delayed until after drawing. Try assigning it in afterViewInit instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. ViewInit was somehow adding a lot of delay I have tried that already. I am using debounce to fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated this PR. Disregard the other one

"rxjs": "~6.6.2",
"tslib": "^2.0.1",
"uuid": "^8.2.0",
Expand Down Expand Up @@ -92,6 +93,7 @@
"@types/node": "^14.11.2",
"@types/uuid": "^8.3.0",
"@types/webpack-env": "^1.14.0",
"resize-observer-polyfill": "^1.5.1",
"codelyzer": "^6.0.0",
"commitizen": "^4.2.1",
"cz-conventional-changelog": "^3.0.2",
Expand Down
1 change: 1 addition & 0 deletions projects/common/src/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export * from './custom-error/custom-error';
// DOM
export { DomElementMeasurerService } from './utilities/dom/dom-element-measurer.service';
export * from './utilities/dom/dom-utilities';
export { fromDomResize } from './utilities/dom/resize/from-dom-event';

// External
export * from './external/external-url-navigator';
Expand Down
19 changes: 19 additions & 0 deletions projects/common/src/utilities/dom/resize/from-dom-event.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import ResizeObserver from 'resize-observer-polyfill';
import { Observable } from 'rxjs';

export const fromDomResize: (element: Element) => Observable<ClientRect> = (element: Element) =>
new Observable(subscriber => {
const resizeObserver = new ResizeObserver(entries => {
entries.forEach(entry => {
if (entry.target === element) {
subscriber.next(entry.contentRect);
}
});
});

resizeObserver.observe(element);

subscriber.next(element.getBoundingClientRect());

return () => resizeObserver.disconnect();
});
3 changes: 2 additions & 1 deletion projects/components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
"d3-array": "^2.2.0",
"d3-axis": "^1.0.12",
"d3-scale": "^3.0.0",
"d3-selection": "^1.4.0"
"d3-selection": "^1.4.0",
"d3-shape": "^1.3.5"
},
"devDependencies": {
"@hypertrace/test-utils": "^0.0.0"
Expand Down
32 changes: 32 additions & 0 deletions projects/components/src/gauge/gauge.component.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
@import 'font';
@import 'color-palette';

.gauge {
width: 100%;
height: 100%;

.gauge-ring {
fill: $gray-2;
}

.input-data {
cursor: default;
}

.value-ring {
transition: transform 0.2s ease-out;
}

.value-display {
font-style: normal;
font-weight: bold;
font-size: 56px;
text-anchor: middle;
}

.label-display {
@include body-1-semibold($gray-7);
font-family: $font-family;
text-anchor: middle;
}
}
61 changes: 61 additions & 0 deletions projects/components/src/gauge/gauge.component.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { Color } from '@hypertrace/common';
import { runFakeRxjs } from '@hypertrace/test-utils';
import { createHostFactory, Spectator } from '@ngneat/spectator/jest';
import { GaugeComponent } from './gauge.component';
import { GaugeModule } from './gauge.module';

describe('Gauge component', () => {
let spectator: Spectator<GaugeComponent>;

const createHost = createHostFactory({
component: GaugeComponent,
declareComponent: false,
imports: [GaugeModule]
});

test('render all data', () => {
spectator = createHost(`<ht-gauge [value]="value" [maxValue]="maxValue" [thresholds]="thresholds"></ht-gauge>`, {
hostProps: {
value: 80,
maxValue: 100,
thresholds: [
{
label: 'Medium',
start: 60,
end: 90,
color: Color.Brown1
},
{
label: 'High',
start: 90,
end: 100,
color: Color.Red5
}
]
}
});

runFakeRxjs(({ expectObservable }) => {
expectObservable(spectator.component.gaugeRendererData$).toBe('200ms (x)', {
x: {
backgroundArc: 'M0,0Z',
origin: {
x: 0,
y: 0
},
data: {
value: 80,
maxValue: 100,
valueArc: 'M0,0Z',
threshold: {
color: '#9e4c41',
end: 90,
label: 'Medium',
start: 60
}
}
}
});
});
});
});
169 changes: 169 additions & 0 deletions projects/components/src/gauge/gauge.component.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
import { ChangeDetectionStrategy, Component, ElementRef, Input, OnChanges } from '@angular/core';
import { Color, fromDomResize, Point, SubscriptionLifecycle } from '@hypertrace/common';
import { Arc, arc, DefaultArcObject } from 'd3-shape';
import { BehaviorSubject, combineLatest, Observable, Subject } from 'rxjs';
import { debounceTime, map } from 'rxjs/operators';

@Component({
selector: 'ht-gauge',
template: `
<svg class="gauge" *ngIf="this.gaugeRendererData$ | async as rendererData">
<g attr.transform="translate({{ rendererData.origin.x }}, {{ rendererData.origin.y }})">
<path class="gauge-ring" [attr.d]="rendererData.backgroundArc" />
<g
class="input-data"
*ngIf="rendererData.data"
htTooltip="{{ rendererData.data.value }} of {{ rendererData.data.maxValue }}"
>
<path
class="value-ring"
[attr.d]="rendererData.data.valueArc"
[attr.fill]="rendererData.data.threshold.color"
/>
<text x="0" y="0" class="value-display" [attr.fill]="rendererData.data.threshold.color">
{{ rendererData.data.value }}
</text>
<text x="0" y="24" class="label-display">{{ rendererData.data.threshold.label }}</text>
</g>
</g>
</svg>
`,
styleUrls: ['./gauge.component.scss'],
providers: [SubscriptionLifecycle],
changeDetection: ChangeDetectionStrategy.OnPush
})
export class GaugeComponent implements OnChanges {
private static readonly GAUGE_RING_WIDTH: number = 20;
private static readonly GAUGE_ARC_CORNER_RADIUS: number = 10;

@Input()
public value?: number;

@Input()
public maxValue?: number;

@Input()
public thresholds: GaugeThreshold[] = [];

public readonly gaugeRendererData$: Observable<GaugeSvgRendererData>;
private readonly inputDataSubject: Subject<GaugeInputData | undefined> = new BehaviorSubject<
GaugeInputData | undefined
>(undefined);
private readonly inputData$: Observable<GaugeInputData | undefined> = this.inputDataSubject.asObservable();

public constructor(public readonly elementRef: ElementRef) {
this.gaugeRendererData$ = this.buildGaugeRendererDataObservable();
}

public ngOnChanges(): void {
this.emitInputData();
}

private buildGaugeRendererDataObservable(): Observable<GaugeSvgRendererData> {
return combineLatest([this.buildDomResizeObservable(), this.inputData$]).pipe(
map(([boundingBox, inputData]) => {
const radius = this.buildRadius(boundingBox);

return {
origin: this.buildOrigin(boundingBox, radius),
backgroundArc: this.buildBackgroundArc(radius),
data: this.buildGaugeData(radius, inputData)
};
})
);
}

private buildDomResizeObservable(): Observable<ClientRect> {
const element = this.elementRef.nativeElement as HTMLElement;

return fromDomResize(element).pipe(debounceTime(200));
}

private buildBackgroundArc(radius: number): string {
return this.buildArcGenerator()({
innerRadius: radius - GaugeComponent.GAUGE_RING_WIDTH,
outerRadius: radius,
startAngle: -Math.PI / 2,
endAngle: Math.PI / 2
})!;
}

private buildGaugeData(radius: number, inputData?: GaugeInputData): GaugeData | undefined {
if (inputData === undefined) {
return undefined;
}

return {
valueArc: this.buildValueArc(radius, inputData),
...inputData
};
}

private buildValueArc(radius: number, inputData: GaugeInputData): string {
return this.buildArcGenerator()({
innerRadius: radius - GaugeComponent.GAUGE_RING_WIDTH,
outerRadius: radius,
startAngle: -Math.PI / 2,
endAngle: -Math.PI / 2 + (inputData.value / inputData.maxValue) * Math.PI
})!;
}

private buildArcGenerator(): Arc<unknown, DefaultArcObject> {
return arc().cornerRadius(GaugeComponent.GAUGE_ARC_CORNER_RADIUS);
}

private buildRadius(boundingBox: ClientRect): number {
return Math.min(boundingBox.height, boundingBox.width) / 2;
}

private buildOrigin(boundingBox: ClientRect, radius: number): Point {
return {
x: boundingBox.width / 2,
y: boundingBox.height / 2 + radius / 2
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems strange, assuming a square bounding box, this makes the origin 3/4 of the way down?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. Because we want the center of the gauge to be at the center of the container. Hence the offset by radius/2.

Copy link
Contributor

Choose a reason for hiding this comment

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

So the radius is basically the height of the gauge right? Which means a gauge takes up half the height of its container (why not the full height with some padding) ?

And am I reading this logic right - it's putting the origin y at the bottom of the gauge (not the middle of the gauge), and the origin x in the middle of the gauge (which is always the center of the bounding box).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

};
}

private emitInputData(): void {
let inputData;
if (this.value !== undefined && this.maxValue !== undefined && this.maxValue > 0 && this.thresholds.length > 0) {
const currentThreshold = this.thresholds.find(
threshold => this.value! >= threshold.start && this.value! < threshold.end
);

if (currentThreshold) {
inputData = {
value: this.value,
maxValue: this.maxValue,
threshold: currentThreshold
};
}
}
this.inputDataSubject.next(inputData);
}
}

export interface GaugeThreshold {
label: string;
start: number;
end: number;
color: Color;
}

interface GaugeSvgRendererData {
origin: Point;
backgroundArc: string;
data?: GaugeData;
}

interface GaugeData {
valueArc: string;
value: number;
maxValue: number;
threshold: GaugeThreshold;
}

interface GaugeInputData {
value: number;
maxValue: number;
threshold: GaugeThreshold;
}
12 changes: 12 additions & 0 deletions projects/components/src/gauge/gauge.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { CommonModule } from '@angular/common';
import { NgModule } from '@angular/core';
import { FormattingModule } from '@hypertrace/common';
import { TooltipModule } from './../tooltip/tooltip.module';
import { GaugeComponent } from './gauge.component';

@NgModule({
declarations: [GaugeComponent],
exports: [GaugeComponent],
imports: [CommonModule, FormattingModule, TooltipModule]
})
export class GaugeModule {}
4 changes: 4 additions & 0 deletions projects/components/src/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ export * from './filtering/filter-modal/in-filter-modal.component';
// Filter Parser
export * from './filtering/filter/parser/filter-parser-lookup.service';

// Gauge
export * from './gauge/gauge.component';
export * from './gauge/gauge.module';

// Header
export * from './header/application/application-header.component';
export * from './header/application/application-header.module';
Expand Down
3 changes: 2 additions & 1 deletion tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
"@hypertrace/distributed-tracing",
"@hypertrace/test-utils",
"@hypertrace/observability",
"ng-mocks"
"ng-mocks",
"resize-observer-polyfill"
]
],
"no-lifecycle-call": true,
Expand Down