refactor(stories): migrate PortRefs to TypeScript - #1129
Merged
Conversation
Port the story to TypeScript and drop the four global prototype mutations it relied on, which leaked into every other story sharing the same classes: - Shape.prototype.getPorts and TriangleShape.prototype.getPorts become a PortRefsCellRenderer that reads a `ports` key from the cell style, so the port sets stay attached to the cells that declare them. - ConstraintHandler.prototype.pointImage becomes a MyCustomConstraintHandler, injected through a custom ConnectionHandler and an explicit plugin list. - EdgeHandler.prototype.isConnectableCell becomes a CustomEdgeHandler, returned by the graph createEdgeHandler hook. Fix two defects found while porting: the triangle 'out1' port sat at the middle of the flat top edge instead of the top right vertex its 'north east' constraint announces, and the port id was bolted onto ConnectionConstraint through an `as any` cast while the class already has a typed `name` field for it. Add a description above the graph explaining what the story demonstrates, use optional chaining instead of the nested null checks in the port lookups, and insert the cells with the object parameter form.
WalkthroughThe PortRefs Storybook story was migrated from JavaScript to TypeScript. The new story preserves port metadata, custom connection handling, sample graph setup, and the exported ChangesPortRefs Story Migration
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/html/stories/PortRefs.stories.ts (1)
64-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a typed story-arguments contract.
Record<string, string>makes the sharedrubberBand,width, andheightargs sound like strings even though they are booleans and numbers, and the story does not providelabel. Give the template the actual story argument shape, or typecreateGraphContainer’s args contract if this type is shared.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 94249578-fc22-476c-8b7c-3fb55fe83026
📒 Files selected for processing (2)
packages/html/stories/PortRefs.stories.jspackages/html/stories/PortRefs.stories.ts
💤 Files with no reviewable changes (1)
- packages/html/stories/PortRefs.stories.js
28 tasks
21 tasks
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
1 participant
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.
Overview
Migrates the
PortRefsStorybook story from JavaScript to TypeScript, and removes the four global prototype mutations it relied on.Those mutations were the main problem:
Shape.prototype,TriangleShape.prototype,ConstraintHandler.prototypeandEdgeHandler.prototypeare shared by every story running in the same Storybook session, so the port definitions and the port image leaked out of this story into the others. They are replaced by per-instance constructs:Shape.prototype.getPorts/TriangleShape.prototype.getPortsPortRefsCellRendererreading aportskey from the cell styleConstraintHandler.prototype.pointImageMyCustomConstraintHandler, injected through a customConnectionHandlerand an explicit plugin listEdgeHandler.prototype.isConnectableCellCustomEdgeHandler, returned by thecreateEdgeHandlerhook of the graphTwo defects found while porting are fixed as well:
out1port sat at the middle of the flat top edge instead of the top right vertex that itsnorth eastconstraint announcesConnectionConstraintthrough anas anycast, while the class already exposes a typednamefield designed for exactly thatThe story also gains a description above the graph explaining what it demonstrates, optional chaining instead of the nested null checks in the port lookups, and the object parameter form of
insertVertex/insertEdge(see #856).Notes
Covers #418 and #1035.
The behaviour of the story is unchanged. Validated manually in Storybook: connecting through ports, reconnecting an edge terminal, and the port image on hover.
This PR is deliberately limited to the story. It is extracted from a larger branch that moves the cell handler factories out of
AbstractGraphinto theSelectionCellsHandlerplugin (#762). Once this is merged, the only remaining difference on this file in that branch is the edge handler wiring, which switches from thecreateEdgeHandlerhook used here to the new plugin factory.Summary by CodeRabbit