fix: consider ManhattanConnector as orthogonal
#707
Conversation
Currently, the implementation of a connector must be explicitly referenced in the code to decide whether it is considered orthogonal. This is not configurable. So, when a new connector is added, its reference must be added. This was not done when `ManhattanConnector` was introduced. ManhattanConnector is now referenced in `Graph.isOrthogonal` and `Graph.createEdgeHandler`. tests have been added to validate the correction and a refactoring of the implementation to simplify it.
WalkthroughA new test file has been added to validate the graph object's Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Graph Client
participant Graph as Graph Class
participant Handler as Edge Handler Instance
Client->>Graph: call createEdgeHandler(state, edgeStyle)
alt edgeStyle is ElbowConnector/Loop
Graph-->>Handler: return ElbowEdgeHandler
else alt edgeStyle is ManhattanConnector/OrthogonalConnector/SegmentConnector
Graph-->>Handler: return EdgeSegmentHandler
else
Graph-->>Handler: return EdgeHandler
end
sequenceDiagram
participant Client as Graph Client
participant Graph as Graph Class
participant Utils as isNullish Utility
Client->>Graph: call isOrthogonal(edge)
Graph->>Utils: check edge's nullish values
Graph-->>Client: return boolean based on orthogonal property and edge style
Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/__tests__/view/Graph.test.ts (1)
95-96: Incorrect test description.The test description states "Expect EdgeSegmentHandler" but the assertion is actually checking for EdgeHandler.
-])('Expect EdgeSegmentHandler for edgeStyle: %s', (_name, edgeStyle) => { +])('Expect EdgeHandler for edgeStyle: %s', (_name, edgeStyle) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/__tests__/view/Graph.test.ts(1 hunks)packages/core/src/view/Graph.ts(3 hunks)
🔇 Additional comments (10)
packages/core/__tests__/view/Graph.test.ts (6)
31-36: Well-structured test for orthogonal property check in CellState.The test correctly validates that a CellState with the orthogonal property set to true returns true when passed to graph.isOrthogonal(). This properly tests the first condition in the isOrthogonal method.
38-41: Good edge case test for undefined style.Testing with no style in the CellState is an important edge case to validate the method returns false when no style is defined.
43-52: Great use of parameterized testing for edge styles.Using test.each for testing multiple edge styles is efficient and maintains test clarity. The explicit inclusion of
ManhattanConnectorin this test case list validates the PR objective.
54-60: Good negative test case for Loop edge style.Testing that the Loop edge style is not considered orthogonal is important for complete test coverage of the isOrthogonal method's behavior.
63-76: Comprehensive test for ElbowEdgeHandler creation.This test correctly validates that the createEdgeHandler method returns an ElbowEdgeHandler for the specified edge styles, including Loop which was added to this condition in the implementation.
78-90: Proper test for EdgeSegmentHandler creation.The test correctly verifies that ManhattanConnector, OrthogonalConnector, and SegmentConnector result in an EdgeSegmentHandler, which aligns with the implementation changes.
packages/core/src/view/Graph.ts (4)
1027-1033: Added EdgeStyle.Loop to the elbow edge handler condition.The Loop edge style is now properly included in the condition that creates an ElbowEdgeHandler. This is a good improvement to the code organization.
1034-1039: ManhattanConnector now correctly recognized for EdgeSegmentHandler.The ManhattanConnector is now properly included in the condition for creating an EdgeSegmentHandler, which fulfills the PR objective to consider ManhattanConnector as orthogonal.
1229-1233: Improved null check using isNullish utility.Replacing the direct null check with the isNullish utility function is a good practice that improves code consistency and handles both null and undefined values.
1235-1249: Enhanced orthogonal edge style detection with ManhattanConnector.The isOrthogonal method now properly includes ManhattanConnector in the list of orthogonal edge styles. The refactoring to use an array includes check improves code readability.
|
Currently, the implementation of a connector must be explicitly referenced in the code to decide whether it is considered orthogonal.
This is not configurable.
So, when a new connector is added, its reference must be added. This was not done when
ManhattanConnectorwas introduced.ManhattanConnector is now referenced in
Graph.isOrthogonalandGraph.createEdgeHandler.Tests have been added to validate the correction and a refactoring of the implementation to simplify it.
Summary by CodeRabbit
isOrthogonalandcreateEdgeHandlermethods, covering various edge cases to ensure robust performance and reliability.