From 671c44438db69f47af27f65a20cd1400b0ff551c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adina=20=C8=9Aeudan?= Date: Mon, 30 Sep 2024 18:30:30 +0300 Subject: [PATCH 1/2] RED-10034: File workload improvementes - WIP --- .../file-workload.component.html | 10 +- .../file-workload/file-workload.component.ts | 121 ++++++++++-------- 2 files changed, 72 insertions(+), 59 deletions(-) diff --git a/apps/red-ui/src/app/modules/file-preview/components/file-workload/file-workload.component.html b/apps/red-ui/src/app/modules/file-preview/components/file-workload/file-workload.component.html index 32b509d90..47d3dc13b 100644 --- a/apps/red-ui/src/app/modules/file-preview/components/file-workload/file-workload.component.html +++ b/apps/red-ui/src/app/modules/file-preview/components/file-workload/file-workload.component.html @@ -32,11 +32,11 @@ -@if (displayedAnnotations$ | async; as annotations) { +@if (filteredAnnotations$ | async; as annotations) {
@@ -112,7 +112,7 @@ > @@ -200,7 +200,7 @@
} @@ -224,7 +224,7 @@ - + diff --git a/apps/red-ui/src/app/modules/file-preview/components/file-workload/file-workload.component.ts b/apps/red-ui/src/app/modules/file-preview/components/file-workload/file-workload.component.ts index 6ccf8e4c7..f65803d1d 100644 --- a/apps/red-ui/src/app/modules/file-preview/components/file-workload/file-workload.component.ts +++ b/apps/red-ui/src/app/modules/file-preview/components/file-workload/file-workload.component.ts @@ -9,6 +9,7 @@ import { OnDestroy, OnInit, TemplateRef, + untracked, viewChild, } from '@angular/core'; import { MatDialog } from '@angular/material/dialog'; @@ -89,23 +90,30 @@ const ALL_HOTKEY_ARRAY = ['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown']; ], }) export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, OnDestroy { - private readonly _annotationsElement = viewChild('annotationsElement'); - private readonly _quickNavigationElement = viewChild('quickNavigation'); readonly multiSelectTemplate = viewChild>('multiSelect'); - readonly #isIqserDevMode = this._userPreferenceService.isIqserDevMode; + readonly annotationsList$: Observable[]>; + readonly allPages = computed(() => Array.from({ length: this.state.file()?.numberOfPages }, (_x, i) => i + 1)); protected readonly iconButtonTypes = IconButtonTypes; protected readonly circleButtonTypes = CircleButtonTypes; - protected readonly displayedAnnotations$: Observable[]>>; + protected readonly filteredAnnotations$: Observable[]>>; protected readonly title = computed(() => this.viewModeService.isEarmarks() ? _('file-preview.tabs.highlights.label') : _('file-preview.tabs.annotations.label'), ); protected readonly currentPageIsExcluded = computed(() => this.state.file().excludedPages.includes(this.pdf.currentPage())); protected readonly translations = workloadTranslations; protected readonly isDocumine = getConfig().IS_DOCUMINE; + readonly showAnalysisDisabledBanner = computed(() => { + const file = this.state.file(); + return this.isDocumine && file.excludedFromAutomaticAnalysis && file.workflowStatus !== WorkflowFileStatuses.APPROVED; + }); protected displayedAnnotations = new Map(); + readonly activeAnnotations = computed(() => this.displayedAnnotations.get(this.pdf.currentPage()) || []); protected displayedPages: number[] = []; protected pagesPanelActive = true; protected enabledFilters = []; + private readonly _annotationsElement = viewChild('annotationsElement'); + private readonly _quickNavigationElement = viewChild('quickNavigation'); + readonly #isIqserDevMode = this._userPreferenceService.isIqserDevMode; #displayedPagesChanged = false; constructor( @@ -129,10 +137,9 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On ) { super(); - // TODO: ngOnDetach is not called here, so we need to unsubscribe manually - this.addActiveScreenSubscription = this.pdf.currentPage$.subscribe(pageNumber => { + effect(() => { this._scrollViews(); - this.scrollAnnotationsToPage(pageNumber, 'always'); + this.scrollAnnotationsToPage(this.pdf.currentPage(), 'always'); }); this.addActiveScreenSubscription = this.listingService.selected$.subscribe(annotationIds => { @@ -146,7 +153,11 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On this.handleKeyEvent($event); }); - this.displayedAnnotations$ = this._displayedAnnotations$; + this.filteredAnnotations$ = this._filteredAnnotations$; + + this.annotationsList$ = combineLatest([this.filteredAnnotations$, this.pdf.currentPage$]).pipe( + map(([annotations, page]) => annotations.get(page)), + ); effect(() => { if (this.multiSelectService.inactive()) { @@ -169,20 +180,11 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On ); } - get activeAnnotations(): AnnotationWrapper[] { - return this.displayedAnnotations.get(this.pdf.currentPage()) || []; - } - - get showAnalysisDisabledBanner() { - const file = this.state.file(); - return this.isDocumine && file.excludedFromAutomaticAnalysis && file.workflowStatus !== WorkflowFileStatuses.APPROVED; - } - private get _firstSelectedAnnotation() { return this.listingService.selected.length ? this.listingService.selected[0] : null; } - private get _displayedAnnotations$(): Observable[]>> { + private get _filteredAnnotations$(): Observable[]>> { const primary$ = this.filterService.getFilterModels$('primaryFilters'); const secondary$ = this.filterService.getFilterModels$('secondaryFilters'); @@ -205,10 +207,6 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On ); } - get #allPages() { - return Array.from({ length: this.state.file()?.numberOfPages }, (_x, i) => i + 1); - } - private static _scrollToFirstElement(elements: HTMLElement[], mode: 'always' | 'if-needed' = 'if-needed') { if (elements.length > 0) { scrollIntoView(elements[0], { @@ -222,12 +220,13 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On ngOnInit(): void { setTimeout(() => { - const showExcludePages = getLocalStorageDataByFileId(this.state.file()?.id, 'show-exclude-pages') ?? false; + const file = untracked(this.state.file); + const showExcludePages = getLocalStorageDataByFileId(file?.id, 'show-exclude-pages') ?? false; if (showExcludePages) { this.excludedPagesService.show(); } - const showDocumentInfo = getLocalStorageDataByFileId(this.state.file()?.id, 'show-document-info') ?? false; + const showDocumentInfo = getLocalStorageDataByFileId(file?.id, 'show-document-info') ?? false; if (showDocumentInfo) { this.documentInfoService.show(); } @@ -235,16 +234,20 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On } selectAllOnActivePage() { - this.listingService.selectAnnotations(this.activeAnnotations); + const activeAnnotations = untracked(this.activeAnnotations); + this.listingService.selectAnnotations(activeAnnotations); } deselectAllOnActivePage(): void { - this.listingService.deselect(this.activeAnnotations); - this.annotationManager.deselect(this.activeAnnotations); + const activeAnnotations = untracked(this.activeAnnotations); + this.listingService.deselect(activeAnnotations); + this.annotationManager.deselect(activeAnnotations); } @HostListener('window:keyup', ['$event']) handleKeyEvent($event: KeyboardEvent): void { + const multiSelectServiceInactive = untracked(this.multiSelectService.inactive); + if ( !ALL_HOTKEY_ARRAY.includes($event.key) || this._dialog.openDialogs.length || @@ -264,7 +267,7 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On // if we activated annotationsPanel - // select first annotation from this page in case there is no // selected annotation on this page and not in multi select mode - if (!this.pagesPanelActive && this.multiSelectService.inactive()) { + if (!this.pagesPanelActive && multiSelectServiceInactive) { this._documentViewer.clearSelection(); this.#selectFirstAnnotationOnCurrentPageIfNecessary(); } @@ -275,7 +278,7 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On if (!this.pagesPanelActive) { // Disable annotation navigation in multi select mode // => TODO: maybe implement selection on enter? - if (this.multiSelectService.inactive()) { + if (multiSelectServiceInactive) { this.navigateAnnotations($event); } } else { @@ -286,7 +289,7 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On } scrollAnnotations(): void { - const currentPage = this.pdf.currentPage(); + const currentPage = untracked(this.pdf.currentPage); if (this._firstSelectedAnnotation?.pageNumber === currentPage) { return; } @@ -294,27 +297,27 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On } scrollAnnotationsToPage(page: number, mode: 'always' | 'if-needed' = 'if-needed'): void { - if (this._annotationsElement()) { - const elements: HTMLElement[] = this._annotationsElement().nativeElement.querySelectorAll( - `div[anotation-page-header="${page}"]`, - ); + const annotationsElement = untracked(this._annotationsElement); + if (annotationsElement) { + const elements: HTMLElement[] = annotationsElement.nativeElement.querySelectorAll(`div[anotation-page-header="${page}"]`); FileWorkloadComponent._scrollToFirstElement(elements, mode); } } @Debounce() scrollToSelectedAnnotation(): void { - if (this.listingService.selected.length === 0 || !this._annotationsElement()) { + const annotationsElement = untracked(this._annotationsElement); + if (this.listingService.selected.length === 0 || annotationsElement) { return; } - const elements: HTMLElement[] = this._annotationsElement().nativeElement.querySelectorAll( + const elements: HTMLElement[] = annotationsElement.nativeElement.querySelectorAll( `[annotation-id="${this._firstSelectedAnnotation?.id}"]`, ); FileWorkloadComponent._scrollToFirstElement(elements); } scrollQuickNavigation(): void { - const currentPage = this.pdf.currentPage(); + const currentPage = untracked(this.pdf.currentPage); let quickNavPageIndex = this.displayedPages.findIndex(p => p >= currentPage); if (quickNavPageIndex === -1 || this.displayedPages[quickNavPageIndex] !== currentPage) { quickNavPageIndex = Math.max(0, quickNavPageIndex - 1); @@ -327,7 +330,8 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On } scrollQuickNavLast() { - this.pdf.navigateTo(this.state.file().numberOfPages); + const file = untracked(this.state.file); + this.pdf.navigateTo(file.numberOfPages); } preventKeyDefault($event: KeyboardEvent): void { @@ -345,11 +349,12 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On } navigateAnnotations($event: KeyboardEvent) { - const currentPage = this.pdf.currentPage(); + const currentPage = untracked(this.pdf.currentPage); + const activeAnnotations = untracked(this.activeAnnotations); if (!this._firstSelectedAnnotation || currentPage !== this._firstSelectedAnnotation.pageNumber) { if (this.displayedPages.indexOf(currentPage) !== -1) { // Displayed page has annotations - return this.listingService.selectAnnotations(this.activeAnnotations ? this.activeAnnotations[0] : null); + return this.listingService.selectAnnotations(activeAnnotations ? activeAnnotations[0] : null); } // Displayed page doesn't have annotations if ($event.key === 'ArrowDown') { @@ -422,14 +427,16 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On secondary: INestedFilter[] = [], componentReferenceIds: string[], ): Map { - const onlyPageWithAnnotations = this.viewModeService.onlyPagesWithAnnotations(); + const onlyPageWithAnnotations = untracked(this.viewModeService.onlyPagesWithAnnotations); + const isRedacted = untracked(this.viewModeService.isRedacted); + const allPages = untracked(this.allPages); if (!primary || primary.length === 0) { - const pages = onlyPageWithAnnotations ? [] : this.#allPages; + const pages = onlyPageWithAnnotations ? [] : allPages; this.#setDisplayedPages(pages); return; } - if (this.viewModeService.isRedacted()) { + if (isRedacted) { annotations = annotations.filter(a => !a.isRemoved); } @@ -447,7 +454,7 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On this.enabledFilters = this.filterService.enabledFlatFilters; if (this.enabledFilters.some(f => f.id === 'pages-without-annotations')) { if (this.enabledFilters.length === 1 && !onlyPageWithAnnotations) { - const pages = this.#allPages.filter(page => !pagesThatDisplayAnnotations.includes(page)); + const pages = allPages.filter(page => !pagesThatDisplayAnnotations.includes(page)); this.#setDisplayedPages(pages); } else { this.#setDisplayedPages([]); @@ -456,7 +463,7 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On } else if (this.enabledFilters.length || onlyPageWithAnnotations || componentReferenceIds) { this.#setDisplayedPages(pagesThatDisplayAnnotations); } else { - this.#setDisplayedPages(this.#allPages); + this.#setDisplayedPages(allPages); } this.displayedPages.sort((a, b) => a - b); @@ -464,18 +471,20 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On } #selectFirstAnnotationOnCurrentPageIfNecessary() { - const currentPage = this.pdf.currentPage(); + const currentPage = untracked(this.pdf.currentPage); + const activeAnnotations = untracked(this.activeAnnotations); if ( (!this._firstSelectedAnnotation || currentPage !== this._firstSelectedAnnotation.pageNumber) && this.displayedPages.indexOf(currentPage) >= 0 && - this.activeAnnotations.length > 0 + activeAnnotations.length > 0 ) { - this.listingService.selectAnnotations(this.activeAnnotations[0]); + this.listingService.selectAnnotations(activeAnnotations[0]); } } #navigatePages($event: KeyboardEvent) { - const pageIdx = this.displayedPages.indexOf(this.pdf.currentPage()); + const currentPage = untracked(this.pdf.currentPage); + const pageIdx = this.displayedPages.indexOf(currentPage); if ($event.key !== 'ArrowDown') { if (pageIdx === -1) { @@ -507,9 +516,10 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On } #nextPageWithAnnotations() { + const currentPage = untracked(this.pdf.currentPage); let idx = 0; for (const page of this.displayedPages) { - if (page > this.pdf.currentPage() && this.displayedAnnotations.get(page)) { + if (page > currentPage && this.displayedAnnotations.get(page)) { break; } ++idx; @@ -518,10 +528,11 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On } #prevPageWithAnnotations() { + const currentPage = untracked(this.pdf.currentPage); let idx = this.displayedPages.length - 1; const reverseDisplayedPages = [...this.displayedPages].reverse(); for (const page of reverseDisplayedPages) { - if (page < this.pdf.currentPage() && this.displayedAnnotations.get(page)) { + if (page < currentPage && this.displayedAnnotations.get(page)) { break; } --idx; @@ -530,8 +541,9 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On } #scrollQuickNavigationToPage(page: number) { - if (this._quickNavigationElement()) { - const elements: HTMLElement[] = this._quickNavigationElement().nativeElement.querySelectorAll(`#quick-nav-page-${page}`); + const quickNavigationElement = untracked(this._quickNavigationElement); + if (quickNavigationElement) { + const elements: HTMLElement[] = quickNavigationElement.nativeElement.querySelectorAll(`#quick-nav-page-${page}`); FileWorkloadComponent._scrollToFirstElement(elements); } } @@ -552,7 +564,8 @@ export class FileWorkloadComponent extends AutoUnsubscribe implements OnInit, On } #scrollToFirstAnnotationPage(annotations: Map[]>) { - if (this.isDocumine && annotations.size && this.#displayedPagesChanged && !this.displayedPages.includes(this.pdf.currentPage())) { + const currentPage = untracked(this.pdf.currentPage); + if (this.isDocumine && annotations.size && this.#displayedPagesChanged && !this.displayedPages.includes(currentPage)) { const page = annotations.keys().next().value; this.pdf.navigateTo(page); } From 9fabb7223a61d829bd22840403c62a14cf8a6539 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adina=20=C8=9Aeudan?= Date: Mon, 7 Oct 2024 19:10:33 +0300 Subject: [PATCH 2/2] RED-10034: More performance improvements --- .../annotation-details.component.html | 34 +++++++++--------- .../annotation-details.component.ts | 35 ++++++++----------- .../annotation-wrapper.component.html | 18 +++++----- .../annotations-list.component.ts | 4 +-- 4 files changed, 44 insertions(+), 47 deletions(-) diff --git a/apps/red-ui/src/app/modules/file-preview/components/annotation-details/annotation-details.component.html b/apps/red-ui/src/app/modules/file-preview/components/annotation-details/annotation-details.component.html index c5dbee02b..4dffb7bf4 100644 --- a/apps/red-ui/src/app/modules/file-preview/components/annotation-details/annotation-details.component.html +++ b/apps/red-ui/src/app/modules/file-preview/components/annotation-details/annotation-details.component.html @@ -1,29 +1,31 @@ -
- -
+@if (_noSelection() && _changesTooltip()) { +
+ +
+} - -
- +@if (_noSelection() && _engines()) { +
+
-
+
- +
- +} diff --git a/apps/red-ui/src/app/modules/file-preview/components/annotation-details/annotation-details.component.ts b/apps/red-ui/src/app/modules/file-preview/components/annotation-details/annotation-details.component.ts index 2c647bd76..a092c15b3 100644 --- a/apps/red-ui/src/app/modules/file-preview/components/annotation-details/annotation-details.component.ts +++ b/apps/red-ui/src/app/modules/file-preview/components/annotation-details/annotation-details.component.ts @@ -1,4 +1,4 @@ -import { Component, inject, Input, OnChanges } from '@angular/core'; +import { Component, computed, inject, input, signal } from '@angular/core'; import { marker as _ } from '@biesbjerg/ngx-translate-extract-marker'; import { KeysOf } from '@iqser/common-ui/lib/utils'; import { AnnotationWrapper } from '@models/file/annotation.wrapper'; @@ -33,30 +33,27 @@ const changesProperties: KeysOf[] = [ ]; @Component({ - selector: 'redaction-annotation-details [annotation]', + selector: 'redaction-annotation-details', templateUrl: './annotation-details.component.html', styleUrls: ['./annotation-details.component.scss'], standalone: true, imports: [NgIf, MatTooltip, MatIcon, CdkOverlayOrigin, NgForOf, CdkConnectedOverlay, TranslateModule], }) -export class AnnotationDetailsComponent implements OnChanges { - @Input() annotation: ListItem; - isPopoverOpen = false; - engines: Engine[]; - changesTooltip: string; - noSelection: boolean; +export class AnnotationDetailsComponent { + readonly annotation = input.required>(); + protected readonly _isPopoverOpen = signal(false); + protected readonly _engines = computed(() => this.#extractEngines(this.annotation().item).filter(engine => engine.show)); private readonly _translateService = inject(TranslateService); - private readonly _multiSelectService = inject(MultiSelectService); + protected readonly _changesTooltip = computed(() => { + const annotation = this.annotation().item; + const changes = changesProperties.filter(key => annotation[key]); - getChangesTooltip(): string | undefined { - const changes = changesProperties.filter(key => this.annotation.item[key]); - - if (!changes.length && !this.annotation.item.engines?.includes(LogEntryEngines.MANUAL)) { + if (!changes.length && !annotation.engines?.includes(LogEntryEngines.MANUAL)) { return; } const details = []; - if (this.annotation.item.engines?.includes(LogEntryEngines.MANUAL)) { + if (annotation.engines?.includes(LogEntryEngines.MANUAL)) { details.push(this._translateService.instant(_('annotation-changes.added-locally'))); } @@ -66,13 +63,9 @@ export class AnnotationDetailsComponent implements OnChanges { const header = this._translateService.instant(_('annotation-changes.header')); return [header, ...details.map(change => `• ${change}`)].join('\n'); - } - - ngOnChanges() { - this.engines = this.#extractEngines(this.annotation.item).filter(engine => engine.show); - this.changesTooltip = this.getChangesTooltip(); - this.noSelection = !this.annotation.isSelected || this._multiSelectService.inactive(); - } + }); + private readonly _multiSelectService = inject(MultiSelectService); + protected readonly _noSelection = computed(() => !this.annotation().isSelected || this._multiSelectService.inactive()); #extractEngines(annotation: AnnotationWrapper): Engine[] { return [ diff --git a/apps/red-ui/src/app/modules/file-preview/components/annotation-wrapper/annotation-wrapper.component.html b/apps/red-ui/src/app/modules/file-preview/components/annotation-wrapper/annotation-wrapper.component.html index e3eeb5b22..8c22e3273 100644 --- a/apps/red-ui/src/app/modules/file-preview/components/annotation-wrapper/annotation-wrapper.component.html +++ b/apps/red-ui/src/app/modules/file-preview/components/annotation-wrapper/annotation-wrapper.component.html @@ -1,6 +1,6 @@
-
+
- -
+ @defer (on hover(annotationDiv)) { +
+ +
+ } }
} diff --git a/apps/red-ui/src/app/modules/file-preview/components/annotations-list/annotations-list.component.ts b/apps/red-ui/src/app/modules/file-preview/components/annotations-list/annotations-list.component.ts index c30366d23..a6364564f 100644 --- a/apps/red-ui/src/app/modules/file-preview/components/annotations-list/annotations-list.component.ts +++ b/apps/red-ui/src/app/modules/file-preview/components/annotations-list/annotations-list.component.ts @@ -11,7 +11,7 @@ import { AnnotationReferencesService } from '../../services/annotation-reference import { AnnotationsListingService } from '../../services/annotations-listing.service'; import { MultiSelectService } from '../../services/multi-select.service'; import { ViewModeService } from '../../services/view-mode.service'; -import { NgForOf, NgIf } from '@angular/common'; +import { JsonPipe, NgForOf, NgIf } from '@angular/common'; import { HighlightsSeparatorComponent } from '../highlights-separator/highlights-separator.component'; import { AnnotationWrapperComponent } from '../annotation-wrapper/annotation-wrapper.component'; import { AnnotationReferencesListComponent } from '../annotation-references-list/annotation-references-list.component'; @@ -21,7 +21,7 @@ import { AnnotationReferencesListComponent } from '../annotation-references-list templateUrl: './annotations-list.component.html', styleUrls: ['./annotations-list.component.scss'], standalone: true, - imports: [NgForOf, NgIf, HighlightsSeparatorComponent, AnnotationWrapperComponent, AnnotationReferencesListComponent], + imports: [NgForOf, NgIf, HighlightsSeparatorComponent, AnnotationWrapperComponent, AnnotationReferencesListComponent, JsonPipe], }) export class AnnotationsListComponent extends HasScrollbarDirective { readonly annotations = input.required[]>();