From 978b072bff1b660464a887c9e603c5f8752b26fb Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Tue, 28 Jan 2025 22:26:30 -0800 Subject: [PATCH] Enhancement: improve doc details close behavior (#8937) --- src-ui/src/app/app.component.ts | 4 +- .../document-detail.component.spec.ts | 13 +++ .../document-detail.component.ts | 8 +- .../services/component-router.service.spec.ts | 102 ++++++++++++++++++ .../app/services/component-router.service.ts | 35 ++++++ 5 files changed, 160 insertions(+), 2 deletions(-) create mode 100644 src-ui/src/app/services/component-router.service.spec.ts create mode 100644 src-ui/src/app/services/component-router.service.ts diff --git a/src-ui/src/app/app.component.ts b/src-ui/src/app/app.component.ts index d22b4ca38..c89f5d4c2 100644 --- a/src-ui/src/app/app.component.ts +++ b/src-ui/src/app/app.component.ts @@ -5,6 +5,7 @@ import { first, Subscription } from 'rxjs' import { ToastsComponent } from './components/common/toasts/toasts.component' import { FileDropComponent } from './components/file-drop/file-drop.component' import { SETTINGS_KEYS } from './data/ui-settings' +import { ComponentRouterService } from './services/component-router.service' import { ConsumerStatusService } from './services/consumer-status.service' import { HotKeyService } from './services/hot-key.service' import { @@ -41,7 +42,8 @@ export class AppComponent implements OnInit, OnDestroy { public tourService: TourService, private renderer: Renderer2, private permissionsService: PermissionsService, - private hotKeyService: HotKeyService + private hotKeyService: HotKeyService, + private componentRouterService: ComponentRouterService ) { let anyWindow = window as any anyWindow.pdfWorkerSrc = 'assets/js/pdf.worker.min.mjs' diff --git a/src-ui/src/app/components/document-detail/document-detail.component.spec.ts b/src-ui/src/app/components/document-detail/document-detail.component.spec.ts index fb2596d4a..0a2e5605f 100644 --- a/src-ui/src/app/components/document-detail/document-detail.component.spec.ts +++ b/src-ui/src/app/components/document-detail/document-detail.component.spec.ts @@ -45,6 +45,7 @@ import { Tag } from 'src/app/data/tag' import { PermissionsGuard } from 'src/app/guards/permissions.guard' import { CustomDatePipe } from 'src/app/pipes/custom-date.pipe' import { DocumentTitlePipe } from 'src/app/pipes/document-title.pipe' +import { ComponentRouterService } from 'src/app/services/component-router.service' import { DocumentListViewService } from 'src/app/services/document-list-view.service' import { OpenDocumentsService } from 'src/app/services/open-documents.service' import { PermissionsService } from 'src/app/services/permissions.service' @@ -127,6 +128,7 @@ describe('DocumentDetailComponent', () => { let settingsService: SettingsService let customFieldsService: CustomFieldsService let httpTestingController: HttpTestingController + let componentRouterService: ComponentRouterService let currentUserCan = true let currentUserHasObjectPermissions = true @@ -264,6 +266,7 @@ describe('DocumentDetailComponent', () => { customFieldsService = TestBed.inject(CustomFieldsService) fixture = TestBed.createComponent(DocumentDetailComponent) httpTestingController = TestBed.inject(HttpTestingController) + componentRouterService = TestBed.inject(ComponentRouterService) component = fixture.componentInstance }) @@ -568,6 +571,16 @@ describe('DocumentDetailComponent', () => { expect(navigateSpy).toHaveBeenCalledWith(['documents']) }) + it('should allow close and navigate to the last view if available', () => { + initNormally() + jest + .spyOn(componentRouterService, 'getComponentURLBefore') + .mockReturnValue('dashboard') + const navigateSpy = jest.spyOn(router, 'navigate') + component.close() + expect(navigateSpy).toHaveBeenCalledWith(['dashboard']) + }) + it('should allow close and navigate to documents by default', () => { initNormally() jest diff --git a/src-ui/src/app/components/document-detail/document-detail.component.ts b/src-ui/src/app/components/document-detail/document-detail.component.ts index 6b65ad335..8d1b35071 100644 --- a/src-ui/src/app/components/document-detail/document-detail.component.ts +++ b/src-ui/src/app/components/document-detail/document-detail.component.ts @@ -59,6 +59,7 @@ import { CustomDatePipe } from 'src/app/pipes/custom-date.pipe' import { DocumentTitlePipe } from 'src/app/pipes/document-title.pipe' import { FileSizePipe } from 'src/app/pipes/file-size.pipe' import { SafeUrlPipe } from 'src/app/pipes/safeurl.pipe' +import { ComponentRouterService } from 'src/app/services/component-router.service' import { DocumentListViewService } from 'src/app/services/document-list-view.service' import { HotKeyService } from 'src/app/services/hot-key.service' import { OpenDocumentsService } from 'src/app/services/open-documents.service' @@ -272,7 +273,8 @@ export class DocumentDetailComponent private userService: UserService, private customFieldsService: CustomFieldsService, private http: HttpClient, - private hotKeyService: HotKeyService + private hotKeyService: HotKeyService, + private componentRouterService: ComponentRouterService ) { super() } @@ -888,6 +890,10 @@ export class DocumentDetailComponent 'view', this.documentListViewService.activeSavedViewId, ]) + } else if (this.componentRouterService.getComponentURLBefore()) { + this.router.navigate([ + this.componentRouterService.getComponentURLBefore(), + ]) } else { this.router.navigate(['documents']) } diff --git a/src-ui/src/app/services/component-router.service.spec.ts b/src-ui/src/app/services/component-router.service.spec.ts new file mode 100644 index 000000000..b11fc8197 --- /dev/null +++ b/src-ui/src/app/services/component-router.service.spec.ts @@ -0,0 +1,102 @@ +import { TestBed } from '@angular/core/testing' +import { ActivationStart, Router } from '@angular/router' +import { Subject } from 'rxjs' +import { ComponentRouterService } from './component-router.service' + +describe('ComponentRouterService', () => { + let service: ComponentRouterService + let router: Router + let eventsSubject: Subject + + beforeEach(() => { + eventsSubject = new Subject() + TestBed.configureTestingModule({ + providers: [ + ComponentRouterService, + { + provide: Router, + useValue: { + events: eventsSubject.asObservable(), + }, + }, + ], + }) + service = TestBed.inject(ComponentRouterService) + router = TestBed.inject(Router) + }) + + it('should add to history and componentHistory on ActivationStart event', () => { + eventsSubject.next( + new ActivationStart({ + url: 'test-url', + component: { name: 'TestComponent' }, + } as any) + ) + + expect((service as any).history).toEqual(['test-url']) + expect((service as any).componentHistory).toEqual(['TestComponent']) + }) + + it('should not add duplicate component names to componentHistory', () => { + eventsSubject.next( + new ActivationStart({ + url: 'test-url-1', + component: { name: 'TestComponent' }, + } as any) + ) + eventsSubject.next( + new ActivationStart({ + url: 'test-url-2', + component: { name: 'TestComponent' }, + } as any) + ) + + expect((service as any).componentHistory.length).toBe(1) + expect((service as any).componentHistory).toEqual(['TestComponent']) + }) + + it('should return the URL of the component before the current one', () => { + eventsSubject.next( + new ActivationStart({ + url: 'test-url-1', + component: { name: 'TestComponent1' }, + } as any) + ) + eventsSubject.next( + new ActivationStart({ + url: 'test-url-2', + component: { name: 'TestComponent2' }, + } as any) + ) + + expect(service.getComponentURLBefore()).toBe('test-url-1') + }) + + it('should update the URL of the current component if the same component is loaded via a different URL', () => { + eventsSubject.next( + new ActivationStart({ + url: 'test-url-1', + component: { name: 'TestComponent' }, + } as any) + ) + eventsSubject.next( + new ActivationStart({ + url: 'test-url-2', + component: { name: 'TestComponent' }, + } as any) + ) + + expect((service as any).history).toEqual(['test-url-2']) + }) + + it('should return null if there is no previous component', () => { + eventsSubject.next( + new ActivationStart({ + url: 'test-url', + component: { name: 'TestComponent' }, + } as any) + ) + + expect(service.getComponentURLBefore()).toBeNull() + }) +}) diff --git a/src-ui/src/app/services/component-router.service.ts b/src-ui/src/app/services/component-router.service.ts new file mode 100644 index 000000000..3f97584b7 --- /dev/null +++ b/src-ui/src/app/services/component-router.service.ts @@ -0,0 +1,35 @@ +import { Injectable } from '@angular/core' +import { ActivationStart, Event, Router } from '@angular/router' +import { filter } from 'rxjs' + +@Injectable({ + providedIn: 'root', +}) +export class ComponentRouterService { + private history: string[] = [] + private componentHistory: any[] = [] + + constructor(private router: Router) { + this.router.events + .pipe(filter((event: Event) => event instanceof ActivationStart)) + .subscribe((event: ActivationStart) => { + if ( + this.componentHistory[this.componentHistory.length - 1] !== + event.snapshot.component.name + ) { + this.history.push(event.snapshot.url.toString()) + this.componentHistory.push(event.snapshot.component.name) + } else { + // Update the URL of the current component in case the same component was loaded via a different URL + this.history[this.history.length - 1] = event.snapshot.url.toString() + } + }) + } + + public getComponentURLBefore(): any { + if (this.componentHistory.length > 1) { + return this.history[this.history.length - 2] + } + return null + } +}