Merge pull request #1219 from atmire/w2p-79597_Fix-thumbnail-a11y-issues

Fix thumbnail a11y issues
DSpace · Jun 24, 2021 · 1187edb · 1187edb
2 parents 0dee03e + 95b98d3
commit 1187edb
Show file tree

Hide file tree

Showing 38 changed files with 395 additions and 295 deletions.
diff --git a/.../+item-page/field-components/metadata-field-wrapper/metadata-field-wrapper.component.html b/.../+item-page/field-components/metadata-field-wrapper/metadata-field-wrapper.component.html
@@ -1,4 +1,4 @@
-<div class="simple-view-element" [class.d-none]="content.textContent.trim().length === 0 && hasNoValue(content.querySelector('img'))">
+<div class="simple-view-element" [class.d-none]="hideIfNoTextContent && content.textContent.trim().length === 0">
   <h5 class="simple-view-element-header" *ngIf="label">{{ label }}</h5>
   <div #content class="simple-view-element-body">
     <ng-content></ng-content>

diff --git a/...tem-page/field-components/metadata-field-wrapper/metadata-field-wrapper.component.spec.ts b/...tem-page/field-components/metadata-field-wrapper/metadata-field-wrapper.component.spec.ts
@@ -1,40 +1,39 @@
-import { Component } from '@angular/core';
+import { Component, Input } from '@angular/core';
 import { waitForAsync, ComponentFixture, TestBed } from '@angular/core/testing';
 
 import { MetadataFieldWrapperComponent } from './metadata-field-wrapper.component';
 
 /* tslint:disable:max-classes-per-file */
 @Component({
     selector: 'ds-component-without-content',
-    template: '<ds-metadata-field-wrapper [label]="\'test label\'">\n' +
+    template: '<ds-metadata-field-wrapper [hideIfNoTextContent]="hideIfNoTextContent" [label]="\'test label\'">\n' +
       '</ds-metadata-field-wrapper>'
 })
-class NoContentComponent {}
+class NoContentComponent {
+  public hideIfNoTextContent = true;
+}
 
 @Component({
     selector: 'ds-component-with-empty-spans',
-    template: '<ds-metadata-field-wrapper [label]="\'test label\'">\n' +
+    template: '<ds-metadata-field-wrapper [hideIfNoTextContent]="hideIfNoTextContent" [label]="\'test label\'">\n' +
       '    <span></span>\n' +
       '    <span></span>\n' +
       '</ds-metadata-field-wrapper>'
 })
-class SpanContentComponent {}
+class SpanContentComponent {
+  @Input() hideIfNoTextContent = true;
+}
 
 @Component({
     selector: 'ds-component-with-text',
-    template: '<ds-metadata-field-wrapper [label]="\'test label\'">\n' +
+    template: '<ds-metadata-field-wrapper [hideIfNoTextContent]="hideIfNoTextContent" [label]="\'test label\'">\n' +
       '    <span>The quick brown fox jumps over the lazy dog</span>\n' +
       '</ds-metadata-field-wrapper>'
 })
-class TextContentComponent {}
+class TextContentComponent {
+  @Input() hideIfNoTextContent = true;
+}
 
