Accessiblity: Img tags missing alt tags + tests (#1483)

Azure · Jul 5, 2018 · 59acc62 · 59acc62
1 parent 771472c
commit 59acc62
Show file tree

Hide file tree

Showing 8 changed files with 107 additions and 23 deletions.
diff --git a/app/components/file/details/image-file-viewer/image-file-viewer.component.spec.ts b/app/components/file/details/image-file-viewer/image-file-viewer.component.spec.ts
@@ -0,0 +1,57 @@
+import { Component, DebugElement, NO_ERRORS_SCHEMA } from "@angular/core";
+import { ComponentFixture, TestBed } from "@angular/core/testing";
+import { By } from "@angular/platform-browser";
+import { Subject, of } from "rxjs";
+import { ImageFileViewerComponent } from "./image-file-viewer.component";
+
+@Component({
+    template: `<bl-image-file-viewer [fileLoader]="fileLoader"></bl-image-file-viewer>`,
+})
+class TestComponent {
+    public fileLoader;
+}
+
+describe("ImageFileViewerComponent", () => {
+    let fixture: ComponentFixture<TestComponent>;
+    let testComponent: TestComponent;
+    let de: DebugElement;
+    let imgEl: DebugElement;
+
+    beforeEach(() => {
+        TestBed.configureTestingModule({
+            imports: [],
+            declarations: [ImageFileViewerComponent, TestComponent],
+            schemas: [NO_ERRORS_SCHEMA],
+        });
+
+        fixture = TestBed.createComponent(TestComponent);
+        testComponent = fixture.componentInstance;
+        testComponent.fileLoader = {
+            filename: "foo.png",
+            fileChanged: new Subject(),
+            cache: jasmine.createSpy().and.returnValue(of("/local/path/to/file.txt")),
+        };
+        de = fixture.debugElement.query(By.css("bl-image-file-viewer"));
+        fixture.detectChanges();
+        imgEl = de.query(By.css("img"));
+    });
+
+    it("chache the image", () => {
+        expect(testComponent.fileLoader.cache).toHaveBeenCalledOnce();
+    });
+
+    it("point to the cached file locally", () => {
+        expect(imgEl).not.toBeFalsy();
+        expect(imgEl.nativeElement.getAttribute("src")).toEqual("file:///local/path/to/file.txt");
+    });
+
+    it("it shows an alt text for the image", () => {
+        expect(imgEl).not.toBeFalsy();
+        expect(imgEl.nativeElement.getAttribute("alt")).toEqual("Displaying image foo.png");
+    });
+
+    it("refresh again when file has changed", () => {
+        testComponent.fileLoader.fileChanged.next(true);
+        expect(testComponent.fileLoader.cache).toHaveBeenCalledTimes(2);
+    });
+});
diff --git a/app/components/file/details/image-file-viewer/image-file-viewer.component.ts b/app/components/file/details/image-file-viewer/image-file-viewer.component.ts
@@ -1,4 +1,4 @@
-import { Component, Input, OnChanges, OnDestroy } from "@angular/core";
+import { ChangeDetectionStrategy, ChangeDetectorRef, Component, Input, OnChanges, OnDestroy } from "@angular/core";
 import { Subscription } from "rxjs";
 
 import { LoadingStatus } from "@batch-flask/ui/loading";
@@ -8,35 +8,49 @@ import "./image-file-viewer.scss";
 @Component({
     selector: "bl-image-file-viewer",
     templateUrl: "image-file-viewer.html",
+    changeDetection: ChangeDetectionStrategy.OnPush,
 })
 export class ImageFileViewerComponent implements OnChanges, OnDestroy {
+
     @Input() public fileLoader: FileLoader;
 
     public src: string;
+    public imageDescription: string;
     public loadingStatus = LoadingStatus.Loading;
 
     private _sub: Subscription;
 
+    constructor(private changeDetector: ChangeDetectorRef) {}
+
     public ngOnChanges(changes) {
         this._loadImage();
         if (changes.fileLoader) {
+            this._cleanup();
+            this._updateImageDescription();
             this._sub = this.fileLoader.fileChanged.subscribe(() => {
                 this._loadImage();
             });
         }
     }
+
     public ngOnDestroy() {
         this._cleanup();
     }
 
     private _loadImage() {
         this.loadingStatus = LoadingStatus.Loading;
+        this.changeDetector.markForCheck();
         this.fileLoader.cache().subscribe((destination) => {
             this.src = `file://${destination}`;
             this.loadingStatus = LoadingStatus.Ready;
+            this.changeDetector.markForCheck();
         });
     }
 
+    private _updateImageDescription() {
+        this.imageDescription = `Displaying image ${this.fileLoader.filename}`;
+    }
+
     private _cleanup() {
         if (this._sub) {
             this._sub.unsubscribe();

diff --git a/app/components/file/details/image-file-viewer/image-file-viewer.html b/app/components/file/details/image-file-viewer/image-file-viewer.html
@@ -1,5 +1,5 @@
 <bl-loading [status]="loadingStatus">
     <div class="img-container">
-        <img [src]="src" *ngIf="src">
+        <img [src]="src" [alt]="imageDescription" *ngIf="src">
     </div>
 </bl-loading>
diff --git a/app/components/market/application-action/choose-action.html b/app/components/market/application-action/choose-action.html
@@ -1,7 +1,7 @@
 <bl-scrollable>
     <div *ngIf="application">
         <h1>
-            <img class="logo" [src]="application.icon" height="30" width="30" alt="logo" *ngIf="application?.icon">
+            <img class="logo" [src]="application.icon" height="30" width="30" alt="Logo for {{application.name}}" *ngIf="application?.icon">
             <span>Action selection for
                 <span [title]="application.description">{{application?.name}}</span>
             </span>

diff --git a/app/components/market/home/market.html b/app/components/market/home/market.html
@@ -19,7 +19,7 @@ <h1>Gallery applications</h1>
                 <div class="name">Local templates</div>
             </bl-card>
             <bl-card *ngFor="let application of displayedApplications;trackBy: trackApplication" class="application" [title]="application.description" [routerLink]="['/market', application.id, 'actions']">
-                <img class="logo" [src]="application.icon">
+                <img class="logo" [src]="application.icon" alt="Logo of {{application.name}}">
                 <div class="name">{{application.name}}</div>
             </bl-card>
         </div>

diff --git a/app/components/settings/default-settings.json b/app/components/settings/default-settings.json
@@ -3,7 +3,7 @@
   "fileTypes": {
     "log": ["txt", "log"],
     "image":  ["png", "jpg", "gif", "svg"],
-    "code": ["json", "ts2", "js", "java", "cs", "cpp", "h", "hpp", "py", "xml", "sh", "cmd", "bat", "yaml", "yml", "ts", "ps1"]
+    "code": ["json", "ts2", "js", "java", "cs", "cpp", "h", "hpp", "py", "xml", "sh", "cmd", "bat", "yaml", "yml", "ts", "ps1", "html", "css", "scss"]
   },
 
   // Default view for the configuration tab of pool, job, task, etc. Can be [pretty,json]

diff --git a/app/services/file/file-loader.ts b/app/services/file/file-loader.ts
@@ -1,11 +1,12 @@
 import * as path from "path";
-import { Observable, Subject } from "rxjs";
+import { Observable, Subject, of } from "rxjs";
 
 import { File } from "app/models";
 import { CloudPathUtils, exists, log } from "app/utils";
+import { concatMap, flatMap, map, share } from "rxjs/operators";
 import { FileSystemService } from "../fs.service";
 
-export type PropertiesFunc = () =>  Observable<File>;
+export type PropertiesFunc = () => Observable<File>;
 export type ContentFunc = (options: FileLoadOptions) => Observable<FileLoadResult>;
 export type DownloadFunc = (destination: string) => Observable<boolean>;
 
@@ -88,7 +89,7 @@ export class FileLoader {
      */
     public getProperties(forceNew = false): Observable<File> {
         if (!forceNew && this._cachedProperties) {
-            return Observable.of(this._cachedProperties);
+            return of(this._cachedProperties);
         }
 
         const obs = this._properties();
@@ -113,12 +114,19 @@ export class FileLoader {
     public download(dest: string): Observable<string> {
         if (this._download) {
             const checkDirObs = Observable.fromPromise(this._fs.ensureDir(path.dirname(dest)));
-            return checkDirObs.flatMap(() => this._download(dest)).map(x => dest).share();
+            return checkDirObs.pipe(
+                flatMap(() => this._download(dest)),
+                map(x => dest),
+                share(),
+            );
         }
 
-        const obs = this.content().concatMap((result) => {
-            return this._fs.saveFile(dest, result.content);
-        }).share();
+        const obs = this.content().pipe(
+            concatMap((result) => {
+                return this._fs.saveFile(dest, result.content);
+            }),
+            share(),
+        );
         obs.subscribe();
 
         return obs;
@@ -129,16 +137,21 @@ export class FileLoader {
      * @returns observable that resolve the path of the cached file when done caching
      */
     public cache(): Observable<string> {
-        return this.getProperties().flatMap((file: File) => {
-            const destination = this._getCacheDestination(file);
-            return Observable.fromPromise(this._fs.exists(destination)).flatMap((exists) => {
-                if (exists) {
-                    return Observable.of(destination);
-                } else {
-                    return this.download(destination);
-                }
-            }).share();
-        });
+        return this.getProperties().pipe(
+            flatMap((file: File) => {
+                const destination = this._getCacheDestination(file);
+                return Observable.fromPromise(this._fs.exists(destination)).pipe(
+                    flatMap((exists) => {
+                        if (exists) {
+                            return of(destination);
+                        } else {
+                            return this.download(destination);
+                        }
+                    }),
+                );
+            }),
+            share(),
+        );
     }
 
     public get displayName() {

diff --git a/src/client/splash-screen/splash-screen.html b/src/client/splash-screen/splash-screen.html
@@ -126,7 +126,7 @@
     </div>
 
     <div class="flask">
-        <img src="./flask.svg"></img>
+        <img src="./flask.svg" role="presentation"></img>
     </div>
 
     <div class="title">