Copilot commented on code in PR #3652:
URL:
https://github.com/apache/incubator-kie-tools/pull/3652#discussion_r3802735799
##########
packages/xyflow-react-kie-diagram/src/maths/DcMaths.ts:
##########
@@ -58,6 +58,12 @@ export function getPointForHandle({
}
}
+/** Midpoint of `node`'s border on the side facing `towards`, by the same
dominant-axis rule as auto-positioned edges. */
+export function getBorderMidpointTowards(node: Bounds, towards: DC__Point):
DC__Point {
+ const [x, y] = getPositionalHandlePosition(node, node, towards);
Review Comment:
`getPositionalHandlePosition(node, node, towards)` passes the same bounds
for both operands, which can make the dominant-axis / zone calculation
degenerate (relative centers become equal) unless `getPositionalHandlePosition`
explicitly ignores the second bounds when `towards` is provided. To make the
behavior unambiguous, consider either (a) passing the *other* node bounds into
this helper (change helper signature), or (b) adding/using a dedicated API that
computes the facing-side handle purely from `node` + target point (e.g., treat
`towards` as a 0×0 bounds at that point).
##########
packages/bpmn-editor/src/mutations/repositionNode.ts:
##########
@@ -20,10 +20,27 @@
import { switchExpression } from "@kie-tools-core/switch-expression-ts";
import { BPMN20__tDefinitions, BPMNDI__BPMNEdge } from
"@kie-tools/bpmn-marshaller/dist/schemas/bpmn-2_0/ts-gen/types";
import { DC__Shape } from
"@kie-tools/xyflow-react-kie-diagram/dist/maths/model";
+import {
+ Bounds,
+ getBoundsCenterPoint,
+ getPositionalHandlePosition,
+} from "@kie-tools/xyflow-react-kie-diagram/dist/maths/Maths";
import { BpmnNodeType } from "../diagram/BpmnDiagramDomain";
import { Normalized } from "../normalization/normalize";
import { addOrGetProcessAndDiagramElements } from
"./addOrGetProcessAndDiagramElements";
+function getShapeBoundsById(diagramElements: { "@_id"?: string }[], shapeId:
string | undefined): Bounds | undefined {
+ if (!shapeId) {
+ return undefined;
+ }
+ const bounds = (diagramElements.find((e) => e["@_id"] === shapeId) as
Normalized<DC__Shape> | undefined)?.[
+ "dc:Bounds"
+ ];
+ return bounds
+ ? { x: bounds["@_x"], y: bounds["@_y"], width: bounds["@_width"], height:
bounds["@_height"] }
+ : undefined;
Review Comment:
This lookup assumes any diagram element with a matching `@_id` can be
treated as a `DC__Shape` with `dc:Bounds`. Making the predicate explicitly
require `dc:Bounds` (or checking the element kind, e.g. `bpmndi:BPMNShape`)
would prevent accidental matches and avoid silent `undefined` bounds that skip
re-anchoring decisions.
##########
packages/bpmn-editor/stories/misc/connectionReanchor/ConnectionReanchor.stories.tsx:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import type { Meta, StoryObj } from "@storybook/react";
+import { getMarshaller } from "@kie-tools/bpmn-marshaller";
+import { BpmnEditorWrapper, StorybookBpmnEditorProps } from
"../../bpmnEditorStoriesWrapper";
+import { BpmnEditor, BpmnEditorProps } from "../../../src/BpmnEditor";
+
+// Start -> First Function -> Second Function -> End. First Function's
incoming flow is pinned to its
+// left border and its outgoing flow to its right border (see the waypoints
below).
+const startToEndProcess = `<?xml version="1.0" encoding="UTF-8"?>
+<definitions
+ xmlns="http://www.omg.org/spec/BPMN/20100524/MODEL";
+ xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI";
+ xmlns:dc="http://www.omg.org/spec/DD/20100524/DC";
+ xmlns:di="http://www.omg.org/spec/DD/20100524/DI";
+ targetNamespace="https://kie.apache.org/bpmn/connection-reanchor";
+ id="connection-reanchor-definitions">
+ <process id="connection_reanchor_process" name="Connection Reanchor"
isExecutable="true">
+ <startEvent id="StartEvent_1" name="Start" />
+ <task id="Task_1" name="First Function" />
+ <task id="Task_2" name="Second Function" />
+ <endEvent id="EndEvent_1" name="End" />
+ <sequenceFlow id="Flow_1" sourceRef="StartEvent_1" targetRef="Task_1" />
+ <sequenceFlow id="Flow_2" sourceRef="Task_1" targetRef="Task_2" />
+ <sequenceFlow id="Flow_3" sourceRef="Task_2" targetRef="EndEvent_1" />
+ </process>
+ <bpmndi:BPMNDiagram id="Diagram_1">
+ <bpmndi:BPMNPlane id="Plane_1" bpmnElement="connection_reanchor_process">
+ <bpmndi:BPMNShape id="Shape_StartEvent_1" bpmnElement="StartEvent_1">
+ <dc:Bounds x="100" y="200" width="56" height="56" />
+ </bpmndi:BPMNShape>
+ <bpmndi:BPMNShape id="Shape_Task_1" bpmnElement="Task_1">
+ <dc:Bounds x="240" y="188" width="160" height="80" />
+ </bpmndi:BPMNShape>
+ <bpmndi:BPMNShape id="Shape_Task_2" bpmnElement="Task_2">
+ <dc:Bounds x="480" y="188" width="160" height="80" />
+ </bpmndi:BPMNShape>
+ <bpmndi:BPMNShape id="Shape_EndEvent_1" bpmnElement="EndEvent_1">
+ <dc:Bounds x="720" y="200" width="56" height="56" />
+ </bpmndi:BPMNShape>
+ <bpmndi:BPMNEdge id="Edge_Flow_1" bpmnElement="Flow_1"
sourceElement="Shape_StartEvent_1" targetElement="Shape_Task_1">
+ <di:waypoint x="128" y="228" />
+ <di:waypoint x="240" y="228" />
+ </bpmndi:BPMNEdge>
+ <bpmndi:BPMNEdge id="Edge_Flow_2" bpmnElement="Flow_2"
sourceElement="Shape_Task_1" targetElement="Shape_Task_2">
+ <di:waypoint x="400" y="228" />
+ <di:waypoint x="480" y="228" />
+ </bpmndi:BPMNEdge>
+ <bpmndi:BPMNEdge id="Edge_Flow_3" bpmnElement="Flow_3"
sourceElement="Shape_Task_2" targetElement="Shape_EndEvent_1">
+ <di:waypoint x="640" y="228" />
+ <di:waypoint x="720" y="228" />
+ </bpmndi:BPMNEdge>
+ </bpmndi:BPMNPlane>
+ </bpmndi:BPMNDiagram>
+</definitions>`;
+
+const meta: Meta<BpmnEditorProps> = {
+ title: "Misc/Connection Reanchor",
+ component: BpmnEditor,
+ includeStories: /^[A-Z]/,
+};
+
+export default meta;
+type Story = StoryObj<StorybookBpmnEditorProps>;
+
+const marshaller = getMarshaller(startToEndProcess, { upgradeTo: "latest" });
+const model = marshaller.parser.parse();
+
+export const StartToEnd: Story = {
+ render: (args) => BpmnEditorWrapper(),
+ args: {
+ model: model,
+ originalVersion: "2.0",
+ externalContextDescription: "The Storybook for the BPMN Editor",
+ externalContextName: "Apache KIE :: BPMN Editor :: Storybook",
+ issueTrackerHref: "",
+ xml: marshaller.builder.build(model),
Review Comment:
The story uses a module-level `model` object shared across renders. If the
editor mutates the model in-place, this can leak state between Storybook
interactions (and between test runs if reused). Prefer creating a fresh model
per render (or deep-cloning the parsed model) so the story is deterministic.
##########
packages/bpmn-editor/tests-e2e/flowElements/connectionReanchorOnNodeMove.spec.ts:
##########
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { test, expect } from "../__fixtures__/base";
+import { JsonModel } from "../__fixtures__/jsonModel";
+import { NodeType } from "../__fixtures__/nodes";
+
+// 0 = node's left border, 0.5 = centre (auto-anchored), 1 = right border.
+async function getEndpointRelativeX(args: {
+ jsonModel: JsonModel;
+ nodeId: string;
+ direction: "incoming" | "outgoing";
+}): Promise<number> {
+ const flows = await args.jsonModel.getSequenceFlows();
+ const flow =
+ args.direction === "incoming"
+ ? flows.find((f) => f["@_targetRef"] === args.nodeId)
+ : flows.find((f) => f["@_sourceRef"] === args.nodeId);
+ expect(flow).toBeTruthy();
+
+ const elements = (await args.jsonModel.getPlane())?.["di:DiagramElement"] ??
[];
+ const shape = elements.find(
+ (e: any) => e.__$$element === "bpmndi:BPMNShape" && e["@_bpmnElement"] ===
args.nodeId
+ ) as any;
+ const edge = elements.find(
+ (e: any) => e.__$$element === "bpmndi:BPMNEdge" && e["@_bpmnElement"] ===
flow!["@_id"]
+ ) as any;
Review Comment:
If `shape` or `edge` isn't found, this will fail later with a non-obvious
runtime error when indexing into `shape[\"dc:Bounds\"]` /
`edge[\"di:waypoint\"]`. Adding explicit assertions (e.g.,
`expect(shape).toBeTruthy()` / `expect(edge).toBeTruthy()`) will make failures
clearer and reduce test flakiness/debug time.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
