aglinxinyuan commented on code in PR #3602:
URL: https://github.com/apache/texera/pull/3602#discussion_r2338436801
##########
core/gui/src/app/workspace/service/joint-ui/joint-ui.service.ts:
##########
@@ -510,6 +510,51 @@ export class JointUIService {
});
}
+ private static RemoveButton: new () => joint.linkTools.Button; // private
static property to cache the button class.
+
+ public getRemoveButton(): new () => joint.linkTools.Button {
Review Comment:
Why do we need to place this function here? Couldn’t we just include it in
`workflow-editor.component.ts` instead?
I noticed that the `getBreakpointButton` function is also in this service,
but I find that placement questionable as well. Does this function rely on any
variables or context that are specific to the service? If not, I’d recommend we
avoid placing `getRemoveButton` in the service, as it doesn't seem to have any
service-specific responsibilities.
##########
core/gui/src/app/workspace/service/joint-ui/joint-ui.service.ts:
##########
@@ -510,6 +510,51 @@ export class JointUIService {
});
}
+ private static RemoveButton: new () => joint.linkTools.Button; // private
static property to cache the button class.
+
+ public getRemoveButton(): new () => joint.linkTools.Button {
+ // Check if the class has already been created.
+ if (!JointUIService.RemoveButton) {
+ // If not, create it ONCE and store it in the static property.
+ JointUIService.RemoveButton = joint.linkTools.Button.extend({
+ name: "remove-button",
+ options: {
+ markup: [
+ {
+ tagName: "circle",
+ selector: "button",
+ attributes: {
+ r: 11,
+ fill: "#D8656A",
+ cursor: "pointer",
+ },
+ },
+ {
+ tagName: "path",
+ selector: "icon",
+ attributes: {
+ d: "M24.778,21.419 19.276,15.917 24.777,10.415 21.949,7.585
16.447,13.087 10.945,7.585 8.117,10.415 13.618,15.917 8.116,21.419
10.946,24.248 16.447,18.746 21.948,24.248z",
Review Comment:
What is the purpose of this shape? In this PR, we’re only adjusting the
placement of existing elements—why do we need to introduce a custom shape here?
##########
core/gui/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -114,6 +116,12 @@ export class WorkflowEditorComponent implements
AfterViewInit, OnDestroy {
this.wrapper = this.workflowActionService.getJointGraphWrapper();
}
+ ngOnInit(): void {
+ // Create the button instances 1 time during initialization.
+ this.removeButton = new (this.jointUIService.getRemoveButton())();
+ this.breakpointButton = new (this.jointUIService.getBreakpointButton())();
Review Comment:
Why do we need to create the button instances once during initialization?
Can’t we just create them when needed, as we previously did with
`breakpointButton`?
--
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]
