diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking-answers/checking-comments/checking-comments.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking-answers/checking-comments/checking-comments.component.ts index 92c82c9d6c8..550bdbdf6fd 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking-answers/checking-comments/checking-comments.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking-answers/checking-comments/checking-comments.component.ts @@ -1,4 +1,4 @@ -import { Component, EventEmitter, Input, OnInit, Output, ViewChild } from '@angular/core'; +import { Component, DestroyRef, EventEmitter, Input, OnInit, Output, ViewChild } from '@angular/core'; import { cloneDeep, sortBy } from 'lodash-es'; import { Operation } from 'realtime-server/lib/esm/common/models/project-rights'; import { Answer } from 'realtime-server/lib/esm/scriptureforge/models/answer'; @@ -8,8 +8,8 @@ import { SF_PROJECT_RIGHTS, SFProjectDomain } from 'realtime-server/lib/esm/scri import { debounceTime } from 'rxjs/operators'; import { DialogService } from 'xforge-common/dialog.service'; import { I18nService } from 'xforge-common/i18n.service'; -import { SubscriptionDisposable } from 'xforge-common/subscription-disposable'; import { UserService } from 'xforge-common/user.service'; +import { quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util'; import { QuestionDoc } from '../../../../core/models/question-doc'; import { SFProjectUserConfigDoc } from '../../../../core/models/sf-project-user-config-doc'; import { AudioAttachment } from '../../checking-audio-player/checking-audio-player.component'; @@ -29,7 +29,7 @@ export interface CommentAction { styleUrls: ['./checking-comments.component.scss'], standalone: false }) -export class CheckingCommentsComponent extends SubscriptionDisposable implements OnInit { +export class CheckingCommentsComponent implements OnInit { @ViewChild(CheckingInputFormComponent) inputComponent?: CheckingInputFormComponent; @Input() project?: SFProjectProfile; @Input() projectUserConfigDoc?: SFProjectUserConfigDoc; @@ -46,10 +46,9 @@ export class CheckingCommentsComponent extends SubscriptionDisposable implements constructor( private readonly dialogService: DialogService, private userService: UserService, - private readonly i18n: I18nService - ) { - super(); - } + private readonly i18n: I18nService, + private readonly destroyRef: DestroyRef + ) {} get showMoreCommentsLabel(): string { const comments = this.getSortedComments(); @@ -148,34 +147,36 @@ export class CheckingCommentsComponent extends SubscriptionDisposable implements if (this.questionDoc != null) { // Give the user two seconds before marking the comment as read. This also prevents SF-624 - prematurely // marking a remotely added comment as read - this.subscribe(this.questionDoc.remoteChanges$.pipe(debounceTime(2000)), () => { - if (this.projectUserConfigDoc == null || this.projectUserConfigDoc.data == null || this.answer == null) { - return; - } - const commentsLength: number = this.answer.comments.filter(comment => !comment.deleted).length; - const defaultCommentsToShow = - commentsLength > this.maxCommentsToShow ? this.maxCommentsToShow - 1 : commentsLength; - const commentsToShow = this.showAllComments ? commentsLength : defaultCommentsToShow; - const commentIdsToMarkRead: string[] = []; - let commentNumber = 1; - // Older comments are displayed above newer comments, so iterate over comments starting with the oldest - for (const comment of this.getSortedComments()) { - if (!this.projectUserConfigDoc.data.commentRefsRead.includes(comment.dataId)) { - commentIdsToMarkRead.push(comment.dataId); - } - commentNumber++; - if (commentNumber > commentsToShow) { - break; + this.questionDoc.remoteChanges$ + .pipe(debounceTime(2000), quietTakeUntilDestroyed(this.destroyRef)) + .subscribe(() => { + if (this.projectUserConfigDoc == null || this.projectUserConfigDoc.data == null || this.answer == null) { + return; } - } - if (commentIdsToMarkRead.length) { - this.projectUserConfigDoc.submitJson0Op(op => { - for (const commentId of commentIdsToMarkRead) { - op.add(puc => puc.commentRefsRead, commentId); + const commentsLength: number = this.answer.comments.filter(comment => !comment.deleted).length; + const defaultCommentsToShow = + commentsLength > this.maxCommentsToShow ? this.maxCommentsToShow - 1 : commentsLength; + const commentsToShow = this.showAllComments ? commentsLength : defaultCommentsToShow; + const commentIdsToMarkRead: string[] = []; + let commentNumber = 1; + // Older comments are displayed above newer comments, so iterate over comments starting with the oldest + for (const comment of this.getSortedComments()) { + if (!this.projectUserConfigDoc.data.commentRefsRead.includes(comment.dataId)) { + commentIdsToMarkRead.push(comment.dataId); } - }); - } - }); + commentNumber++; + if (commentNumber > commentsToShow) { + break; + } + } + if (commentIdsToMarkRead.length) { + this.projectUserConfigDoc.submitJson0Op(op => { + for (const commentId of commentIdsToMarkRead) { + op.add(puc => puc.commentRefsRead, commentId); + } + }); + } + }); } } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking-answers/checking-question/checking-question.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking-answers/checking-question/checking-question.component.ts index 925bc48045e..14c655ec2c2 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking-answers/checking-question/checking-question.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking-answers/checking-question/checking-question.component.ts @@ -20,7 +20,7 @@ import { import { Subscription } from 'rxjs'; import { I18nService } from 'xforge-common/i18n.service'; import { RealtimeQuery } from 'xforge-common/models/realtime-query'; -import { SubscriptionDisposable } from 'xforge-common/subscription-disposable'; +import { quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util'; import { QuestionDoc } from '../../../../core/models/question-doc'; import { TextAudioDoc } from '../../../../core/models/text-audio-doc'; import { SFProjectService } from '../../../../core/sf-project.service'; @@ -33,14 +33,14 @@ import { SingleButtonAudioPlayerComponent } from '../../single-button-audio-play styleUrls: ['./checking-question.component.scss'], standalone: false }) -export class CheckingQuestionComponent extends SubscriptionDisposable implements OnChanges, OnDestroy { +export class CheckingQuestionComponent implements OnChanges, OnDestroy { @Output() audioPlayed: EventEmitter = new EventEmitter(); @ViewChild('questionAudio') questionAudio?: SingleButtonAudioPlayerComponent; @ViewChild('scriptureAudio') set scriptureAudio(comp: SingleButtonAudioPlayerComponent) { if (this._scriptureAudio === comp) return; this._scriptureAudio = comp; if (comp) { - this.subscribe(comp.hasFinishedPlayingOnce$, newVal => { + comp.hasFinishedPlayingOnce$.pipe(quietTakeUntilDestroyed(this.destroyRef)).subscribe(newVal => { if (newVal) { this.selectQuestion(); this.updateUserRefsPlayed(); @@ -64,9 +64,7 @@ export class CheckingQuestionComponent extends SubscriptionDisposable implements private readonly destroyRef: DestroyRef, private readonly projectService: SFProjectService, private readonly i18n: I18nService - ) { - super(); - } + ) {} get focusedText(): string { return this._focusedText; @@ -181,7 +179,7 @@ export class CheckingQuestionComponent extends SubscriptionDisposable implements async ngOnChanges(changes: SimpleChanges): Promise { if (changes['questionDoc']) { - this.dispose(); + this.audioQuery?.dispose(); const projectId: string = this._questionDoc!.data!.projectRef; if (projectId === this.projectId) { @@ -209,7 +207,6 @@ export class CheckingQuestionComponent extends SubscriptionDisposable implements ngOnDestroy(): void { this.isDestroyed = true; - super.ngOnDestroy(); this._audioChangeSub?.unsubscribe(); this.questionDocSub?.unsubscribe(); this.audioQuery?.dispose(); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/single-button-audio-player/single-button-audio-player.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/single-button-audio-player/single-button-audio-player.component.ts index a97291922f9..37f57332db1 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/single-button-audio-player/single-button-audio-player.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/single-button-audio-player/single-button-audio-player.component.ts @@ -1,6 +1,7 @@ -import { Component, Input, OnChanges, OnDestroy } from '@angular/core'; +import { Component, DestroyRef, Input, OnChanges, OnDestroy } from '@angular/core'; import { BehaviorSubject, Subscription } from 'rxjs'; import { OnlineStatusService } from 'xforge-common/online-status.service'; +import { quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util'; import { AudioPlayer, AudioStatus } from '../../../shared/audio/audio-player'; import { AudioPlayerBaseComponent } from '../../../shared/audio/audio-player-base/audio-player-base.component'; import { AudioSegmentPlayer } from '../../../shared/audio/audio-segment-player'; @@ -26,8 +27,8 @@ export class SingleButtonAudioPlayerComponent extends AudioPlayerBaseComponent i this._source = source; } - constructor(onlineStatusService: OnlineStatusService) { - super(onlineStatusService); + constructor(onlineStatusService: OnlineStatusService, destroyRef: DestroyRef) { + super(onlineStatusService, destroyRef); } get progressInDegrees(): string { @@ -71,13 +72,13 @@ export class SingleButtonAudioPlayerComponent extends AudioPlayerBaseComponent i this.audio = new AudioPlayer(this._source, this.onlineStatusService); } - this.subscribe(this.audio.status$, newVal => { + this.audio.status$.pipe(quietTakeUntilDestroyed(this.destroyRef)).subscribe(newVal => { if (newVal === AudioStatus.Available) { this.isAudioAvailable$.next(true); this.calculateProgress(); } }); - this.subscribe(this.audio.finishedPlaying$, () => { + this.audio.finishedPlaying$.pipe(quietTakeUntilDestroyed(this.destroyRef)).subscribe(() => { if (!this.hasFinishedPlayingOnce$.value) { this.hasFinishedPlayingOnce$.next(true); } @@ -90,7 +91,7 @@ export class SingleButtonAudioPlayerComponent extends AudioPlayerBaseComponent i } } - ngOnDestroy(): void { + override ngOnDestroy(): void { super.ngOnDestroy(); this._timeUpdatedSubscription?.unsubscribe(); } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/audio/audio-player-base/audio-player-base.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/audio/audio-player-base/audio-player-base.component.ts index 5cddbdf28a6..31493426c3a 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/audio/audio-player-base/audio-player-base.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/audio/audio-player-base/audio-player-base.component.ts @@ -1,23 +1,25 @@ -import { Component, Input, OnDestroy } from '@angular/core'; +import { Component, DestroyRef, Input, OnDestroy } from '@angular/core'; import { BehaviorSubject } from 'rxjs'; import { OnlineStatusService } from 'xforge-common/online-status.service'; -import { SubscriptionDisposable } from 'xforge-common/subscription-disposable'; +import { quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util'; import { AudioPlayer, AudioStatus } from '../audio-player'; @Component({ template: ``, standalone: false }) -export abstract class AudioPlayerBaseComponent extends SubscriptionDisposable implements OnDestroy { +export abstract class AudioPlayerBaseComponent implements OnDestroy { readonly isAudioAvailable$: BehaviorSubject = new BehaviorSubject(false); private _audioStatus: AudioStatus | undefined; private _hasProblem: boolean = false; private _isAudioAvailable: boolean = false; private _audio: AudioPlayer | undefined; - constructor(protected readonly onlineStatusService: OnlineStatusService) { - super(); - this.subscribe(this.isAudioAvailable$, newVal => { + constructor( + protected readonly onlineStatusService: OnlineStatusService, + protected readonly destroyRef: DestroyRef + ) { + this.isAudioAvailable$.pipe(quietTakeUntilDestroyed(this.destroyRef)).subscribe(newVal => { this._isAudioAvailable = newVal; this.audio?.setSeek(0); }); @@ -38,7 +40,7 @@ export abstract class AudioPlayerBaseComponent extends SubscriptionDisposable im protected set audio(value: AudioPlayer | undefined) { this._audio = value; if (this._audio !== undefined) { - this.subscribe(this._audio.status$, newVal => { + this._audio.status$.pipe(quietTakeUntilDestroyed(this.destroyRef)).subscribe(newVal => { this._audioStatus = newVal; if (newVal === AudioStatus.Available) { this.hasProblem = false; @@ -73,8 +75,7 @@ export abstract class AudioPlayerBaseComponent extends SubscriptionDisposable im } } - override ngOnDestroy(): void { - super.ngOnDestroy(); + ngOnDestroy(): void { this.audio?.dispose(); } } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/audio/audio-player/audio-player.component.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/audio/audio-player/audio-player.component.ts index 22d13709a09..bda5b7fd8d5 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/shared/audio/audio-player/audio-player.component.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/shared/audio/audio-player/audio-player.component.ts @@ -1,4 +1,4 @@ -import { Component, OnDestroy } from '@angular/core'; +import { Component, DestroyRef, OnDestroy } from '@angular/core'; import { MatSliderDragEvent } from '@angular/material/slider'; import { Subscription } from 'rxjs'; import { I18nService } from 'xforge-common/i18n.service'; @@ -18,9 +18,10 @@ export class AudioPlayerComponent extends AudioPlayerBaseComponent implements On constructor( onlineStatusService: OnlineStatusService, - readonly i18n: I18nService + readonly i18n: I18nService, + destroyRef: DestroyRef ) { - super(onlineStatusService); + super(onlineStatusService, destroyRef); } get currentTime(): number { @@ -50,7 +51,7 @@ export class AudioPlayerComponent extends AudioPlayerBaseComponent implements On }); } - ngOnDestroy(): void { + override ngOnDestroy(): void { super.ngOnDestroy(); this._timeUpdatedSubscription?.unsubscribe(); } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/pwa.service.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/pwa.service.spec.ts index 25b85f11524..f891caacae8 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/pwa.service.spec.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/pwa.service.spec.ts @@ -1,3 +1,4 @@ +import { DestroyRef } from '@angular/core'; import { fakeAsync, tick } from '@angular/core/testing'; import { SwUpdate, VersionEvent, VersionReadyEvent } from '@angular/service-worker'; import { BehaviorSubject, Subject } from 'rxjs'; @@ -72,6 +73,7 @@ class TestEnvironment { readonly pwaService: PwaService; private versionUpdates$: Subject = new Subject(); static isRunningInstalledApp$: BehaviorSubject = new BehaviorSubject(false); + private readonly mockDestroyRef: DestroyRef = { onDestroy: (_cb: () => void) => () => {} }; constructor() { TestEnvironment.isRunningInstalledApp$.next(false); @@ -80,7 +82,8 @@ class TestEnvironment { instance(mockedSwUpdate), instance(mockedLocationService), instance(mockedLocalSettingsService), - mockedWindow + mockedWindow, + this.mockDestroyRef ); when(mockedSwUpdate.versionUpdates).thenReturn(this.versionUpdates$); when(mockedSwUpdate.checkForUpdate()).thenResolve(true); @@ -88,7 +91,7 @@ class TestEnvironment { } dispose(): void { - this.pwaService.dispose(); + // No longer need to call dispose since we're using DestroyRef } triggerVersionEvent(type: 'VERSION_DETECTED' | 'VERSION_READY' | 'VERSION_INSTALLATION_FAILED'): void { diff --git a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/pwa.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/pwa.service.ts index 59ad22e7944..8d17e9b755b 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/pwa.service.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/pwa.service.ts @@ -1,10 +1,10 @@ -import { Inject, Injectable } from '@angular/core'; +import { DestroyRef, Inject, Injectable } from '@angular/core'; import { SwUpdate, VersionReadyEvent } from '@angular/service-worker'; import { BehaviorSubject, interval, Observable } from 'rxjs'; -import { filter, takeUntil } from 'rxjs/operators'; +import { filter } from 'rxjs/operators'; import { WINDOW } from 'xforge-common/browser-globals'; import { LocalSettingsService } from 'xforge-common/local-settings.service'; -import { SubscriptionDisposable } from 'xforge-common/subscription-disposable'; +import { quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util'; import { LocationService } from './location.service'; export const PWA_CHECK_FOR_UPDATES = 30_000; @@ -26,7 +26,7 @@ export interface InstallPromptOutcome { @Injectable({ providedIn: 'root' }) -export class PwaService extends SubscriptionDisposable { +export class PwaService { private readonly _canInstall$: BehaviorSubject = new BehaviorSubject(false); private promptEvent?: BeforeInstallPromptEvent; @@ -34,20 +34,18 @@ export class PwaService extends SubscriptionDisposable { private readonly updates: SwUpdate, private readonly locationService: LocationService, private readonly localSettings: LocalSettingsService, - @Inject(WINDOW) private window: Window + @Inject(WINDOW) private window: Window, + private readonly destroyRef: DestroyRef ) { - super(); - // Check for updates periodically if enabled and the browser supports it if (this.updates.isEnabled) { - const checkForUpdatesInterval$ = interval(PWA_CHECK_FOR_UPDATES) - .pipe(takeUntil(this.ngUnsubscribe)) - .subscribe(() => - this.updates.checkForUpdate().catch((error: any) => { - // Stop checking for updates and throw the error - checkForUpdatesInterval$.unsubscribe(); - throw new Error(error); - }) + interval(PWA_CHECK_FOR_UPDATES) + .pipe(quietTakeUntilDestroyed(this.destroyRef)) + .subscribe( + () => + void this.updates.checkForUpdate().catch((error: any) => { + throw new Error(error); + }) ); } @@ -87,7 +85,7 @@ export class PwaService extends SubscriptionDisposable { } activateUpdates(): void { - this.updates.activateUpdate(); + void this.updates.activateUpdate(); this.locationService.reload(); } diff --git a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/sharedb-realtime-remote-store.ts b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/sharedb-realtime-remote-store.ts index 3d590bebef7..d3fcb9b4ba9 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/sharedb-realtime-remote-store.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/xforge-common/sharedb-realtime-remote-store.ts @@ -247,8 +247,8 @@ export class SharedbRealtimeDocAdapter implements RealtimeDocAdapter { this.idle$ = fromEvent(this.doc, 'no write pending').pipe(map(() => undefined)); this.create$ = fromEvent(this.doc, 'create').pipe(map(() => undefined)); this.delete$ = fromEvent(this.doc, 'del').pipe(map(() => undefined)); - this.changes$ = fromEvent<[any, any]>(this.doc, 'op').pipe(map(([ops]) => ops)); - this.remoteChanges$ = fromEvent<[any, any]>(this.doc, 'op').pipe( + this.changes$ = (fromEvent(this.doc, 'op') as Observable<[any, any]>).pipe(map(([ops]) => ops)); + this.remoteChanges$ = (fromEvent(this.doc, 'op') as Observable<[any, any]>).pipe( filter(([, source]) => !source), map(([ops]) => ops) );