Skip to content

Commit 11f2517

Browse files
committed
fix issue where thumnails of embargoed bitstreams wouldn't show up for users with access rights
1 parent d31e178 commit 11f2517

File tree

3 files changed

+47
-36
lines changed

3 files changed

+47
-36
lines changed

src/app/thumbnail/thumbnail.component.html

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
<div class="thumbnail" [class.limit-width]="limitWidth">
2-
<div *ngIf="isLoading" class="thumbnail-content outer">
2+
<div *ngIf="(isLoading$ | async)" class="thumbnail-content outer">
33
<div class="inner">
44
<div class="centered">
55
<ds-themed-loading [spinner]="true"></ds-themed-loading>
66
</div>
77
</div>
88
</div>
9-
<!-- don't use *ngIf="!isLoading" so the thumbnail can load in while the animation is playing -->
10-
<img *ngIf="src !== null" class="thumbnail-content img-fluid" [ngClass]="{'d-none': isLoading}"
11-
[src]="src | dsSafeUrl" [alt]="alt | translate" (error)="errorHandler()" (load)="successHandler()">
12-
<div *ngIf="src === null && !isLoading" class="thumbnail-content outer">
9+
<!-- don't use *ngIf="!(isLoading$ | async)" so the thumbnail can load in while the animation is playing -->
10+
<img *ngIf="(src$ | async) !== null" class="thumbnail-content img-fluid" [ngClass]="{'d-none': (isLoading$ | async)}"
11+
[src]="(src$ | async) | dsSafeUrl" [alt]="alt | translate" (error)="errorHandler()" (load)="successHandler()">
12+
<div *ngIf="(src$ | async) === null && !(isLoading$ | async)" class="thumbnail-content outer">
1313
<div class="inner">
1414
<div class="thumbnail-placeholder centered lead">
1515
{{ placeholder | translate }}

