-
Notifications
You must be signed in to change notification settings - Fork 199
Description
Is your feature request related to a problem? Please describe
Currently, the Graph.defaultLoopStyle property directly references the EdgeStyle.Loop implementation. This hardcoded link limits flexibility and makes tree shaking impossible for users who do not rely on loop edges in their application.
Warning
This request assumes that missing a loop edge style won't break rendering. This needs to be verified when stories covering loop behavior are implemented (see the related issue's task for that check).
Describe the solution you'd like
We’d like to make Graph.defaultLoopStyle accept a string name referring to a registered edge style, not the direct function reference. For example: "loopEdgeStyle" instead of EdgeStyle.Loop.
- It should support both a string or a function (like other edge-related props).
- The default value can stay
"loopEdgeStyle", to ensure backward compatibility. - This change would reduce unnecessary bundle size for apps that don’t use loops.
Notes
- this will introduce a breaking change for people relying on the default loop implementation
- now, they will have to be sure that the loop edge style is registered when using BaseGraph
- when using Graph, there will be no change as the loop edge style is registered in that case
Describe alternatives you've considered
Keeping the function reference in place hardcodes the dependency and prevents tree shaking.
This is not the same as the default edge, vertex, and label shapes defined in CellRenderer. Those are critical default implementations: without defaultVertexShape, defaultEdgeShape, or defaultTextShape, cells wouldn’t render at all.
These are the only concrete shape implementations that should be part of the codebase. Everything else - including loop styles - should ideally be configurable through registration.
defaultVertexShape = RectangleShapedefaultEdgeShape = ConnectorShapedefaultTextShape = TextShape
But unlike these core shapes, the loop edge style is not essential and shouldn’t be hardcoded.
Additional context
- Allow string values for
Graph.defaultLoopStyle. This would make the loop style consistent with other configurable elements via registration.
Graph.defaultLoopStyle = "loopEdgeStyle";
// OR
Graph.defaultLoopStyle = EdgeStyle.Loop;- We should also:
- Add a Storybook story showing different loop configurations:
- default loop from
Graph - custom style with
loopStyle - custom style with
noLoop - custom style with
orthogonalLoop
- default loop from
⚠️ Fix mismatch in theCellStateStyle.orthogonalLoopJSDoc: currently mentionsOrthConnector, but it's not used. Either the code or the JSDoc should be updated to reflect the actual implementation.
- Add a Storybook story showing different loop configurations:
Important
Be aware that it is planned to move Graph.defaultLoopStyle to GraphView.defaultLoopStyle as part of #762