refactor: migrate the FixedPoints story to TypeScript #839
Conversation
WalkthroughA new Storybook MDX documentation file for "Fixed Points" was added. The corresponding story implementation was refactored for improved type safety and modularity, introducing stricter TypeScript typings, enhanced handler overrides, and updated edge and vertex setup. The code now uses custom handlers and plugins for connection constraints and edge management. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Storybook
participant Graph
participant CustomConnectionHandler
participant CustomConstraintHandler
participant CustomElbowEdgeHandler
User->>Storybook: Select "Fixed Points" story
Storybook->>Graph: Instantiate with plugins (handlers)
Graph->>CustomConnectionHandler: Use for connection logic
CustomConnectionHandler->>CustomConstraintHandler: Create for constraint checks
CustomConnectionHandler->>CustomElbowEdgeHandler: Create for edge handling
Graph->>CustomElbowEdgeHandler: Delegate edge handling
CustomElbowEdgeHandler->>CustomConstraintHandler: Use for constraint checks
User->>Graph: Interact (e.g., connect edges)
Graph->>CustomConnectionHandler: Handle connections (with fixed points)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use ✨ Finishing Touches
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
This eases the maintenance and allows to detect errors earlier. Also, correctly display the crosses related to the connection constraints when hovering on a vertex.
tbouffard
force-pushed
the
refactor/story_fixedPoints_migrate_to_ts
branch
from
401cfa6 to
1024412
Compare
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/html/stories/FixedPoints.stories.ts (3)
65-80: Enhanced type safety in constraint handler! ✅Good improvements with proper type annotations and the override keyword. The null-safe check aligns with the parent class implementation.
Consider using a safer null check pattern if there's any possibility of
icon.boundsbeing null:- mathUtils.intersects(icon.bounds!, rectangle) + (icon.bounds && mathUtils.intersects(icon.bounds, rectangle))
154-160: Implementation works but has acknowledged limitations.The forced use of
CustomElbowEdgeHandlerfor all edges might be restrictive. Good that this is documented for future improvement in PR #823.Consider adding a TODO comment with more context about what specific improvements are planned.
246-303: Consider modernizing the commented alternative implementation.The prototype modification approach in the commented code is outdated and could cause issues. When implementing this feature with Storybook args, consider using proper class extension or composition patterns instead of prototype modification.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/html/stories/FixedPoints.mdx(1 hunks)packages/html/stories/FixedPoints.stories.ts(6 hunks)
🔇 Additional comments (8)
packages/html/stories/FixedPoints.stories.ts (7)
18-45: Well-structured imports with proper TypeScript types! 👍The addition of type imports and the new
configureImagesBasePathimport enhances type safety and maintainability.
61-63: Good addition of type safety and configuration setup!The type annotation for the
argsparameter and the call toconfigureImagesBasePath()ensure proper initialization.
141-152: Clean implementation of custom edge handler!Well-structured class that properly extends
ElbowEdgeHandlerand maintains consistency by delegatingisConnectableCellto the ConnectionHandler plugin.
161-179: Well-defined fixed connection points!The implementation provides a comprehensive set of 8 connection points (corners and midpoints) for vertices, which perfectly demonstrates the "Fixed Points" concept.
🧰 Tools
🪛 Biome (1.9.4)
[error] 165-165: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
182-193: Excellent modular plugin configuration!The plugins array pattern improves code organization and the conditional RubberBandHandler addition enables dynamic configuration through Storybook args.
216-244: Great improvements to edge configuration!Using boolean values instead of numeric (0/1) for
exitPerimeter,entryPerimeter, andorthogonalproperties improves code readability and type safety.
82-102:❓ Verification inconclusive
Type safety improvements look good, but verify the ignored parameter behavior.
The enhanced type annotations improve code maintainability. However, the comment about the ignored 2nd parameter in
getAllConnectionConstraintssuggests a potential API mismatch.Verify that ignoring the 2nd parameter doesn't cause issues:
🏁 Script executed:
#!/bin/bash # Check the getAllConnectionConstraints signature in the base class ast-grep --pattern $'getAllConnectionConstraints($_,$_)'Length of output: 57
Unable to locate
getAllConnectionConstraintssignature – please verify its API
We couldn’t find a definition forgetAllConnectionConstraintsin the repo, so it’s unclear whether the second (boolean) parameter can safely be ignored. Ensure that your custom handler’s assumption matches the actual Graph implementation you’re extending.packages/html/stories/FixedPoints.mdx (1)
1-9: Clear and concise documentation!The MDX file provides appropriate documentation for the Fixed Points story with a clear title and description.
1 participant
This eases the maintenance and allows to detect errors earlier.
Also, correctly display the crosses related to the connection constraints when hovering on a vertex.
Notes
Depends on #838
See also #840
Summary by CodeRabbit
New Features
Refactor