-
Notifications
You must be signed in to change notification settings - Fork 11
feat: custom messages for load async #1217
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
Changes from all commits
8a1375f
f387581
274bce2
a2ad67d
d9f5b14
33f25fc
984f279
35c7d50
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
export const enum ImagesAssetPath { | ||
ErrorPage = 'assets/images/error-page.svg', | ||
LoaderSpinner = 'assets/images/loader-spinner.gif', | ||
LoaderPage = 'assets/images/loader-page.gif', | ||
LoaderExpandableRow = 'assets/images/loader-expandable-row.gif' | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import { CommonModule } from '@angular/common'; | ||
import { ImagesAssetPath } from '@hypertrace/assets-library'; | ||
import { createHostFactory, SpectatorHost } from '@ngneat/spectator/jest'; | ||
import { LoaderType } from '../load-async.service'; | ||
import { LoaderComponent } from './loader.component'; | ||
|
||
describe('Loader component', () => { | ||
let spectator: SpectatorHost<LoaderComponent>; | ||
|
||
const createHost = createHostFactory({ | ||
component: LoaderComponent, | ||
imports: [CommonModule] | ||
}); | ||
|
||
test('Loader component when loader type is page', () => { | ||
spectator = createHost(`<ht-loader [loaderType]="'${LoaderType.Page}'"></ht-loader>`); | ||
|
||
expect(spectator.query('.ht-loader')).toExist(); | ||
expect(spectator.query('.ht-loader img')).toExist(); | ||
expect(spectator.query('.ht-loader img')).toHaveClass(LoaderType.Page); | ||
expect(spectator.query('.ht-loader img')).toHaveAttribute('src', ImagesAssetPath.LoaderPage); | ||
}); | ||
|
||
test('Loader component when loader type is not passed', () => { | ||
spectator = createHost(`<ht-loader></ht-loader>`); | ||
|
||
expect(spectator.query('.ht-loader')).toExist(); | ||
expect(spectator.query('.ht-loader img')).toExist(); | ||
expect(spectator.query('.ht-loader img')).toHaveClass(LoaderType.Spinner); | ||
expect(spectator.query('.ht-loader img')).toHaveAttribute('src', ImagesAssetPath.LoaderSpinner); | ||
}); | ||
|
||
test('Loader component when loader type is spinner', () => { | ||
spectator = createHost(`<ht-loader [loaderType]="'${LoaderType.Spinner}'"></ht-loader>`); | ||
|
||
expect(spectator.query('.ht-loader')).toExist(); | ||
expect(spectator.query('.ht-loader img')).toExist(); | ||
expect(spectator.query('.ht-loader img')).toHaveClass(LoaderType.Spinner); | ||
expect(spectator.query('.ht-loader img')).toHaveAttribute('src', ImagesAssetPath.LoaderSpinner); | ||
}); | ||
|
||
test('Loader component loader type is expandable row', () => { | ||
spectator = createHost(`<ht-loader [loaderType]="'${LoaderType.ExpandableRow}'"></ht-loader>`); | ||
|
||
expect(spectator.query('.ht-loader')).toExist(); | ||
expect(spectator.query('.ht-loader img')).toExist(); | ||
expect(spectator.query('.ht-loader img')).toHaveClass(LoaderType.ExpandableRow); | ||
expect(spectator.query('.ht-loader img')).toHaveAttribute('src', ImagesAssetPath.LoaderExpandableRow); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,36 @@ | ||
import { ChangeDetectionStrategy, Component } from '@angular/core'; | ||
import { IconType } from '@hypertrace/assets-library'; | ||
import { IconSize } from '../../icon/icon-size'; | ||
import { ChangeDetectionStrategy, Component, Input, OnChanges } from '@angular/core'; | ||
import { ImagesAssetPath } from '@hypertrace/assets-library'; | ||
import { LoaderType } from '../load-async.service'; | ||
|
||
@Component({ | ||
selector: 'ht-loader', | ||
styleUrls: ['./loader.component.scss'], | ||
changeDetection: ChangeDetectionStrategy.OnPush, | ||
template: ` | ||
<div class="ht-loader"> | ||
<ht-icon icon="${IconType.Loading}" size="${IconSize.Large}"></ht-icon> | ||
<img [ngClass]="[this.currentLoaderType]" [src]="this.getImagePathFromType(this.currentLoaderType)" /> | ||
</div> | ||
`, | ||
changeDetection: ChangeDetectionStrategy.OnPush | ||
` | ||
}) | ||
export class LoaderComponent {} | ||
export class LoaderComponent implements OnChanges { | ||
@Input() | ||
public loaderType?: LoaderType; | ||
|
||
public currentLoaderType: LoaderType = LoaderType.Spinner; | ||
|
||
public ngOnChanges(): void { | ||
this.currentLoaderType = this.loaderType ?? LoaderType.Spinner; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could just set the image path here. no need to maintain There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We needed this, as if someone passes the So I needed to reassign this var. |
||
} | ||
|
||
public getImagePathFromType(loaderType: LoaderType): ImagesAssetPath { | ||
switch (loaderType) { | ||
case LoaderType.ExpandableRow: | ||
return ImagesAssetPath.LoaderExpandableRow; | ||
case LoaderType.Page: | ||
return ImagesAssetPath.LoaderPage; | ||
case LoaderType.Spinner: | ||
default: | ||
return ImagesAssetPath.LoaderSpinner; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ import { IconType } from '@hypertrace/assets-library'; | |
import { Observable } from 'rxjs'; | ||
import { switchMap, tap } from 'rxjs/operators'; | ||
import { LoadAsyncStateType } from '../load-async-state.type'; | ||
import { AsyncState, ErrorAsyncState, LoadAsyncContext } from '../load-async.service'; | ||
import { AsyncState, LoadAsyncConfig, LoadAsyncContext, LoaderType } from '../load-async.service'; | ||
|
||
export const ASYNC_WRAPPER_PARAMETERS$ = new InjectionToken<Observable<LoadAsyncWrapperParameters>>( | ||
'ASYNC_WRAPPER_PARAMETERS$' | ||
|
@@ -14,7 +14,7 @@ export const ASYNC_WRAPPER_PARAMETERS$ = new InjectionToken<Observable<LoadAsync | |
template: ` | ||
<div *ngIf="this.state$ | async as state" class="fill-container" [ngSwitch]="state.type"> | ||
<ng-container *ngSwitchCase="'${LoadAsyncStateType.Loading}'"> | ||
<ht-loader></ht-loader> | ||
<ht-loader [loaderType]="this.loaderType"></ht-loader> | ||
</ng-container> | ||
<ng-container *ngSwitchCase="'${LoadAsyncStateType.Success}'"> | ||
<ng-container *ngTemplateOutlet="this.content; context: state.context"></ng-container> | ||
|
@@ -34,36 +34,49 @@ export class LoadAsyncWrapperComponent { | |
public readonly state$: Observable<AsyncState>; | ||
|
||
public icon?: IconType; | ||
public loaderType?: LoaderType; | ||
public title?: string; | ||
public description: string = ''; | ||
|
||
public content?: TemplateRef<LoadAsyncContext>; | ||
public config?: LoadAsyncConfig; | ||
|
||
public constructor(@Inject(ASYNC_WRAPPER_PARAMETERS$) parameters$: Observable<LoadAsyncWrapperParameters>) { | ||
this.state$ = parameters$.pipe( | ||
tap(params => (this.content = params.content)), | ||
tap(params => { | ||
this.content = params.content; | ||
this.config = params.config; | ||
}), | ||
switchMap(parameter => parameter.state$), | ||
tap(state => this.updateMessage(state.type, (state as Partial<ErrorAsyncState>).description)) | ||
tap(state => this.updateMessage(state)) | ||
); | ||
} | ||
|
||
private updateMessage(stateType: LoadAsyncStateType, description: string = ''): void { | ||
this.description = description; | ||
|
||
switch (stateType) { | ||
private updateMessage(state: AsyncState): void { | ||
switch (state.type) { | ||
case LoadAsyncStateType.Loading: | ||
this.loaderType = this.config?.load?.loaderType; | ||
break; | ||
case LoadAsyncStateType.NoData: | ||
this.icon = IconType.NoData; | ||
this.title = 'No Data'; | ||
this.icon = this.config?.noData?.icon ?? IconType.NoData; | ||
this.title = this.config?.noData?.title ?? 'No Data'; | ||
this.description = this.config?.noData?.description ?? ''; | ||
break; | ||
case LoadAsyncStateType.GenericError: | ||
this.icon = this.config?.error?.icon ?? IconType.Error; | ||
this.title = this.config?.error?.title ?? 'Error'; | ||
this.description = state.description ?? this.config?.error?.description ?? ''; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I went back and forth which order these two should come in. I think I'm leaning towards a configured error description should take precedence over one from an error itself. We can leave it as is for now, may change later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, I was also confused about that!! |
||
break; | ||
default: | ||
this.icon = IconType.Error; | ||
this.title = 'Error'; | ||
this.icon = undefined; | ||
this.title = ''; | ||
this.description = ''; | ||
} | ||
} | ||
} | ||
|
||
export interface LoadAsyncWrapperParameters { | ||
state$: Observable<AsyncState>; | ||
content: TemplateRef<LoadAsyncContext>; | ||
config?: LoadAsyncConfig; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this different than icon? I think I missed the context of this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one, I was not very keen on. But Michael had the new loaders as gifs. So, we would have to use image. ht-icon won't work as per Alok. I think he tried.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you remember why it wouldn't work? I think icon already handles svgs and font ligatures, it just may need some changes to add gif support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember. @itssharmasandeep Can we try using ht-icons for this?