From 84841e21baa8fae5f8a2a85e19efe75632972e8b Mon Sep 17 00:00:00 2001 From: jayden <6512715003@QQ.com> Date: Wed, 31 Dec 2025 10:10:07 +0800 Subject: [PATCH 1/2] fix: prevent preview conversation interruption when switching panels - Add DestroyRef to track component destruction state in ChatConversationPreviewComponent - Guard all subscription callbacks with destroyed check to prevent NG0953 error - Keep preview component mounted when switching panels using CSS display control - Add previewCreated signal and showPreview computed to preserve component state - Fixes #330 --- .../@shared/chat/preview/preview.component.ts | 25 ++++++++++- .../xpert/studio/panel/panel.component.html | 41 ++++++++++--------- .../xpert/studio/panel/panel.component.ts | 17 +++++++- 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/apps/cloud/src/app/@shared/chat/preview/preview.component.ts b/apps/cloud/src/app/@shared/chat/preview/preview.component.ts index eb5d0ed8db..8f10cdcde2 100644 --- a/apps/cloud/src/app/@shared/chat/preview/preview.component.ts +++ b/apps/cloud/src/app/@shared/chat/preview/preview.component.ts @@ -5,6 +5,7 @@ import { booleanAttribute, Component, computed, + DestroyRef, effect, ElementRef, inject, @@ -96,6 +97,9 @@ export class ChatConversationPreviewComponent { readonly confirmDel = injectConfirmDelete() readonly #audioRecorder = inject(AudioRecorderService) readonly #synthesizeService = inject(SynthesizeService) + readonly #destroyRef = inject(DestroyRef) + + #destroyed = false // Inputs readonly conversationId = model() @@ -246,6 +250,11 @@ export class ChatConversationPreviewComponent { effect(() => this.#audioRecorder.canvasRef.set(this.canvasRef()), { allowSignalWrites: true }) effect(() => this.#audioRecorder.xpert.set(this.xpert() as IXpert), { allowSignalWrites: true }) effect(() => this.input.set(this.#audioRecorder.text()), { allowSignalWrites: true }) + + // Set flag when component is destroyed to prevent operations on destroyed component + this.#destroyRef.onDestroy(() => { + this.#destroyed = true + }) } chat(options?: { input?: string; confirm?: boolean; @@ -312,6 +321,11 @@ export class ChatConversationPreviewComponent { ) .subscribe({ next: (msg) => { + // Check if component is destroyed to avoid operations on destroyed component + if (this.#destroyed) { + return + } + if (msg.event === 'error') { this.onChatError(msg.data) } else { @@ -331,6 +345,7 @@ export class ChatConversationPreviewComponent { this.output.update((state) => state + event.data) } } else if (event.type === ChatMessageTypeEnum.EVENT) { + // Component destroyed check is done at the beginning of next callback, emit directly here this.chatEvent.emit(event) switch (event.event) { case ChatMessageEventTypeEnum.ON_CONVERSATION_START: @@ -369,9 +384,16 @@ export class ChatConversationPreviewComponent { } }, error: (err) => { - this.onChatError(getErrorMessage(err)) + // Check if component is destroyed before handling error + if (!this.#destroyed) { + this.onChatError(getErrorMessage(err)) + } }, complete: () => { + // Check if component is destroyed to avoid unnecessary operations + if (this.#destroyed) { + return + } this.loading.set(false) if (this.currentMessage()) { this.appendMessage({ ...this.currentMessage() }) @@ -388,6 +410,7 @@ export class ChatConversationPreviewComponent { } onChatError(message: string) { + // #destroyed check is already done in error callback, no need to check here this.loading.set(false) if (this.currentMessage()) { this.appendMessage({ ...this.currentMessage() }) diff --git a/apps/cloud/src/app/features/xpert/studio/panel/panel.component.html b/apps/cloud/src/app/features/xpert/studio/panel/panel.component.html index 64f0cae5c8..c38fcd707a 100644 --- a/apps/cloud/src/app/features/xpert/studio/panel/panel.component.html +++ b/apps/cloud/src/app/features/xpert/studio/panel/panel.component.html @@ -39,27 +39,28 @@ } - @if (sidePanel(); as sidePanel) { -
- @switch (sidePanel) { - @case ('preview') { - @if (executionId()) { - - } - +
+ + @if (showPreview()) { +
+ @if (sidePanel() === 'preview' && executionId()) { + } + +
+ } - @case ('variables') { - - } - @case ('environments') { - - } - } + + @if (sidePanel() === 'variables') { + + } -
- } + + @if (sidePanel() === 'environments') { + + } +
\ No newline at end of file diff --git a/apps/cloud/src/app/features/xpert/studio/panel/panel.component.ts b/apps/cloud/src/app/features/xpert/studio/panel/panel.component.ts index 9f0b612594..f69acc4a11 100644 --- a/apps/cloud/src/app/features/xpert/studio/panel/panel.component.ts +++ b/apps/cloud/src/app/features/xpert/studio/panel/panel.component.ts @@ -1,6 +1,6 @@ import { CdkMenuModule } from '@angular/cdk/menu' import { CommonModule } from '@angular/common' -import { Component, computed, HostListener, inject, model, signal } from '@angular/core' +import { Component, computed, effect, HostListener, inject, model, signal } from '@angular/core' import { listFadeIn } from 'apps/cloud/src/app/@core' import { SelectionService } from '../domain' import { XpertStudioComponent } from '../studio.component' @@ -48,6 +48,12 @@ export class XpertStudioPanelComponent { return node ? [node] : [] }) + // Track if preview component has been created, keep it mounted once created + readonly previewCreated = signal(false) + readonly showPreview = computed(() => + this.sidePanel() === 'preview' || this.previewCreated() + ) + readonly minPanelWidth = 420 readonly maxPanelWidth = 720 readonly panelWidth = signal(this.minPanelWidth) @@ -55,6 +61,15 @@ export class XpertStudioPanelComponent { private startX = 0 private startWidth = 0 + constructor() { + // Mark preview component as created when switching to preview panel + effect(() => { + if (this.sidePanel() === 'preview') { + this.previewCreated.set(true) + } + }, { allowSignalWrites: true }) + } + close() { this.selectionService.selectNode(null) } From 7ce6cae6e935b13f89a85309959afa232f704dcc Mon Sep 17 00:00:00 2001 From: jayden <6512715003@QQ.com> Date: Wed, 7 Jan 2026 14:46:50 +0800 Subject: [PATCH 2/2] refactor: improve subscription cleanup and preview visibility logic - Add takeUntilDestroyed() operator to chat subscription for automatic cleanup - Add manual chatSubscription cleanup in onDestroy as extra safety measure - Add destroyed check before chatEvent.emit to prevent operations on destroyed component - Fix showPreview logic to prevent preview from showing when sidePanel is null These changes address Copilot review suggestions and improve code robustness. --- .../@shared/chat/preview/preview.component.ts | 19 ++++++++++++++----- .../xpert/studio/panel/panel.component.ts | 2 +- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/apps/cloud/src/app/@shared/chat/preview/preview.component.ts b/apps/cloud/src/app/@shared/chat/preview/preview.component.ts index 8f10cdcde2..33979eeba6 100644 --- a/apps/cloud/src/app/@shared/chat/preview/preview.component.ts +++ b/apps/cloud/src/app/@shared/chat/preview/preview.component.ts @@ -51,7 +51,7 @@ import { MarkdownModule } from 'ngx-markdown' import { derivedAsync } from 'ngxtension/derived-async' import { map, Observable, of, timer, switchMap, tap, Subscription } from 'rxjs' import { effectAction } from '@metad/ocap-angular/core' -import { toObservable } from '@angular/core/rxjs-interop' +import { takeUntilDestroyed, toObservable } from '@angular/core/rxjs-interop' import { injectConfirmDelete } from '@metad/ocap-angular/common' import { CdkMenuModule } from '@angular/cdk/menu' import { XpertPreviewAiMessageComponent } from './ai-message/message.component' @@ -252,8 +252,12 @@ export class ChatConversationPreviewComponent { effect(() => this.input.set(this.#audioRecorder.text()), { allowSignalWrites: true }) // Set flag when component is destroyed to prevent operations on destroyed component + // Also clean up chatSubscription as an extra safety measure this.#destroyRef.onDestroy(() => { this.#destroyed = true + if (this.chatSubscription && !this.chatSubscription.closed) { + this.chatSubscription.unsubscribe() + } }) } @@ -319,9 +323,11 @@ export class ChatConversationPreviewComponent { isDraft: true } ) + .pipe(takeUntilDestroyed(this.#destroyRef)) .subscribe({ next: (msg) => { - // Check if component is destroyed to avoid operations on destroyed component + // takeUntilDestroyed ensures subscription is cleaned up on component destruction, + // but we keep the check as an extra safety measure if (this.#destroyed) { return } @@ -345,7 +351,10 @@ export class ChatConversationPreviewComponent { this.output.update((state) => state + event.data) } } else if (event.type === ChatMessageTypeEnum.EVENT) { - // Component destroyed check is done at the beginning of next callback, emit directly here + // Ensure component is not destroyed before emitting chat events + if (this.#destroyed) { + return + } this.chatEvent.emit(event) switch (event.event) { case ChatMessageEventTypeEnum.ON_CONVERSATION_START: @@ -384,13 +393,13 @@ export class ChatConversationPreviewComponent { } }, error: (err) => { - // Check if component is destroyed before handling error + // takeUntilDestroyed ensures subscription is cleaned up, but keep check for safety if (!this.#destroyed) { this.onChatError(getErrorMessage(err)) } }, complete: () => { - // Check if component is destroyed to avoid unnecessary operations + // takeUntilDestroyed ensures subscription is cleaned up, but keep check for safety if (this.#destroyed) { return } diff --git a/apps/cloud/src/app/features/xpert/studio/panel/panel.component.ts b/apps/cloud/src/app/features/xpert/studio/panel/panel.component.ts index f69acc4a11..51dd1c716e 100644 --- a/apps/cloud/src/app/features/xpert/studio/panel/panel.component.ts +++ b/apps/cloud/src/app/features/xpert/studio/panel/panel.component.ts @@ -51,7 +51,7 @@ export class XpertStudioPanelComponent { // Track if preview component has been created, keep it mounted once created readonly previewCreated = signal(false) readonly showPreview = computed(() => - this.sidePanel() === 'preview' || this.previewCreated() + this.sidePanel() === 'preview' || (this.previewCreated() && this.sidePanel() !== null) ) readonly minPanelWidth = 420