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 76de1c1e1b2..14491742159 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts @@ -526,4 +526,87 @@ describe("MenuComponent", () => { expect(config.nzTitle).toBe("Export All Operators Result"); expect(config.nzData).toEqual(expect.objectContaining({ workflowName: "report-wf", sourceTriggered: "menu" })); }); + + describe("canvas display toggles", () => { + // A fake JointJS element that records `attr(path, value)` calls and answers `get("type")`. + function fakeElement(type: string) { + return { + type, + attrs: {} as Record, + get(key: string) { + return key === "type" ? this.type : undefined; + }, + attr: vi.fn(function (this: { attrs: Record }, path: string, value: unknown) { + this.attrs[path] = value; + }), + }; + } + + // Stubs getJointGraphWrapper() with a paper element + model/graph backed by the given elements. + function stubWrapper(elements: ReturnType[]) { + const el = document.createElement("div"); + const wrapper = { + mainPaper: { el, model: { getElements: () => elements } }, + jointGraph: { getElements: () => elements }, + }; + vi.spyOn(workflowActionService, "getJointGraphWrapper").mockReturnValue(wrapper as any); + return el; + } + + describe("toggleRegion", () => { + it("publishes the displayed flag to the joint graph wrapper when enabled", () => { + const setSpy = vi.spyOn(workflowActionService.getJointGraphWrapper(), "setRegionsDisplayed"); + + component.showRegion = true; + component.toggleRegion(); + + expect(setSpy).toHaveBeenCalledWith(true); + }); + + it("publishes the displayed flag to the joint graph wrapper when disabled", () => { + const setSpy = vi.spyOn(workflowActionService.getJointGraphWrapper(), "setRegionsDisplayed"); + + component.showRegion = false; + component.toggleRegion(); + + expect(setSpy).toHaveBeenCalledWith(false); + }); + }); + + describe("toggleStatus", () => { + it("removes hide-operator-status when enabled and repositions the status label", () => { + const operator = fakeElement("operator"); + const el = stubWrapper([operator]); + el.classList.add("hide-operator-status"); + + component.showStatus = true; + component.showNumWorkers = false; + component.toggleStatus(); + + expect(el.classList.contains("hide-operator-status")).toBe(false); + expect(operator.attr).toHaveBeenCalledWith(".texera-operator-state/ref-x", -10); + expect(operator.attr).toHaveBeenCalledWith(".texera-operator-state/ref-y", -35); + }); + + it("adds hide-operator-status when disabled", () => { + const operator = fakeElement("operator"); + const el = stubWrapper([operator]); + + component.showStatus = false; + component.toggleStatus(); + + expect(el.classList.contains("hide-operator-status")).toBe(true); + }); + + it("offsets the status label higher when worker counts are shown", () => { + const operator = fakeElement("operator"); + stubWrapper([operator]); + + component.showNumWorkers = true; + component.toggleStatus(); + + expect(operator.attr).toHaveBeenCalledWith(".texera-operator-state/ref-y", -55); + }); + }); + }); }); diff --git a/frontend/src/app/workspace/component/menu/menu.component.ts b/frontend/src/app/workspace/component/menu/menu.component.ts index 6375fbcee4e..b5621d06d3a 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.ts @@ -321,8 +321,8 @@ export class MenuComponent implements OnInit, OnDestroy { const refY = this.showNumWorkers ? -55 : -35; const paperModel = this.workflowActionService.getJointGraphWrapper().mainPaper.model as any; paperModel.getElements().forEach((el: any) => { - el.attr(".operator-status/ref-x", -10); - el.attr(".operator-status/ref-y", refY); + el.attr(".texera-operator-state/ref-x", -10); + el.attr(".texera-operator-state/ref-y", refY); }); } @@ -542,11 +542,9 @@ export class MenuComponent implements OnInit, OnDestroy { } public toggleRegion(): void { - this.workflowActionService - .getJointGraphWrapper() - .jointGraph.getElements() - .filter(el => el.get("type") === "region") // small improvement here too - .forEach(el => el.attr("body/visibility", this.showRegion ? "visible" : "hidden")); + // The editor owns applying this to the shared JointJS model (both canvas and mini-map) and + // reapplies it whenever regions are recreated during execution (see #5120, #4027). + this.workflowActionService.getJointGraphWrapper().setRegionsDisplayed(this.showRegion); } /** diff --git a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.scss b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.scss index 422f743b717..482dac56a26 100644 --- a/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.scss +++ b/frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.scss @@ -25,10 +25,6 @@ height: 100%; } -::ng-deep .hide-region .region { - display: none; -} - ::ng-deep .agent-action { // Agent action highlights - temporary 5-second visual indicators // Styles are defined inline in JointJS element, but this provides a hook for customization @@ -39,6 +35,10 @@ display: none; } +::ng-deep .hide-operator-status .texera-operator-state { + display: none; +} + // Inline panels overlay container .panels-container { position: absolute; 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 38c8c8b1138..a3cb50774c2 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 @@ -117,6 +117,64 @@ describe("WorkflowEditorComponent", () => { expect(component).toBeTruthy(); }); + it("should hide operator status on the canvas by default", () => { + // keeps the Status toggle off until the user enables it + const editor = (component as any).editor as HTMLElement; + expect(editor.classList.contains("hide-operator-status")).toBe(true); + }); + + // 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 { + const operatorID = `region_op_${regionId}`; + const operator = new joint.shapes.basic.Rect({ position: { x: 0, y: 0 }, size: { width: 80, height: 40 } }); + operator.set("id", operatorID); + jointGraph.addCell(operator); + const executeWorkflowService = TestBed.inject(ExecuteWorkflowService); + (executeWorkflowService as any).regionUpdateStream.next({ regions: [[regionId, [operatorID]]] }); + return operatorID; + } + + it("should create region elements hidden so the Regions toggle starts off on canvas and mini-map", () => { + emitRegionUpdate(1); + + const region = jointGraph.getCell("region-1"); + expect(region).toBeTruthy(); + // region visibility is a shared-model attribute, so hidden-by-default applies to both surfaces + expect(region.attr("body/visibility")).toBe("hidden"); + }); + + it("should show regions created during execution when the toggle is already on", () => { + // user enables Regions, then execution emits region updates + const wrapper = TestBed.inject(WorkflowActionService).getJointGraphWrapper(); + wrapper.setRegionsDisplayed(true); + emitRegionUpdate(1); + + expect(jointGraph.getCell("region-1").attr("body/visibility")).toBe("visible"); + }); + + it("should keep regions visible when they are recreated on a later execution update", () => { + const wrapper = TestBed.inject(WorkflowActionService).getJointGraphWrapper(); + wrapper.setRegionsDisplayed(true); + emitRegionUpdate(1); + // a subsequent update removes and recreates the region elements + emitRegionUpdate(2); + + expect(jointGraph.getCell("region-2").attr("body/visibility")).toBe("visible"); + }); + + it("should toggle visibility of existing regions when the displayed flag changes", () => { + const wrapper = TestBed.inject(WorkflowActionService).getJointGraphWrapper(); + emitRegionUpdate(1); + expect(jointGraph.getCell("region-1").attr("body/visibility")).toBe("hidden"); + + wrapper.setRegionsDisplayed(true); + expect(jointGraph.getCell("region-1").attr("body/visibility")).toBe("visible"); + + wrapper.setRegionsDisplayed(false); + expect(jointGraph.getCell("region-1").attr("body/visibility")).toBe("hidden"); + }); + it("should create element in the UI after adding operator in the model", () => { const operatorID = "test_one_operator_1"; 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 979f131ad3c..54e9ec9a4e8 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 @@ -275,6 +275,7 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy height: this.editor.offsetHeight, }); this.editor.classList.add("hide-worker-count"); + this.editor.classList.add("hide-operator-status"); } private handleDisableJointPaperInteractiveness(): void { @@ -361,7 +362,6 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy } private handleRegionEvents(): void { - this.editor.classList.add("hide-region"); const Region = joint.dia.Element.define( "region", { @@ -369,7 +369,9 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy body: { fill: "rgba(158,158,158,0.2)", pointerEvents: "none", - class: "region", + // Regions start hidden and are revealed via the View > Regions toggle. Driving visibility + // through this model attribute keeps the main canvas and the mini-map in sync (see #4027). + visibility: "hidden", }, }, }, @@ -396,8 +398,16 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy this.updateRegionElement(element, ops); return { regionElement: element, operators: ops }; }); + // regions are recreated on every update, so reapply the current toggle state to the new elements + this.setRegionsVisibility(this.wrapper.getRegionsDisplayed()); }); + // apply the View > Regions toggle to all existing region elements (canvas and mini-map share the model) + this.wrapper + .getRegionsDisplayedStream() + .pipe(untilDestroyed(this)) + .subscribe(displayed => this.setRegionsVisibility(displayed)); + this.paper.model.on("change:position", operator => { regionMap .filter(region => region.operators.includes(operator)) @@ -418,6 +428,13 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy }); } + private setRegionsVisibility(displayed: boolean): void { + this.paper.model + .getElements() + .filter(element => element.get("type") === "region") + .forEach(element => element.attr("body/visibility", displayed ? "visible" : "hidden")); + } + private updateRegionElement(regionElement: joint.dia.Element, operators: joint.dia.Cell[]) { const points = operators.flatMap(op => { const { x, y, width, height } = op.getBBox(), diff --git a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts index 495208f5dce..c212722314d 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.spec.ts @@ -793,4 +793,29 @@ describe("JointGraphWrapperService", () => { }) ); }); + + describe("regions displayed flag", () => { + it("defaults to not displayed", () => { + expect(jointGraphWrapper.getRegionsDisplayed()).toBe(false); + }); + + it("updates the value when set", () => { + jointGraphWrapper.setRegionsDisplayed(true); + expect(jointGraphWrapper.getRegionsDisplayed()).toBe(true); + + jointGraphWrapper.setRegionsDisplayed(false); + expect(jointGraphWrapper.getRegionsDisplayed()).toBe(false); + }); + + it("emits the current value to new subscribers and on every change", () => { + const emitted: boolean[] = []; + jointGraphWrapper.getRegionsDisplayedStream().subscribe(displayed => emitted.push(displayed)); + + jointGraphWrapper.setRegionsDisplayed(true); + jointGraphWrapper.setRegionsDisplayed(false); + + // BehaviorSubject replays the initial false, then each subsequent change + expect(emitted).toEqual([false, true, false]); + }); + }); }); diff --git a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts index f47aa8ab87d..b7a2bf05b51 100644 --- a/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts +++ b/frontend/src/app/workspace/service/workflow-graph/model/joint-graph-wrapper.ts @@ -17,7 +17,7 @@ * under the License. */ -import { fromEvent, Observable, ReplaySubject, Subject } from "rxjs"; +import { BehaviorSubject, fromEvent, Observable, ReplaySubject, Subject } from "rxjs"; import { filter, map } from "rxjs/operators"; import { LogicalPort, Point } from "../../../types/workflow-common.interface"; import * as joint from "jointjs"; @@ -103,6 +103,11 @@ export class JointGraphWrapper { private mainJointPaperAttachedStream: Subject = new ReplaySubject(1); + // Whether region hulls are shown on the canvas (View > Regions toggle). Kept here so that it + // survives the region elements being recreated on every execution update, and so the editor can + // reapply it to the shared model (covering both the main canvas and the mini-map). + private regionsDisplayedStream = new BehaviorSubject(false); + private elementPositions: Map = new Map(); private listenPositionChange: boolean = true; @@ -209,6 +214,21 @@ export class JointGraphWrapper { return this.mainJointPaperAttachedStream; } + /** + * Sets whether region hulls should be displayed on the canvas (and mini-map). + */ + public setRegionsDisplayed(displayed: boolean): void { + this.regionsDisplayedStream.next(displayed); + } + + public getRegionsDisplayed(): boolean { + return this.regionsDisplayedStream.value; + } + + public getRegionsDisplayedStream(): Observable { + return this.regionsDisplayedStream.asObservable(); + } + /** * This method is used to toggle the multiselect mode. * @param multiSelect