Skip to content

feat: added ht-status-display component #1775

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { Pipe, PipeTransform } from '@angular/core';
import { IconType } from '@hypertrace/assets-library';

@Pipe({
name: 'htStatusDisplayIcon'
})
export class StatusDisplayIconPipe implements PipeTransform {
public transform(statusCode: string | number, status?: string): IconType | undefined {
if (status === 'FAIL') {
return IconType.AlertFill;
}
if (status === 'SUCCESS') {
return IconType.CheckCircleFill;
}
/**
* GRPC Success -> 0
* HTTP Success -> 100-300
*/
if (+statusCode === 0 || (+statusCode >= 100 && +statusCode <= 399)) {
return IconType.CheckCircleFill;
}

return IconType.AlertFill;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { Pipe, PipeTransform } from '@angular/core';
import { displayString } from '@hypertrace/common';
import { isNil } from 'lodash-es';

@Pipe({
name: 'htStatusDisplayText'
})
export class StatusDisplayTextPipe implements PipeTransform {
/**
* Appends the `statusMessage` with a `-` if it's available.
* If not just show the `statusCode`
*
* @param statusCode - status code
* @param statusMessage - status message
*/
public transform(statusCode: string | number, statusMessage?: string): string {
let statusDisplayText = displayString(statusCode);
if (!isNil(statusMessage) && statusMessage !== 'null') {
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we looking for string null here?

statusDisplayText = `${statusDisplayText} - ${statusMessage}`;
}

return statusDisplayText;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
@import 'mixins';

.status {
display: flex;
align-items: center;
gap: 4px;
padding: 0 4px;
background-color: $gray-1;
border-radius: 4px;
font-size: 12px;
color: $gray-5;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { IconType } from '@hypertrace/assets-library';
import { IconComponent } from '@hypertrace/components';
import { createHostFactory } from '@ngneat/spectator/jest';
import { MockComponent } from 'ng-mocks';
import { StatusDisplayIconPipe } from './status-display-icon.pipe';
import { StatusDisplayTextPipe } from './status-display-text.pipe';
import { StatusDisplayComponent } from './status-display.component';

describe('Status Display Component', () => {
const createHost = createHostFactory({
component: StatusDisplayComponent,
shallow: true,
declarations: [MockComponent(IconComponent), StatusDisplayTextPipe, StatusDisplayIconPipe]
});

test('should display the status code properly with check icon', () => {
const spectator = createHost(`<ht-status-display [statusCode]="statusCode"></ht-status-display>`, {
hostProps: {
statusCode: 200
}
});

expect(spectator.query(IconComponent)?.icon).toBe(IconType.CheckCircleFill);
expect(spectator.query('.text')?.textContent).toBe('200');
});

test('should display the fail icon', () => {
const spectator = createHost(
`<ht-status-display [statusCode]="statusCode" [status]="status"></ht-status-display>`,
{
hostProps: {
status: 'FAIL',
statusCode: 400
}
}
);

expect(spectator.query(IconComponent)?.icon).toBe(IconType.AlertFill);
});

test('should display the fail icon', () => {
const spectator = createHost(
`<ht-status-display [statusCode]="statusCode" [status]="status"></ht-status-display>`,
{
hostProps: {
statusCode: 400
}
}
);

expect(spectator.query(IconComponent)?.icon).toBe(IconType.AlertFill);
});

test('should display the check icon if unknown status is send', () => {
const spectator = createHost(
`<ht-status-display [statusCode]="statusCode" [status]="status"></ht-status-display>`,
{
hostProps: {
status: 'SOMETHING',
statusCode: 200
}
}
);

expect(spectator.query(IconComponent)?.icon).toBe(IconType.CheckCircleFill);
});

test('should display the status code with status message', () => {
const spectator = createHost(
`<ht-status-display [statusCode]="statusCode" [statusMessage]="statusMessage"></ht-status-display>`,
{
hostProps: {
statusCode: 200,
statusMessage: 'OK'
}
}
);

expect(spectator.query('.text')?.textContent).toBe('200 - OK');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { ChangeDetectionStrategy, Component, Input } from '@angular/core';
import { IconSize } from '@hypertrace/components';

@Component({
selector: 'ht-status-display',
template: ` <div class="status" *ngIf="this.statusCode">
<ng-container *ngIf="this.statusCode | htStatusDisplayIcon: this.status as icon">
<ht-icon [icon]="icon" size="${IconSize.ExtraSmall}"></ht-icon>
</ng-container>
<span class="text">{{ this.statusCode | htStatusDisplayText: this.statusMessage }}</span>
</div>`,
styleUrls: ['./status-display.component.scss'],
changeDetection: ChangeDetectionStrategy.OnPush
})
export class StatusDisplayComponent {
@Input()
public statusCode?: number | string;

@Input()
public status?: string;

@Input()
public statusMessage?: string;

// TODO: Add display for different styles if required
Copy link
Contributor

Choose a reason for hiding this comment

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

@arjunlalb @adisreyaj
We have this component as gray background and since we want this to be used at the status table cell renderer (which has different styling according to the status), how do we wanna approach this?

  1. Should we replace that styling with this new one
  2. Or add that styling as a config here as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should use a single style. I recommend using the existing style. After consulting with design team, we can switch it over to the new one or keep the old one.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree on keeping the same styling throughout the product and yeah, the existing one seems better to me as well. But let's wait for @adisreyaj to comment on this since I think he has more context around that.

I think, We can leave this here as it is until we get final confirmation.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { NgModule } from '@angular/core';

import { CommonModule } from '@angular/common';
import { FormattingModule, MemoizeModule } from '@hypertrace/common';
import { IconModule } from '@hypertrace/components';
import { StatusDisplayIconPipe } from './status-display-icon.pipe';
import { StatusDisplayTextPipe } from './status-display-text.pipe';
import { StatusDisplayComponent } from './status-display.component';

@NgModule({
imports: [IconModule, MemoizeModule, CommonModule, FormattingModule],
exports: [StatusDisplayComponent],
declarations: [StatusDisplayComponent, StatusDisplayTextPipe, StatusDisplayIconPipe]
})
export class StatusDisplayModule {}