From 19687787b4ca16058f71a64f9ae4e38c36580b31 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Sun, 18 Oct 2026 23:13:28 -0700 Subject: [PATCH] fix(workflow-form): give the Form View a code editor host, so its step panel's code button opens the editor The code editor renders into an outlet the operator canvas published once (`` in WorkspaceComponent, handed to CodeEditorService as `vc`) and never let go of. The Form View has no outlet of its own, yet its step panel shows the operator's real property frame and, in edit mode, the ordinary "Edit code content" button. Pressing it opened the editor into whatever `vc` held: nothing, from a direct open of the Form View, or the destroyed canvas's outlet after the routed switch between the views (#8581), where Monaco rendered into a detached view and nothing appeared on screen. Verified on main: the button is there, the click does nothing, no error. Each view that shows operator properties now carries its own outlet, marked `texeraCodeEditorHost` on an ``: the directive attaches the outlet to CodeEditorService when the view's template is built and detaches it -- only its own -- when the view is destroyed, which also closes an editor still open in it. The views are sibling routes, so there is one host at a time. The button opens into whichever host is up and guards the case of none (every view that mounts a codearea hosts the editor now, so that is a guard rather than a state the button shows): a co-editor's open reaching a codearea with no host neither throws nor marks that operator's editor open, and reaches only the codearea on that operator. The editor is told its operator by its opener instead of reading the highlight in its constructor, and may be dragged within the Form View's page as well as the canvas. Deliberately not in this change: exposing a code property as a form input. The form's rules keep the code editor out of what an author may expose (NON_FORM_FIELD_TYPES) and fall back from it on an old workflow that carries one (CANVAS_ONLY_FORMLY_TYPES); editing code is not filling in a value, and a form reader should not be writing code. The step panel is the author's own editing surface, which is what this fixes. Closes #8760 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY --- .../code-editor-host.directive.spec.ts | 89 +++++++++++ .../code-editor-host.directive.ts | 47 ++++++ .../code-editor.component.browser.spec.ts | 44 +++--- .../code-editor.component.html | 2 +- .../code-editor.component.spec.ts | 46 +++++- .../code-editor.component.ts | 23 ++- .../codearea-custom-template.component.html | 6 +- ...codearea-custom-template.component.spec.ts | 144 ++++++++++++++++-- .../codearea-custom-template.component.ts | 41 ++++- ...ator-property-edit-frame.component.spec.ts | 20 +++ .../operator-property-edit-frame.component.ts | 7 + .../workflow-form.component.html | 5 + .../workflow-form.component.scss | 8 + .../workflow-form/workflow-form.component.ts | 2 + .../workflow-form.rendered.spec.ts | 14 ++ .../workflow-form.spec-harness.ts | 1 + .../component/workspace.component.html | 3 +- .../component/workspace.component.spec.ts | 43 +++--- .../component/workspace.component.ts | 19 +-- .../code-editor/code-editor.service.spec.ts | 29 ++++ .../code-editor/code-editor.service.ts | 25 ++- 21 files changed, 538 insertions(+), 80 deletions(-) create mode 100644 frontend/src/app/workspace/component/code-editor-dialog/code-editor-host.directive.spec.ts create mode 100644 frontend/src/app/workspace/component/code-editor-dialog/code-editor-host.directive.ts diff --git a/frontend/src/app/workspace/component/code-editor-dialog/code-editor-host.directive.spec.ts b/frontend/src/app/workspace/component/code-editor-dialog/code-editor-host.directive.spec.ts new file mode 100644 index 00000000000..71a58711553 --- /dev/null +++ b/frontend/src/app/workspace/component/code-editor-dialog/code-editor-host.directive.spec.ts @@ -0,0 +1,89 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Component, OnDestroy } from "@angular/core"; +import { TestBed } from "@angular/core/testing"; +import { CodeEditorHostDirective } from "./code-editor-host.directive"; +import { CodeEditorService } from "../../service/code-editor/code-editor.service"; + +/** A view that hosts the editor, as the operator canvas and the Form View do. */ +@Component({ + template: "", + imports: [CodeEditorHostDirective], +}) +class HostingViewComponent {} + +/** Stands in for the editor: what the host holds, and what must be torn down with the view. */ +@Component({ template: "editor" }) +class HostedComponent implements OnDestroy { + static destroyed = 0; + ngOnDestroy(): void { + HostedComponent.destroyed++; + } +} + +describe("CodeEditorHostDirective", () => { + let service: CodeEditorService; + + beforeEach(async () => { + HostedComponent.destroyed = 0; + await TestBed.configureTestingModule({ imports: [HostingViewComponent, HostedComponent] }).compileComponents(); + service = TestBed.inject(CodeEditorService); + }); + + it("attaches its outlet as the editor's host when the view is built", () => { + expect(service.host).toBeUndefined(); + + const fixture = TestBed.createComponent(HostingViewComponent); + fixture.detectChanges(); + + expect(service.host).toBeDefined(); + // The real outlet: the editor is created into it. + const ref = service.host!.createComponent(HostedComponent); + expect(ref.location.nativeElement.textContent).toBe("editor"); + }); + + it("closes what it hosts and lets go of the host when the view is destroyed", () => { + const fixture = TestBed.createComponent(HostingViewComponent); + fixture.detectChanges(); + service.host!.createComponent(HostedComponent); + + fixture.destroy(); + + // The editor's own teardown ran, by the view's (it writes awareness and saves its geometry + // there) ... + expect(HostedComponent.destroyed).toBe(1); + // ... and nothing claims to host an editor any more. + expect(service.host).toBeUndefined(); + }); + + it("lets go only of its own outlet, not of a host that has since replaced it", () => { + const first = TestBed.createComponent(HostingViewComponent); + first.detectChanges(); + const firstHost = service.host; + const second = TestBed.createComponent(HostingViewComponent); + second.detectChanges(); + expect(service.host).not.toBe(firstHost); + + first.destroy(); + + expect(service.host).toBeDefined(); + expect(service.host).not.toBe(firstHost); + }); +}); diff --git a/frontend/src/app/workspace/component/code-editor-dialog/code-editor-host.directive.ts b/frontend/src/app/workspace/component/code-editor-dialog/code-editor-host.directive.ts new file mode 100644 index 00000000000..d7fb95d6127 --- /dev/null +++ b/frontend/src/app/workspace/component/code-editor-dialog/code-editor-host.directive.ts @@ -0,0 +1,47 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Directive, OnDestroy, ViewContainerRef } from "@angular/core"; +import { CodeEditorService } from "../../service/code-editor/code-editor.service"; + +/** + * Marks the outlet the code editor opens into, on an `` in a view's template. Each view + * that shows operator properties -- the operator canvas and the Form View -- carries one, and the + * view in front is the editor's host: its outlet is attached to CodeEditorService when the view's + * template is built, and detached when the view is destroyed -- which closes an editor still open in + * it too, since the view's teardown destroys what its outlet holds, before this hook runs. The views + * are sibling routes and never overlap, so there is one host at a time; a late destroy still lets go + * only of its own. + */ +@Directive({ + selector: "[texeraCodeEditorHost]", + standalone: true, +}) +export class CodeEditorHostDirective implements OnDestroy { + constructor( + private readonly outlet: ViewContainerRef, + private readonly codeEditorService: CodeEditorService + ) { + this.codeEditorService.attachHost(this.outlet); + } + + ngOnDestroy(): void { + this.codeEditorService.detachHost(this.outlet); + } +} diff --git a/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.browser.spec.ts b/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.browser.spec.ts index fac2b91dac9..28ef4424377 100644 --- a/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.browser.spec.ts +++ b/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.browser.spec.ts @@ -232,16 +232,33 @@ describe("CodeEditorComponent (browser)", () => { vi.restoreAllMocks(); }); - // Builds a fixture for the highlighted operator, but defers the - // detectChanges/ngAfterViewInit step so the caller can stage `code` / - // `formControl` (and the EditorApp's fake editor) before the subscribe body - // fires. Returns the fixture, the fake editor, and the component instance. - function makeFixtureWithFakes() { - const predicate = { ...mockJavaUDFPredicate }; + // Builds a fixture for a Java UDF operator, told its operator the way the + // codearea button tells it, but defers the detectChanges step (ngOnInit + + // ngAfterViewInit) so the caller can stage `formControl`, the shared text + // and the EditorApp's fake editor before the subscribe body fires. The + // shared text is staged on the graph, where ngOnInit reads it from: a `code` + // property on the predicate becomes a Y.Text on the shared + // operator-properties map, and that is what the subscribe body binds to (and + // crosses the `if (!this.code) return;` gate with). Returns the fixture, the + // fake editor, the component instance and that shared text (undefined when + // staged without one). + function makeFixtureWithFakes(options: { withSharedCode?: boolean } = {}) { + const { withSharedCode = true } = options; + const predicate = { ...mockJavaUDFPredicate, operatorProperties: withSharedCode ? { code: "" } : {} }; workflowActionService.addOperator(predicate, mockPoint); workflowActionService.getJointGraphWrapper().highlightOperators(predicate.operatorID); + const sharedCode = withSharedCode + ? (( + workflowActionService + .getTexeraGraph() + .getSharedOperatorType(predicate.operatorID) + .get("operatorProperties") as any + ).get("code") as Y.Text) + : undefined; const fixture = TestBed.createComponent(CodeEditorComponent); + // The opener names the operator; see makeFixture in the jsdom spec. + fixture.componentRef.setInput("currentOperatorId", predicate.operatorID); const editor = makeFakeEditor(); // The component pulls the editor back out of `EditorApp.getEditor()` inside // its start path (and again from `adjustEditorSize`), so the mocked @@ -249,11 +266,7 @@ describe("CodeEditorComponent (browser)", () => { editorAppMock.getEditor.mockReturnValue(editor); const c = fixture.componentInstance as any; c.formControl = new FormControl({ value: "", disabled: false }); - // A YText must live inside a Y.Doc to be useful; the binding stub doesn't - // care, but we stage it as if it came from the shared model so the - // subscribe body crosses the `if (!this.code) return;` gate. - c.code = new Y.Doc().getText("code"); - return { fixture, editor, c: c as CodeEditorComponent }; + return { fixture, editor, c: c as CodeEditorComponent, sharedCode }; } // The component's subscribe path runs `from(startEditor()).pipe(...)`, @@ -367,8 +380,7 @@ describe("CodeEditorComponent (browser)", () => { }); it("initializeMonacoEditor: leaves the binding unwired when the operator has no shared code", async () => { - const { fixture, editor, c } = makeFixtureWithFakes(); - (c as any).code = undefined; + const { fixture, editor } = makeFixtureWithFakes({ withSharedCode: false }); fixture.detectChanges(); await flush(); @@ -412,10 +424,8 @@ describe("CodeEditorComponent (browser)", () => { }); it("initializeDiffEditor: diffs this operator's latest-version code against the shared text", async () => { - const { fixture, c } = makeFixtureWithFakes(); - const sharedText = new Y.Doc().getText("code"); - sharedText.insert(0, "shared editing text"); - (c as any).code = sharedText; + const { fixture, sharedCode } = makeFixtureWithFakes(); + sharedCode!.insert(0, "shared editing text"); // The decoy operator is listed FIRST, so picking anything other than the // entry matching `currentOperatorId` surfaces the wrong version. vi.spyOn(workflowActionService, "getTempWorkflow").mockReturnValue({ diff --git a/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.html b/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.html index 4de5a6ef22a..145a68d9169 100644 --- a/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.html +++ b/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.html @@ -22,7 +22,7 @@ id="code-editor-container" class="box" cdkDrag - cdkDragBoundary="texera-workspace" + cdkDragBoundary="texera-workspace, texera-workflow-form" (focusin)="onFocus()">
{ workflowActionService.addOperator(predicate, mockPoint); workflowActionService.getJointGraphWrapper().highlightOperators(predicate.operatorID); const fixture = TestBed.createComponent(CodeEditorComponent); + // The opener names the operator (the panel's button reads the highlight, the form's its + // binding); the highlight above is for what still reads it, such as onFocus. + fixture.componentRef.setInput("currentOperatorId", predicate.operatorID); + // The opener hands the editor the field's control before the first check; the AI actions read + // its disabled state. Editable here; the read-only test stages its own. + fixture.componentInstance.formControl = new FormControl(""); fixture.detectChanges(); return fixture; } - it("creates with the highlighted operator", () => { + it("binds to the operator it is given", () => { const fixture = makeFixture(mockJavaUDFPredicate); expect(fixture.componentInstance).toBeTruthy(); expect(fixture.componentInstance.currentOperatorId).toBe(mockJavaUDFPredicate.operatorID); }); + it("keeps the operator it is given when another one is highlighted (a Form View editor)", () => { + // The form's field names its operator; the highlight, if any, is a step the reader is looking at. + const given = { ...buildPredicateWithCode("given-op", "PythonUDFV2", "x = 1"), customDisplayName: "Given" }; + const highlighted = buildPredicate("highlighted-op", "PythonUDFV2"); + workflowActionService.addOperator(given, mockPoint); + workflowActionService.addOperator(highlighted, mockPoint); + workflowActionService.getJointGraphWrapper().highlightOperators(highlighted.operatorID); + const fixture = TestBed.createComponent(CodeEditorComponent); + fixture.componentRef.setInput("currentOperatorId", given.operatorID); + + fixture.detectChanges(); + + expect(fixture.componentInstance.currentOperatorId).toBe(given.operatorID); + expect(fixture.componentInstance.title).toBe("Given"); + }); + it("shows UI parameter parser errors for the edited operator", () => { const predicate = buildPredicate("python-with-invalid-parameters", "PythonUDFV2"); makeFixture(predicate); @@ -278,9 +300,9 @@ describe("CodeEditorComponent", () => { expect(notificationServiceMock.error).not.toHaveBeenCalled(); }); - // Language detection — the constructor maps `RUDFSource` / `RUDF` to `r`, + // Language detection — ngOnInit maps `RUDFSource` / `RUDF` to `r`, // the three V2-era Python operator types to `python`, and anything else - // to `java`. The exact branch lives in the constructor; the public + // to `java`. The exact branch lives in ngOnInit; the public // `language` field is what the rest of the editor (LSP wiring, file- // suffix selection) keys off. @@ -381,6 +403,7 @@ describe("CodeEditorComponent", () => { fixture.componentInstance.onFocus(); expect(highlightSpy).toHaveBeenCalledWith(mockJavaUDFPredicate.operatorID); }); + }); describe("rejectCurrentAnnotation", () => { @@ -454,6 +477,21 @@ describe("CodeEditorComponent", () => { }); describe("AI assistant action wiring", () => { + // Monaco's readOnly stops typing, not the context-menu actions, and accepting a type annotation + // inserts into the shared code; a read-only viewer's editor must not offer them (#8760). + it("registers no AI actions for a read-only editor, and both for an editable one", () => { + const fixture = makeFixture(mockJavaUDFPredicate); + const readOnly = makeFakeEditor(); + fixture.componentInstance.formControl = new FormControl({ value: "", disabled: true }); + (fixture.componentInstance as any).setupAIAssistantActions(asEditor(readOnly)); + expect(readOnly.addAction).not.toHaveBeenCalled(); + + const editable = makeFakeEditor(); + fixture.componentInstance.formControl = new FormControl(""); + (fixture.componentInstance as any).setupAIAssistantActions(asEditor(editable)); + expect(editable.addAction).toHaveBeenCalledTimes(2); + }); + // setupAIAssistantActions checks an AI-provider flag (OpenAI vs others) // before deciding whether to register the per-selection 'Add Type // Annotation' action. We can't drive the action body without a real @@ -477,6 +515,7 @@ describe("CodeEditorComponent", () => { wfActions.addOperator(mockJavaUDFPredicate, mockPoint); wfActions.getJointGraphWrapper().highlightOperators(mockJavaUDFPredicate.operatorID); const fixture = TestBed.createComponent(CodeEditorComponent); + fixture.componentRef.setInput("currentOperatorId", mockJavaUDFPredicate.operatorID); fixture.detectChanges(); const checked = await TestBed.inject(AIAssistantService).checkAIAssistantEnabled().toPromise(); expect(checked).toBe("OpenAI"); @@ -1313,6 +1352,7 @@ describe("CodeEditorComponent", () => { if (tempOperators) stageTempWorkflow(tempOperators); TestBed.inject(WorkflowVersionService).setDisplayParticularVersion(true); const fixture = TestBed.createComponent(CodeEditorComponent); + fixture.componentRef.setInput("currentOperatorId", predicate.operatorID); fixture.detectChanges(); await waitFor(() => Boolean(diffModelOf(fixture.componentInstance)), "the mounted diff editor"); return fixture.componentInstance; diff --git a/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.ts b/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.ts index d68128312eb..18b28ff5a90 100644 --- a/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.ts +++ b/frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.ts @@ -23,7 +23,9 @@ import { ComponentRef, ElementRef, HostListener, + Input, OnDestroy, + OnInit, Type, ViewChild, } from "@angular/core"; @@ -93,14 +95,18 @@ export const LANGUAGE_SERVER_CONNECTION_TIMEOUT_MS = 1000; AnnotationSuggestionComponent, ], }) -export class CodeEditorComponent implements AfterViewInit, SafeStyle, OnDestroy { +export class CodeEditorComponent implements OnInit, AfterViewInit, SafeStyle, OnDestroy { @ViewChild("editor", { static: true }) editorElement!: ElementRef; @ViewChild("container", { static: true }) containerElement!: ElementRef; @ViewChild(AnnotationSuggestionComponent) annotationSuggestion!: AnnotationSuggestionComponent; @ViewChild(BreakpointConditionInputComponent) breakpointConditionInput!: BreakpointConditionInputComponent; private code?: YText; private workflowVersionStreamSubject: Subject = new Subject(); - public currentOperatorId!: string; + /** + * The operator whose code this edits, given by the opener: the operator property panel's button + * reads the highlighted operator, the Form View's the operator its field is bound to. + */ + @Input() currentOperatorId!: string; public title: string | undefined; public formControl!: FormControl; @@ -150,8 +156,10 @@ export class CodeEditorComponent implements AfterViewInit, SafeStyle, OnDestroy private config: GuiConfigService, private uiUdfParametersSyncService: UiUdfParametersSyncService, private notificationService: NotificationService - ) { - this.currentOperatorId = this.workflowActionService.getJointGraphWrapper().getCurrentHighlightedOperatorIDs()[0]; + ) {} + + ngOnInit(): void { + // Everything here keys on currentOperatorId, an input set after construction. const operatorType = this.workflowActionService.getTexeraGraph().getOperator(this.currentOperatorId).operatorType; if (CodeEditorComponent.PYTHON_OPERATOR_TYPES.has(operatorType)) { this.language = "python"; @@ -468,6 +476,11 @@ export class CodeEditorComponent implements AfterViewInit, SafeStyle, OnDestroy } private setupAIAssistantActions(editor: MonacoEditor) { + // Monaco's readOnly stops typing, not the context-menu actions, and accepting a type annotation + // inserts into the shared code: a read-only viewer's editor offers none (#8760). + if (this.formControl.disabled) { + return; + } // Check if the AI provider is "openai" this.aiAssistantService .checkAIAssistantEnabled() @@ -679,6 +692,8 @@ export class CodeEditorComponent implements AfterViewInit, SafeStyle, OnDestroy this.editorApp?.getEditor()?.layout(); } onFocus() { + // The wrapper's own highlight, as it always was, not WorkflowActionService's: that one also + // announces the highlight to co-editors and switches multi-select mode, neither wanted on focus. this.workflowActionService.getJointGraphWrapper().highlightOperators(this.currentOperatorId); } } diff --git a/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.html b/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.html index cb3a4cef63a..0d91439c474 100644 --- a/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.html +++ b/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.html @@ -24,8 +24,8 @@ nzShape="round" nzType="primary" [disabled]="isEditorOpen" - [ngClass]="{ 'readonly': this.field.formControl.disabled }"> -
Edit code content
-
View code content
+ [ngClass]="{ 'readonly': this.field.formControl.disabled || inspect }"> +
Edit code content
+
View code content
diff --git a/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.spec.ts b/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.spec.ts index 7bf63d610e5..eb2005ac8c0 100644 --- a/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.spec.ts +++ b/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.spec.ts @@ -17,8 +17,10 @@ * under the License. */ +import { reflectComponentType } from "@angular/core"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { CodeareaCustomTemplateComponent } from "./codearea-custom-template.component"; +import { CodeEditorComponent } from "../code-editor-dialog/code-editor.component"; import { HttpClientTestingModule } from "@angular/common/http/testing"; import { WorkflowActionService } from "../../service/workflow-graph/model/workflow-action.service"; import { OperatorMetadataService } from "../../service/operator-metadata/operator-metadata.service"; @@ -56,6 +58,52 @@ describe("CodeareaCustomTemplateComponent", () => { expect(component).toBeTruthy(); }); + describe("what the button says", () => { + const label = () => + (fixture.nativeElement.querySelector("#editor-button button") as HTMLElement).textContent!.trim(); + const greyed = () => + (fixture.nativeElement.querySelector("#editor-button button") as HTMLElement).classList.contains("readonly"); + + it("offers an edit while the control is editable", () => { + fixture.detectChanges(); + + expect(label()).toBe("Edit code content"); + expect(greyed()).toBe(false); + }); + + // The canvas, for a workflow one cannot write: the control is read-only and the button still + // opens the editor, which then shows the code without letting it be changed. + it("offers a view while the control is read-only and the button still works", () => { + component.field.formControl.disable(); + fixture.detectChanges(); + + expect(label()).toBe("View code content"); + expect(greyed()).toBe(true); + }); + + // The Form View's read-only inspection of a step: the whole panel is inert, so the button + // cannot be pressed at all. It keeps its usual face there, greyed with the rest, rather than + // promising a view it cannot open (#8760). + it("keeps its usual face, greyed, on a read-only inspection where it cannot be pressed", () => { + component.field = { props: { inspect: true }, formControl: new FormControl({ value: "", disabled: true }) }; + fixture.detectChanges(); + + expect(label()).toBe("Edit code content"); + expect(greyed()).toBe(true); + }); + + // The inspecting frame disables its form group as it is built, and formly adds the controls to + // that group afterwards, so whether this control ends up disabled depends on the order. The + // button's face does not follow that race. + it("is greyed on an inspection even when its control was left enabled", () => { + component.field = { props: { inspect: true }, formControl: new FormControl() }; + fixture.detectChanges(); + + expect(label()).toBe("Edit code content"); + expect(greyed()).toBe(true); + }); + }); + /** * The component's whole job is to keep one shared, per-operator "is the editor open" flag in * sync across three inputs: this component's own open/close, the same operator's editor opened @@ -64,13 +112,14 @@ describe("CodeareaCustomTemplateComponent", () => { * reopen) the wrong operator's editor. * * `openEditor` is driven directly rather than through the template button: it calls - * `codeEditorService.vc.createComponent`, and `vc` is a ViewContainerRef the real workspace + * `codeEditorService.host.createComponent`, and the host is a ViewContainerRef the view in front * supplies. The tests below install a fake one so the component's own bookkeeping is observable * without standing up a Monaco editor. */ describe("editor-open state", () => { let codeEditorService: CodeEditorService; let destroyCallbacks: (() => void)[]; + let editorHost: { createComponent: () => any }; // The highlighted operator is what getOperatorID reads, and it is the key every // setEditorState/getEditorState call is scoped by. @@ -103,15 +152,24 @@ describe("CodeareaCustomTemplateComponent", () => { codeEditorService = TestBed.inject(CodeEditorService); destroyCallbacks = []; - // Stand-in for the workspace's ViewContainerRef. Records the onDestroy hook the component - // registers so the close path can be triggered without a real component teardown. - codeEditorService.vc = { - createComponent: () => ({ - instance: {} as any, - onDestroy: (cb: () => void) => destroyCallbacks.push(cb), - destroy: () => destroyCallbacks.forEach(cb => cb()), - }), - } as any; + // Stand-in for a view's editor outlet. Records the inputs the component sets on the editor and + // the onDestroy hook it registers, so the close path can be triggered without a real + // component teardown. + editorHost = { + createComponent: () => { + const ref = { + instance: {} as any, + inputs: {} as Record, + setInput: (name: string, value: unknown) => { + ref.inputs[name] = value; + }, + onDestroy: (cb: () => void) => destroyCallbacks.push(cb), + destroy: () => destroyCallbacks.forEach(cb => cb()), + }; + return ref; + }, + }; + codeEditorService.attachHost(editorHost as any); }); it("marks the operator's editor open and hands the field's control to it", () => { @@ -125,6 +183,21 @@ describe("CodeareaCustomTemplateComponent", () => { expect(publishedEditorState(highlightedOperatorId())).toBe(true); }); + // The fake host records whatever name setInput is given; the real ComponentRef would log and + // ignore an input the editor does not declare, so the names are checked against its declaration. + it("sets only inputs the editor declares", () => { + component.openEditor(); + + const declared = reflectComponentType(CodeEditorComponent)!.inputs.map(input => input.templateName); + expect(declared).toEqual(expect.arrayContaining(Object.keys((component.componentRef as any).inputs))); + }); + + it("tells the editor its operator, which is the highlighted one", () => { + component.openEditor(); + + expect((component.componentRef as any).inputs).toEqual({ currentOperatorId: highlightedOperatorId() }); + }); + it("clears the flag again when the editor component is destroyed", () => { component.openEditor(); component.componentRef!.destroy(); @@ -158,6 +231,24 @@ describe("CodeareaCustomTemplateComponent", () => { expect(remoteComponent.componentRef).toBeDefined(); }); + // The Form View can show several exposed code cards at once; a shadowed co-editor's open names + // one operator, and only that operator's editor opens here (#8760). + it("stays closed when a co-editor opens another operator's editor", () => { + const opened = new Subject<{ operatorId: string }>(); + vi.spyOn(TestBed.inject(CoeditorPresenceService), "getCoeditorOpenedCodeEditorSubject").mockReturnValue( + opened.asObservable() + ); + const remoteFixture = TestBed.createComponent(CodeareaCustomTemplateComponent); + const remoteComponent = remoteFixture.componentInstance; + remoteComponent.field = { props: {}, formControl: new FormControl() } as any; + remoteFixture.detectChanges(); + + opened.next({ operatorId: "another-operator" }); + + expect(remoteComponent.isEditorOpen).toBe(false); + expect(remoteComponent.componentRef).toBeUndefined(); + }); + it("tears this client's editor down when any co-editor closes one, whichever operator it names", () => { // The mirror image of the open case above, and the half that was missing: a co-editor // closing the dialog has to close it here too, or this client keeps typing into an editor @@ -273,5 +364,38 @@ describe("CodeareaCustomTemplateComponent", () => { codeEditorService.setEditorState(highlightedOperatorId(), false); expect(component.isEditorOpen).toBe(false); }); + + // Every view that mounts a codearea hosts the editor (#8760), so this is a guard, not a state: + // a click, or a co-editor's open, must not throw or render Monaco into nothing. + describe("without an editor host", () => { + beforeEach(() => { + codeEditorService.detachHost(editorHost as any); + fixture.detectChanges(); + }); + + it("opens nothing and marks nothing open on a click", () => { + expect(() => component.openEditor()).not.toThrow(); + + expect(component.isEditorOpen).toBe(false); + expect(component.componentRef).toBeUndefined(); + expect(publishedEditorState(highlightedOperatorId())).toBe(false); + }); + + it("stays closed when a co-editor opens theirs, instead of throwing", () => { + const opened = new Subject<{ operatorId: string }>(); + vi.spyOn(TestBed.inject(CoeditorPresenceService), "getCoeditorOpenedCodeEditorSubject").mockReturnValue( + opened.asObservable() + ); + const remoteFixture = TestBed.createComponent(CodeareaCustomTemplateComponent); + const remoteComponent = remoteFixture.componentInstance; + remoteComponent.field = { props: {}, formControl: new FormControl() } as any; + remoteFixture.detectChanges(); + + expect(() => opened.next({ operatorId: highlightedOperatorId() })).not.toThrow(); + + expect(remoteComponent.isEditorOpen).toBe(false); + expect(remoteComponent.componentRef).toBeUndefined(); + }); + }); }); }); diff --git a/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.ts b/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.ts index fc852bdfa19..dd92ce6894e 100644 --- a/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.ts +++ b/frontend/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.ts @@ -55,6 +55,27 @@ export class CodeareaCustomTemplateComponent extends FieldType public isEditorOpen: boolean = false; private operatorID: string = ""; + /** + * Whether the button offers a look at the code rather than an edit: the control is read-only and + * the button still works, as on the canvas for a workflow one cannot write. Not on the Form View's + * read-only inspection of a step, where the whole panel is inert and nothing in it can be pressed: + * there the button keeps its usual face, "Edit code content", greyed out with the rest of the + * panel, rather than promising a view it cannot open (#8760). + */ + get viewOnly(): boolean { + return this.field.formControl.disabled && !this.inspect; + } + + /** + * A read-only inspection mount (see viewOnly). The button is greyed out on it whatever its + * control's state: the inspecting frame disables its form group as it is built, and formly adds + * the controls to that group afterwards, so whether this one ends up disabled depends on the + * order -- the button's face should not. + */ + get inspect(): boolean { + return this.field.props?.["inspect"] === true; + } + constructor( private coeditorPresenceService: CoeditorPresenceService, private codeEditorService: CodeEditorService, @@ -62,10 +83,16 @@ export class CodeareaCustomTemplateComponent extends FieldType private workflowActionService: WorkflowActionService ) { super(); + // A shadowed co-editor opening their editor opens the same operator's here, not whatever + // operator this codearea happens to be on: only the codearea on that operator answers (#8760). this.coeditorPresenceService .getCoeditorOpenedCodeEditorSubject() .pipe(untilDestroyed(this)) - .subscribe(_ => this.openEditor()); + .subscribe(({ operatorId }) => { + if (operatorId === this.operatorID) { + this.openEditor(); + } + }); this.coeditorPresenceService .getCoeditorClosedCodeEditorSubject() .pipe(untilDestroyed(this)) @@ -84,7 +111,17 @@ export class CodeareaCustomTemplateComponent extends FieldType } openEditor(): void { - this.componentRef = this.codeEditorService.vc.createComponent(CodeEditorComponent); + // Without a host there is nothing to open into. Every view that mounts a codearea hosts the + // editor today (the operator canvas and the Form View), so this is a guard, not a state the + // button shows: a co-editor's open reaching a codearea with no host must not throw or mark + // this operator's editor open. + const host = this.codeEditorService.host; + if (!host) { + return; + } + this.componentRef = host.createComponent(CodeEditorComponent); + // The editor is told its operator by its opener, rather than reading the highlight itself. + this.componentRef.setInput("currentOperatorId", this.operatorID); this.componentRef.instance.componentRef = this.componentRef; this.componentRef.instance.formControl = this.field.formControl; this.isEditorOpen = true; diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts index fbdf39213cf..728b96fcd65 100644 --- a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.spec.ts @@ -2038,6 +2038,26 @@ describe("OperatorPropertyEditFrameComponent", () => { }); expect(getField("code")?.type).toBe("codearea"); }); + + // The Form View's read-only inspection of a step mounts this frame with actsAsEditor=false and + // inert, so its code button cannot be pressed; told so, the button keeps its usual face rather + // than offering a view it cannot open (#8760). + it("marks the code field as an inspection when not acting as the editor", () => { + component.actsAsEditor = false; + component.setFormlyFormBinding({ + type: "object", + properties: { code: { type: "string", description: "Input your code here" } }, + }); + expect(getField("code")?.props?.["inspect"]).toBe(true); + }); + + it("leaves the code field unmarked when acting as the editor", () => { + component.setFormlyFormBinding({ + type: "object", + properties: { code: { type: "string", description: "Input your code here" } }, + }); + expect(getField("code")?.props?.["inspect"]).toBeUndefined(); + }); }); // ────────────────────────────────────────────────────────────────────────── diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts index 17101bc598d..4eec26fb0de 100644 --- a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts @@ -960,6 +960,13 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On if (customType) { mappedField.type = customType; } + // A read-only inspection of a step (the Form View outside edit mode mounts this frame with + // actsAsEditor=false, and inert) cannot press anything in it, the code button included. The + // button is told, so it keeps its usual face there -- "Edit code content", greyed out with the + // rest -- instead of offering a view it cannot open (#8760). + if (customType === "codearea" && !this.actsAsEditor) { + mappedField.props = { ...mappedField.props, inspect: true }; + } if (this.currentOperatorSchema?.operatorType === "HuggingFace") { applyHuggingFaceFieldBindings(mappedField); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html index e80ade70bae..53c42cc741a 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.html @@ -590,3 +590,8 @@

Results shown here

+ + + diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss index 3889c57e24f..dcb3369a2b0 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.scss @@ -1104,3 +1104,11 @@ $shell: #fafafa; } } } + +/* The code editor opens into this page's outlet (texeraCodeEditorHost), so it is a descendant and + can be dragged under the sticky title row (z-index 20) and the panel's close button; keep the + editor above them. The canvas's chrome never exceeds that, so the shared editor keeps its own + value and only this page lifts it. The class is what beats the editor's own rule in specificity. */ +:host ::ng-deep #code-editor-container.box { + z-index: 21; +} diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts index 610488e05aa..54663d2b9cb 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts @@ -63,6 +63,7 @@ import { ExecutionState, ExecutionStateInfo } from "../../types/execute-workflow import { OperatorPredicate, Point } from "../../types/workflow-common.interface"; import { ComputingUnitSelectionComponent } from "../power-button/computing-unit-selection.component"; import { PropertyEditorComponent } from "../property-editor/property-editor.component"; +import { CodeEditorHostDirective } from "../code-editor-dialog/code-editor-host.directive"; import { ResultTableFrameComponent } from "../result-panel/result-table-frame/result-table-frame.component"; import { VisualizationFrameContentComponent } from "../visualization-panel-content/visualization-frame-content.component"; import { WorkflowEditorComponent } from "../workflow-editor/workflow-editor.component"; @@ -169,6 +170,7 @@ interface ResultChoice { MiniMapComponent, CoeditorUserIconComponent, ReadOnlyCardDirective, + CodeEditorHostDirective, ], }) export class WorkflowFormComponent implements OnInit, OnDestroy { diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts index 07b0048102c..821728c3765 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.rendered.spec.ts @@ -72,6 +72,7 @@ import { WorkflowPveService } from "../../service/virtual-environment/virtual-en import { NzModalService } from "ng-zorro-antd/modal"; import { ExecutionState } from "../../types/execute-workflow.interface"; import { GuiConfigService } from "../../../common/service/gui-config.service"; +import { CodeEditorService } from "../../service/code-editor/code-editor.service"; /** * The direct-construction spec exercises the component's logic without a DOM; this one stands @@ -815,6 +816,19 @@ describe("WorkflowFormComponent (rendered template)", () => { expect(el(".result .result-body")).not.toBeNull(); }); + // The page hosts the code editor while it is in front (#8760): the step panel's code button and + // an exposed code property's open the editor here, and leaving the page closes it. + it("hosts the code editor while the page is up, and lets go of the host when it is destroyed", () => { + const codeEditorService = TestBed.inject(CodeEditorService); + fixture.detectChanges(); + finishLoad(); + expect(codeEditorService.host).toBeDefined(); + + fixture.destroy(); + + expect(codeEditorService.host).toBeUndefined(); + }); + it("mounts the step panel from the start, hidden until a step is selected", () => { fixture.detectChanges(); finishLoad(); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts index 0bce6e34ff9..4476175853b 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts @@ -139,6 +139,7 @@ export function setupHarness() { setHeatmapView: vi.fn(), }; const workflowActionService = { + setOperatorProperty: vi.fn(), resetAsNewWorkflow: vi.fn(), setNewSharedModel: vi.fn(), reloadWorkflow: vi.fn(), diff --git a/frontend/src/app/workspace/component/workspace.component.html b/frontend/src/app/workspace/component/workspace.component.html index c83432238d2..2d42b90adb3 100644 --- a/frontend/src/app/workspace/component/workspace.component.html +++ b/frontend/src/app/workspace/component/workspace.component.html @@ -36,4 +36,5 @@ [agentIdToActivate]="agentIdToActivate"> - + + diff --git a/frontend/src/app/workspace/component/workspace.component.spec.ts b/frontend/src/app/workspace/component/workspace.component.spec.ts index 118fc9e4118..db49d9783df 100644 --- a/frontend/src/app/workspace/component/workspace.component.spec.ts +++ b/frontend/src/app/workspace/component/workspace.component.spec.ts @@ -36,6 +36,7 @@ import { UserService } from "../../common/service/user/user.service"; import { WorkflowPersistService } from "../../common/service/workflow-persist/workflow-persist.service"; import { Workflow } from "../../common/type/workflow"; import { CodeEditorService } from "../service/code-editor/code-editor.service"; +import { CodeEditorHostDirective } from "./code-editor-dialog/code-editor-host.directive"; import { WorkflowCompilingService } from "../service/compile-workflow/workflow-compiling.service"; import { OperatorMetadataService } from "../service/operator-metadata/operator-metadata.service"; import { UndoRedoService } from "../service/undo-redo/undo-redo.service"; @@ -148,7 +149,7 @@ describe("WorkspaceComponent", () => { notificationService = { error: vi.fn() }; hubService = { postView: vi.fn().mockReturnValue(of(0)) }; - codeEditorService = { vc: undefined }; + codeEditorService = { attachHost: vi.fn(), detachHost: vi.fn() }; messageService = { error: vi.fn() }; // `getCurrentNavigation` answers what the page is being destroyed for: null stands for no @@ -174,15 +175,15 @@ describe("WorkspaceComponent", () => { workflowConsoleService = { clearConsoleMessages: vi.fn() }; workflowResultService = { clearResults: vi.fn() }; - // Drop the standalone component's child imports and allow unknown elements via - // CUSTOM_ELEMENTS_SCHEMA. The template still renders, so `` - // is wired up and the @ViewChild query resolves to a real ViewContainerRef, while + // Drop the standalone component's child imports -- all but the code editor host directive, a + // leaf with no children of its own -- and allow unknown elements via CUSTOM_ELEMENTS_SCHEMA. The + // template still renders, so the `texeraCodeEditorHost` outlet is built and attached, while // the children's transitive dependencies stay out of the test build. // TODO(coverage): rewrite using stub child components via remove/add so the // template participates in coverage. See TESTING.md anti-pattern #9. /* eslint-disable no-restricted-syntax */ TestBed.overrideComponent(WorkspaceComponent, { - set: { imports: [], providers: [], schemas: [CUSTOM_ELEMENTS_SCHEMA] }, + set: { imports: [CodeEditorHostDirective], providers: [], schemas: [CUSTOM_ELEMENTS_SCHEMA] }, }); /* eslint-enable no-restricted-syntax */ @@ -216,12 +217,6 @@ describe("WorkspaceComponent", () => { fixture = TestBed.createComponent(WorkspaceComponent); component = fixture.componentInstance; - // ngOnDestroy clears the ViewContainerRef bound to `#codeEditor`. Tests that - // exercise individual methods skip change detection, so the @ViewChild query - // is never resolved; assign a stub to keep TestBed teardown from throwing. - // Tests that exercise `fixture.detectChanges()` will overwrite this with - // the live ViewContainerRef during ngAfterViewInit. - component.codeEditorViewRef = { clear: vi.fn() } as any; } describe("ngOnInit", () => { @@ -676,19 +671,23 @@ describe("WorkspaceComponent", () => { }); }); - // Exercises the rendered template: the `` outlet is - // present, so the @ViewChild query resolves to a live ViewContainerRef and - // ngAfterViewInit can publish it to CodeEditorService. - describe("child rendering side effects", () => { - it("publishes the resolved ViewContainerRef to CodeEditorService.vc on view init", async () => { - codeEditorService.vc = undefined; + // Exercises the rendered template: the `texeraCodeEditorHost` outlet is in it, so the canvas + // hosts the code editor while it is up and lets go of the host when it is destroyed. The Form + // View carries the same outlet, which is how the host changes hands between the two (#8760). + describe("the code editor host", () => { + it("is attached when the canvas renders, and detached when the canvas is destroyed", async () => { await createFixture(); fixture.detectChanges(); - // createEmbeddedView is present on a real ViewContainerRef but not on the - // pre-fixture stub, so checking it distinguishes the resolved query from - // the placeholder. - expect(codeEditorService.vc).toBe(component.codeEditorViewRef); - expect(typeof codeEditorService.vc.createEmbeddedView).toBe("function"); + expect(codeEditorService.attachHost).toHaveBeenCalledTimes(1); + // A real ViewContainerRef (the directive's own), not a placeholder: the editor is created + // into it. + const host = codeEditorService.attachHost.mock.calls[0][0]; + expect(typeof host.createComponent).toBe("function"); + expect(codeEditorService.detachHost).not.toHaveBeenCalled(); + + fixture.destroy(); + + expect(codeEditorService.detachHost).toHaveBeenCalledWith(host); }); }); }); diff --git a/frontend/src/app/workspace/component/workspace.component.ts b/frontend/src/app/workspace/component/workspace.component.ts index 3e43c07acd9..2adbda18ce4 100644 --- a/frontend/src/app/workspace/component/workspace.component.ts +++ b/frontend/src/app/workspace/component/workspace.component.ts @@ -18,17 +18,7 @@ */ import { Location, NgIf } from "@angular/common"; -import { - AfterViewInit, - ChangeDetectorRef, - Component, - HostListener, - Input, - OnDestroy, - OnInit, - ViewChild, - ViewContainerRef, -} from "@angular/core"; +import { AfterViewInit, ChangeDetectorRef, Component, HostListener, Input, OnDestroy, OnInit } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; import { UserService } from "../../common/service/user/user.service"; import { WorkflowPersistService } from "../../common/service/workflow-persist/workflow-persist.service"; @@ -44,7 +34,7 @@ import { isDefined } from "../../common/util/predicate"; import { NotificationService } from "src/app/common/service/notification/notification.service"; import { WorkflowConsoleService } from "../service/workflow-console/workflow-console.service"; import { OperatorReuseCacheStatusService } from "../service/workflow-status/operator-reuse-cache-status.service"; -import { CodeEditorService } from "../service/code-editor/code-editor.service"; +import { CodeEditorHostDirective } from "./code-editor-dialog/code-editor-host.directive"; import { WorkflowMetadata } from "src/app/dashboard/type/workflow-metadata.interface"; import { EntityType, HubService } from "../../hub/service/hub.service"; import { THROTTLE_TIME_MS } from "../../hub/component/workflow/detail/hub-workflow-detail.component"; @@ -88,12 +78,12 @@ export const SAVE_DEBOUNCE_TIME_IN_MS = 5000; AgentPanelComponent, PropertyEditorComponent, JupyterNotebookPanelComponent, + CodeEditorHostDirective, ], }) export class WorkspaceComponent implements AfterViewInit, OnInit, OnDestroy { public writeAccess: boolean = false; public isLoading: boolean = false; - @ViewChild("codeEditor", { read: ViewContainerRef }) codeEditorViewRef!: ViewContainerRef; /** * Optional agent ID to activate when the workspace loads. @@ -134,7 +124,6 @@ export class WorkspaceComponent implements AfterViewInit, OnInit, OnDestroy { private router: Router, private notificationService: NotificationService, private hubService: HubService, - private codeEditorService: CodeEditorService, private config: GuiConfigService, private changeDetectorRef: ChangeDetectorRef, private computingUnitStatusService: ComputingUnitStatusService, @@ -185,7 +174,6 @@ export class WorkspaceComponent implements AfterViewInit, OnInit, OnDestroy { this.onWIDChange(); this.updateViewCount(); this.registerLoadOperatorMetadata(); - this.codeEditorService.vc = this.codeEditorViewRef; } /** @@ -210,7 +198,6 @@ export class WorkspaceComponent implements AfterViewInit, OnInit, OnDestroy { ngOnDestroy() { this.persistBeforeLeaving(); - this.codeEditorViewRef.clear(); // Tear down the connection and all websocket-derived session state so a // re-entered workflow starts clean instead of reusing the previous one -- unless the Form // View of this same workflow is taking over, in which case the session is handed to it diff --git a/frontend/src/app/workspace/service/code-editor/code-editor.service.spec.ts b/frontend/src/app/workspace/service/code-editor/code-editor.service.spec.ts index 6f514b46c12..1dc53664715 100644 --- a/frontend/src/app/workspace/service/code-editor/code-editor.service.spec.ts +++ b/frontend/src/app/workspace/service/code-editor/code-editor.service.spec.ts @@ -17,6 +17,7 @@ * under the License. */ +import { ViewContainerRef } from "@angular/core"; import { CodeEditorService } from "./code-editor.service"; describe("CodeEditorService", () => { @@ -70,4 +71,32 @@ describe("CodeEditorService", () => { expect(valueA).toBe(true); expect(valueB).toBe(false); }); + + // The outlet the editor opens into, attached by whichever view is in front (#8760). + describe("the editor host", () => { + const outlet = (name: string) => ({ name }) as unknown as ViewContainerRef; + + it("has no host until a view attaches one", () => { + expect(service.host).toBeUndefined(); + + const canvas = outlet("canvas"); + service.attachHost(canvas); + + expect(service.host).toBe(canvas); + }); + + it("lets go of the host that detaches, and keeps one that has replaced it", () => { + const canvas = outlet("canvas"); + const form = outlet("form"); + service.attachHost(canvas); + service.attachHost(form); + + // The canvas's late destroy must not take the form's host with it. + service.detachHost(canvas); + expect(service.host).toBe(form); + + service.detachHost(form); + expect(service.host).toBeUndefined(); + }); + }); }); diff --git a/frontend/src/app/workspace/service/code-editor/code-editor.service.ts b/frontend/src/app/workspace/service/code-editor/code-editor.service.ts index 8dd534e60dc..8fb0dcd5bda 100644 --- a/frontend/src/app/workspace/service/code-editor/code-editor.service.ts +++ b/frontend/src/app/workspace/service/code-editor/code-editor.service.ts @@ -24,7 +24,30 @@ import { BehaviorSubject, Observable } from "rxjs"; providedIn: "root", }) export class CodeEditorService { - public vc!: ViewContainerRef; + /** + * Where the code editor renders: the outlet of the view in front, marked `texeraCodeEditorHost` + * on an `` in its template (see CodeEditorHostDirective). The operator canvas and the + * Form View each carry one; a view's outlet is attached when its template is built and detached + * when the view is destroyed. The editor is a floating box, so where the outlet sits in the page + * does not matter -- only that the view in front has one. + */ + private editorHost?: ViewContainerRef; + + /** The outlet to open the editor into, while a view hosts one. */ + get host(): ViewContainerRef | undefined { + return this.editorHost; + } + + attachHost(host: ViewContainerRef): void { + this.editorHost = host; + } + + /** Lets go of this host only: a host that has since replaced it stays attached. */ + detachHost(host: ViewContainerRef): void { + if (this.editorHost === host) { + this.editorHost = undefined; + } + } private editorStates: Map> = new Map();