This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-5160-a1045ce55e9169deb94f8ae3bf195692ef994ec5 in repository https://gitbox.apache.org/repos/asf/texera.git
commit e6509191d21a4df280bd98b3b73d6dca9f84a761 Author: Jiadong Bai <[email protected]> AuthorDate: Sat May 30 18:34:24 2026 -0700 fix(frontend): restore Regions and Status canvas toggles (#5160) ### What changes were proposed in this PR? Restores the **Regions** and **Status** toggles under View, both of which broke after #4495 switched these elements from JointJS `visibility` attributes to CSS classes. - **Regions**: region visibility is now a shared, persistent flag on `JointGraphWrapper` (`setRegionsDisplayed` / `getRegionsDisplayed` / stream). The editor applies it to the shared JointJS model both when the flag changes and whenever regions are recreated on a `RegionUpdateEvent`. Because the model is shared, the toggle drives both the main canvas (#5120) and the mini-map (#4027), and it survives the region elements being recreated during execution (previously regions went hidden mid-run even with the toggle on). The menu toggle just sets the flag; the editor owns applying it. The `hide-region` CSS class/SCSS rule are removed. - **Status** (`workflow-editor.component.scss`, `workflow-editor.component.ts`, `menu.component.ts`): re-added the `hide-operator-status` SCSS rule (retargeted to the renamed `.texera-operator-state` element), added the default class so Status starts hidden, and fixed the stale `.operator-status` selector in `applyOperatorStatusPosition()`. Unit tests added for the toggle delegation, status repositioning, the default `hide-operator-status` class, and region visibility across creation / recreation / toggling. Here is a demo of the style after fix: <img width="1707" height="1162" alt="2026-05-30 11 35 33" src="https://github.com/user-attachments/assets/ae5fc0d3-76f1-4bf8-914e-2644f89b46c2"; /> ### Any related issues, documentation, discussions? Closes #5120 Regression from #4495. ### How was this PR tested? Manual + unit tests. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Opus 4.8 (1M context) --------- Co-authored-by: Bob Bai <[email protected]> --- .../component/menu/menu.component.spec.ts | 83 ++++++++++++++++++++++ .../app/workspace/component/menu/menu.component.ts | 12 ++-- .../workflow-editor/workflow-editor.component.scss | 8 +-- .../workflow-editor.component.spec.ts | 58 +++++++++++++++ .../workflow-editor/workflow-editor.component.ts | 21 +++++- .../model/joint-graph-wrapper.spec.ts | 25 +++++++ .../workflow-graph/model/joint-graph-wrapper.ts | 22 +++++- 7 files changed, 215 insertions(+), 14 deletions(-) diff --git a/frontend/src/app/workspace/component/menu/menu.component.spec.ts b/frontend/src/app/workspace/component/menu/menu.component.spec.ts index 76de1c1e1b..1449174215 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<string, unknown>, + get(key: string) { + return key === "type" ? this.type : undefined; + }, + attr: vi.fn(function (this: { attrs: Record<string, unknown> }, 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<typeof fakeElement>[]) { + 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 6375fbcee4..b5621d06d3 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 422f743b71..482dac56a2 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 38c8c8b113..a3cb50774c 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-<id> 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 979f131ad3..54e9ec9a4e 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 495208f5dc..c212722314 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 f47aa8ab87..b7a2bf05b5 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<joint.dia.Paper> = 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<boolean>(false); + private elementPositions: Map<string, PositionInfo> = new Map<string, PositionInfo>(); 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<boolean> { + return this.regionsDisplayedStream.asObservable(); + } + /** * This method is used to toggle the multiselect mode. * @param multiSelect
