From 8716498770d731833d7df68eb4e15c006f36358e Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Sun, 6 Sep 2026 22:24:51 -0700 Subject: [PATCH 1/2] feat(gui): author the Form View in place with an edit mode Add an Edit/Done toggle (write access only) that turns the Form View into in-place authoring without leaving the page: rename an input or its sub-fields by clicking the title, hide a sub-field (kept faded, not removed), reorder inputs by drag, expose a new one by clicking a step on the embedded preview (the property panel goes live with tick boxes), remove an input, write the instruction (write/preview), and pick which steps report their results. An author sees broken inputs (their operator was deleted) to repair or remove them; a reader never does. Leaving edit mode locks everything back to read-only. Every edit goes through the same shared graph and form-binding config the operator canvas edits. Adds a reusable editable-label formly wrapper (renders a plain label for a reader, an editable input with an optional hide eye for an author). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY --- .../editable-label-wrapper.component.html | 71 +++ .../editable-label-wrapper.component.scss | 113 +++++ .../editable-label-wrapper.component.spec.ts | 190 ++++++++ .../editable-label-wrapper.component.ts | 79 ++++ .../src/app/common/formly/formly-config.ts | 2 + .../property-editor.component.ts | 17 +- .../context-menu/context-menu.component.html | 17 +- .../context-menu.component.spec.ts | 57 +++ .../context-menu/context-menu.component.ts | 16 +- .../workflow-editor.component.html | 2 +- .../workflow-editor.component.spec.ts | 19 +- .../workflow-editor.component.ts | 8 +- .../workflow-form.component.html | 343 +++++++++++--- .../workflow-form.component.scss | 331 ++++++++++++- .../workflow-form.component.spec.ts | 433 +++++++++++++++++- .../workflow-form/workflow-form.component.ts | 393 ++++++++++++++-- .../workflow-form.rendered.spec.ts | 95 +++- .../workflow-form.spec-harness.ts | 9 + .../form-binding/form-binding.service.ts | 8 +- 19 files changed, 2067 insertions(+), 136 deletions(-) create mode 100644 frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html create mode 100644 frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.scss create mode 100644 frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.spec.ts create mode 100644 frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.ts diff --git a/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html new file mode 100644 index 00000000000..9eac0c84a0c --- /dev/null +++ b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.html @@ -0,0 +1,71 @@ + + + +
+ + + +
+ + + + + + + +
+ +
diff --git a/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.scss b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.scss new file mode 100644 index 00000000000..52ea5c3cc06 --- /dev/null +++ b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.scss @@ -0,0 +1,113 @@ +/** + * 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. + */ + +.lbl-row { + display: flex; + align-items: center; + gap: 6px; + margin-bottom: 2px; +} + +/* A faint dashed box at rest signals the title is editable (authors could not tell it was + a field before); it firms up on hover and turns into a real input on focus. */ +.lbl-input { + flex: 1; + min-width: 0; + border: 1px dashed #dcdcdc; + border-radius: 4px; + background: none; + padding: 1px 5px; + font-size: 13px; + font-weight: 500; + color: rgba(0, 0, 0, 0.85); + outline: none; + cursor: text; + + &::placeholder { + color: rgba(0, 0, 0, 0.45); + font-weight: 400; + } + + &:hover { + border-style: solid; + border-color: #b0b0b0; + } + + &:focus { + border-style: solid; + border-color: #1890ff; + background: #fff; + } +} + +.lbl-eye { + flex: none; + border: 0; + background: none; + cursor: pointer; + line-height: 0; + padding: 3px; + border-radius: 4px; + color: rgba(0, 0, 0, 0.45); + font-size: 14px; + + &:hover { + color: rgba(0, 0, 0, 0.85); + background: #f5f5f5; + } + + &:focus-visible { + outline: 2px solid #1890ff; + outline-offset: 1px; + } + + /* Hidden is a state worth seeing at a glance, so it keeps the accent rather than + fading like the rest of the row. */ + &.off { + color: #1890ff; + } +} + +/* Present for assistive technology only: the visible name box is the label's editor, not the + control's label. Standard visually-hidden recipe: off-layout, clipped, never display:none (which + would remove it from the accessibility tree too). */ +.lbl-sr-only { + position: absolute; + width: 1px; + height: 1px; + padding: 0; + margin: -1px; + overflow: hidden; + clip: rect(0, 0, 0, 0); + white-space: nowrap; + border: 0; +} + +.lbl-static { + display: block; + font-size: 13px; + color: rgba(0, 0, 0, 0.85); + margin-bottom: 2px; +} + +/* A hidden field stays visible to its author, faded, so they can see what they removed + and put it back. The reader never renders it at all. */ +.dimmed { + opacity: 0.4; +} diff --git a/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.spec.ts b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.spec.ts new file mode 100644 index 00000000000..72b2b30febe --- /dev/null +++ b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.spec.ts @@ -0,0 +1,190 @@ +/** + * 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 { ComponentFixture, TestBed } from "@angular/core/testing"; +import { FormlyFieldConfig } from "@ngx-formly/core"; +import { EditableLabelWrapperComponent } from "./editable-label-wrapper.component"; + +describe("EditableLabelWrapperComponent", () => { + let component: EditableLabelWrapperComponent; + let fixture: ComponentFixture; + + const state = (overrides: Partial[1]> = {}) => ({ + authoring: true, + name: "My input", + hidden: false, + fallback: "File Key", + ...overrides, + }); + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [EditableLabelWrapperComponent], + }).compileComponents(); + fixture = TestBed.createComponent(EditableLabelWrapperComponent); + component = fixture.componentInstance; + }); + + describe("decorate", () => { + it("appends the wrapper after any existing ones", () => { + const config: FormlyFieldConfig = { key: "k", wrappers: ["form-field"] }; + EditableLabelWrapperComponent.decorate( + config, + state(), + () => {}, + () => {} + ); + expect(config.wrappers).toEqual(["form-field", "editable-label-wrapper"]); + }); + + it("blanks formly's own label so the wrapper does not print it twice", () => { + const config: FormlyFieldConfig = { key: "k", props: { label: "File Key" } }; + EditableLabelWrapperComponent.decorate( + config, + state(), + () => {}, + () => {} + ); + expect(config.props?.["label"]).toBe(""); + }); + + it("maps the naming state and callbacks into props", () => { + const rename = vi.fn(); + const setHidden = vi.fn(); + const config: FormlyFieldConfig = { key: "k" }; + EditableLabelWrapperComponent.decorate(config, state({ name: "Genes", hidden: true }), rename, setHidden); + expect(config.props?.["authoring"]).toBe(true); + expect(config.props?.["authorName"]).toBe("Genes"); + expect(config.props?.["authorHidden"]).toBe(true); + expect(config.props?.["schemaLabel"]).toBe("File Key"); + expect(config.props?.["renameField"]).toBe(rename); + expect(config.props?.["setFieldHidden"]).toBe(setHidden); + }); + + it("defaults canHide to true, and honors an explicit false", () => { + const shown: FormlyFieldConfig = { key: "k" }; + EditableLabelWrapperComponent.decorate( + shown, + state(), + () => {}, + () => {} + ); + expect(shown.props?.["canHide"]).toBe(true); + + const locked: FormlyFieldConfig = { key: "k" }; + EditableLabelWrapperComponent.decorate( + locked, + state({ canHide: false }), + () => {}, + () => {} + ); + expect(locked.props?.["canHide"]).toBe(false); + }); + }); + + describe("handlers", () => { + it("onRename forwards the input's value to renameField", () => { + const rename = vi.fn(); + component.field = { props: { renameField: rename } } as unknown as FormlyFieldConfig; + component.onRename({ target: { value: "New name" } } as unknown as Event); + expect(rename).toHaveBeenCalledWith("New name"); + }); + + it("onRename is inert on a reader mount, where decorate omitted rename", () => { + const config: FormlyFieldConfig = { key: "k", props: { label: "Predicates" } }; + EditableLabelWrapperComponent.decorate(config, { + authoring: false, + name: "Predicate", + hidden: false, + fallback: "Predicates", + canHide: false, + }); + component.field = config; + + expect(config.props?.["label"]).toBe(""); + expect(() => component.onRename({ target: { value: "x" } } as unknown as Event)).not.toThrow(); + }); + + it("onToggleHidden flips the current hidden state through setFieldHidden", () => { + const setHidden = vi.fn(); + component.field = { props: { authorHidden: false, setFieldHidden: setHidden } } as unknown as FormlyFieldConfig; + component.onToggleHidden(); + expect(setHidden).toHaveBeenCalledWith(true); + }); + }); + + describe("template", () => { + it("while authoring, renders the name input seeded with authorName and the schema label as placeholder", () => { + component.field = { + props: { authoring: true, authorName: "Genes", schemaLabel: "File Key", canHide: true, authorHidden: false }, + } as unknown as FormlyFieldConfig; + fixture.detectChanges(); + + const input = fixture.nativeElement.querySelector("input.lbl-input") as HTMLInputElement; + expect(input).toBeTruthy(); + expect(input.value).toBe("Genes"); + expect(input.placeholder).toBe("File Key"); + // the hide toggle is offered when canHide is not false + expect(fixture.nativeElement.querySelector("button.lbl-eye")).toBeTruthy(); + }); + + it("while authoring, still names the control for assistive technology, following the current name", () => { + // The visible box edits the label; it is not the control's label. decorate blanks formly's + // own, so without this the control below would have no accessible name while authoring. + component.field = { + id: "formly_3_genes", + props: { authoring: true, authorName: "Genes", schemaLabel: "File Key", canHide: true }, + } as unknown as FormlyFieldConfig; + fixture.detectChanges(); + + const label = fixture.nativeElement.querySelector("label.lbl-sr-only") as HTMLLabelElement; + expect(label.getAttribute("for")).toBe("formly_3_genes"); + expect(label.textContent?.trim()).toBe("Genes"); + // The name box itself is labelled on its own and never points at the control. + expect(fixture.nativeElement.querySelector("input.lbl-input").getAttribute("aria-label")).toBeTruthy(); + + component.field.props!["authorName"] = ""; + fixture.detectChanges(); + expect(label.textContent?.trim()).toBe("File Key"); + }); + + it("hides the eye toggle when canHide is false", () => { + component.field = { + props: { authoring: true, authorName: "Genes", schemaLabel: "File Key", canHide: false }, + } as unknown as FormlyFieldConfig; + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector("button.lbl-eye")).toBeNull(); + }); + + it("for a reader, renders a plain static label from authorName, tied to the control by id", () => { + // decorate blanks formly's own label, so this one is the control's only label: without `for` + // the reader's input would show a name but have no accessible name at all. + component.field = { + id: "formly_3_genes", + props: { authoring: false, authorName: "Genes", schemaLabel: "File Key" }, + } as unknown as FormlyFieldConfig; + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector("input.lbl-input")).toBeNull(); + const label = fixture.nativeElement.querySelector("label.lbl-static") as HTMLLabelElement; + expect(label.textContent?.trim()).toBe("Genes"); + expect(label.getAttribute("for")).toBe("formly_3_genes"); + }); + }); +}); diff --git a/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.ts b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.ts new file mode 100644 index 00000000000..61c0188e557 --- /dev/null +++ b/frontend/src/app/common/formly/editable-label-wrapper/editable-label-wrapper.component.ts @@ -0,0 +1,79 @@ +/** + * 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 } from "@angular/core"; +import { NgIf } from "@angular/common"; +import { NzIconDirective } from "ng-zorro-antd/icon"; +import { FieldWrapper, FormlyFieldConfig } from "@ngx-formly/core"; +import { merge } from "lodash-es"; + +/** + * Lets an author rename or hide one field of the form in place: the label itself becomes + * the input, so what they type is exactly what the reader sees, where they see it (the + * schema's own labels -- "File Key", "Alias" -- describe the operator, not the reader's + * task). Renders as a plain label for anyone not authoring. + */ +@Component({ + selector: "texera-editable-label-wrapper", + templateUrl: "./editable-label-wrapper.component.html", + styleUrls: ["./editable-label-wrapper.component.scss"], + imports: [NgIf, NzIconDirective], +}) +export class EditableLabelWrapperComponent extends FieldWrapper { + /** Add this wrapper to a field with its naming + callbacks; `fallback` (the schema label) + * is the placeholder, so the author sees what leaving it blank yields. `rename` may be omitted + * for a reader mount (`authoring: false`), which renders a static label and no name input; + * `setHidden` may be omitted only with `canHide: false`, where the hide control is never + * rendered. */ + public static decorate( + config: FormlyFieldConfig, + state: { authoring: boolean; name: string; hidden: boolean; fallback: string; canHide?: boolean }, + rename?: (name: string) => void, + setHidden?: (hidden: boolean) => void + ): void { + merge(config, { + wrappers: [...(config.wrappers ?? []), "editable-label-wrapper"], + props: { + ...config.props, + // The wrapper draws the label itself; leaving formly's own label on would print + // it twice. + label: "", + authoring: state.authoring, + authorName: state.name, + authorHidden: state.hidden, + canHide: state.canHide !== false, + schemaLabel: state.fallback, + renameField: rename, + setFieldHidden: setHidden, + }, + }); + } + + public onRename(event: Event): void { + // Optional-chained for the same reason as setFieldHidden below: a reader mount omits rename and + // never renders the input this handles, but the handler should not depend on that. + this.props["renameField"]?.((event.target as HTMLInputElement).value); + } + + public onToggleHidden(): void { + // Optional-chained: decorate may omit setHidden when canHide is false, and although this handler + // is unreachable then (the hide control is not rendered), a plain call would couple that to luck. + this.props["setFieldHidden"]?.(!this.props["authorHidden"]); + } +} diff --git a/frontend/src/app/common/formly/formly-config.ts b/frontend/src/app/common/formly/formly-config.ts index 61cc13e639e..2b42d1239b7 100644 --- a/frontend/src/app/common/formly/formly-config.ts +++ b/frontend/src/app/common/formly/formly-config.ts @@ -27,6 +27,7 @@ import { PresetWrapperComponent } from "./preset-wrapper/preset-wrapper.componen import { DatasetFileSelectorComponent } from "../../workspace/component/dataset-file-selector/dataset-file-selector.component"; import { CollabWrapperComponent } from "./collab-wrapper/collab-wrapper/collab-wrapper.component"; import { ExposePropertyWrapperComponent } from "./expose-property-wrapper/expose-property-wrapper.component"; +import { EditableLabelWrapperComponent } from "./editable-label-wrapper/editable-label-wrapper.component"; import { FormlyRepeatDndComponent } from "./repeat-dnd/repeat-dnd.component"; import { UiUdfParametersComponent } from "../../workspace/component/ui-udf-parameters/ui-udf-parameters.component"; import { DatasetVersionSelectorComponent } from "../../workspace/component/dataset-version-selector/dataset-version-selector.component"; @@ -94,6 +95,7 @@ export const TEXERA_FORMLY_CONFIG = { { name: "preset-wrapper", component: PresetWrapperComponent }, { name: "collab-wrapper", component: CollabWrapperComponent }, { name: "expose-property-wrapper", component: ExposePropertyWrapperComponent }, + { name: "editable-label-wrapper", component: EditableLabelWrapperComponent }, ], }; diff --git a/frontend/src/app/workspace/component/property-editor/property-editor.component.ts b/frontend/src/app/workspace/component/property-editor/property-editor.component.ts index 5952b942101..63b97a71c7a 100644 --- a/frontend/src/app/workspace/component/property-editor/property-editor.component.ts +++ b/frontend/src/app/workspace/component/property-editor/property-editor.component.ts @@ -235,15 +235,16 @@ export class PropertyEditorComponent implements OnInit, OnDestroy, OnChanges { ngOnDestroy(): void { // The Form View's read-only copy (persistPlacement=false) must not persist geometry: it is not // the docked canvas panel, so writing these keys would overwrite the real panel's saved size. - if (!this.persistPlacement) { - return; - } - localStorage.setItem("right-panel-width", String(this.width)); - localStorage.setItem("right-panel-height", String(this.height)); + // Guarding the block rather than returning early keeps any teardown added below it running for + // both mounts. + if (this.persistPlacement) { + localStorage.setItem("right-panel-width", String(this.width)); + localStorage.setItem("right-panel-height", String(this.height)); - const rightContainer = document.getElementById("right-container"); - if (rightContainer) { - localStorage.setItem("right-panel-style", rightContainer.style.cssText); + const rightContainer = document.getElementById("right-container"); + if (rightContainer) { + localStorage.setItem("right-panel-style", rightContainer.style.cssText); + } } } diff --git a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.html b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.html index 4465d65cb27..7d423f58d52 100644 --- a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.html +++ b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.html @@ -17,6 +17,9 @@ under the License. --> +
  • { expect(spy).toHaveBeenCalledTimes(1); }); + // The Form View's edit mode turns modification on for the property panel while its embedded + // preview stays structure-locked; the lock has to reach the menu, or right-click on the preview + // could still re-shape the graph. + describe("under a structure lock", () => { + const renderedLabels = () => + fixture.debugElement.queryAll(By.css("li[nz-menu-item]")).map(li => norm(li.nativeElement.textContent)); + + it("keeps cut, delete, disable and enable off even with modification enabled", () => { + highlightedOperatorsSubject.next(["op1"]); + component.isWorkflowModifiable = true; + component.structureLocked = true; + operatorMenuService.isDisableOperator = true; + operatorMenuService.isDisableOperatorClickable = true; + operatorMenuService.isToViewResult = true; + operatorMenuService.isToViewResultClickable = true; + fixture.detectChanges(); + + const labels = renderedLabels(); + expect(labels).not.toContain("cut"); + expect(labels).not.toContain("delete"); + expect(labels).not.toContain("disable"); + // Copying and the result toggle do not re-shape the graph, so they stay. + expect(labels).toContain("copy"); + expect(labels).toContain("view result"); + + operatorMenuService.isDisableOperator = false; + fixture.detectChanges(); + expect(renderedLabels()).not.toContain("enable"); + }); + + it("keeps paste and link deletion off", () => { + component.isWorkflowModifiable = true; + component.structureLocked = true; + highlightedOperatorsSubject.next([]); + highlightedCommentBoxesSubject.next([]); + fixture.detectChanges(); + expect(renderedLabels()).not.toContain("paste"); + + jointGraphWrapperSpy.getCurrentHighlightedLinkIDs.mockReturnValue(["link1"]); + fixture.detectChanges(); + expect(renderedLabels()).not.toContain("delete"); + }); + + it("canModify needs modification on and no structure lock", () => { + component.isWorkflowModifiable = true; + component.structureLocked = false; + expect(component.canModify).toBe(true); + + component.structureLocked = true; + expect(component.canModify).toBe(false); + + component.structureLocked = false; + component.isWorkflowModifiable = false; + expect(component.canModify).toBe(false); + }); + }); + it("execute to this operator invokes executeUpToOperator", () => { highlightedOperatorsSubject.next(["op1"]); component.isWorkflowModifiable = true; diff --git a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts index 019f77f7afa..911172b6ba3 100644 --- a/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts +++ b/frontend/src/app/workspace/component/workflow-editor/context-menu/context-menu/context-menu.component.ts @@ -17,7 +17,7 @@ * under the License. */ -import { Component } from "@angular/core"; +import { Component, Input } from "@angular/core"; import { OperatorMenuService } from "src/app/workspace/service/operator-menu/operator-menu.service"; import { WorkflowActionService } from "src/app/workspace/service/workflow-graph/model/workflow-action.service"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; @@ -41,6 +41,20 @@ import { NzIconDirective } from "ng-zorro-antd/icon"; }) export class ContextMenuComponent { public isWorkflowModifiable: boolean = false; + + /** + * Set by an embedded, structure-locked editor (the Form View's preview). Its edit mode turns + * workflow modification back on for the property panel, which alone would also bring the + * re-shaping commands here back; the lock keeps them off. Copy, the result toggles and export + * do not change the graph and are left to their own rules. + */ + @Input() structureLocked = false; + + /** Whether the graph may be re-shaped from this menu: modification on, and no structure lock. */ + public get canModify(): boolean { + return this.isWorkflowModifiable && !this.structureLocked; + } + public highlightedOperatorIds: readonly string[] = []; public highlightedCommentBoxIds: readonly string[] = []; diff --git a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.html b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.html index 0b652d1a76c..43d25120e9c 100644 --- a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.html +++ b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.html @@ -34,7 +34,7 @@ - + diff --git a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts index 95972294825..182811c5c00 100644 --- a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts @@ -56,7 +56,8 @@ import { OperatorLink, OperatorPredicate } from "../../types/workflow-common.int import { tap } from "rxjs/operators"; import { WorkflowVersionService } from "../../../dashboard/service/user/workflow-version/workflow-version.service"; import { config as rxjsConfig, of, Subject } from "rxjs"; -import { NzContextMenuService, NzDropDownModule } from "ng-zorro-antd/dropdown"; +import { NzContextMenuService, NzDropDownModule, NzDropdownMenuComponent } from "ng-zorro-antd/dropdown"; +import { By } from "@angular/platform-browser"; import { ActivatedRoute, Router } from "@angular/router"; import { RouterTestingModule } from "@angular/router/testing"; import { ContextMenuComponent } from "./context-menu/context-menu/context-menu.component"; @@ -140,6 +141,22 @@ describe("WorkflowEditorComponent", () => { expect(editor.classList.contains("hide-operator-status")).toBe(true); }); + it("carries its structure lock into the right-click menu", () => { + // The Form View's edit mode re-enables workflow modification for the property panel while its + // preview stays structure-locked; the menu must see the lock, or right-click could still cut, + // paste or delete from the preview. + component.structureLocked = true; + fixture.detectChanges(); + const menu = fixture.debugElement.query(By.directive(NzDropdownMenuComponent)).componentInstance; + component.nzContextMenu.create(new MouseEvent("contextmenu", { clientX: 5, clientY: 5 }), menu); + fixture.detectChanges(); + + const contextMenu = fixture.debugElement.query(By.directive(ContextMenuComponent)); + expect(contextMenu).not.toBeNull(); + expect((contextMenu.componentInstance as ContextMenuComponent).structureLocked).toBe(true); + component.nzContextMenu.close(); + }); + // Drives the region-update stream the editor subscribes to in handleRegionEvents, creating // region- elements around the given operator, and returns the operator id used. function emitRegionUpdate(regionId: number): string { diff --git a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts index 9234d3338c5..e889f49b42d 100644 --- a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts +++ b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts @@ -128,10 +128,10 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy * Set by a view that shows the graph but must never re-shape it. Separate from the * workflow-modification lock (which also gates property editing, so reusing that alone would * disable the property panel a later authoring mode needs). It locks the paper's own - * interactions -- dragging, linking, and the keyboard delete/cut/port commands. The right-click - * menu's structural commands follow the modification lock instead, so a read-only view like the - * Form View, which also disables modification, is fully locked; an authoring view that re-enables - * modification will need to carry this lock into the menu too. + * interactions -- dragging, linking, and the keyboard delete/cut/port commands -- and is passed on + * to the right-click menu, whose re-shaping commands otherwise follow the modification lock alone: + * the Form View's edit mode re-enables modification for the property panel, and without the + * hand-off the preview's menu would cut, paste and delete again. */ @Input() structureLocked = false; 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 64b30e323a7..56e1f3203cf 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 @@ -61,6 +61,30 @@
    + +
    +

    + Choose what people fill in, then run to see it as they will. +

    + + +
    +
    - +
    - + [class.authoring]="authoring"> + + + + + + + + +
    + + +
    + + +
    + + + +

    Markdown. Add a picture with ![alt](https://…).

    +     + +
    +
    Inputs + Drag to reorder. Click a step in the workflow to add more
    - This workflow has no inputs to fill in. + {{ authoring ? "No inputs yet. Open the workflow below and click a step to expose its settings." : "This workflow + has no inputs to fill in." }}
    -
    +
    - -
    - -
    + *ngFor="let r of rendered; let i = index; trackBy: trackByRendered" + cdkDrag + [cdkDragDisabled]="!authoring"> + +
    + +
    + From {{ r.resolved.operatorLabel }} +
    + + + + +
    - -

    - {{ r.resolved.binding.helpText }} -

    + +
    + {{ r.resolved.brokenReason }} +
    + + + +
    + +
    + + +

    + {{ r.resolved.binding.helpText }} +

    +     + + +
    + +
    + Set on {{ r.resolved.operatorLabel }}: {{ r.resolved.binding.propertyKey }} + + +
    +
    @@ -207,7 +404,10 @@

    {{ instructionTitle || "How to use this" }}

    aria-hidden="true"> Workflow - Optional. The steps that will run. + {{ authoring ? "Click a step to choose which of its settings people fill in." : "Optional. The steps that + will run." }} @@ -237,17 +437,21 @@

    {{ instructionTitle || "How to use this" }}

    aria-hidden="true"> - +
    +

    Results shown here

    +

    The final result always shows. Toggle an earlier step here to feature its result too.

    +
    + +

    + No earlier steps to add yet. Turn on a step's result view on the canvas to feature it 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 6202f803a29..164e154c33e 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 @@ -195,20 +195,18 @@ $shell: #fafafa; .instr { margin-bottom: 26px; + /* The header row. Reading, its one child is the full-width toggle button, which carries the row's + padding so the whole row stays clickable; authoring, the row itself is padded and holds the + icon, the title input and a small chevron toggle. */ .instr-bar { display: flex; align-items: center; gap: 10px; width: 100%; - padding: 13px 16px; - cursor: pointer; - user-select: none; - appearance: none; - border: 0; - background: none; - color: inherit; - font: inherit; - text-align: left; + + &.authoring { + padding: 13px 16px; + } h2 { margin: 0; @@ -228,6 +226,40 @@ $shell: #fafafa; } } + .instr-toggle { + display: flex; + align-items: center; + gap: 10px; + flex: 1; + padding: 13px 16px; + cursor: pointer; + user-select: none; + appearance: none; + border: 0; + background: none; + color: inherit; + font: inherit; + text-align: left; + + &:focus-visible { + outline: 2px solid $blue; + outline-offset: -2px; + border-radius: 8px; + } + } + + /* The authoring toggle is only the chevron: the row's padding is on the row, and the title input + takes the width. */ + .instr-chev { + flex: none; + padding: 4px; + border-radius: 4px; + + &:focus-visible { + outline-offset: 0; + } + } + &.open .instr-bar .chev { transform: rotate(0deg); } @@ -787,3 +819,284 @@ $shell: #fafafa; } } } + +/* ============================================================================ + Author mode: the in-place editing UI (Edit/Done). Everything here shows only + while authoring; a reader never renders these elements. + ============================================================================ */ + +.pc-head { + display: flex; + align-items: center; + gap: 14px; + margin-top: 18px; + + .lede { + margin: 0; + color: $text-2; + } + + .spacer { + flex: 1; + } +} + +.hint { + font-size: 12px; + color: $text-2; +} + +/* Author edits the instruction heading right here: looks like the h2, with a faint dashed + box so it reads as editable; firms up on hover, a real input on focus. */ +.instr-title-input { + flex: 1; + min-width: 0; + margin: 0; + font-size: 15px; + font-weight: 600; + color: inherit; + background: none; + border: 1px dashed #dcdcdc; + border-radius: 4px; + padding: 2px 6px; + cursor: text; + + &:hover { + border-style: solid; + border-color: #b0b0b0; + } + + &:focus { + border-style: solid; + border-color: $blue; + background: #fff; + outline: none; + } +} + +.tabs { + display: flex; + gap: 2px; + border-bottom: 1px solid $divider; + margin: -16px -16px 14px; + padding: 0 16px; + + button { + background: none; + border: none; + border-bottom: 2px solid transparent; + padding: 9px 12px; + color: $text-2; + font-weight: 500; + cursor: pointer; + + &[aria-current="true"] { + color: $blue; + border-bottom-color: $blue; + } + + &:focus-visible { + outline: 2px solid $blue; + outline-offset: -2px; + } + } +} + +.md-input { + width: 100%; + min-height: 180px; + resize: vertical; + border: 1px solid $border; + border-radius: 6px; + padding: 10px 12px; + font-family: "SFMono-Regular", Consolas, Menlo, monospace; + font-size: 13px; + line-height: 1.65; + background: $shell; + + &:focus { + background: #fff; + border-color: $blue; + outline: none; + } +} + +.field-top { + display: flex; + align-items: flex-start; + gap: 10px; +} + +.field-top-spacer { + flex: 1; +} + +.grip { + color: rgba(0, 0, 0, 0.3); + cursor: grab; + padding-top: 3px; +} + +/* Move up / Move down: quiet icon buttons at the row's right edge, the keyboard route the drag + handle does not give. Disabled at the ends rather than hidden, so the row does not reflow as a + card reaches the top or bottom. */ +.move { + appearance: none; + border: 0; + background: none; + padding: 2px 3px; + border-radius: 3px; + color: $text-2; + cursor: pointer; + font: inherit; + line-height: 1; + + &:hover:not(:disabled) { + color: $text; + } + + &:disabled { + color: rgba(0, 0, 0, 0.15); + cursor: default; + } + + &:focus-visible { + outline: 2px solid $blue; + outline-offset: 1px; + } +} + +.field-help { + font-size: 13px; + color: $text-2; + margin-top: 2px; +} + +.broken { + margin-top: 4px; + font-size: 13px; + color: #874d00; +} + +/* A dragged card lifts; the gap it left fades. */ +.cdk-drag-preview { + box-shadow: 0 6px 20px rgba(0, 0, 0, 0.14); +} + +.cdk-drag-placeholder { + opacity: 0.35; +} + +.edit { + margin-top: 14px; + padding-top: 14px; + border-top: 1px dashed $border; + display: grid; + grid-template-columns: 1fr; + gap: 12px 18px; + + .field { + display: flex; + flex-direction: column; + gap: 4px; + + .label { + font-size: 12px; + color: $text-2; + } + + input { + border: 1px solid $border; + border-radius: 6px; + padding: 6px 10px; + font-size: 13px; + + &:focus { + border-color: $blue; + outline: none; + } + } + } + + .field.wide { + grid-column: 1 / -1; + } + + .edit-foot { + grid-column: 1 / -1; + display: flex; + align-items: center; + + .spacer { + flex: 1; + } + + .remove { + background: none; + border: 1px solid $border; + border-radius: 6px; + padding: 4px 10px; + color: #ff4d4f; + cursor: pointer; + + &:hover { + border-color: #ff4d4f; + } + + &:focus-visible { + outline: 2px solid $blue; + outline-offset: 2px; + } + } + } +} + +.respick { + margin-top: 26px; + padding: 15px 18px; + + h3 { + margin: 0 0 3px; + font-size: 14px; + font-weight: 600; + } + + p { + margin: 0 0 11px; + font-size: 13px; + color: $text-2; + } + + .opts { + display: flex; + flex-wrap: wrap; + gap: 8px; + } + + .pill { + padding: 5px 12px; + border-radius: 15px; + font-size: 13px; + border: 1px solid $border; + background: #fff; + color: $text-2; + cursor: pointer; + + &:hover { + border-color: $blue; + color: $blue; + } + + &:focus-visible { + outline: 2px solid $blue; + outline-offset: 2px; + } + + &.on { + background: rgba(24, 144, 255, 0.1); + border-color: $blue; + color: $blue; + font-weight: 500; + } + } +} diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts index 77613168670..40cc6c13230 100644 --- a/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts +++ b/frontend/src/app/workspace/component/workflow-form/workflow-form.component.spec.ts @@ -332,14 +332,43 @@ describe("WorkflowFormComponent", () => { vi.useRealTimers(); }); - it("saves before handing over to the operator canvas", () => { + it("saves, then hands over to the operator canvas only once the save has completed", () => { enableSave(); build(formViewWorkflow).ngOnInit(); workflowPersistService.persistWorkflow.mockClear(); + const navigate = vi.spyOn(component as any, "openCanvasPage").mockImplementation(() => {}); component.openRegularCanvas(); + // The full-page load aborts a request still in flight, so the navigation waits for the save + // to complete (the persist mock completes synchronously here). expect(workflowPersistService.persistWorkflow).toHaveBeenCalled(); + expect(navigate).toHaveBeenCalledTimes(1); + }); + + it("stays on the form and reports it when the save before the switch fails", () => { + enableSave(); + build(formViewWorkflow).ngOnInit(); + workflowPersistService.persistWorkflow.mockReturnValue(throwError(() => new Error("nope"))); + const navigate = vi.spyOn(component as any, "openCanvasPage").mockImplementation(() => {}); + + component.openRegularCanvas(); + + expect(navigate).not.toHaveBeenCalled(); + expect(h.notificationService.error).toHaveBeenCalledWith( + "Could not save. Your latest changes are not stored yet." + ); + }); + + it("hands a reader with nothing to save straight over to the canvas", () => { + build({ ...formViewWorkflow, readonly: true }).ngOnInit(); + workflowPersistService.persistWorkflow.mockClear(); + const navigate = vi.spyOn(component as any, "openCanvasPage").mockImplementation(() => {}); + + component.openRegularCanvas(); + + expect(workflowPersistService.persistWorkflow).not.toHaveBeenCalled(); + expect(navigate).toHaveBeenCalledTimes(1); }); it("saves once more on the way out", () => { @@ -703,6 +732,41 @@ describe("WorkflowFormComponent", () => { return component.rendered[0].fields[0] as any; }; + // The shared array widget prints its label at the bottom beside its add button, so a repeated + // input's title would sit above the rows in edit mode and jump below them on Done. A reader gets + // the same static title above instead, and the widget's own label is blanked. + it("gives a repeated input its title above in reader mode, not the array widget's bottom label", () => { + build(formViewWorkflow).ngOnInit(); + h.formlyJsonschema.toFieldConfig = () => ({ + fieldGroup: [ + { + key: "predicates", + type: "array", + props: { label: "Predicates" }, + fieldArray: () => ({ fieldGroup: [{ key: "alias", props: { label: "Alias" } }] }), + }, + ], + }); + + const field = expose({ id: "p", operatorID: "op-1", propertyKey: "predicates", displayName: "Predicate" }); + + expect(field.wrappers).toContain("editable-label-wrapper"); + expect(field.props.authoring).toBe(false); + expect(field.props.authorName).toBe("Predicate"); + expect(field.props.schemaLabel).toBe("Predicates"); + expect(field.props.label).toBe(""); + }); + + it("leaves a scalar input's label to formly in reader mode", () => { + build(formViewWorkflow).ngOnInit(); + + const field = expose({ id: "n", operatorID: "op-1", propertyKey: "n_hvg", displayName: "How many" }); + + // Above the control already, with formly's required marker; nothing to wrap. + expect(field.wrappers ?? []).not.toContain("editable-label-wrapper"); + expect(field.props.label).toBe("How many"); + }); + it("renames and hides an overridden sub-field of an object property", () => { build(formViewWorkflow).ngOnInit(); @@ -822,17 +886,64 @@ describe("WorkflowFormComponent", () => { expect(rebuild).toHaveBeenCalled(); }); - it("does not rebuild under the cursor of someone typing", async () => { + it("holds a rebuild while someone is typing and runs it once the focus leaves", async () => { build(formViewWorkflow).ngOnInit(); - vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); + const typing = vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); const rebuild = vi.spyOn(component as any, "readConfig"); h.compilationChanged.next("Succeeded"); await new Promise(r => setTimeout(r, FORM_DEBOUNCE_TIME_MS + 50)); + expect(rebuild).not.toHaveBeenCalled(); + + // The cursor leaves the field: the held rebuild runs, once. Held rather than dropped, or the + // compiled schema would never reach the cards until something else rebuilt them. + typing.mockReturnValue(false); + component.onFocusOut(); + await new Promise(r => setTimeout(r, 10)); + + expect(rebuild).toHaveBeenCalledTimes(1); + }); + + it("keeps a held rebuild held when the focus only moves to another text field", async () => { + build(formViewWorkflow).ngOnInit(); + vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); + const rebuild = vi.spyOn(component as any, "readConfig"); + workflowActionService.formBindingChanged$.next(undefined); + + component.onFocusOut(); // tabbed to the next input: still typing when the check runs + await new Promise(r => setTimeout(r, 10)); expect(rebuild).not.toHaveBeenCalled(); }); + it("rebuilds nothing on a focusout with no rebuild held", async () => { + build(formViewWorkflow).ngOnInit(); + const rebuild = vi.spyOn(component as any, "readConfig"); + + component.onFocusOut(); + await new Promise(r => setTimeout(r, 10)); + + expect(rebuild).not.toHaveBeenCalled(); + }); + + // Leaving the text field by clicking a tick box: focusout queues the held rebuild, then the tick + // box's own change rebuilds at once and clears the hold. The queued callback must notice and + // not rebuild the same cards a second time. + it("does not rebuild twice when the control that took the focus already rebuilt", async () => { + build(formViewWorkflow).ngOnInit(); + const typing = vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); + const rebuild = vi.spyOn(component as any, "readConfig"); + workflowActionService.formBindingChanged$.next(undefined); // held + component.onFocusOut(); // queued + + typing.mockReturnValue(false); + workflowActionService.formBindingChanged$.next(undefined); // the tick box's own change: rebuilds now + expect(rebuild).toHaveBeenCalledTimes(1); + await new Promise(r => setTimeout(r, 10)); // the queued callback fires + + expect(rebuild).toHaveBeenCalledTimes(1); + }); + it("re-reads the config when a property is exposed or un-exposed", () => { build(formViewWorkflow).ngOnInit(); const before = formBindingService.resolveFields.mock.calls.length; @@ -843,15 +954,37 @@ describe("WorkflowFormComponent", () => { }); // Once #8351 makes this stream fire for a co-editor's change, a rebuild under the cursor would - // discard a half-entered value -- so the binding path skips typing, like the compilation path. - it("does not re-read the config on a binding change while the reader is typing", () => { + // discard a half-entered value -- so the binding path holds it while typing, like the + // compilation path, and runs it when the focus leaves. + it("holds a binding-change rebuild while the reader is typing, then runs it on focusout", async () => { build(formViewWorkflow).ngOnInit(); - vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); + const typing = vi.spyOn(component as any, "isTypingInTheForm").mockReturnValue(true); const rebuild = vi.spyOn(component as any, "readConfig"); workflowActionService.formBindingChanged$.next(undefined); - expect(rebuild).not.toHaveBeenCalled(); + + typing.mockReturnValue(false); + component.onFocusOut(); + await new Promise(r => setTimeout(r, 10)); + + expect(rebuild).toHaveBeenCalledTimes(1); + }); + + // The bug this guards against: ticking a property in the step panel focuses the tick box, an + // inside this page. Counted as typing, the rebuild that should add the + // card was held back, so the tick looked like it did nothing until something else rebuilt. + it("does not count a focused tick box as typing", () => { + build(formViewWorkflow).ngOnInit(); + const box = document.createElement("input"); + box.type = "checkbox"; + document.body.appendChild(box); + (component as any).host = { nativeElement: { contains: () => true, querySelector: () => null } }; + box.focus(); + + expect((component as any).isTypingInTheForm()).toBe(false); + + document.body.removeChild(box); }); it("reports typing when a form field inside the page is focused", () => { @@ -1521,4 +1654,290 @@ describe("WorkflowFormComponent", () => { expect(component.selectedOperatorId).toBeUndefined(); }); }); + + describe("author mode", () => { + it("enters edit mode: opens the workflow, enables modification, re-reads the config", () => { + build(formViewWorkflow).ngOnInit(); + const read = vi.spyOn(component as any, "readConfig"); + + component.toggleAuthoring(); + + expect(component.authoring).toBe(true); + expect(component.workflowOpen).toBe(true); + expect(h.workflowActionService.enableWorkflowModification).toHaveBeenCalled(); + expect(read).toHaveBeenCalled(); + }); + + it("leaves edit mode: collapses the workflow and locks modification back", () => { + build(formViewWorkflow).ngOnInit(); + component.toggleAuthoring(); + (h.workflowActionService.disableWorkflowModification as any).mockClear(); + + component.toggleAuthoring(); + + expect(component.authoring).toBe(false); + expect(component.workflowOpen).toBe(false); + expect(h.workflowActionService.disableWorkflowModification).toHaveBeenCalled(); + }); + + it("refuses to enter edit mode without write access, at the method and not only the button", () => { + // The Edit button is not rendered for a reader, but every authoring action writes the shared + // config, so the method itself is the boundary: a reader stays a reader whoever calls it. + build({ ...formViewWorkflow, readonly: true }).ngOnInit(); + expect(component.canEdit).toBe(false); + const read = vi.spyOn(component as any, "readConfig"); + + component.toggleAuthoring(); + + expect(component.authoring).toBe(false); + expect(read).not.toHaveBeenCalled(); + expect(h.workflowActionService.enableWorkflowModification).not.toHaveBeenCalled(); + }); + + it("always allows leaving edit mode, even if write access is gone", () => { + build(formViewWorkflow).ngOnInit(); + component.toggleAuthoring(); + expect(component.authoring).toBe(true); + component.canEdit = false; + + component.toggleAuthoring(); + + expect(component.authoring).toBe(false); + expect(h.workflowActionService.disableWorkflowModification).toHaveBeenCalled(); + }); + + it("shows broken inputs to an author but never to a reader", () => { + build(formViewWorkflow).ngOnInit(); + (component as any).parameters = [resolved("b", "B", { brokenReason: "gone" })]; + + expect(component.visibleFields).toEqual([]); + component.authoring = true; + expect(component.visibleFields.length).toBe(1); + }); + + it("renders a broken input as an empty card carrying its reason", () => { + build(formViewWorkflow).ngOnInit(); + component.authoring = true; + h.formBindingService.resolveFields.mockReturnValue([resolved("b", "B", { brokenReason: "gone" })]); + + (component as any).readConfig(); + + expect(component.rendered[0].fields).toEqual([]); + expect(component.rendered[0].resolved.brokenReason).toBe("gone"); + }); + + it("keeps an input whose operator is gone, in either mode, for the author to remove explicitly", () => { + // Re-reading the config must not rewrite it. A broken input reaches the author as a card with + // its reason (the two tests above) and leaves only through onRemoveBinding; dropping it on the + // way in would be a silent config write and would hide where the input went. + build(formViewWorkflow).ngOnInit(); + (component as any).loading = false; + h.hasOperatorIds.add("op-1"); + h.formBindingService.getConfig.mockReturnValue({ + instruction: undefined, + fields: [ + { id: "gone", operatorID: "removed" }, + { id: "b", operatorID: "op-1" }, + ], + resultOperatorIds: [], + }); + + (component as any).readConfig(); + component.authoring = true; + (component as any).readConfig(); + + expect(h.formBindingService.setFields).not.toHaveBeenCalled(); + expect(h.formBindingService.removeBinding).not.toHaveBeenCalled(); + }); + + it("lists viewed and chosen intermediate steps, never the always-shown terminal", () => { + build(formViewWorkflow).ngOnInit(); + component.authoring = true; + h.graphOperators.push({ operatorID: "viewed-mid", operatorType: "Filter" }); + h.graphOperators.push({ operatorID: "chosen-mid", operatorType: "Filter" }); + h.graphOperators.push({ operatorID: "plain-mid", operatorType: "Filter" }); + h.graphOperators.push({ operatorID: "last", operatorType: "Limit" }); + h.viewResultIds.add("viewed-mid"); + h.terminalIds.add("last"); // terminal always shows, so it is not the author's to toggle here + h.formBindingService.getConfig.mockReturnValue({ + instruction: undefined, + fields: [], + resultOperatorIds: ["chosen-mid"], + }); + + (component as any).readConfig(); + + const ids = component.resultChoices.map(c => c.operatorID); + expect(ids).toContain("viewed-mid"); // has the eye on the canvas + expect(ids).toContain("chosen-mid"); // already chosen + expect(ids).not.toContain("plain-mid"); // no eye, not chosen -> nothing to show, not offered + expect(ids).not.toContain("last"); // terminal always shows; never offered in the picker + expect(component.resultChoices.find(c => c.operatorID === "chosen-mid")?.shown).toBe(true); + expect(component.resultChoices.find(c => c.operatorID === "viewed-mid")?.shown).toBe(false); + }); + + it("adds a step to the picker the moment its eye is turned on, without a re-read", () => { + build(formViewWorkflow).ngOnInit(); + component.authoring = true; + h.graphOperators.push({ operatorID: "mid", operatorType: "Filter" }); + h.formBindingService.getConfig.mockReturnValue({ instruction: undefined, fields: [], resultOperatorIds: [] }); + (component as any).readConfig(); + expect(component.resultChoices.map(c => c.operatorID)).not.toContain("mid"); + + // The author gives "mid" the eye on the canvas: the view-result set changes, emitting no result + // update, so the picker must react to that stream directly (or the option would not appear). + h.viewResultIds.add("mid"); + h.viewResultChanged.next({}); + + expect(component.resultChoices.map(c => c.operatorID)).toContain("mid"); + }); + + it("does not build the result picker for a reader", () => { + build(formViewWorkflow).ngOnInit(); + h.graphOperators.push({ operatorID: "op-1", operatorType: "Filter" }); + + (component as any).readConfig(); + + expect(component.resultChoices).toEqual([]); + }); + + it("routes structural edits through the binding service and re-reads", () => { + build(formViewWorkflow).ngOnInit(); + const read = vi.spyOn(component as any, "readConfig"); + + component.onRemoveBinding(resolved("n", "N", {})); + expect(h.formBindingService.removeBinding).toHaveBeenCalledWith("n"); + component.onToggleResult({ operatorID: "op-1", label: "Filter", shown: false }); + expect(h.formBindingService.toggleResultOperator).toHaveBeenCalledWith("op-1"); + expect(read).toHaveBeenCalledTimes(2); + }); + + it("saves help text without rebuilding the form (no readConfig on every keystroke)", () => { + build(formViewWorkflow).ngOnInit(); + const read = vi.spyOn(component as any, "readConfig"); + + component.onEditHelpText(resolved("n", "N", {}), "help"); + + expect(h.formBindingService.updateBinding).toHaveBeenCalledWith("n", { helpText: "help" }); + expect(read).not.toHaveBeenCalled(); + }); + + it("reorders the saved field the dragged card names, not the raw rendered index", () => { + build(formViewWorkflow).ngOnInit(); + // rendered is shorter than the saved fields: 'b' rendered no card (its schema was unavailable). + component.rendered = [{ resolved: { binding: { id: "a" } } }, { resolved: { binding: { id: "c" } } }] as any; + h.formBindingService.getConfig.mockReturnValue({ + instruction: undefined, + fields: [{ id: "a" }, { id: "b" }, { id: "c" }], + resultOperatorIds: [], + }); + + // Drag rendered[1] ("c", saved index 2) to the top (onto rendered[0] "a", saved index 0). + component.onDrop({ previousIndex: 1, currentIndex: 0 } as any); + + expect(h.formBindingService.reorder).toHaveBeenCalledWith(2, 0); + }); + + it("moves a card one place from the keyboard through the same reorder as the drag", () => { + build(formViewWorkflow).ngOnInit(); + // Three cards, but 'b' is not among the saved fields' neighbours in the same order (a saved + // field that rendered no card sits between), so the move has to resolve by id, as the drag does. + const cards = [ + { resolved: { binding: { id: "a" } } }, + { resolved: { binding: { id: "b" } } }, + { resolved: { binding: { id: "c" } } }, + ] as any; + component.rendered = cards; + h.formBindingService.getConfig.mockReturnValue({ + instruction: undefined, + fields: [{ id: "a" }, { id: "hidden" }, { id: "b" }, { id: "c" }], + resultOperatorIds: [], + }); + + component.onMoveBinding(cards[1], -1); // 'b' (saved 2) up onto 'a' (saved 0) + expect(h.formBindingService.reorder).toHaveBeenCalledWith(2, 0); + + // The move re-reads the config, which rebuilds `rendered` from the (mocked, empty) resolved + // fields; put the cards back to move again. + component.rendered = cards; + component.onMoveBinding(cards[1], 1); // 'b' (saved 2) down onto 'c' (saved 3) + expect(h.formBindingService.reorder).toHaveBeenCalledWith(2, 3); + }); + + it("moves nothing off either end", () => { + build(formViewWorkflow).ngOnInit(); + const cards = [{ resolved: { binding: { id: "a" } } }, { resolved: { binding: { id: "b" } } }] as any; + component.rendered = cards; + h.formBindingService.getConfig.mockReturnValue({ + instruction: undefined, + fields: [{ id: "a" }, { id: "b" }], + resultOperatorIds: [], + }); + + component.onMoveBinding(cards[0], -1); + component.onMoveBinding(cards[1], 1); + + expect(h.formBindingService.reorder).not.toHaveBeenCalled(); + }); + + it("drops a reorder whose card names a field the config no longer holds", () => { + build(formViewWorkflow).ngOnInit(); + // A card left over from a config that has since changed: its binding is gone from the saved + // fields, so neither end of the drag resolves. Reordering on those -1s would move the wrong + // field, so the drag is dropped instead. + component.rendered = [{ resolved: { binding: { id: "gone" } } }, { resolved: { binding: { id: "a" } } }] as any; + h.formBindingService.getConfig.mockReturnValue({ + instruction: undefined, + fields: [{ id: "a" }], + resultOperatorIds: [], + }); + const read = vi.spyOn(component as any, "readConfig"); + + component.onDrop({ previousIndex: 0, currentIndex: 1 } as any); + + expect(h.formBindingService.reorder).not.toHaveBeenCalled(); + expect(read).not.toHaveBeenCalled(); + }); + + it("saves the instruction as the author types, and previews on demand", () => { + build(formViewWorkflow).ngOnInit(); + component.instructionTitle = "T"; + component.instructionBody = "B"; + + component.onInstructionChange(); + expect(h.formBindingService.updateConfig).toHaveBeenCalledWith({ instruction: { title: "T", body: "B" } }); + + const render = vi.spyOn(component as any, "renderInstruction"); + component.setInstructionMode("write"); + expect(render).not.toHaveBeenCalled(); + component.setInstructionMode("preview"); + expect(component.instructionMode).toBe("preview"); + expect(render).toHaveBeenCalled(); + }); + + it("wires the editable title to rename the input and a sub-field to rename or hide it", () => { + build(formViewWorkflow).ngOnInit(); + component.authoring = true; + h.hasOperatorIds.add("op-1"); + h.formBindingService.resolveFields.mockReturnValue([ + resolved("n", "N", { + binding: { id: "n", operatorID: "op-1", propertyKey: "nested", displayName: "N", overrides: {} }, + }), + ]); + + (component as any).readConfig(); + const root = component.rendered[0].fields[0] as any; + + // The input's own title renames the whole binding. + root.props.renameField("New name"); + expect(h.formBindingService.updateBinding).toHaveBeenCalledWith("n", { displayName: "New name" }); + + // A sub-field's editable label renames it and its eye hides it, both keyed by path. + const sub = root.fieldGroup[0]; + sub.props.renameField("Sub name"); + expect(h.formBindingService.setFieldOverride).toHaveBeenCalledWith("n", "sub", { displayName: "Sub name" }); + sub.props.setFieldHidden(true); + expect(h.formBindingService.setFieldOverride).toHaveBeenCalledWith("n", "sub", { hidden: true }); + }); + }); }); 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 c9396a6069e..6b262dc3eda 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 @@ -34,7 +34,9 @@ import { MarkdownService } from "ngx-markdown"; import { EMPTY, forkJoin, Subject, timer } from "rxjs"; import { debounceTime, switchMap, takeUntil, tap } from "rxjs/operators"; +import { CdkDragDrop, DragDropModule } from "@angular/cdk/drag-drop"; import { USER_WORKFLOW, USER_WORKSPACE } from "../../../app-routing.constant"; +import { EditableLabelWrapperComponent } from "../../../common/formly/editable-label-wrapper/editable-label-wrapper.component"; import { FormFieldBinding, Workflow, WorkflowContent } from "../../../common/type/workflow"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; import { ComputingUnitState } from "../../../common/type/computing-unit-connection.interface"; @@ -56,7 +58,7 @@ import { WorkflowResultService } from "../../service/workflow-result/workflow-re import { PanelResizeService } from "../../service/workflow-result/panel-resize/panel-resize.service"; import { WorkflowWebsocketService } from "../../service/workflow-websocket/workflow-websocket.service"; import { ExecutionState } from "../../types/execute-workflow.interface"; -import { Point } from "../../types/workflow-common.interface"; +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 { ResultTableFrameComponent } from "../result-panel/result-table-frame/result-table-frame.component"; @@ -67,6 +69,22 @@ import { CoeditorUserIconComponent } from "../menu/coeditor-user-icon/coeditor-u import { CoeditorPresenceService } from "../../service/workflow-graph/model/coeditor-presence.service"; import { SAVE_DEBOUNCE_TIME_IN_MS } from "../workspace.component"; +/** + * Input types that take a click, not text. Focusing one is not "typing", so a rebuild that arrives + * while one has the focus loses nothing and must not be held back (see isTypingInTheForm). + */ +const NON_TEXT_INPUT_TYPES = new Set([ + "checkbox", + "radio", + "button", + "submit", + "reset", + "range", + "color", + "file", + "image", +]); + /** * One rendered input: the resolved binding plus the operator's own formly field for that property. * Building the field from the operator's JSON schema (not guessing from the value) is what gives a @@ -79,6 +97,14 @@ interface RenderedField { model: Record; } +/** One row of the author's "which results to show" picker: a candidate step and whether it is + * currently chosen. */ +interface ResultChoice { + operatorID: string; + label: string; + shown: boolean; +} + /** * The Form View: a second way to use a workflow. On top of the title-bar frame and the collapsible * read-only workflow preview, this PR renders the inputs an author exposed -- each as its @@ -91,9 +117,12 @@ interface RenderedField { * underneath -- the final step's output plus the author's chosen view-result steps, each a table, a * visualisation, or a compact "no result yet" -- reading the canvas's view-result set and never * writing it. A reader can also click a step on the embedded preview to open its property panel - * read-only: the panel writes nothing to the shared workflow and its content is inert. The - * authoring mode that turns that panel live and picks what to show is a later PR. A view, not a new - * object: it opens the same workflow the canvas does. + * read-only: the panel writes nothing to the shared workflow and its content is inert. With write + * access, an Edit toggle turns the page into in-place authoring: rename an input or its sub-fields, + * hide a sub-field, reorder inputs by drag, expose a new one by clicking a step (the panel goes + * live and its writes turn on), remove one, write the instruction, and pick which extra results to + * feature. A view, not a new object: every graph edit goes through the same shared graph the + * operator canvas edits, and the form-binding config is local until #8351 shares it. */ @UntilDestroy() @Component({ @@ -105,6 +134,7 @@ interface RenderedField { FormsModule, ReactiveFormsModule, FormlyModule, + DragDropModule, NzAvatarModule, NzIconModule, NzButtonModule, @@ -127,6 +157,14 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { public autoSaveState = ""; /** Write access: only then does a filled-in value write back, and only then does the page save. */ public canEdit = false; + /** Edit mode: a writer authoring the form in place (rename/hide/reorder/expose/remove inputs, + * edit the instruction, pick results). Off, the page is the read-only form a reader sees. */ + public authoring = false; + /** While authoring, the instruction is edited as raw markdown ("write") or shown rendered + * ("preview"); a reader always sees it rendered. */ + public instructionMode: "write" | "preview" = "write"; + /** The author's "which results to show" picker: every candidate step with its chosen flag. */ + public resultChoices: ResultChoice[] = []; /** The exposed inputs, resolved against the live graph, and the formly field built for each. */ private parameters: ResolvedField[] = []; @@ -184,6 +222,8 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { /** Set on teardown so deferred callbacks stop touching a view that is gone. */ private destroyed = false; + /** A rebuild of the inputs that arrived while the reader was typing, held until the typing ends. */ + private rebuildDeferred = false; /** * Operator positions as loaded, kept only as a fallback: a save writes the live positions @@ -298,7 +338,11 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { .getViewResultOperatorsChangedStream() .pipe(untilDestroyed(this)) .subscribe(() => { + // An eye toggled on the canvas changes both what shows (a newly-viewed step) and what the + // author can pick, so rebuild the picker here too -- otherwise a just-eyed step would not + // appear as an option (and an un-eyed one would linger) until the next full re-read. this.refreshShownResults(); + this.rebuildResultChoices(); this.cdr.markForCheck(); }); @@ -389,30 +433,60 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { // Attribute boxes become dropdowns only after compilation writes the column enums into each // operator's dynamic schema -- which lands after these cards were built. Rebuild on the // compilation-state stream, a ReplaySubject(1) so a late subscriber (this page reloads fresh - // on every Canvas<->Form switch) gets the current state at once. Skip it while someone is - // typing, so a rebuild does not throw away a half-entered value under the cursor. + // on every Canvas<->Form switch) gets the current state at once. Held, not dropped, while + // someone is typing (see rebuildFormOrDefer), so it neither throws away a half-entered value + // under the cursor nor goes missing. this.workflowCompilingService .getCompilationStateInfoChangedStream() .pipe(debounceTime(FORM_DEBOUNCE_TIME_MS), untilDestroyed(this)) - .subscribe(() => { - if (this.isTypingInTheForm()) { - return; - } - this.readConfig(); - }); + .subscribe(() => this.rebuildFormOrDefer(false)); // Exposing or un-exposing a property in the panel changes the definition; the inputs above have // to follow at once, which is the whole point of editing them side by side. Today this fires for // this client's own edits; once #8351 moves formBinding into the shared model it also fires for - // a co-editor's -- so, like the compilation path, skip the rebuild while the reader is typing, or - // a remote change would throw away a half-entered value under the cursor. - this.workflowActionService.formBindingChanged$.pipe(untilDestroyed(this)).subscribe(() => { - if (this.isTypingInTheForm()) { - return; - } - this.readConfig(); + // a co-editor's -- so, like the compilation path, the rebuild is held while the reader is typing + // (a remote change would otherwise throw away a half-entered value under the cursor) and runs + // the moment the typing ends. + this.workflowActionService.formBindingChanged$ + .pipe(untilDestroyed(this)) + .subscribe(() => this.rebuildFormOrDefer(true)); + } + + /** + * Rebuild the inputs from the config now or, while the reader is typing, hold the rebuild until + * the focus leaves the text control (onFocusOut). Held rather than dropped: the change that asked + * for it (a property exposed in the panel, a schema compiled) still has to reach the page, only + * not under the cursor. Dropping it left an exposed property's card missing until something else + * happened to rebuild, which read as the tick box doing nothing. + */ + private rebuildFormOrDefer(detect: boolean): void { + if (this.isTypingInTheForm()) { + this.rebuildDeferred = true; + return; + } + this.rebuildDeferred = false; + this.readConfig(); + if (detect) { this.cdr.detectChanges(); - }); + } + } + + /** + * focusout fires before the next element takes the focus, so the held rebuild is decided after + * the current tick: a reader who merely tabbed to another text field keeps it held, anyone else + * gets it now. The hold is re-checked when that tick fires: the very click that took the focus + * can be a control whose own change rebuilds at once (the expose tick box), clearing the hold in + * between, and a stale callback that rebuilt regardless would only rebuild the same cards twice. + */ + @HostListener("focusout") + public onFocusOut(): void { + if (this.rebuildDeferred) { + this.later(() => { + if (this.rebuildDeferred) { + this.rebuildFormOrDefer(true); + } + }, 0); + } } private load(wid: number): void { @@ -462,34 +536,93 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { } /** - * Show the workflow rather than edit it: the graph shape and its properties are read-only - * on this page. A later PR's authoring mode makes properties editable with write access. + * Lock or unlock editing: the graph and its properties are read-only unless a writer is in edit + * mode, which is the only state that unlocks them (see toggleAuthoring). */ private applyEditability(): void { - this.workflowActionService.disableWorkflowModification(); + // Edit mode with write access is the only state that makes the operator properties (and the + // embedded canvas) modifiable here; every other state locks them, so a reader -- or a writer + // just viewing -- cannot change the workflow through this page. + if (this.authoring && this.canEdit) { + this.workflowActionService.enableWorkflowModification(); + } else { + this.workflowActionService.disableWorkflowModification(); + } } // --------------------------------------------------------------------------- // Inputs: the exposed properties, rendered as their operators' own fields // --------------------------------------------------------------------------- - /** Whether the cursor is currently inside one of this page's inputs. */ + /** + * Whether the reader is mid-way through typing somewhere on this page: the caret is in a control + * that holds text (a text-like input, a textarea, a select, a content-editable). A tick box, radio + * or button also takes the focus when clicked but holds no half-entered value, so it is not typing + * -- a tick box (the step panel's expose boxes, once that panel is live for authoring) is precisely + * the click that has to rebuild the cards at once, and counting it as typing held that rebuild back. + */ private isTypingInTheForm(): boolean { const active = document.activeElement as HTMLElement | null; if (!active || !this.host.nativeElement.contains(active)) { return false; } - return ["INPUT", "TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable; + if (active.tagName === "INPUT") { + return !NON_TEXT_INPUT_TYPES.has((active as HTMLInputElement).type); + } + return ["TEXTAREA", "SELECT"].includes(active.tagName) || active.isContentEditable; + } + + private operators(): OperatorPredicate[] { + return this.workflowActionService.getTexeraGraph().getAllOperators(); + } + + /** + * The author's picker for the extra results: every non-terminal step with view-result ("the eye") + * on the canvas is offered, plus any already-chosen step (so a pick never vanishes from its own + * picker). The terminal step is not offered -- its result always shows and is not the author's to + * toggle. The shown flag mirrors the saved resultOperatorIds. + */ + private rebuildResultChoices(): void { + // The picker is only shown while authoring, so a reader does no per-operator work. + if (!this.authoring) { + this.resultChoices = []; + return; + } + const viewed = this.workflowActionService.getTexeraGraph().getOperatorsToViewResult(); + const chosen = new Set(this.formBindingService.getConfig().resultOperatorIds); + // The terminal result always shows and is not the author's to toggle, so it is not offered here. + // The picker curates only the extra intermediate steps -- those given view-result (the eye) on the + // canvas. Reuse the one terminal rule (terminalOperatorIds) rather than a second copy. Already-chosen + // ids stay listed so the author can un-pick them. + const terminals = new Set(this.terminalOperatorIds()); + this.resultChoices = this.operators() + .filter(op => !terminals.has(op.operatorID) && (viewed.has(op.operatorID) || chosen.has(op.operatorID))) + .map(op => ({ + operatorID: op.operatorID, + label: this.formBindingService.operatorLabel(op), + shown: chosen.has(op.operatorID), + })); } + /** + * Re-read the saved form config and rebuild everything derived from it. An input whose operator + * has since been deleted is NOT dropped here: resolveFields marks it broken, a reader never sees + * it (visibleFields), and an author sees it as an empty card with the reason and removes it + * explicitly. Deleting it silently on entering edit mode would be a config write nobody asked for, + * and would leave the author guessing where an input went. + */ private readConfig(): void { const config = this.formBindingService.getConfig(); this.parameters = this.formBindingService.resolveFields(); this.instructionTitle = config.instruction?.title ?? ""; this.instructionBody = config.instruction?.body ?? ""; this.refreshShownResults(); - // A reader always sees the instruction as rendered markdown. - void this.renderInstruction(); + this.rebuildResultChoices(); + // Readers always see the instruction rendered; an author sees it rendered only while previewing + // (otherwise they are editing the raw markdown in the textarea). + if (!this.authoring || this.instructionMode === "preview") { + void this.renderInstruction(); + } this.buildForm(); } @@ -542,6 +675,12 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { private renderField(resolved: ResolvedField): RenderedField | undefined { const { binding } = resolved; + // A broken input (its operator gone) has no schema to build a field from. Only an author ever + // sees it (visibleFields drops it for readers), rendered as an empty card so the author can + // remove it; a reader never reaches here for one. + if (resolved.brokenReason) { + return { resolved, fields: [], form: new FormGroup({}), model: {} }; + } const schema = this.operatorSchemaFor(binding.operatorID); if (!schema) { return undefined; @@ -628,7 +767,7 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { field.props = { ...(field.props ?? {}), disabled: true }; } - this.applyFieldOverrides(field, binding); + this.applyFieldOverrides(field, binding, schemaLabel); return { resolved, fields: [field], form, model }; } @@ -672,11 +811,36 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { * stored per-sub-field overrides (rename, hide), keyed by field path. A repeated section builds * its row template on demand, so its builder is wrapped to decorate every row formly ever makes. */ - private applyFieldOverrides(field: FormlyFieldConfig, binding: FormFieldBinding): void { + private applyFieldOverrides(field: FormlyFieldConfig, binding: FormFieldBinding, schemaLabel: string): void { const walk = (node: FormlyFieldConfig, path: string): void => { // Drop the schema's own description on every field, nested ones included: on this page the // one piece of guidance is the help text the form's author writes, rendered once by the card. node.props = { ...(node.props ?? {}), description: "" }; + // Author mode, the input itself (root path): its name is renamed in place by clicking the + // title, like every nested field. No eye here -- a whole input leaves via Remove, not a hide + // toggle. The editable label becomes the single title, so formly's own label is cleared to + // avoid printing it twice. + if (!path && this.authoring) { + EditableLabelWrapperComponent.decorate( + node, + { authoring: true, name: binding.displayName ?? "", hidden: false, fallback: schemaLabel, canHide: false }, + name => this.onBindingNamed(binding.id, name) + ); + node.props = { ...(node.props ?? {}), label: "" }; + } else if (!path && node.type === "array") { + // Reader mode, a repeated input: the shared array widget prints its label at the BOTTOM, + // beside its add button (the canvas panel's convention), while every other widget and the + // author's editable title sit above. Left alone, the title would jump from above the rows + // in edit mode to below them on Done. Give it the same static title above instead; the + // wrapper blanks the widget's own label. + EditableLabelWrapperComponent.decorate(node, { + authoring: false, + name: binding.displayName ?? "", + hidden: false, + fallback: schemaLabel, + canHide: false, + }); + } // Apply the author's stored overrides so a reader sees each sub-field renamed and hidden as // set up. The root (path "") carries the binding's own displayName, set in renderField. if (path) { @@ -684,7 +848,22 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { if (override.displayName) { node.props = { ...(node.props ?? {}), label: override.displayName }; } - if (override.hidden) { + if (this.authoring) { + // An author edits the sub-field's label where it appears and keeps hidden fields on + // screen (faded, via the wrapper) so they can be brought back, rather than removed from + // the DOM as they are for a reader. + EditableLabelWrapperComponent.decorate( + node, + { + authoring: true, + name: override.displayName ?? "", + hidden: override.hidden === true, + fallback: (node.props?.label as string) || path, + }, + name => this.onSubFieldNamed(binding.id, path, name), + hidden => this.onSubFieldHiddenAt(binding.id, path, hidden) + ); + } else if (override.hidden) { node.hide = true; // Hidden means "not shown", not "cleared". Formly 7's resetFieldOnHide extra defaults to // true, so a field that renders hidden has its value stripped from the model -- and this @@ -764,11 +943,13 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { /** * The inputs a reader is offered. Broken bindings (the operator was deleted, or the property key - * no longer exists) are left out, since filling one in could not affect a run; the author's view - * of them, to repair them, is added by the authoring PR. + * no longer exists) are left out, since filling one in could not affect a run; an author sees them + * (below), to repair or remove them. */ public get visibleFields(): ResolvedField[] { - return this.parameters.filter(field => !field.brokenReason); + // A reader never sees a broken input (its operator is gone, so filling it could not affect the + // run); an author sees it, to repair or remove it. + return this.authoring ? this.parameters : this.parameters.filter(field => !field.brokenReason); } public trackByRendered(_: number, rendered: RenderedField): string { @@ -926,6 +1107,125 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { this.instructionOpen = !this.instructionOpen; } + // --------------------------------------------------------------------------- + // Author mode: editing the form in place (write access only). Graph edits go through the same + // shared graph the operator canvas edits; form-binding edits go through the form-binding config + // (local until #8351 shares it). Each edit then re-reads the config. + // --------------------------------------------------------------------------- + + public toggleAuthoring(): void { + // Entering edit mode needs write access. The Edit button is only rendered for a writer, but the + // guard belongs here, at the method, so no other caller can put a reader into a mode whose every + // action writes the shared config. Leaving edit mode is always allowed. + if (!this.authoring && !this.canEdit) { + return; + } + this.authoring = !this.authoring; + if (this.authoring) { + // An author picks fields off the workflow, so show it. + this.showWorkflow(); + } else { + this.workflowOpen = false; + } + // Edit mode is what makes operator properties (and the embedded canvas) editable here. + this.applyEditability(); + this.readConfig(); + } + + /** + * Only presentation is editable here (the input's shown name and its help text). Which operator + * property an input drives is decided by ticking it in the property panel, so there is nothing to + * type and no way to point an input at a property that does not exist. + */ + public onEditHelpText(resolved: ResolvedField, value: string): void { + // Help text is presentation only and does not change which inputs the form has, so it is NOT + // followed by readConfig: rebuilding the whole form on every keystroke would churn every card + // (heavy file/model widgets included) and jump the cursor. Like the instruction, it is saved to + // the config and reflected on the next full re-read. (A binding's shown name is edited through + // the editable title, not here -- see onBindingNamed.) + this.formBindingService.updateBinding(resolved.binding.id, { helpText: value }); + } + + public onRemoveBinding(resolved: ResolvedField): void { + this.formBindingService.removeBinding(resolved.binding.id); + this.readConfig(); + } + + public onDrop(event: CdkDragDrop): void { + this.moveRenderedCard(event.previousIndex, event.currentIndex); + } + + /** + * Keyboard counterpart of the drag: the Move up / Move down buttons on an author's card step it + * one place. CDK drag-drop offers no keyboard path of its own and the drag handle is decorative, + * so without these a keyboard-only author could not reorder at all. + */ + public onMoveBinding(card: RenderedField, delta: -1 | 1): void { + const at = this.rendered.indexOf(card); + this.moveRenderedCard(at, at + delta); + } + + /** + * Move the card at one rendered position onto another. The positions are indices into `rendered`, + * which can be shorter than the saved fields (a binding whose operator is live but whose schema is + * momentarily unavailable renders no card), so reordering the saved fields by those raw indices + * could move the wrong one. Translate both ends to the saved field they name, by binding id, and + * reorder those. A position off either end, or a card the config no longer holds, moves nothing. + */ + private moveRenderedCard(fromIndex: number, toIndex: number): void { + const fields = this.formBindingService.getConfig().fields; + const movedId = this.rendered[fromIndex]?.resolved.binding.id; + const targetId = this.rendered[toIndex]?.resolved.binding.id; + const from = fields.findIndex(f => f.id === movedId); + const to = fields.findIndex(f => f.id === targetId); + if (from === -1 || to === -1) { + return; + } + this.formBindingService.reorder(from, to); + this.readConfig(); + } + + public onInstructionChange(): void { + this.formBindingService.updateConfig({ + instruction: { title: this.instructionTitle, body: this.instructionBody }, + }); + } + + public setInstructionMode(mode: "write" | "preview"): void { + this.instructionMode = mode; + if (mode === "preview") { + void this.renderInstruction(); + } + } + + public onToggleResult(choice: ResultChoice): void { + this.formBindingService.toggleResultOperator(choice.operatorID); + this.readConfig(); + } + + private showWorkflow(): void { + if (!this.workflowOpen) { + this.workflowOpen = true; + this.openWorkflowStrip(); + } + } + + /** Renaming the input itself, from its own title. */ + private onBindingNamed(bindingId: string, value: string): void { + this.formBindingService.updateBinding(bindingId, { displayName: value }); + this.readConfig(); + } + + private onSubFieldNamed(bindingId: string, path: string, value: string): void { + this.formBindingService.setFieldOverride(bindingId, path, { displayName: value }); + this.readConfig(); + } + + private onSubFieldHiddenAt(bindingId: string, path: string, hidden: boolean): void { + this.formBindingService.setFieldOverride(bindingId, path, { hidden }); + this.readConfig(); + } + // --------------------------------------------------------------------------- // Running the same workflow the canvas runs, through the same execute/kill service. The canvas // wraps its run with completion-email options (executeWorkflowWithEmailNotification); this page @@ -1221,11 +1521,24 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { * of yourself, broken runs. A fresh document is the reliable handover. */ public openRegularCanvas(): void { - this.save(); - /* v8 ignore start -- full-document navigation; jsdom cannot navigate */ + // Save first and hand over only once the save has completed: the full-page load unloads this + // document, and a request still in flight at that moment is aborted, so navigating right after + // firing the save could lose the very edit the switch is meant to carry across. A save that + // fails keeps the author here with the error shown, rather than leaving with changes that were + // never stored. A reader, who has nothing to save, goes straight over. + this.save(() => this.openCanvasPage()); + } + + /** + * The full-page handover to the operator canvas, apart from the save so the order is testable. + * Excluded from coverage as a whole: jsdom cannot navigate, so the specs stub this method and + * assert when it is called rather than what it does. + */ + /* v8 ignore start */ + private openCanvasPage(): void { window.location.href = `${USER_WORKSPACE}/${this.wid}`; - /* v8 ignore stop */ } + /* v8 ignore stop */ /** * Save the same way the operator canvas does. Both views edit one workflow, so the @@ -1244,18 +1557,23 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { * workflow when the payload has no id, so saving whatever the graph holds would spawn * stray "Untitled workflow" rows when the page is left before its workflow loaded. */ - private save(): void { + private save(afterwards?: () => void): void { // A read-only viewer can open and run the form (execution is gated on computing-unit access, // not workflow access) but must never persist: every such save is a guaranteed 403 that would // spam "Could not save" on each debounce. Their inputs are non-editable, so nothing is lost. + // `afterwards` runs once the save has completed, or at once when there is nothing to save; + // it does not run when the save fails, so a caller that navigates on it stays put instead. if (!this.canEdit) { + afterwards?.(); return; } if (!this.userService.isLogin() || !this.workflowPersistService.isWorkflowPersistEnabled()) { + afterwards?.(); return; } const workflow = this.workflowActionService.getWorkflow(); if (workflow.wid === undefined || workflow.wid !== this.wid) { + afterwards?.(); return; } const preserved: Workflow = { @@ -1277,6 +1595,7 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { // A save that fails silently is the worst thing this page can do: the author walks // away believing the form they just built is stored. error: () => this.notificationService.error("Could not save. Your latest changes are not stored yet."), + complete: () => afterwards?.(), }); } 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 4dfa516d6f4..9639b325624 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 @@ -37,6 +37,7 @@ import { LockOutline, MinusOutline, PlusOutline, + UpOutline, } from "@ant-design/icons-angular/icons"; import { EMPTY, of, Subject } from "rxjs"; @@ -272,6 +273,7 @@ describe("WorkflowFormComponent (rendered template)", () => { LockOutline, MinusOutline, PlusOutline, + UpOutline, ], }, DatePipe, @@ -405,8 +407,59 @@ describe("WorkflowFormComponent (rendered template)", () => { expect(el(".instr .instr-bar h2")?.textContent?.trim()).toBe("How to use this"); expect(el(".instr .md")?.innerHTML).toContain("Fill in the inputs."); - (el(".instr-bar") as HTMLButtonElement).click(); + // Reading, the whole header row is one toggle button, wired to the body it opens. + const toggle = el(".instr-toggle") as HTMLButtonElement; + expect(toggle.getAttribute("aria-controls")).toBe("instr-body"); + expect(el("#instr-body")).not.toBeNull(); + toggle.click(); expect(fixture.componentInstance.instructionOpen).toBe(false); + fixture.detectChanges(); + expect(toggle.getAttribute("aria-expanded")).toBe("false"); + }); + + it("keeps the author's title input outside the toggle button", async () => { + fixture.detectChanges(); + finishLoad(); + const c = fixture.componentInstance; + c.canEdit = true; + c.authoring = true; + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + + // An input nested in a button is invalid interactive content; the author's header is a row with + // the input as a sibling of a chevron button that does the toggling. + const input = el(".instr-title-input") as HTMLInputElement; + expect(input).not.toBeNull(); + expect(input.closest("button")).toBeNull(); + const chevron = el(".instr-chev") as HTMLButtonElement; + expect(chevron.getAttribute("aria-controls")).toBe("instr-body"); + expect(chevron.getAttribute("aria-expanded")).toBe("true"); + chevron.click(); + expect(c.instructionOpen).toBe(false); + }); + + it("offers Move up / Move down on an author's cards, disabled at the ends", () => { + fixture.detectChanges(); + finishLoad(); + const c = fixture.componentInstance; + c.canEdit = true; + c.authoring = true; + (c as any).parameters = [{ binding: { id: "b1" } }, { binding: { id: "b2" } }]; + c.rendered = [ + { resolved: { binding: { id: "b1" }, operatorLabel: "Scan" }, fields: [], form: new FormGroup({}), model: {} }, + { resolved: { binding: { id: "b2" }, operatorLabel: "Filter" }, fields: [], form: new FormGroup({}), model: {} }, + ] as any; + const move = vi.spyOn(c, "onMoveBinding"); + fixture.detectChanges(); + + const buttons = Array.from(fixture.nativeElement.querySelectorAll(".param .move")) as HTMLButtonElement[]; + expect(buttons.map(b => b.getAttribute("aria-label"))).toEqual(["Move up", "Move down", "Move up", "Move down"]); + // First card cannot move up, last card cannot move down; the other two are live. + expect(buttons.map(b => b.disabled)).toEqual([true, false, false, true]); + + buttons[1].click(); + expect(move).toHaveBeenCalledWith(c.rendered[0], 1); }); it("renders the run bar with the run button and the computing-unit selector", () => { @@ -518,6 +571,29 @@ describe("WorkflowFormComponent (rendered template)", () => { expect(el(".panel")!.getAttribute("aria-label")).toBe("Step settings, read-only"); }); + it("turns that same panel live in edit mode: writes on, tick boxes on, inert off", () => { + fixture.detectChanges(); + finishLoad(); + const c = fixture.componentInstance; + c.selectedOperatorId = "op-1"; + c.authoring = true; + fixture.detectChanges(); + + const panel = fixture.debugElement.query(By.directive(MockPropertyEditorComponent)) + .componentInstance as MockPropertyEditorComponent; + // Authoring is a real edit of the shared graph, the same edit the canvas makes, so the panel + // acts as an editor; and its tick boxes are how the author picks what the form exposes. + expect(panel.actsAsEditor).toBe(true); + expect(panel.exposeChoosing).toBe(true); + // Still not the docked canvas panel, so it still makes no claim on that panel's geometry. + expect(panel.persistPlacement).toBe(false); + expect(el("texera-property-editor")!.hasAttribute("inert")).toBe(false); + // The container's tab stop existed only because inert content cannot hold focus. With the form + // focusable again it would just sit in front of it, so it goes away with inert. + expect(el(".panel")!.hasAttribute("tabindex")).toBe(false); + expect(el(".panel")!.getAttribute("aria-label")).toBe("Step settings"); + }); + it("tears the workflow down when the browser unloads (the beforeunload host binding)", () => { fixture.detectChanges(); finishLoad(); @@ -527,4 +603,21 @@ describe("WorkflowFormComponent (rendered template)", () => { expect(workflowActionService.clearWorkflow).toHaveBeenCalled(); }); + + // The held rebuild is drained by a real blur: a focusout bubbling up from a control inside the + // page reaches the host listener. Dispatching the DOM event (not calling the handler) is what + // would catch the listener being removed or miswired. + it("runs a held rebuild when a control inside the page loses focus (the focusout host binding)", async () => { + fixture.detectChanges(); + finishLoad(); + const c = fixture.componentInstance; + const rebuild = vi.spyOn(c as any, "readConfig"); + (c as any).rebuildDeferred = true; + + el("input.wf-name")!.dispatchEvent(new FocusEvent("focusout", { bubbles: true })); + await new Promise(r => setTimeout(r, 10)); + + expect(rebuild).toHaveBeenCalledTimes(1); + expect((c as any).rebuildDeferred).toBe(false); + }); }); 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 7acede23180..19e7640ca5c 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 @@ -164,6 +164,15 @@ export function setupHarness() { writeValue: vi.fn(), // A result card's friendly label; the mock returns the operator's display name or its id. operatorLabel: (op: any) => op?.customDisplayName ?? op?.operatorType ?? op?.operatorID, + // Author-mode writes: the component calls these then re-reads config. Spied so a test can + // assert the edit was made without needing a real binding store. + updateBinding: vi.fn(), + setFieldOverride: vi.fn(), + removeBinding: vi.fn(), + reorder: vi.fn(), + toggleResultOperator: vi.fn(), + updateConfig: vi.fn(), + setFields: vi.fn(), }; // A field per property the tests expose. Real formly json-schema conversion is exercised by the // property panel's own spec; here a deterministic map keeps these tests about the component's diff --git a/frontend/src/app/workspace/service/form-binding/form-binding.service.ts b/frontend/src/app/workspace/service/form-binding/form-binding.service.ts index 8990c62a95f..3d182eb2ade 100644 --- a/frontend/src/app/workspace/service/form-binding/form-binding.service.ts +++ b/frontend/src/app/workspace/service/form-binding/form-binding.service.ts @@ -179,10 +179,10 @@ export class FormBindingService { } /** - * Choose whether an operator's output is shown on the form after a run. This records the - * form's own selection only and never changes the canvas's view-result flags: the form - * offers exactly the operators the workflow already views, so their results are already - * materialised and nothing here needs to touch the graph. Read-only, one direction. + * Choose whether an operator's output is featured on the form after a run, on top of the final + * step's result, which always shows. This records the form's own selection only and never changes + * the canvas's view-result flags: the picker offers view-result operators, whose results are + * already materialised, so nothing here needs to touch the graph. Read-only, one direction. */ public toggleResultOperator(operatorID: string): void { const shown = this.getConfig().resultOperatorIds; From 01d0e2d351fccf955ebb39d5597559dcbefd1975 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Thu, 10 Sep 2026 20:18:16 -0700 Subject: [PATCH 2/2] test(gui): stub the export download through FileSaverService The workspace menu's export test module-mocked the CommonJS file-saver package (vi.mock("file-saver")). Under the Angular unit-test builder that call ends up inside the built spec chunk rather than at its top level (Vitest warns about it on every platform), and once this stack shifted the shared chunk graph the mock stopped applying on the Windows leg: the real saveAs ran and the spy saw 0 calls (7 of the last 8 Windows runs on #8455 and #8456, while main and #8499 pass). Route the download through the existing injectable FileSaverService, as the dashboard downloads already do, and stub that with TestBed, which does not depend on mock hoisting. No behaviour change. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY --- .../workspace/component/menu/menu.component.spec.ts | 10 +++++----- .../src/app/workspace/component/menu/menu.component.ts | 10 +++++++--- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/frontend/src/app/workspace/component/menu/menu.component.spec.ts b/frontend/src/app/workspace/component/menu/menu.component.spec.ts index c726bfec659..da69bf3adf0 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts @@ -45,7 +45,7 @@ import { ExecutionState } from "../../types/execute-workflow.interface"; import { HeatmapView } from "../../service/heatmap/heatmap-scoring"; import { ComputingUnitState } from "../../../common/type/computing-unit-connection.interface"; import { mockPoint, mockScanPredicate } from "../../service/workflow-graph/model/mock-workflow-data"; -import { saveAs } from "file-saver"; +import { FileSaverService } from "../../../dashboard/service/user/file/file-saver.service"; import type { ModalOptions } from "ng-zorro-antd/modal"; import type { ComputingUnitSelectionComponent } from "../power-button/computing-unit-selection.component"; import { WorkflowContent } from "../../../common/type/workflow"; @@ -57,8 +57,6 @@ import { MockGuiConfigService } from "../../../common/service/gui-config.service import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; import type { Mocked } from "vitest"; -vi.mock("file-saver", () => ({ saveAs: vi.fn() })); - describe("MenuComponent", () => { let component: MenuComponent; let fixture: ComponentFixture; @@ -112,7 +110,6 @@ describe("MenuComponent", () => { fixture = TestBed.createComponent(MenuComponent); component = fixture.componentInstance; fixture.detectChanges(); - vi.mocked(saveAs).mockClear(); }); it("should create", () => { @@ -525,6 +522,9 @@ describe("MenuComponent", () => { describe("onClickExportWorkflow (save)", () => { it("serializes the workflow content as JSON and downloads it under the workflow name", () => { + // Stubbed on the injected wrapper rather than by module-mocking file-saver: that CommonJS + // mock is order-sensitive under the unit-test builder and was failing on the Windows leg. + const saveAs = vi.spyOn(TestBed.inject(FileSaverService), "saveAs").mockImplementation(() => {}); const fakeContent = { operators: [{ operatorID: "op1" }], links: [], @@ -537,7 +537,7 @@ describe("MenuComponent", () => { component.onClickExportWorkflow(); expect(saveAs).toHaveBeenCalledTimes(1); - const [blobArg, fileNameArg] = vi.mocked(saveAs).mock.calls[0] as [Blob, string]; + const [blobArg, fileNameArg] = saveAs.mock.calls[0] as [Blob, string]; expect(fileNameArg).toBe("my-workflow.json"); expect(blobArg).toBeInstanceOf(Blob); expect(blobArg.type).toBe("text/plain;charset=utf-8"); diff --git a/frontend/src/app/workspace/component/menu/menu.component.ts b/frontend/src/app/workspace/component/menu/menu.component.ts index b5b209b3eb0..c3ad4aa979e 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.ts @@ -35,7 +35,7 @@ import { catchError, debounceTime, switchMap, tap } from "rxjs/operators"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; import { WorkflowUtilService } from "../../service/workflow-graph/util/workflow-util.service"; import { WorkflowVersionService } from "../../../dashboard/service/user/workflow-version/workflow-version.service"; -import { saveAs } from "file-saver"; +import { FileSaverService } from "../../../dashboard/service/user/file/file-saver.service"; import { NotificationService } from "src/app/common/service/notification/notification.service"; import { OperatorMenuService } from "../../service/operator-menu/operator-menu.service"; import { CoeditorPresenceService } from "../../service/workflow-graph/model/coeditor-presence.service"; @@ -188,7 +188,8 @@ export class MenuComponent implements OnInit, OnDestroy { private computingUnitStatusService: ComputingUnitStatusService, protected config: GuiConfigService, private router: Router, - private jupyterPanelService: JupyterPanelService + private jupyterPanelService: JupyterPanelService, + private fileSaverService: FileSaverService ) { workflowWebsocketService .subscribeToEvent("ExecutionDurationUpdateEvent") @@ -619,7 +620,10 @@ export class MenuComponent implements OnInit, OnDestroy { const workflowContent: WorkflowContent = this.workflowActionService.getWorkflowContent(); const workflowContentJson = JSON.stringify(workflowContent, null, 2); const fileName = this.currentWorkflowName + ".json"; - saveAs(new Blob([workflowContentJson], { type: "text/plain;charset=utf-8" }), fileName); + // Through the injectable wrapper (as the dashboard downloads already do), so a spec stubs it + // with TestBed instead of module-mocking the CommonJS file-saver package, which the unit-test + // builder cannot hoist reliably. + this.fileSaverService.saveAs(new Blob([workflowContentJson], { type: "text/plain;charset=utf-8" }), fileName); } /**