src/app/thumbnail/thumbnail.component.spec.ts

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,31 +67,31 @@ describe('ThumbnailComponent', () => {
6767

6868
describe('loading', () => {
6969
it('should start out with isLoading$ true', () => {
70-
expect(comp.isLoading).toBeTrue();
70+
expect(comp.isLoading$.getValue()).toBeTrue();
7171
});
7272

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

7979
it('should set isLoading$ to false once the src is set to null', () => {
8080
comp.setSrc(null);
81-
expect(comp.isLoading).toBeFalse();
81+
expect(comp.isLoading$.getValue()).toBeFalse();
8282
});
8383

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

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

9292
describe('with a thumbnail image', () => {
9393
beforeEach(() => {
94-
comp.src = 'https://bit.stream';
94+
comp.src$.next('https://bit.stream');
9595
fixture.detectChanges();
9696
});
9797

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

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

112112
describe('without a thumbnail image', () => {
113113
beforeEach(() => {
114-
comp.src = null;
114+
comp.src$.next(null);
115115
fixture.detectChanges();
116116
});
117117

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

121-
comp.isLoading = false;
121+
comp.isLoading$.next(false);
122122
fixture.detectChanges();
123123
expect(fixture.debugElement.query(By.css('div.thumbnail-placeholder'))).toBeTruthy();
124124
});
@@ -214,14 +214,14 @@ describe('ThumbnailComponent', () => {
214214
describe('fallback', () => {
215215
describe('if there is a default image', () => {
216216
it('should display the default image', () => {
217-
comp.src = 'http://bit.stream';
217+
comp.src$.next('http://bit.stream');
218218
comp.defaultImage = 'http://default.img';
219219
comp.errorHandler();
220-
expect(comp.src).toBe(comp.defaultImage);
220+
expect(comp.src$.getValue()).toBe(comp.defaultImage);
221221
});
222222

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

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

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

241241
fixture.detectChanges();
242242
const placeholder = fixture.debugElement.query(By.css('div.thumbnail-placeholder')).nativeElement;
@@ -328,7 +328,7 @@ describe('ThumbnailComponent', () => {
328328
it('should show the default image', () => {
329329
comp.defaultImage = 'default/image.jpg';
330330
comp.ngOnChanges({});
331-
expect(comp.src).toBe('default/image.jpg');
331+
expect(comp.src$.getValue()).toBe('default/image.jpg');
332332
});
333333
});
334334
});

src/app/thumbnail/thumbnail.component.ts

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
1-
import { Component, Input, OnChanges, SimpleChanges } from '@angular/core';
1+
import { Component, Inject, Input, OnChanges, PLATFORM_ID, SimpleChanges } from '@angular/core';
22
import { Bitstream } from '../core/shared/bitstream.model';
33
import { hasNoValue, hasValue } from '../shared/empty.util';
44
import { RemoteData } from '../core/data/remote-data';
5-
import { of as observableOf } from 'rxjs';
5+
import { of as observableOf, BehaviorSubject } from 'rxjs';
66
import { switchMap } from 'rxjs/operators';
77
import { FeatureID } from '../core/data/feature-authorization/feature-id';
88
import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service';
99
import { AuthService } from '../core/auth/auth.service';
1010
import { FileService } from '../core/shared/file.service';
11+
import { isPlatformBrowser } from '@angular/common';
1112

1213
/**
1314
* This component renders a given Bitstream as a thumbnail.
@@ -34,7 +35,7 @@ export class ThumbnailComponent implements OnChanges {
3435
/**
3536
* The src attribute used in the template to render the image.
3637
*/
37-
src: string = undefined;
38+
src$: BehaviorSubject<string> = new BehaviorSubject<string>(undefined);
3839

3940
retriedWithToken = false;
4041

@@ -57,9 +58,10 @@ export class ThumbnailComponent implements OnChanges {
5758
* Whether the thumbnail is currently loading
5859
* Start out as true to avoid flashing the alt text while a thumbnail is being loaded.
5960
*/
60-
isLoading = true;
61+
isLoading$: BehaviorSubject<boolean> = new BehaviorSubject(true);
6162

6263
constructor(
64+
@Inject(PLATFORM_ID) private platformID: any,
6365
protected auth: AuthService,
6466
protected authorizationService: AuthorizationDataService,
6567
protected fileService: FileService,
@@ -71,16 +73,25 @@ export class ThumbnailComponent implements OnChanges {
7173
* Use a default image if no actual image is available.
7274
*/
7375
ngOnChanges(changes: SimpleChanges): void {
74-
if (hasNoValue(this.thumbnail)) {
75-
this.setSrc(this.defaultImage);
76-
return;
77-
}
76+
if (isPlatformBrowser(this.platformID)) {
77+
// every time the inputs change we need to start the loading animation again, as it's possible
78+
// that thumbnail is first set to null when the parent component initializes and then set to
79+
// the actual value
80+
if (this.isLoading$.getValue() === false) {
81+
this.isLoading$.next(true);
82+
}
7883

79-
const src = this.contentHref;
80-
if (hasValue(src)) {
81-
this.setSrc(src);
82-
} else {
83-
this.setSrc(this.defaultImage);
84+
if (hasNoValue(this.thumbnail)) {
85+
this.setSrc(this.defaultImage);
86+
return;
87+
}
88+
89+
const src = this.contentHref;
90+
if (hasValue(src)) {
91+
this.setSrc(src);
92+
} else {
93+
this.setSrc(this.defaultImage);
94+
}
8495
}
8596
}
8697

@@ -110,7 +121,7 @@ export class ThumbnailComponent implements OnChanges {
110121
* Otherwise, fall back to the default image or a HTML placeholder
111122
*/
112123
errorHandler() {
113-
const src = this.src;
124+
const src = this.src$.getValue();
114125
const thumbnail = this.bitstream;
115126
const thumbnailSrc = thumbnail?._links?.content?.href;
116127

@@ -162,16 +173,16 @@ export class ThumbnailComponent implements OnChanges {
162173
* @param src
163174
*/
164175
setSrc(src: string): void {
165-
this.src = src;
176+
this.src$.next(src);
166177
if (src === null) {
167-
this.isLoading = false;
178+
this.isLoading$.next(false);
168179
}
169180
}
170181

171182
/**
172183
* Stop the loading animation once the thumbnail is successfully loaded
173184
*/
174185
successHandler() {
175-
this.isLoading = false;
186+
this.isLoading$.next(false);
176187
}
177188
}

0 commit comments

Comments
 (0)