fix: harden edge geometry and dynamic group behavior - #2416
Merged
Conversation
🦋 Changeset detectedLatest commit: 2bfd527 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens edge-geometry handling in Core and improves Extension behaviors around curved-edge path generation and DynamicGroup copy/paste, adding targeted tests and runnable regression examples to prevent NaN/throw scenarios from interrupting interaction.
Changes:
- Core: prevent non-finite polyline endpoints when dragging across rounded-rectangle straight-side regions; add regression tests.
- Extension: make
getCurvedEdgePathreturn safely for empty/single/malformed point lists; add tests. - Extension/examples: add dynamic-group selection copy/paste regression scenario and a feature-example reproducer for the rounded-rect NaN case; add patch changesets.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/extension/src/materials/curved-edge/index.ts | Adds defensive validation and safe early-returns in getCurvedEdgePath. |
| packages/extension/src/dynamic-group/index.ts | Changes DynamicGroup’s addElements default distance behavior. |
| packages/extension/test/materials/curved-edge/curved-edge.test.ts | Adds regression coverage for empty/single/malformed curved-edge point lists. |
| packages/core/src/util/node.ts | Ensures rounded-rect intersection selection falls back to straight-side crosspoints when arcs can’t intersect. |
| packages/core/src/model/edge/PolylineEdgeModel.ts | Guards against replacing endpoints with non-finite intersection results. |
| packages/core/tests/util/node.test.ts | Adds tests for rounded-rectangle straight-side vs corner intersection behavior. |
| packages/core/tests/model/polyline-edge.test.ts | Adds a regression test preventing NaN endpoints during rounded-rect dragging. |
| examples/feature-examples/src/pages/edges/custom/rounded-rect-nan/index.tsx | Adds a UI reproducer page for the rounded-rect NaN drag issue. |
| examples/feature-examples/src/pages/edges/custom/rounded-rect-nan/index.less | Styles for the reproducer page overlays and data panel. |
| examples/feature-examples/.umirc.ts | Registers the new rounded-rect NaN regression route. |
| examples/dynamic-group-regression/src/scenarios/selectionCopyPaste.ts | Adds a focused selection copy/paste regression scenario and diagnostics. |
| examples/dynamic-group-regression/src/scenarios/index.ts | Wires the new selection copy/paste scenario into the workbench. |
| examples/dynamic-group-regression/src/components/SelectionCopyControls.tsx | Adds scenario controls for selection, copy, paste, diagnose, and move-to-repro separation. |
| examples/dynamic-group-regression/src/components/RegressionWorkbench.tsx | Enables SelectionSelect plugin to support box-selection in the new scenario. |
| examples/dynamic-group-regression/README.md | Documents the new regression scenario and how to reproduce it. |
| .changeset/fix-rounded-rect-edge-nan.md | Adds a Core patch changeset for finite polyline endpoints during rounded-rect dragging. |
| .changeset/fix-invalid-polyline-path-rendering.md | Adds an Extension patch changeset for safe curved-edge path generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
903
to
906
| lf.addElements = ( | ||
| { nodes: selectedNodes, edges: selectedEdges }: GraphConfigData, | ||
| distance = 40, | ||
| distance = 0, | ||
| ): GraphElements => { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
addElementspaste translation with caller-managed offsets and add focused regression scenarios.getCurvedEdgePathsafely handle empty, single-point, and malformed point lists without changing the surrounding edge View behavior.Motivation and Context
Degenerate edge geometry could produce
NaNcoordinates or throw while rendering a curved edge, which could interrupt graph interaction. DynamicGroup selection paste could also apply an unexpected default translation. These changes keep geometry finite, preserve the renderer boundary for malformed curved paths, and make paste positioning explicit.UI verification is available in:
examples/feature-examples: rounded rectangle NaN reproducer.examples/dynamic-group-regression: selection copy and paste scenarios.Types of changes
Verification
pnpm test -- packages/core/__tests__/model/polyline-edge.test.ts packages/core/__tests__/util/node.test.ts packages/extension/__test__/dynamic-group packages/extension/__test__/materials/curved-edge/curved-edge.test.ts --runInBand— 11 suites and 79 tests passed.pnpm --filter @logicflow/core build— passed.pnpm --filter @logicflow/extension build— passed when run after the Core build.pnpm --dir examples/feature-examples build— passed.pnpm --dir examples/dynamic-group-regression build— still fails at the pre-existingsrc/scenarios/index.ts:631model assertion; this is unrelated to the changed scenarios.pnpm exec changeset status— passed.Self Check before Merge