Skip to content

Conversation

@DymoneLewis
Copy link
Collaborator

No description provided.

@DymoneLewis DymoneLewis requested a review from Copilot November 10, 2025 06:59
@changeset-bot
Copy link

changeset-bot bot commented Nov 10, 2025

⚠️ No Changeset found

Latest commit: e40d655

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a new EDGE_TOP overlap mode (value -1) to LogicFlow's element stacking system. This mode ensures edges are always rendered above nodes, complementing the existing DEFAULT (0) and INCREASE (1) modes.

  • Introduces OverlapMode.EDGE_TOP = -1 enum value with sorting logic that renders nodes first, then edges
  • Updates model classes to handle zIndex for EDGE_TOP mode similar to INCREASE mode
  • Adds comprehensive documentation and interactive demo controls for testing the new mode

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/core/src/constant/index.ts Adds EDGE_TOP enum value (-1) to OverlapMode
packages/core/src/model/GraphModel.ts Implements special sorting logic for EDGE_TOP mode in sortElements computed property and toFront method; fixes spelling errors
packages/core/src/model/node/BaseNodeModel.ts Extends zIndex handling to include EDGE_TOP mode
packages/core/src/model/edge/BaseEdgeModel.ts Extends zIndex handling to include EDGE_TOP mode
sites/docs/docs/api/model/graphModel.zh.md Documents EDGE_TOP mode in Chinese with behavior description
sites/docs/docs/api/model/graphModel.en.md Documents EDGE_TOP mode in English with behavior description
sites/docs/docs/api/detail/constructor.zh.md Updates constructor documentation for overlapMode parameter in Chinese
sites/docs/docs/api/detail/constructor.en.md Updates constructor documentation for overlapMode parameter in English
sites/docs/examples/graph/basic/demo/graph.tsx Adds interactive demo controls for testing EDGE_TOP mode
examples/feature-examples/src/pages/graph/index.tsx Adds interactive demo controls for testing EDGE_TOP mode with refactored handler functions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +254 to +291
const nodesSorted = [...this.nodes].sort((a, b) => a.zIndex - b.zIndex)
const edgesSorted = [...this.edges].sort((a, b) => a.zIndex - b.zIndex)

const visibleNodes: (BaseNodeModel | BaseEdgeModel)[] = []
const visibleEdges: (BaseNodeModel | BaseEdgeModel)[] = []
const visibleLt: PointTuple = [
-DEFAULT_VISIBLE_SPACE,
-DEFAULT_VISIBLE_SPACE,
]
const visibleRb: PointTuple = [
this.width + DEFAULT_VISIBLE_SPACE,
this.height + DEFAULT_VISIBLE_SPACE,
]

for (let i = 0; i < nodesSorted.length; i++) {
const item = nodesSorted[i]
if (
item.visible &&
(!this.partial ||
item.isSelected ||
this.isElementInArea(item, visibleLt, visibleRb, false, false))
) {
visibleNodes.push(item)
}
}
for (let i = 0; i < edgesSorted.length; i++) {
const item = edgesSorted[i]
if (
item.visible &&
(!this.partial ||
item.isSelected ||
this.isElementInArea(item, visibleLt, visibleRb, false, false))
) {
visibleEdges.push(item)
}
}

return [...visibleNodes, ...visibleEdges]
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EDGE_TOP sorting logic duplicates the visibility filtering logic from the default path (lines 294-322). Consider extracting the visibility filtering into a helper function to avoid code duplication and improve maintainability.

Copilot uses AI. Check for mistakes.
if (!lf) return
const { edges } = lf.getGraphData() as GraphData
forEach(edges, (edge) => {
if ((edge as any).id) lf.openEdgeAnimation((edge as any).id)
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary type casting and null check. The edges array from getGraphData() returns EdgeData[] which always has an id property. Remove the type cast and condition: lf.openEdgeAnimation(edge.id)

Copilot uses AI. Check for mistakes.
if (!lf) return
const { edges } = lf.getGraphData() as GraphData
forEach(edges, (edge) => {
if ((edge as any).id) lf.closeEdgeAnimation((edge as any).id)
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary type casting and null check. The edges array from getGraphData() returns EdgeData[] which always has an id property. Remove the type cast and condition: lf.closeEdgeAnimation(edge.id)

Suggested change
if ((edge as any).id) lf.closeEdgeAnimation((edge as any).id)
lf.closeEdgeAnimation(edge.id)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants