Copilot commented on code in PR #3999:
URL: https://github.com/apache/texera/pull/3999#discussion_r2464619676
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -376,6 +376,18 @@ export class WorkflowEditorComponent implements OnInit,
AfterViewInit, OnDestroy
.filter(region => region.operators.includes(operator))
.forEach(region => this.updateRegionElement(region.regionElement,
region.operators));
});
+
+ this.executeWorkflowService
+ .getRegionStateStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(region => {
+ const colorMap: Record<string, string> = {
+ ExecutingDependeePortsPhase: "rgba(244,67,54,0.2)",
+ ExecutingNonDependeePortsPhase: "rgba(255,213,79,0.2)",
+ Completed: "rgba(76,175,80,0.2)",
+ };
+ this.paper.getModelById("region-" + region.id).attr("body/fill",
colorMap[region.state]);
Review Comment:
The code assumes that `getModelById` will always return a valid element. If
the region element doesn't exist in the paper (e.g., due to timing issues or
race conditions), this will throw a runtime error. Add a null check before
calling `attr`.
```suggestion
const regionElement = this.paper.getModelById("region-" + region.id);
if (regionElement) {
regionElement.attr("body/fill", colorMap[region.state]);
}
```
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -376,6 +376,18 @@ export class WorkflowEditorComponent implements OnInit,
AfterViewInit, OnDestroy
.filter(region => region.operators.includes(operator))
.forEach(region => this.updateRegionElement(region.regionElement,
region.operators));
});
+
+ this.executeWorkflowService
+ .getRegionStateStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(region => {
+ const colorMap: Record<string, string> = {
+ ExecutingDependeePortsPhase: "rgba(244,67,54,0.2)",
+ ExecutingNonDependeePortsPhase: "rgba(255,213,79,0.2)",
+ Completed: "rgba(76,175,80,0.2)",
+ };
+ this.paper.getModelById("region-" + region.id).attr("body/fill",
colorMap[region.state]);
Review Comment:
The code does not handle the `Unexecuted` state or any unknown state values.
If `region.state` is `Unexecuted` or any other unmapped state,
`colorMap[region.state]` will be `undefined`, which could cause rendering
issues. Add a fallback color or a check for undefined states.
```suggestion
const fillColor = colorMap[region.state] || "rgba(158,158,158,0.2)";
// fallback color for unmapped states
this.paper.getModelById("region-" + region.id).attr("body/fill",
fillColor);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
