refactor: migrate the Control story to TypeScript - #821
Conversation
Ease the maintenance and detect the errors earlier. Extra improvements: - Disable the context menu to make the panning correctly work - Add a "reset zoom" button - Add more cells to better illustrate the feature - Add a Storybook argument to enable Graph.resizeContainer that was previously commented - Add documentation
WalkthroughA new Storybook MDX story ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Storybook
participant MyCustomGraph
participant MyCustomCellRenderer
User->>Storybook: Opens Control story
Storybook->>MyCustomGraph: Initialize with container and plugins
MyCustomGraph->>MyCustomCellRenderer: createCellRenderer()
MyCustomGraph->>MyCustomCellRenderer: Render cells with controls
User->>MyCustomCellRenderer: Clicks delete control on vertex
MyCustomCellRenderer->>MyCustomGraph: Remove vertex from graph
User->>Storybook: Uses UI buttons (Zoom In/Out, Reset Zoom)
Storybook->>MyCustomGraph: Adjust graph view accordingly
Possibly related PRs
Suggested labels
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)packages/html/stories/Control.stories.ts (3)
🔇 Additional comments (15)
✨ 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
packages/html/stories/Control.stories.ts (2)
75-96: 🛠️ Refactor suggestionAvoid narrowing the parameter type in overridden methods
CellRenderer.createControlexpects aCellState.
By overriding it withcreateControl(state: CustomCellState)you’re narrowing the accepted type, which breaks the Liskov Substitution Principle and will fail to compile if--exactOptionalPropertyTypesor stricter variance checks are enabled in the consuming project.Change the signature to accept the original super-type and down-cast internally:
-override createControl(state: CustomCellState) { +override createControl(state: CellState) { + const customState = state as CustomCellState; super.createControl(state); - const { graph } = state.view; + const { graph } = customState.view; …
121-136:⚠️ Potential issuePossible null dereference when comparing bounds
getDeleteControlBoundsmay returnnull, yetbounds!.equals(...)is called unconditionally, which will throw at runtime.-const bounds = this.getDeleteControlBounds(state); -if ( - state.deleteControl.scale !== s || - !state.deleteControl.bounds!.equals(bounds) -) { +const bounds = this.getDeleteControlBounds(state); +if ( + bounds && // ✅ guard + (state.deleteControl.scale !== s || + !state.deleteControl.bounds!.equals(bounds)) +) { state.deleteControl.bounds = bounds; state.deleteControl.scale = s; state.deleteControl.redraw(); }
🧹 Nitpick comments (3)
packages/html/stories/Control.mdx (1)
1-15: Minor MDX nit – add a trailing empty lineStorybook occasionally complains when an MDX document doesn’t end with a newline (depends on editor settings and OS).
@@ -<Controls /> +<Controls /> +packages/html/stories/Control.stories.ts (2)
161-164: Don’t mutate the array returned bygetDefaultPlugins
getDefaultPlugins()may decide to cache and reuse its array in the future.
Instead of mutating it withpush, create a shallow copy before extending:-const plugins = getDefaultPlugins(); -if (args.rubberBand) plugins.push(RubberBandHandler); +const plugins = [ + ...getDefaultPlugins(), + ...(args.rubberBand ? [RubberBandHandler] : []), +];
65-65: Unused destructured property
labelis destructured from the args but never used, producing a lint warning.-const Template = ({ label, ...args }: Record<string, any>) => { +const Template = ({ /* label */, ...args }: Record<string, any>) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/html/stories/Boundary.mdx(1 hunks)packages/html/stories/Control.mdx(1 hunks)packages/html/stories/Control.stories.ts(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/html/stories/Control.stories.ts (3)
packages/core/src/types.ts (1)
GraphPluginConstructor(1172-1175)packages/core/src/view/plugins/index.ts (1)
getDefaultPlugins(46-55)packages/html/stories/Layers.stories.js (1)
v3(63-63)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (windows-2022)
- GitHub Check: build (macos-14)
- GitHub Check: build
🔇 Additional comments (1)
packages/html/stories/Boundary.mdx (1)
1-1: Import clean-up looks goodRemoving the unused
ArgTypesandStoryimports keeps the file lean and avoids false-positive linter warnings.
|
Ease the maintenance and detect the errors earlier.
Extra improvements:
Summary by CodeRabbit
New Features
Documentation
Refactor