Skip to content
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

[Port dspace-7_x] Removed unnecessary *ngVars from ThumbnailComponent #3391

Merged
merged 1 commit into from
Oct 8, 2024
Merged
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
20 changes: 9 additions & 11 deletions src/app/thumbnail/thumbnail.component.html
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
<div class="thumbnail" [class.limit-width]="limitWidth" *ngVar="(isLoading$ | async) as isLoading">
<div class="thumbnail" [class.limit-width]="limitWidth">
<div *ngIf="isLoading" class="thumbnail-content outer">
<div class="inner">
<div class="centered">
<ds-themed-loading [spinner]="true"></ds-themed-loading>
</div>
</div>
</div>
<ng-container *ngVar="(src$ | async) as src">
<!-- don't use *ngIf="!isLoading" so the thumbnail can load in while the animation is playing -->
<img *ngIf="src !== null" class="thumbnail-content img-fluid" [ngClass]="{'d-none': isLoading}"
[src]="src | dsSafeUrl" [alt]="alt | translate" (error)="errorHandler()" (load)="successHandler()">
<div *ngIf="src === null && !isLoading" class="thumbnail-content outer">
<div class="inner">
<div class="thumbnail-placeholder centered lead">
{{ placeholder | translate }}
</div>
<!-- don't use *ngIf="!isLoading" so the thumbnail can load in while the animation is playing -->
<img *ngIf="src !== null" class="thumbnail-content img-fluid" [ngClass]="{'d-none': isLoading}"
[src]="src | dsSafeUrl" [alt]="alt | translate" (error)="errorHandler()" (load)="successHandler()">
<div *ngIf="src === null && !isLoading" class="thumbnail-content outer">
<div class="inner">
<div class="thumbnail-placeholder centered lead">
{{ placeholder | translate }}
</div>
</div>
</ng-container>
</div>
</div>
28 changes: 14 additions & 14 deletions src/app/thumbnail/thumbnail.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,31 +67,31 @@ describe('ThumbnailComponent', () => {

describe('loading', () => {
it('should start out with isLoading$ true', () => {
expect(comp.isLoading$.getValue()).toBeTrue();
expect(comp.isLoading).toBeTrue();
});

it('should set isLoading$ to false once an image is successfully loaded', () => {
comp.setSrc('http://bit.stream');
fixture.debugElement.query(By.css('img.thumbnail-content')).triggerEventHandler('load', new Event('load'));
expect(comp.isLoading$.getValue()).toBeFalse();
expect(comp.isLoading).toBeFalse();
});

it('should set isLoading$ to false once the src is set to null', () => {
comp.setSrc(null);
expect(comp.isLoading$.getValue()).toBeFalse();
expect(comp.isLoading).toBeFalse();
});

it('should show a loading animation while isLoading$ is true', () => {
expect(de.query(By.css('ds-themed-loading'))).toBeTruthy();

comp.isLoading$.next(false);
comp.isLoading = false;
fixture.detectChanges();
expect(fixture.debugElement.query(By.css('ds-themed-loading'))).toBeFalsy();
});

describe('with a thumbnail image', () => {
beforeEach(() => {
comp.src$.next('https://bit.stream');
comp.src = 'https://bit.stream';
fixture.detectChanges();
});

Expand All @@ -100,7 +100,7 @@ describe('ThumbnailComponent', () => {
expect(img).toBeTruthy();
expect(img.classes['d-none']).toBeTrue();

comp.isLoading$.next(false);
comp.isLoading = false;
fixture.detectChanges();
img = fixture.debugElement.query(By.css('img.thumbnail-content'));
expect(img).toBeTruthy();
Expand All @@ -111,14 +111,14 @@ describe('ThumbnailComponent', () => {

describe('without a thumbnail image', () => {
beforeEach(() => {
comp.src$.next(null);
comp.src = null;
fixture.detectChanges();
});

it('should only show the HTML placeholder once done loading', () => {
expect(fixture.debugElement.query(By.css('div.thumbnail-placeholder'))).toBeFalsy();

comp.isLoading$.next(false);
comp.isLoading = false;
fixture.detectChanges();
expect(fixture.debugElement.query(By.css('div.thumbnail-placeholder'))).toBeTruthy();
});
Expand Down Expand Up @@ -214,14 +214,14 @@ describe('ThumbnailComponent', () => {
describe('fallback', () => {
describe('if there is a default image', () => {
it('should display the default image', () => {
comp.src$.next('http://bit.stream');
comp.src = 'http://bit.stream';
comp.defaultImage = 'http://default.img';
comp.errorHandler();
expect(comp.src$.getValue()).toBe(comp.defaultImage);
expect(comp.src).toBe(comp.defaultImage);
});

it('should include the alt text', () => {
comp.src$.next('http://bit.stream');
comp.src = 'http://bit.stream';
comp.defaultImage = 'http://default.img';
comp.errorHandler();

Expand All @@ -233,10 +233,10 @@ describe('ThumbnailComponent', () => {

describe('if there is no default image', () => {
it('should display the HTML placeholder', () => {
comp.src$.next('http://default.img');
comp.src = 'http://default.img';
comp.defaultImage = null;
comp.errorHandler();
expect(comp.src$.getValue()).toBe(null);
expect(comp.src).toBe(null);

fixture.detectChanges();
const placeholder = fixture.debugElement.query(By.css('div.thumbnail-placeholder')).nativeElement;
Expand Down Expand Up @@ -328,7 +328,7 @@ describe('ThumbnailComponent', () => {
it('should show the default image', () => {
comp.defaultImage = 'default/image.jpg';
comp.ngOnChanges({});
expect(comp.src$.getValue()).toBe('default/image.jpg');
expect(comp.src).toBe('default/image.jpg');
});
});
});
Expand Down
14 changes: 7 additions & 7 deletions src/app/thumbnail/thumbnail.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Component, Input, OnChanges, SimpleChanges } from '@angular/core';
import { Bitstream } from '../core/shared/bitstream.model';
import { hasNoValue, hasValue } from '../shared/empty.util';
import { RemoteData } from '../core/data/remote-data';
import { BehaviorSubject, of as observableOf } from 'rxjs';
import { of as observableOf } from 'rxjs';
import { switchMap } from 'rxjs/operators';
import { FeatureID } from '../core/data/feature-authorization/feature-id';
import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service';
Expand Down Expand Up @@ -34,7 +34,7 @@ export class ThumbnailComponent implements OnChanges {
/**
* The src attribute used in the template to render the image.
*/
src$ = new BehaviorSubject<string>(undefined);
src: string = undefined;

retriedWithToken = false;

Expand All @@ -57,7 +57,7 @@ export class ThumbnailComponent implements OnChanges {
* Whether the thumbnail is currently loading
* Start out as true to avoid flashing the alt text while a thumbnail is being loaded.
*/
isLoading$ = new BehaviorSubject(true);
isLoading = true;

constructor(
protected auth: AuthService,
Expand Down Expand Up @@ -110,7 +110,7 @@ export class ThumbnailComponent implements OnChanges {
* Otherwise, fall back to the default image or a HTML placeholder
*/
errorHandler() {
const src = this.src$.getValue();
const src = this.src;
const thumbnail = this.bitstream;
const thumbnailSrc = thumbnail?._links?.content?.href;

Expand Down Expand Up @@ -162,16 +162,16 @@ export class ThumbnailComponent implements OnChanges {
* @param src
*/
setSrc(src: string): void {
this.src$.next(src);
this.src = src;
if (src === null) {
this.isLoading$.next(false);
this.isLoading = false;
}
}

/**
* Stop the loading animation once the thumbnail is successfully loaded
*/
successHandler() {
this.isLoading$.next(false);
this.isLoading = false;
}
}
Loading