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/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); } /** 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;