-
Notifications
You must be signed in to change notification settings - Fork 11
Initial draft for new loaders using gifs #1071
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
7d84a9d
5b1e115
9569dfa
3ef430d
752b534
579ae96
ede6f6e
ff67f22
aa03ccb
b34414a
b7444b2
0470ca4
d51d783
c78b305
416837b
76d5ffd
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,12 @@ | ||
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' | ||
} | ||
|
||
export const enum LoaderType { | ||
Spinner = 'spinner', | ||
ExpandableRow = 'expandable-row', | ||
Page = 'page' | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ import { | |
TemplateRef, | ||
ViewContainerRef | ||
} from '@angular/core'; | ||
import { LoaderType } from '@hypertrace/assets-library'; | ||
import { Observable, ReplaySubject } from 'rxjs'; | ||
import { LoadAsyncContext, LoadAsyncService } from './load-async.service'; | ||
import { | ||
|
@@ -23,8 +24,11 @@ import { | |
export class LoadAsyncDirective implements OnChanges, OnDestroy { | ||
@Input('htLoadAsync') | ||
public data$?: Observable<unknown>; | ||
@Input('htLoadAsyncLoaderType') | ||
public loaderType?: LoaderType; | ||
|
||
private readonly wrapperParamsSubject: ReplaySubject<LoadAsyncWrapperParameters> = new ReplaySubject(1); | ||
private readonly wrapperInjector: Injector; | ||
private readonly wrapperInjector!: Injector; | ||
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. Is this required? |
||
private wrapperView?: ComponentRef<LoadAsyncWrapperComponent>; | ||
|
||
public constructor( | ||
|
@@ -49,6 +53,7 @@ export class LoadAsyncDirective implements OnChanges, OnDestroy { | |
this.wrapperView = this.wrapperView || this.buildWrapperView(); | ||
this.wrapperParamsSubject.next({ | ||
state$: this.loadAsyncService.mapObservableState(this.data$), | ||
loaderType: this.loaderType, | ||
content: this.templateRef | ||
}); | ||
} else { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import { CommonModule } from '@angular/common'; | ||
import { ImagesAssetPath, LoaderType } from '@hypertrace/assets-library'; | ||
import { createHostFactory, SpectatorHost } from '@ngneat/spectator/jest'; | ||
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 [type]="'${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 [type]="'${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 [type]="'${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,37 @@ | ||
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, LoaderType } from '@hypertrace/assets-library'; | ||
import { assertUnreachable } from '@hypertrace/common'; | ||
|
||
@Component({ | ||
selector: 'ht-loader', | ||
styleUrls: ['./loader.component.scss'], | ||
template: ` | ||
<div class="ht-loader"> | ||
<ht-icon icon="${IconType.Loading}" size="${IconSize.Large}"></ht-icon> | ||
<img [ngClass]="[this.type]" [src]="this.imagePath" /> | ||
anandtiwary marked this conversation as resolved.
Show resolved
Hide resolved
|
||
</div> | ||
`, | ||
changeDetection: ChangeDetectionStrategy.OnPush | ||
}) | ||
export class LoaderComponent {} | ||
export class LoaderComponent implements OnChanges { | ||
@Input() | ||
public type: LoaderType = LoaderType.Spinner; | ||
|
||
public imagePath: ImagesAssetPath = ImagesAssetPath.LoaderSpinner; | ||
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. If we keep it unassigned, would it get the same value from |
||
|
||
public ngOnChanges(): void { | ||
this.imagePath = this.getImagePathFromType(); | ||
} | ||
|
||
private getImagePathFromType(): ImagesAssetPath { | ||
switch (this.type) { | ||
case LoaderType.ExpandableRow: | ||
return ImagesAssetPath.LoaderExpandableRow; | ||
case LoaderType.Page: | ||
return ImagesAssetPath.LoaderPage; | ||
case LoaderType.Spinner: | ||
return ImagesAssetPath.LoaderSpinner; | ||
default: | ||
return assertUnreachable(this.type); | ||
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. may be we can leave 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. Actually, No. When a consumer is passing a wrong value it should throw an error, on the other hand, if the consumer is not passing a value it should show default value. |
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,21 @@ | ||
import { ChangeDetectionStrategy, Component, Inject, InjectionToken, TemplateRef } from '@angular/core'; | ||
import { IconType } from '@hypertrace/assets-library'; | ||
import { Observable } from 'rxjs'; | ||
import { IconType, LoaderType } from '@hypertrace/assets-library'; | ||
import { BehaviorSubject, Observable } from 'rxjs'; | ||
import { switchMap, tap } from 'rxjs/operators'; | ||
import { LoadAsyncStateType } from '../load-async-state.type'; | ||
import { AsyncState, ErrorAsyncState, LoadAsyncContext } from '../load-async.service'; | ||
|
||
export const ASYNC_WRAPPER_PARAMETERS$ = new InjectionToken<Observable<LoadAsyncWrapperParameters>>( | ||
'ASYNC_WRAPPER_PARAMETERS$' | ||
); | ||
|
||
@Component({ | ||
selector: 'ht-load-async-wrapper', | ||
template: ` | ||
<div *ngIf="this.state$ | async as state" class="fill-container" [ngSwitch]="state.type"> | ||
<ng-container *ngSwitchCase="'${LoadAsyncStateType.Loading}'"> | ||
<ht-loader></ht-loader> | ||
<ng-container *ngIf="this.loaderType$ | async as loaderType"> | ||
<ht-loader [type]="loaderType"></ht-loader> | ||
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. ng-container is not needed. You can use 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. can do 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. loaderType here will be defined since we are setting default value. |
||
</ng-container> | ||
</ng-container> | ||
<ng-container *ngSwitchCase="'${LoadAsyncStateType.Success}'"> | ||
<ng-container *ngTemplateOutlet="this.content; context: state.context"></ng-container> | ||
|
@@ -39,9 +40,15 @@ export class LoadAsyncWrapperComponent { | |
|
||
public content?: TemplateRef<LoadAsyncContext>; | ||
|
||
private readonly loaderTypeSubject: BehaviorSubject<LoaderType | undefined> = new BehaviorSubject< | ||
LoaderType | undefined | ||
>(undefined); | ||
public readonly loaderType$: Observable<LoaderType | undefined> = this.loaderTypeSubject.asObservable(); | ||
|
||
public constructor(@Inject(ASYNC_WRAPPER_PARAMETERS$) parameters$: Observable<LoadAsyncWrapperParameters>) { | ||
this.state$ = parameters$.pipe( | ||
tap(params => (this.content = params.content)), | ||
tap(params => this.loaderTypeSubject.next(params.loaderType)), | ||
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. do we gain anything by storing it as observable? If we store the |
||
switchMap(parameter => parameter.state$), | ||
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. As per prev comments, we can add loadertype as part of LoadingState. We wouldn't need to store another subject here. We can reference it from the state object directly. |
||
tap(state => this.updateMessage(state.type, (state as Partial<ErrorAsyncState>).description)) | ||
); | ||
|
@@ -65,5 +72,6 @@ export class LoadAsyncWrapperComponent { | |
|
||
export interface LoadAsyncWrapperParameters { | ||
state$: Observable<AsyncState>; | ||
loaderType?: LoaderType; | ||
content: TemplateRef<LoadAsyncContext>; | ||
} |
Uh oh!
There was an error while loading. Please reload this page.