116728: Removed unnecessary *ngVars from the ThumbnailComponent

alexandrevryghem · Jul 26, 2024 · d31e178 · d31e178
1 parent 404ccd9
commit d31e178
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 32 deletions.
diff --git a/src/app/thumbnail/thumbnail.component.html b/src/app/thumbnail/thumbnail.component.html
@@ -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>
diff --git a/src/app/thumbnail/thumbnail.component.spec.ts b/src/app/thumbnail/thumbnail.component.spec.ts
@@ -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();
       });
 
@@ -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();
@@ -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();
       });
@@ -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();
 
@@ -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;
@@ -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');
       });
     });
   });

diff --git a/src/app/thumbnail/thumbnail.component.ts b/src/app/thumbnail/thumbnail.component.ts
@@ -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';
@@ -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;
 
@@ -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,
@@ -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;
 
@@ -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;
   }
 }