refactor: migrate the Control story to TypeScript - #821
Conversation
Sorry, something went wrong.
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 (Control.mdx) and its corresponding story implementation (Control.stories.ts) were added to demonstrate interactive controls within a graph, such as deleting vertices. The implementation introduces custom graph and cell renderer classes, enhanced typing, plugin-based initialization, and expanded UI controls. An existing MDX file was also updated to clean up unused imports. 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 labelsdocumentation Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review details Configuration used: CodeRabbit UI Reviewing files that changed from the base of the PR and between 3056c25 and 9f38877. 📒 Files selected for processing (1)
packages/core/src/types.ts (1)🔇 Additional comments (15) packages/html/stories/Control.stories.ts (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. ❤️ Share 🪧 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 (.coderabbit.yaml)
Documentation and Community
|
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)packages/html/stories/Control.stories.ts (2)🧹 Nitpick comments (3)75-96: 🛠️ Refactor suggestion
Avoid narrowing the parameter type in overridden methods
CellRenderer.createControl expects a CellState.
By overriding it with createControl(state: CustomCellState) you’re narrowing the accepted type, which breaks the Liskov Substitution Principle and will fail to compile if --exactOptionalPropertyTypes or 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 issue
Possible null dereference when comparing bounds
getDeleteControlBounds may return null, yet bounds!.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(); }
packages/html/stories/Control.mdx (1)📜 Review detailspackages/html/stories/Control.stories.ts (2)1-15: Minor MDX nit – add a trailing empty line
Storybook occasionally complains when an MDX document doesn’t end with a newline (depends on editor settings and OS).
@@ -<Controls /> +<Controls /> +161-164: Don’t mutate the array returned by getDefaultPlugins
getDefaultPlugins() may decide to cache and reuse its array in the future.
Instead of mutating it with push, 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
label is 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>) => {
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Reviewing files that changed from the base of the PR and between c0c4c41 and 3056c25.
📒 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)
packages/core/src/types.ts (1)⏰ Context from checks skipped due to timeout of 90000ms (4)packages/core/src/view/plugins/index.ts (1)
- GraphPluginConstructor (1172-1175)
packages/html/stories/Layers.stories.js (1)
- getDefaultPlugins (46-55)
- v3 (63-63)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (windows-2022)
- GitHub Check: build (macos-14)
- GitHub Check: build
packages/html/stories/Boundary.mdx (1)1-1: Import clean-up looks good
Removing the unused ArgTypes and Story imports keeps the file lean and avoids false-positive linter warnings.
Sorry, something went wrong.
|
Sorry, something went wrong.
Ease the maintenance and detect the errors earlier.
Extra improvements:
Summary by CodeRabbit
New Features
Documentation
Refactor