-@Component({
-    selector: 'ds-component-with-image',
-    template: '<ds-metadata-field-wrapper [label]="\'test label\'">\n' +
-      '    <img src="https://some/image.png" alt="an alt text">\n' +
-      '</ds-metadata-field-wrapper>'
-})
-class ImgContentComponent {}
 /* tslint:enable:max-classes-per-file */
 
 describe('MetadataFieldWrapperComponent', () => {
@@ -43,7 +42,7 @@ describe('MetadataFieldWrapperComponent', () => {
 
   beforeEach(waitForAsync(() => {
     TestBed.configureTestingModule({
-      declarations: [MetadataFieldWrapperComponent, NoContentComponent, SpanContentComponent, TextContentComponent, ImgContentComponent]
+      declarations: [MetadataFieldWrapperComponent, NoContentComponent, SpanContentComponent, TextContentComponent]
     }).compileComponents();
   }));
 
@@ -58,38 +57,60 @@ describe('MetadataFieldWrapperComponent', () => {
     expect(component).toBeDefined();
   });
 
-  it('should not show the component when there is no content', () => {
-    const parentFixture = TestBed.createComponent(NoContentComponent);
-    parentFixture.detectChanges();
-    const parentNative = parentFixture.nativeElement;
-    const nativeWrapper = parentNative.querySelector(wrapperSelector);
-    expect(nativeWrapper.classList.contains('d-none')).toBe(true);
-  });
+  describe('with hideIfNoTextContent=true', () => {
+    it('should not show the component when there is no content', () => {
+      const parentFixture = TestBed.createComponent(NoContentComponent);
+      parentFixture.detectChanges();
+      const parentNative = parentFixture.nativeElement;
+      const nativeWrapper = parentNative.querySelector(wrapperSelector);
+      expect(nativeWrapper.classList.contains('d-none')).toBe(true);
+    });
 
-  it('should not show the component when there is DOM content but not text or an image', () => {
-    const parentFixture = TestBed.createComponent(SpanContentComponent);
-    parentFixture.detectChanges();
-    const parentNative = parentFixture.nativeElement;
-    const nativeWrapper = parentNative.querySelector(wrapperSelector);
-    expect(nativeWrapper.classList.contains('d-none')).toBe(true);
-  });
+    it('should not show the component when there is no text content', () => {
+      const parentFixture = TestBed.createComponent(SpanContentComponent);
+      parentFixture.detectChanges();
+      const parentNative = parentFixture.nativeElement;
+      const nativeWrapper = parentNative.querySelector(wrapperSelector);
+      expect(nativeWrapper.classList.contains('d-none')).toBe(true);
+    });
 
-  it('should show the component when there is text content', () => {
-    const parentFixture = TestBed.createComponent(TextContentComponent);
-    parentFixture.detectChanges();
-    const parentNative = parentFixture.nativeElement;
-    const nativeWrapper = parentNative.querySelector(wrapperSelector);
-    parentFixture.detectChanges();
-    expect(nativeWrapper.classList.contains('d-none')).toBe(false);
+    it('should show the component when there is text content', () => {
+      const parentFixture = TestBed.createComponent(TextContentComponent);
+      parentFixture.detectChanges();
+      const parentNative = parentFixture.nativeElement;
+      const nativeWrapper = parentNative.querySelector(wrapperSelector);
+      parentFixture.detectChanges();
+      expect(nativeWrapper.classList.contains('d-none')).toBe(false);
+    });
   });
 
-  it('should show the component when there is img content', () => {
-    const parentFixture = TestBed.createComponent(ImgContentComponent);
-    parentFixture.detectChanges();
-    const parentNative = parentFixture.nativeElement;
-    const nativeWrapper = parentNative.querySelector(wrapperSelector);
-    parentFixture.detectChanges();
-    expect(nativeWrapper.classList.contains('d-none')).toBe(false);
-  });
+  describe('with hideIfNoTextContent=false', () => {
+    it('should show the component when there is no content', () => {
+      const parentFixture = TestBed.createComponent(NoContentComponent);
+      parentFixture.componentInstance.hideIfNoTextContent = false;
+      parentFixture.detectChanges();
+      const parentNative = parentFixture.nativeElement;
+      const nativeWrapper = parentNative.querySelector(wrapperSelector);
+      expect(nativeWrapper.classList.contains('d-none')).toBe(false);
+    });
 
+    it('should show the component when there is no text content', () => {
+      const parentFixture = TestBed.createComponent(SpanContentComponent);
+      parentFixture.componentInstance.hideIfNoTextContent = false;
+      parentFixture.detectChanges();
+      const parentNative = parentFixture.nativeElement;
+      const nativeWrapper = parentNative.querySelector(wrapperSelector);
+      expect(nativeWrapper.classList.contains('d-none')).toBe(false);
+    });
+
+    it('should show the component when there is text content', () => {
+      const parentFixture = TestBed.createComponent(TextContentComponent);
+      parentFixture.componentInstance.hideIfNoTextContent = false;
+      parentFixture.detectChanges();
+      const parentNative = parentFixture.nativeElement;
+      const nativeWrapper = parentNative.querySelector(wrapperSelector);
+      parentFixture.detectChanges();
+      expect(nativeWrapper.classList.contains('d-none')).toBe(false);
+    });
+  });
 });
diff --git a/...pp/+item-page/field-components/metadata-field-wrapper/metadata-field-wrapper.component.ts b/...pp/+item-page/field-components/metadata-field-wrapper/metadata-field-wrapper.component.ts
@@ -1,5 +1,4 @@
 import { Component, Input } from '@angular/core';
-import { hasNoValue } from '../../../shared/empty.util';
 
 /**
  * This component renders any content inside this wrapper.
@@ -17,10 +16,5 @@ export class MetadataFieldWrapperComponent {
    */
   @Input() label: string;
 
-  /**
-   * Make hasNoValue() available in the template
-   */
-  hasNoValue(o: any): boolean {
-    return hasNoValue(o);
-  }
+  @Input() hideIfNoTextContent = true;
 }
diff --git a/src/app/+item-page/full/field-components/file-section/full-file-section.component.html b/src/app/+item-page/full/field-components/file-section/full-file-section.component.html
@@ -11,7 +11,7 @@ <h5 class="simple-view-element-header">{{"item.page.filesection.original.bundle"
                            [retainScrollPosition]="true">
 
 
-                <div class="file-section row" *ngFor="let file of originals?.page;">
+                <div class="file-section row mb-3" *ngFor="let file of originals?.page;">
                     <div class="col-3">
                         <ds-thumbnail [thumbnail]="(file.thumbnail | async)?.payload"></ds-thumbnail>
                     </div>

diff --git a/src/app/+item-page/simple/item-types/publication/publication.component.html b/src/app/+item-page/simple/item-types/publication/publication.component.html
@@ -9,8 +9,8 @@ <h2 class="item-page-title-field mr-auto">
 <div class="row">
   <div class="col-xs-12 col-md-4">
     <ng-container *ngIf="!mediaViewer.image">
-      <ds-metadata-field-wrapper>
-        <ds-thumbnail [thumbnail]="getThumbnail() | async"></ds-thumbnail>
+      <ds-metadata-field-wrapper [hideIfNoTextContent]="false">
+        <ds-thumbnail [thumbnail]="thumbnail$ | async"></ds-thumbnail>
       </ds-metadata-field-wrapper>
     </ng-container>
     <ng-container *ngIf="mediaViewer.image">

diff --git a/src/app/+item-page/simple/item-types/shared/item.component.ts b/src/app/+item-page/simple/item-types/shared/item.component.ts
@@ -1,11 +1,12 @@
 import { Component, Input, OnInit } from '@angular/core';
-import { Observable } from 'rxjs';
 import { environment } from '../../../../../environments/environment';
 import { BitstreamDataService } from '../../../../core/data/bitstream-data.service';
 import { Bitstream } from '../../../../core/shared/bitstream.model';
 import { Item } from '../../../../core/shared/item.model';
-import { getFirstSucceededRemoteDataPayload } from '../../../../core/shared/operators';
+import { takeUntilCompletedRemoteData } from '../../../../core/shared/operators';
 import { getItemPageRoute } from '../../../item-page-routing-paths';
+import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject';
+import { RemoteData } from '../../../../core/data/remote-data';
 
 @Component({
   selector: 'ds-item',
@@ -17,6 +18,11 @@ import { getItemPageRoute } from '../../../item-page-routing-paths';
 export class ItemComponent implements OnInit {
   @Input() object: Item;
 
+  /**
+   * The Item's thumbnail
+   */
+  thumbnail$: BehaviorSubject<RemoteData<Bitstream>>;
+
   /**
    * Route to the item page
    */
@@ -28,12 +34,12 @@ export class ItemComponent implements OnInit {
 
   ngOnInit(): void {
     this.itemPageRoute = getItemPageRoute(this.object);
-  }
 
-  // TODO refactor to return RemoteData, and thumbnail template to deal with loading
-  getThumbnail(): Observable<Bitstream> {
-    return this.bitstreamDataService.getThumbnailFor(this.object).pipe(
-      getFirstSucceededRemoteDataPayload()
-    );
+    this.thumbnail$ = new BehaviorSubject<RemoteData<Bitstream>>(undefined);
+    this.bitstreamDataService.getThumbnailFor(this.object).pipe(
+      takeUntilCompletedRemoteData(),
+    ).subscribe((rd: RemoteData<Bitstream>) => {
+      this.thumbnail$.next(rd);
+    });
   }
 }
diff --git a/src/app/+item-page/simple/item-types/untyped-item/untyped-item.component.html b/src/app/+item-page/simple/item-types/untyped-item/untyped-item.component.html
@@ -9,8 +9,8 @@ <h2 class="item-page-title-field mr-auto">
 <div class="row">
   <div class="col-xs-12 col-md-4">
     <ng-container *ngIf="!mediaViewer.image">
-      <ds-metadata-field-wrapper>
-        <ds-thumbnail [thumbnail]="getThumbnail() | async"></ds-thumbnail>
+      <ds-metadata-field-wrapper [hideIfNoTextContent]="false">
+        <ds-thumbnail [thumbnail]="thumbnail$ | async"></ds-thumbnail>
       </ds-metadata-field-wrapper>
     </ng-container>
     <ng-container *ngIf="mediaViewer.image">

diff --git a/...esult-grid-elements/journal-issue/journal-issue-search-result-grid-element.component.html b/...esult-grid-elements/journal-issue/journal-issue-search-result-grid-element.component.html
@@ -8,14 +8,14 @@
        rel="noopener noreferrer" [routerLink]="[itemPageRoute]"
        class="card-img-top full-width">
       <div>
-        <ds-grid-thumbnail [thumbnail]="getThumbnail() | async">
-        </ds-grid-thumbnail>
+        <ds-thumbnail [thumbnail]="getThumbnail() | async" [limitWidth]="false">
+        </ds-thumbnail>
       </div>
     </a>
     <span *ngIf="linkType == linkTypes.None" class="card-img-top full-width">
             <div>
-                <ds-grid-thumbnail [thumbnail]="getThumbnail() | async">
-                </ds-grid-thumbnail>
+                <ds-thumbnail [thumbnail]="getThumbnail() | async" [limitWidth]="false">
+                </ds-thumbnail>
             </div>
         </span>
     <div class="card-body">

diff --git a/...ult-grid-elements/journal-volume/journal-volume-search-result-grid-element.component.html b/...ult-grid-elements/journal-volume/journal-volume-search-result-grid-element.component.html
@@ -8,14 +8,14 @@
        rel="noopener noreferrer" [routerLink]="[itemPageRoute]"
        class="card-img-top full-width">
       <div>
-        <ds-grid-thumbnail [thumbnail]="getThumbnail() | async">
-        </ds-grid-thumbnail>
+        <ds-thumbnail [thumbnail]="getThumbnail() | async" [limitWidth]="false">
+        </ds-thumbnail>
       </div>
     </a>
     <span *ngIf="linkType == linkTypes.None" class="card-img-top full-width">
                 <div>
-                    <ds-grid-thumbnail [thumbnail]="getThumbnail() | async">
-                    </ds-grid-thumbnail>
+                    <ds-thumbnail [thumbnail]="getThumbnail() | async" [limitWidth]="false">
+                    </ds-thumbnail>
                 </div>
             </span>
     <div class="card-body">

diff --git a/...nts/search-result-grid-elements/journal/journal-search-result-grid-element.component.html b/...nts/search-result-grid-elements/journal/journal-search-result-grid-element.component.html
@@ -8,14 +8,14 @@
        rel="noopener noreferrer" [routerLink]="[itemPageRoute]"
        class="card-img-top full-width">
       <div>
-        <ds-grid-thumbnail [thumbnail]="getThumbnail() | async">
-        </ds-grid-thumbnail>
+        <ds-thumbnail [thumbnail]="getThumbnail() | async" [limitWidth]="false">
+        </ds-thumbnail>
       </div>
     </a>
     <span *ngIf="linkType == linkTypes.None" class="card-img-top full-width">
             <div>
-                <ds-grid-thumbnail [thumbnail]="getThumbnail() | async">
-                </ds-grid-thumbnail>
+                <ds-thumbnail [thumbnail]="getThumbnail() | async" [limitWidth]="false">
+                </ds-thumbnail>
             </div>
         </span>
     <div class="card-body">

diff --git a/src/app/entity-groups/journal-entities/item-pages/journal-issue/journal-issue.component.html b/src/app/entity-groups/journal-entities/item-pages/journal-issue/journal-issue.component.html
@@ -8,8 +8,8 @@ <h2 class="item-page-title-field mr-auto">
 </div>
 <div class="row">
   <div class="col-xs-12 col-md-4">
-    <ds-metadata-field-wrapper>
-      <ds-thumbnail [thumbnail]="getThumbnail() | async"></ds-thumbnail>
+    <ds-metadata-field-wrapper [hideIfNoTextContent]="false">
+      <ds-thumbnail [thumbnail]="thumbnail$ | async"></ds-thumbnail>
     </ds-metadata-field-wrapper>
     <ds-generic-item-page-field [item]="object"
       [fields]="['publicationvolume.volumeNumber']"

diff --git a/...pp/entity-groups/journal-entities/item-pages/journal-volume/journal-volume.component.html b/...pp/entity-groups/journal-entities/item-pages/journal-volume/journal-volume.component.html
@@ -8,8 +8,8 @@ <h2 class="item-page-title-field mr-auto">
 </div>
 <div class="row">
   <div class="col-xs-12 col-md-4">
-    <ds-metadata-field-wrapper>
-      <ds-thumbnail [thumbnail]="getThumbnail() | async"></ds-thumbnail>
+    <ds-metadata-field-wrapper [hideIfNoTextContent]="false">
+      <ds-thumbnail [thumbnail]="thumbnail$ | async"></ds-thumbnail>
     </ds-metadata-field-wrapper>
     <ds-generic-item-page-field [item]="object"
       [fields]="['publicationvolume.volumeNumber']"

diff --git a/src/app/entity-groups/journal-entities/item-pages/journal/journal.component.html b/src/app/entity-groups/journal-entities/item-pages/journal/journal.component.html
@@ -8,8 +8,8 @@ <h2 class="item-page-title-field mr-auto">
 </div>
 <div class="row">
   <div class="col-xs-12 col-md-4">
-    <ds-metadata-field-wrapper>
-      <ds-thumbnail [thumbnail]="getThumbnail() | async"></ds-thumbnail>
+    <ds-metadata-field-wrapper [hideIfNoTextContent]="false">
+      <ds-thumbnail [thumbnail]="thumbnail$ | async"></ds-thumbnail>
     </ds-metadata-field-wrapper>
     <ds-generic-item-page-field class="item-page-fields" [item]="object"
       [fields]="['creativeworkseries.issn']"

diff --git a/...s/search-result-grid-elements/org-unit/org-unit-search-result-grid-element.component.html b/...s/search-result-grid-elements/org-unit/org-unit-search-result-grid-element.component.html
@@ -8,14 +8,14 @@
        rel="noopener noreferrer" [routerLink]="[itemPageRoute]"
        class="card-img-top full-width">
       <div>
-        <ds-grid-thumbnail [thumbnail]="getThumbnail() | async">
-        </ds-grid-thumbnail>
+        <ds-thumbnail [thumbnail]="getThumbnail() | async" [limitWidth]="false">
+        </ds-thumbnail>
       </div>
     </a>
     <span *ngIf="linkType == linkTypes.None" class="card-img-top full-width">
             <div>
-                <ds-grid-thumbnail [thumbnail]="getThumbnail() | async">
-                </ds-grid-thumbnail>
+                <ds-thumbnail [thumbnail]="getThumbnail() | async" [limitWidth]="false">
+                </ds-thumbnail>
             </div>
         </span>
     <div class="card-body">

diff --git a/...ments/search-result-grid-elements/person/person-search-result-grid-element.component.html b/...ments/search-result-grid-elements/person/person-search-result-grid-element.component.html
@@ -8,14 +8,14 @@
        rel="noopener noreferrer" [routerLink]="[itemPageRoute]"
        class="card-img-top full-width">
       <div>
-        <ds-grid-thumbnail [thumbnail]="getThumbnail() | async">
-        </ds-grid-thumbnail>
+        <ds-thumbnail [thumbnail]="getThumbnail() | async" [limitWidth]="false">
+        </ds-thumbnail>
       </div>
     </a>
     <span *ngIf="linkType == linkTypes.None" class="card-img-top full-width">
             <div>
-                <ds-grid-thumbnail [thumbnail]="getThumbnail() | async">
-                </ds-grid-thumbnail>
+                <ds-thumbnail [thumbnail]="getThumbnail() | async" [limitWidth]="false">
+                </ds-thumbnail>
             </div>
         </span>
     <div class="card-body">