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 99a30acdd18..9234d3338c5 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 @@ -17,7 +17,7 @@ * under the License. */ -import { AfterViewInit, ChangeDetectorRef, Component, ElementRef, OnDestroy, OnInit } from "@angular/core"; +import { AfterViewInit, ChangeDetectorRef, Component, ElementRef, Input, OnDestroy, OnInit } from "@angular/core"; import { combineLatest, fromEvent, merge, Subject } from "rxjs"; import { NzModalCommentBoxComponent } from "./comment-box-modal/nz-modal-comment-box.component"; import { NzModalRef, NzModalService } from "ng-zorro-antd/modal"; @@ -118,7 +118,26 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy metricLabel: string; heatLabel: string; } | null = null; - private interactive: boolean = true; + private paperInteractive: boolean = true; + // Keeps the paper sized to its OWN container (not just the window) and rebuilds cell geometry + // when the container goes 0 -> real size. Needed by embedded previews like the Form View strip, + // which toggles this editor's container via display:none. + private paperResizeObserver?: ResizeObserver; + + /** + * 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. + */ + @Input() structureLocked = false; + + private get interactive(): boolean { + return this.paperInteractive && !this.structureLocked; + } private _onProcessKeyboardActionObservable: Subject = new Subject(); private wrapper; private currentOpenedOperatorID: string | null = null; @@ -196,6 +215,7 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy this.handlePaperRestoreDefaultOffset(); this.handlePaperZoom(); this.handleWindowResize(); + this.handleContainerResize(); this.handleViewDeleteOperator(); if (this.workflowActionService.getHighlightingEnabled()) { this.handleCellHighlight(); @@ -235,6 +255,7 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy } ngOnDestroy(): void { + this.paperResizeObserver?.disconnect(); document.removeEventListener("keydown", this._handleKeyboardAction.bind(this)); // The overlay belongs to the canvas being viewed, but the wrapper holding // the view is root-provided and outlives this component, while the menu's @@ -288,7 +309,7 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy // marks all the available magnets or elements when a link is dragged markAvailable: true, // disable jointjs default action of adding vertexes to the link - interactive: defaultInteractiveOption, + interactive: this.interactive ? defaultInteractiveOption : disableInteractiveOption, // set a default link element used by jointjs when user creates a link on UI defaultLink: JointUIService.getDefaultLinkCell(), // disable jointjs default action that stops propagate click events on jointjs paper @@ -316,13 +337,8 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy .getWorkflowModificationEnabledStream() .pipe(untilDestroyed(this)) .subscribe(enabled => { - if (enabled) { - this.interactive = true; - this.paper.setInteractivity(defaultInteractiveOption); - } else { - this.interactive = false; - this.paper.setInteractivity(disableInteractiveOption); - } + this.paperInteractive = enabled; + this.paper.setInteractivity(this.interactive ? defaultInteractiveOption : disableInteractiveOption); this.changeDetectorRef.detectChanges(); }); } @@ -733,6 +749,48 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy .subscribe(() => this.paper.setDimensions(this.editorWrapper.offsetWidth, this.editorWrapper.offsetHeight)); } + /** + * The window:resize handler reacts only to the window. When this editor is embedded in a + * self-resizing container -- e.g. the Form View's "Workflow" strip toggled via display:none -- + * the window never fires, so the paper is mis-sized and cells that repainted while 0-sized cache + * their port anchors at the origin (links tangle across boxes). A ResizeObserver on the editor's + * own container fixes both when the real size lands, instead of guessing with timers. + */ + /* v8 ignore start -- ResizeObserver + JointJS paper geometry; jsdom has no layout to observe or measure */ + private handleContainerResize(): void { + // Only an embedded, structure-locked preview needs this: its container is toggled by a parent + // (display:none) without any window resize. The operator canvas keeps its window:resize + // handling untouched -- observing its container would rebuild cell geometry on every panel drag. + if (!this.structureLocked) { + return; + } + this.paperResizeObserver = new ResizeObserver(() => this.resizePaperToContainer()); + this.paperResizeObserver.observe(this.editorWrapper); + } + + private resizePaperToContainer(): void { + if (!this.paper) { + return; + } + const width = this.editorWrapper.offsetWidth; + const height = this.editorWrapper.offsetHeight; + // Collapsed/hidden container: do NOT measure or rebuild against zero -- that is exactly what + // caches the broken geometry in the first place. + if (width === 0 || height === 0) { + return; + } + this.paper.setDimensions(width, height); + // Rebuild cell geometry against the now-real DOM. Re-rendering each element recomputes its + // port anchors; re-routing each link then draws it between the real ports. A plain + // setDimensions (or a viewport zoom-to-fit) cannot undo anchors already cached at 0,0. + this.paper.model.getElements().forEach(element => this.paper.findViewByModel(element)?.update()); + this.paper.model.getLinks().forEach(link => { + const linkView = this.paper.findViewByModel(link) as joint.dia.LinkView | undefined; + linkView?.requestConnectionUpdate(); + }); + } + /* v8 ignore stop */ + private handleCellHighlight(): void { this.handleHighlightMouseDBClickInput(); this.handleHighlightMouseInput(); @@ -1207,8 +1265,44 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy .getOperatorValidationStream() .pipe(untilDestroyed(this)) .subscribe(value => this.applyOperatorBorder(value.operatorID, value.validation)); + + // Operators already in the graph when this editor mounts produced no add event and won't hit + // the validation stream until they change, so nothing corrects the border they were drawn + // with. The operator canvas mounts before the workflow loads and hears every event, so it + // already looks right; the Form View builds its preview only once opened, leaving operators in + // unvalidated red with a completed run's colours missing. Repaint only there -- a view that + // locks the structure is exactly the one that mounts its editor late. + /* v8 ignore start -- JointJS repaint on a paper that only exists once rendered */ + if (this.structureLocked) { + this.paintCurrentOperatorState(); + } + /* v8 ignore stop */ } + /* v8 ignore start -- JointJS paper repaint; needs a rendered paper jsdom cannot provide */ + private paintCurrentOperatorState(): void { + this.workflowActionService + .getTexeraGraph() + .getAllOperators() + .forEach(operator => { + const statistics = this.workflowStatusService.getCurrentStatus()[operator.operatorID]; + if (statistics) { + this.jointUIService.changeOperatorStatistics( + this.paper, + operator.operatorID, + statistics, + this.isSource(operator.operatorID), + this.isSink(operator.operatorID) + ); + } + this.applyOperatorBorder( + operator.operatorID, + this.validationWorkflowService.validateOperator(operator.operatorID) + ); + }); + } + /* v8 ignore stop */ + /** * This function is provided to JointJS to disable some invalid connections on the UI. * If the connection is invalid, users are not able to connect the links on the UI. @@ -1596,6 +1690,14 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy * Handles mouse events to enable shared cursor. */ private handlePointerEvents(): void { + // A shared cursor is for co-editing the graph; a read-only view shouldn't broadcast one. The + // Form View otherwise left a stranded dot with the reader's name on the operator canvas, its + // last mouse position lingering in awareness state after the page was gone. + /* v8 ignore start -- read-only preview guard; the locked view only mounts once rendered */ + if (this.structureLocked) { + return; + } + /* v8 ignore stop */ fromEvent(this.editor, "mousemove") .pipe(untilDestroyed(this)) .subscribe(e => { 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 a1402b04f26..523e32b5d9d 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 @@ -69,7 +69,39 @@ Loading… - -
+
+ +
+ + +
+ + +
+
+
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 c1662549750..6110a0f2f76 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 @@ -22,6 +22,7 @@ $blue: #1890ff; $text: rgba(0, 0, 0, 0.85); $text-2: rgba(0, 0, 0, 0.45); $divider: #f0f0f0; +$border: #d9d9d9; $shell: #fafafa; :host { @@ -181,3 +182,95 @@ $shell: #fafafa; color: $text-2; padding: 40px 0; } + +/* A section of the page: the workflow preview here, the inputs and results in later PRs. */ +.card { + border: 1px solid $border; + border-radius: 8px; + background: #fff; +} + +/* ---------- workflow preview ---------- */ + +.wf { + margin-top: 28px; + + // A native button so it is keyboard-operable and announces its expanded state, reset to + // read as the plain card header it looks like. + .wf-bar { + display: flex; + align-items: center; + gap: 12px; + width: 100%; + padding: 13px 16px; + cursor: pointer; + user-select: none; + appearance: none; + border: 0; + background: none; + color: inherit; + font: inherit; + text-align: left; + + .wf-title { + display: block; + font-size: 15px; + font-weight: 600; + } + + .wf-sub { + display: block; + margin-top: 2px; + font-size: 13px; + color: $text-2; + } + + .chev { + color: $text-2; + transform: rotate(-90deg); + transition: transform 0.2s; + } + + .spacer { + flex: 1; + } + + &:focus-visible { + outline: 2px solid $blue; + outline-offset: -2px; + } + } + + &.open .wf-bar .chev { + transform: rotate(0deg); + } + + .wf-body { + position: relative; + border-top: 1px solid $divider; + height: 380px; + resize: vertical; + overflow: hidden; + background: #f7f8f9; + + texera-workflow-editor { + display: block; + height: 100%; + width: 100%; + } + + // The canvas mini-map, parked in the corner for navigating a large graph. Reused as-is at + // its natural 300x200 (as the workspace/Hub do) -- no size override, so the whole graph + // shows instead of being clipped behind the toolbar. No overflow:hidden either: the inner + // #mini-map-container already clips the paper, and clipping here would hide the collapsed + // mini-map's own re-open button, so it could never be brought back. + .box { + position: absolute; + right: 10px; + bottom: 10px; + border: 1px solid $divider; + border-radius: 6px; + background: #fff; + } + } +} 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 ebd55cf3f02..f081b4b9464 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 @@ -341,4 +341,65 @@ describe("WorkflowFormComponent", () => { vi.useRealTimers(); }); }); + + // JointJS measures the paper once, when the editor is created. Creating it in the same pass + // that uncollapses the strip races the browser's layout, and losing that race draws links up + // and over the boxes -- so the strip opens first, and the canvas is built a frame later. + describe("the workflow preview", () => { + const frame = () => new Promise(r => requestAnimationFrame(() => r(null))); + + it("opens the strip but does not build the canvas in the same pass", () => { + build(formViewWorkflow).ngOnInit(); + + component.toggleWorkflow(); + + expect(component.workflowOpen).toBe(true); + expect(component.workflowEverOpened).toBe(false); + }); + + it("builds the canvas a frame after the strip opens, then centres it", async () => { + build(formViewWorkflow).ngOnInit(); + + component.toggleWorkflow(); + await frame(); + expect(component.workflowEverOpened).toBe(true); + + await frame(); + expect(h.triggerCenterEvent).toHaveBeenCalled(); + }); + + it("closes the strip again without rebuilding the canvas", () => { + build(formViewWorkflow).ngOnInit(); + component.toggleWorkflow(); + + component.toggleWorkflow(); + + expect(component.workflowOpen).toBe(false); + }); + + // Opening then immediately collapsing must not build the children into a hidden (0-sized) + // strip -- the mini-map has no resize observer and would be stuck blank on the next open. + it("does not build the canvas if the strip is collapsed again before the frame", async () => { + build(formViewWorkflow).ngOnInit(); + + component.toggleWorkflow(); // open -> schedules the deferred build + component.toggleWorkflow(); // collapse again in the same tick, before the frame + await frame(); + + expect(component.workflowEverOpened).toBe(false); + }); + + // Leaving for the dashboard is an ordinary in-app navigation, so a reader can walk out in the + // frame between opening the strip and the canvas being built; that deferred build must not run + // on a page that is gone (detectChanges would throw on a destroyed view). + it("does not build the canvas for a page that has been left", async () => { + build(formViewWorkflow).ngOnInit(); + + component.toggleWorkflow(); + component.ngOnDestroy(); + await frame(); + + expect(component.workflowEverOpened).toBe(false); + }); + }); }); 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 c1db653f032..d438572133b 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 @@ -23,6 +23,7 @@ import { FormsModule } from "@angular/forms"; import { ActivatedRoute, Router } from "@angular/router"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; import { NzAvatarModule } from "ng-zorro-antd/avatar"; +import { NzIconModule } from "ng-zorro-antd/icon"; import { UserIconComponent } from "../../../dashboard/component/user/user-icon/user-icon.component"; import { forkJoin } from "rxjs"; import { debounceTime } from "rxjs/operators"; @@ -40,23 +41,34 @@ import { GuiConfigService } from "../../../common/service/gui-config.service"; import { WorkflowConsoleService } from "../../service/workflow-console/workflow-console.service"; import { WorkflowResultService } from "../../service/workflow-result/workflow-result.service"; import { Point } from "../../types/workflow-common.interface"; +import { WorkflowEditorComponent } from "../workflow-editor/workflow-editor.component"; +import { MiniMapComponent } from "../workflow-editor/mini-map/mini-map.component"; import { CoeditorUserIconComponent } from "../menu/coeditor-user-icon/coeditor-user-icon.component"; import { CoeditorPresenceService } from "../../service/workflow-graph/model/coeditor-presence.service"; import { SAVE_DEBOUNCE_TIME_IN_MS } from "../workspace.component"; /** - * The Form View: a second way to use a workflow. Building on the page shell, this PR adds the - * title bar -- the workflow name (renamable, exactly as on the operator canvas), its "Saved - * at ..." state, and the debounced save that both views share so an edit in one view is not - * lost in the other. The read-only preview, the inputs, running and results are added by later - * PRs. A view, not a new object: it opens the same workflow the canvas does. + * The Form View: a second way to use a workflow. On top of the title-bar frame, this PR adds + * the collapsible read-only workflow preview -- the same workflow editor and mini-map the canvas + * uses, embedded here with the graph shape locked (its own `structureLocked`), built the first + * time the reader opens the strip. The inputs, running and results are added by later PRs. A + * view, not a new object: it opens the same workflow the canvas does. */ @UntilDestroy() @Component({ selector: "texera-workflow-form", templateUrl: "./workflow-form.component.html", styleUrls: ["./workflow-form.component.scss"], - imports: [CommonModule, FormsModule, NzAvatarModule, UserIconComponent, CoeditorUserIconComponent], + imports: [ + CommonModule, + FormsModule, + NzAvatarModule, + NzIconModule, + UserIconComponent, + WorkflowEditorComponent, + MiniMapComponent, + CoeditorUserIconComponent, + ], }) export class WorkflowFormComponent implements OnInit, OnDestroy { public wid?: number; @@ -65,6 +77,11 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { /** "Saved at …", worded and formatted exactly as on the operator canvas. */ public autoSaveState = ""; + /** The collapsible workflow preview: closed until the reader opens it. */ + public workflowOpen = false; + /** The embedded canvas is built the first time the strip opens, never while collapsed. */ + public workflowEverOpened = false; + /** Set on teardown so deferred callbacks stop touching a view that is gone. */ private destroyed = false; @@ -158,6 +175,40 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { this.workflowActionService.disableWorkflowModification(); } + /** Open or close the workflow preview; opening it builds the canvas the first time. */ + public toggleWorkflow(): void { + this.workflowOpen = !this.workflowOpen; + if (this.workflowOpen) { + this.openWorkflowStrip(); + } + } + + /** + * Reveal the strip, then build the canvas a frame later (so JointJS measures the strip's + * real size, not a zero-sized frame that misroutes links), then centre the graph a frame + * after that so the fit runs against a canvas that exists. The editor keeps its own paper + * sized via its container ResizeObserver, so nothing more is needed here. + * + * Each deferred step rechecks `workflowOpen`: a reader who opens then immediately collapses + * the strip must not have the children mounted into a now-hidden (0-sized) body -- the + * embedded mini-map is a fixed-size widget with no resize observer, so mounting it collapsed + * would leave it blank on the next open. + */ + private openWorkflowStrip(): void { + this.later(() => { + if (!this.workflowOpen) { + return; + } + this.workflowEverOpened = true; + this.cdr.detectChanges(); + this.later(() => { + if (this.workflowOpen) { + this.workflowActionService.getTexeraGraph().triggerCenterEvent(); + } + }); + }); + } + /** * Size the name field to its text, the way the operator canvas does, so what follows * it starts at the same place in both views instead of after a fixed-width box. @@ -298,16 +349,21 @@ export class WorkflowFormComponent implements OnInit, OnDestroy { } /** - * Run after a short delay, unless the page is gone by then: the callback touches the view, - * and detectChanges on a destroyed view throws -- reachable by navigating away while the - * name field is still waiting to be measured. + * Run after the current frame (or a delay), unless the page is gone by then: these callbacks + * touch the view, and detectChanges on a destroyed view throws -- reachable by navigating away + * while the name field is waiting to be measured, or the preview canvas to be built. */ - private later(fn: () => void, delayMs = 0): void { - setTimeout(() => { + private later(fn: () => void, delayMs?: number): void { + const run = () => { if (!this.destroyed) { fn(); } - }, delayMs); + }; + if (delayMs === undefined) { + requestAnimationFrame(run); + } else { + setTimeout(run, delayMs); + } } /** 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 af7ba64f078..fc50486634a 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 @@ -50,11 +50,13 @@ describe("WorkflowFormComponent (rendered template)", () => { const configure = async () => { workflow$ = new Subject(); - // Blank out ONLY the two child icons: their ng-zorro dropdown/menu needs a host context - // this page does not set up. The override is on the children, not the page, so the page's - // own .component.html renders as shipped and stays covered -- which is the point of this - // spec, and why the no-restricted-syntax guard (aimed at blanking the component under test) - // does not apply here. + // Blank out ONLY the two child icons: their ng-zorro dropdown/menu needs a host context this + // page does not set up. The override is on the children, not the page, so the page's own + // .component.html renders as shipped and stays covered -- which is the point of this spec, and + // why the no-restricted-syntax guard (aimed at blanking the component under test) does not + // apply here. The embedded workflow editor / mini-map are never instantiated (they sit behind + // *ngIf="workflowEverOpened", and these tests never open the strip -- a real JointJS paper + // needs layout jsdom lacks), so they need no override. /* eslint-disable no-restricted-syntax */ TestBed.overrideComponent(UserIconComponent, { set: { template: "" } }); TestBed.overrideComponent(CoeditorUserIconComponent, { set: { template: "" } }); @@ -162,6 +164,17 @@ describe("WorkflowFormComponent (rendered template)", () => { expect(el(".pc-loading")).toBeNull(); }); + it("toggles the workflow preview when its bar is clicked", () => { + fixture.detectChanges(); + finishLoad(); + // Spied so the click only exercises the template binding, without building the JointJS canvas. + const spy = vi.spyOn(fixture.componentInstance, "toggleWorkflow").mockImplementation(() => {}); + + el(".wf-bar")!.click(); + + expect(spy).toHaveBeenCalled(); + }); + it("tears the workflow down when the browser unloads (the beforeunload host binding)", () => { fixture.detectChanges(); finishLoad(); diff --git a/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts b/frontend/src/app/workspace/component/workflow-form/workflow-form.spec-harness.ts index 2fc33a6a192..ed0a1c977b7 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 @@ -36,6 +36,8 @@ export function setupHarness() { const router = { navigate: vi.fn() }; const workflowChangedStream = new Subject(); const workflowMetaDataChangedStream = new Subject(); + // The preview centres the embedded graph once it is built; tests assert this fired. + const triggerCenterEvent = vi.fn(); const workflowActionService = { resetAsNewWorkflow: vi.fn(), @@ -50,6 +52,7 @@ export function setupHarness() { getWorkflowMetadata: () => ({ name: "scGPT", lastModifiedTime: 1767225600000 }), setWorkflowName: vi.fn(), setWorkflowMetadata: vi.fn(), + getTexeraGraph: () => ({ triggerCenterEvent }), }; const workflowPersistService = { retrieveWorkflow: vi.fn().mockReturnValue(of(formViewWorkflow)), @@ -101,5 +104,6 @@ export function setupHarness() { config, workflowChangedStream, workflowMetaDataChangedStream, + triggerCenterEvent, }; }