From 588db58b5d2036f8ddea8b365e9866375b0448d7 Mon Sep 17 00:00:00 2001 From: Cilibiu Bogdan Date: Fri, 19 Feb 2021 18:59:55 +0200 Subject: [PATCH] [ADF-5339] ESCAPE should close opened dialog not the overlay viewer (#6696) * don't close viewer if Escape was performed on opened dialog * subscribe only in overlayMode * tests * lint * move logic to method --- .../components/viewer.component.spec.ts | 38 +++++++++- .../viewer/components/viewer.component.ts | 74 ++++++++++++------- 2 files changed, 83 insertions(+), 29 deletions(-) diff --git a/lib/core/viewer/components/viewer.component.spec.ts b/lib/core/viewer/components/viewer.component.spec.ts index d1805a0716..33c906b776 100644 --- a/lib/core/viewer/components/viewer.component.spec.ts +++ b/lib/core/viewer/components/viewer.component.spec.ts @@ -30,6 +30,8 @@ import { AlfrescoApiServiceMock } from '../../mock/alfresco-api.service.mock'; import { NodeEntry, VersionEntry } from '@alfresco/js-api'; import { CoreTestingModule } from '../../testing/core.testing.module'; import { TranslateModule } from '@ngx-translate/core'; +import { MatDialog } from '@angular/material/dialog'; +import { NoopAnimationsModule } from '@angular/platform-browser/animations'; @Component({ selector: 'adf-viewer-container-toolbar', @@ -72,6 +74,13 @@ class ViewerWithCustomToolbarActionsComponent { class ViewerWithCustomSidebarComponent { } +@Component({ + selector: 'adf-dialog-dummy', + template: `` +}) +class DummyDialogComponent { +} + @Component({ selector: 'adf-viewer-container-open-with', template: ` @@ -126,9 +135,11 @@ describe('ViewerComponent', () => { let fixture: ComponentFixture; let alfrescoApiService: AlfrescoApiService; let element: HTMLElement; + let dialog: MatDialog; setupTestBed({ imports: [ + NoopAnimationsModule, TranslateModule.forRoot(), CoreTestingModule ], @@ -149,7 +160,8 @@ describe('ViewerComponent', () => { } }, RenderingQueueServices, - { provide: Location, useClass: SpyLocation } + { provide: Location, useClass: SpyLocation }, + MatDialog ] }); @@ -159,6 +171,7 @@ describe('ViewerComponent', () => { component = fixture.componentInstance; alfrescoApiService = TestBed.inject(AlfrescoApiService); + dialog = TestBed.inject(MatDialog); }); describe('Extension Type Test', () => { @@ -776,7 +789,7 @@ describe('ViewerComponent', () => { }); it('should Esc button hide the viewer', (done) => { - EventMock.keyUp(27); + EventMock.keyDown(27); fixture.detectChanges(); fixture.whenStable().then(() => { @@ -784,6 +797,27 @@ describe('ViewerComponent', () => { done(); }); }); + + it('should not close the viewer on Escape event if dialog was opened', (done) => { + const event = new KeyboardEvent('keydown', { + bubbles: true, + keyCode: 27 + } as KeyboardEventInit ); + const dialogRef = dialog.open(DummyDialogComponent); + + dialogRef.afterClosed().subscribe(() => { + document.body.dispatchEvent(event); + fixture.detectChanges(); + expect(element.querySelector('.adf-viewer-content')).toBeNull(); + done(); + }); + + fixture.detectChanges(); + + document.body.dispatchEvent(event); + fixture.detectChanges(); + expect(element.querySelector('.adf-viewer-content')).not.toBeNull(); + }); }); describe('Overlay mode false', () => { diff --git a/lib/core/viewer/components/viewer.component.ts b/lib/core/viewer/components/viewer.component.ts index af5263eee2..49ba41bbaf 100644 --- a/lib/core/viewer/components/viewer.component.ts +++ b/lib/core/viewer/components/viewer.component.ts @@ -28,10 +28,11 @@ import { ViewerMoreActionsComponent } from './viewer-more-actions.component'; import { ViewerOpenWithComponent } from './viewer-open-with.component'; import { ViewerSidebarComponent } from './viewer-sidebar.component'; import { ViewerToolbarComponent } from './viewer-toolbar.component'; -import { Subscription } from 'rxjs'; +import { fromEvent, Subject } from 'rxjs'; import { ViewUtilService } from '../services/view-util.service'; import { AppExtensionService, ViewerExtensionRef } from '@alfresco/adf-extensions'; -import { filter } from 'rxjs/operators'; +import { filter, skipWhile, takeUntil } from 'rxjs/operators'; +import { MatDialog } from '@angular/material/dialog'; @Component({ selector: 'adf-viewer', @@ -224,8 +225,6 @@ export class ViewerComponent implements OnChanges, OnInit, OnDestroy { private cacheBusterNumber; - private subscriptions: Subscription[] = []; - // Extensions that are supported by the Viewer without conversion private extensions = { image: ['png', 'jpg', 'jpeg', 'gif', 'bpm', 'svg'], @@ -242,11 +241,16 @@ export class ViewerComponent implements OnChanges, OnInit, OnDestroy { media: ['video/mp4', 'video/webm', 'video/ogg', 'audio/mpeg', 'audio/mp3', 'audio/ogg', 'audio/wav'] }; + private onDestroy$ = new Subject(); + private shouldCloseViewer = true; + private keyDown$ = fromEvent(document, 'keydown'); + constructor(private apiService: AlfrescoApiService, private viewUtilService: ViewUtilService, private logService: LogService, private extensionService: AppExtensionService, - private el: ElementRef) { + private el: ElementRef, + public dialog: MatDialog) { viewUtilService.maxRetries = this.maxRetries; } @@ -255,23 +259,20 @@ export class ViewerComponent implements OnChanges, OnInit, OnDestroy { } ngOnInit() { - this.subscriptions.push( - this.apiService.nodeUpdated.pipe( - filter((node) => node && node.id === this.nodeId && node.name !== this.fileName) - ).subscribe((node) => this.onNodeUpdated(node)) - ); + this.apiService.nodeUpdated.pipe( + filter((node) => node && node.id === this.nodeId && node.name !== this.fileName), + takeUntil(this.onDestroy$) + ).subscribe((node) => this.onNodeUpdated(node)); - this.subscriptions.push( - this.viewUtilService.viewerTypeChange.subscribe((type: string) => { - this.viewerType = type; - }) - ); - this.subscriptions.push( - this.viewUtilService.urlFileContentChange.subscribe((content: string) => { - this.urlFileContent = content; - }) - ); + this.viewUtilService.viewerTypeChange.pipe(takeUntil(this.onDestroy$)).subscribe((type: string) => { + this.viewerType = type; + }); + this.viewUtilService.urlFileContentChange.pipe(takeUntil(this.onDestroy$)).subscribe((content: string) => { + this.urlFileContent = content; + }); + + this.closeOverlayManager(); this.loadExtensions(); } @@ -284,8 +285,8 @@ export class ViewerComponent implements OnChanges, OnInit, OnDestroy { } ngOnDestroy() { - this.subscriptions.forEach((subscription) => subscription.unsubscribe()); - this.subscriptions = []; + this.onDestroy$.next(true); + this.onDestroy$.complete(); } private onNodeUpdated(node: Node) { @@ -591,11 +592,6 @@ export class ViewerComponent implements OnChanges, OnInit, OnDestroy { const key = event.keyCode; - // Esc - if (key === 27 && this.overlayMode) { // esc - this.close(); - } - // Left arrow if (key === 37 && this.canNavigateBefore) { event.preventDefault(); @@ -682,6 +678,30 @@ export class ViewerComponent implements OnChanges, OnInit, OnDestroy { this.viewerType = 'unknown'; } + private closeOverlayManager() { + this.dialog.afterOpened.pipe( + skipWhile(() => !this.overlayMode), + takeUntil(this.onDestroy$) + ).subscribe(() => this.shouldCloseViewer = false); + + this.dialog.afterAllClosed.pipe( + skipWhile(() => !this.overlayMode), + takeUntil(this.onDestroy$) + ).subscribe(() => this.shouldCloseViewer = true); + + this.keyDown$.pipe( + skipWhile(() => !this.overlayMode), + filter((e: KeyboardEvent) => e.keyCode === 27), + takeUntil(this.onDestroy$) + ).subscribe( (event: KeyboardEvent) => { + event.preventDefault(); + + if (this.shouldCloseViewer) { + this.close(); + } + }); + } + private generateCacheBusterNumber() { this.cacheBusterNumber = Date.now(); }