This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new 7daf8d7841 refactor(frontend): make redundant operator border repaints
a no-op (#6927)
7daf8d7841 is described below
commit 7daf8d78415d5f40fc017678738ce57b137065c5
Author: Prateek Ganigi <[email protected]>
AuthorDate: Wed Aug 12 18:51:52 2026 +0000
refactor(frontend): make redundant operator border repaints a no-op (#6927)
### What changes were proposed in this PR?
When an operator is added, its border was painted by two paths - the
operator-add restore and the validation pass, producing the same color.
Harmless, but a redundant repaint.
This PR adds a guarded border setter (`paintOperatorBorder`) in
`JointUIService` that writes `rect.body/stroke` only when the color
actually changes. Both `changeOperatorColor` and `changeOperatorState`
route their border write through it, so a repaint with the color the
border already has becomes a no-op, effectively "painted once" -
including on the navigation-return (reload) path.
**Deviation from the approach suggested on the issue:** the issue
suggested dropping the `applyOperatorBorder` call from the operator-add
handler and letting the validation pass set the border. I kept that call
and used the guard instead, because `changeOperatorStatistics` already
paints the border via `changeOperatorState` *without* checking validity.
Dropping `applyOperatorBorder` would make an invalid operator with a
cached "completed" status rely on the validation pass firing afterward
to correct green→red, reintroducing the order-dependent border fragility
that #5146 removed (and it would break in the edge case where
`setDynamicSchema` skips its emit because the schema is unchanged). The
guard reaches the same no-redundant-repaint goal while keeping the
border validity-correct regardless of event timing.
### Any related issues, documentation, discussions?
Part of #5726
### How was this PR tested?
Unit tests:
- `JointUIService`: the guarded setter skips the write when the border
is already the requested color, and writes when it differs.
- `WorkflowEditorComponent`: added a navigation-return test for a cached
**Running** operator (orange), alongside the existing completed (green),
default (gray), invalid (red), and invalid-over-cached-priority cases.
- Full frontend suite: 3739 passing.
Manual (navigated away from a running workflow and back), border
restored correctly for:
- Completed operators → green
- Running operators → orange
- Invalid operators → red
- Valid, not run → default gray
### Was this PR authored or co-authored using generative AI tooling?
This PR was co-authored using Claude Code (Anthropic Claude Opus 4.7) in
compliance with ASF.
---
.../workflow-editor.component.spec.ts | 19 ++++++++++++++++---
.../service/joint-ui/joint-ui.service.spec.ts | 15 +++++++++++++++
.../workspace/service/joint-ui/joint-ui.service.ts | 21 +++++++++++++++++----
3 files changed, 48 insertions(+), 7 deletions(-)
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 d2ecfda618..2c8807a9ec 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
@@ -887,15 +887,16 @@ describe("WorkflowEditorComponent", () => {
*/
describe("operator border restoration after navigation", () => {
let workflowStatusService: WorkflowStatusService;
- const cachedCompleted = {
+ const cachedStatus = (operatorState: OperatorState) => ({
[mockScanPredicate.operatorID]: {
- operatorState: OperatorState.Completed,
+ operatorState,
aggregatedInputRowCount: 0,
inputPortMetrics: {},
aggregatedOutputRowCount: 0,
outputPortMetrics: {},
},
- };
+ });
+ const cachedCompleted = cachedStatus(OperatorState.Completed);
const getStroke = (operatorID: string): string =>
component.paper.getModelById(operatorID).attr("rect.body/stroke") as
string;
@@ -913,6 +914,18 @@ describe("WorkflowEditorComponent", () => {
expect(getStroke(mockScanPredicate.operatorID)).toBe("green");
});
+ it("paints the execution-state stroke (orange) for a valid operator with
a cached Running status", () => {
+ // Navigation-return with a mid-run operator: the border must be
restored
+ // to the running color, not the default (see #3614).
+ vi.spyOn(workflowStatusService,
"getCurrentStatus").mockReturnValue(cachedStatus(OperatorState.Running));
+ vi.spyOn(validationWorkflowService,
"validateOperator").mockReturnValue({ isValid: true });
+
+ workflowActionService.addOperator(mockScanPredicate, mockPoint);
+ fixture.detectChanges();
+
+ expect(getStroke(mockScanPredicate.operatorID)).toBe("orange");
+ });
+
it("falls back to the default valid stroke (#CFCFCF) when no cached
status exists", () => {
vi.spyOn(workflowStatusService,
"getCurrentStatus").mockReturnValue({});
vi.spyOn(validationWorkflowService,
"validateOperator").mockReturnValue({ isValid: true });
diff --git
a/frontend/src/app/workspace/service/joint-ui/joint-ui.service.spec.ts
b/frontend/src/app/workspace/service/joint-ui/joint-ui.service.spec.ts
index d34a9eb2b3..c383e0963d 100644
--- a/frontend/src/app/workspace/service/joint-ui/joint-ui.service.spec.ts
+++ b/frontend/src/app/workspace/service/joint-ui/joint-ui.service.spec.ts
@@ -361,6 +361,21 @@ describe("JointUIService", () => {
service.changeOperatorColor(paper, "op-1", false);
expect(attrSpy).toHaveBeenCalledWith("rect.body/stroke", "red");
});
+ it("skips the write when the border is already the requested color", () =>
{
+ const { paper, attrSpy } = makePaperWithModel();
+ // model reports it is already neutral; the guarded setter must not
rewrite it
+ attrSpy.mockImplementation((selector: string) => (selector ===
"rect.body/stroke" ? "#CFCFCF" : undefined));
+ const service = new JointUIService(emptyMetadataStub as never);
+ service.changeOperatorColor(paper, "op-1", true);
+ expect(attrSpy).not.toHaveBeenCalledWith("rect.body/stroke", "#CFCFCF");
+ });
+ it("writes the border when the current color differs", () => {
+ const { paper, attrSpy } = makePaperWithModel();
+ attrSpy.mockImplementation((selector: string) => (selector ===
"rect.body/stroke" ? "red" : undefined));
+ const service = new JointUIService(emptyMetadataStub as never);
+ service.changeOperatorColor(paper, "op-1", true);
+ expect(attrSpy).toHaveBeenCalledWith("rect.body/stroke", "#CFCFCF");
+ });
});
describe("changeOperatorState", () => {
diff --git a/frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts
b/frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts
index d069a270eb..bb6fbff0f0 100644
--- a/frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts
+++ b/frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts
@@ -491,11 +491,24 @@ export class JointUIService {
* @param isOperatorValid
*/
public changeOperatorColor(jointPaper: joint.dia.Paper, operatorID: string,
isOperatorValid: boolean): void {
- if (isOperatorValid) {
- jointPaper.getModelById(operatorID).attr("rect.body/stroke", "#CFCFCF");
- } else {
- jointPaper.getModelById(operatorID).attr("rect.body/stroke", "red");
+ this.paintOperatorBorder(jointPaper, operatorID, isOperatorValid ?
"#CFCFCF" : "red");
+ }
+
+ /**
+ * Sets the operator's border stroke, returning early when it is already that
+ * color. A same-value attr() write would not re-render (Backbone's Model.set
+ * no-ops via _.isEqual), but attr() still deep-clones and deep-compares the
+ * whole attrs tree before reaching that check. On operator add the
validation
+ * pass and the operator-add restore both request a border color for the same
+ * operator, so returning early here skips that clone/compare on the second
+ * call.
+ */
+ private paintOperatorBorder(jointPaper: joint.dia.Paper, operatorID: string,
color: string): void {
+ const model = jointPaper.getModelById(operatorID);
+ if (model.attr("rect.body/stroke") === color) {
+ return;
}
+ model.attr("rect.body/stroke", color);
}
public changeOperatorDisableStatus(jointPaper: joint.dia.Paper, operator:
OperatorPredicate): void {
