refactor: migrate the Stylesheet story to TypeScript #878

tbouffard · 2025-07-20T15:00:27Z

This eases the maintenance and detects error earlier. Also use the insert cells methods taken a single parameter, as the legacy methods with several parameters will be deprecated in the future.

In addition:

AbstractGraph.getLabel now accepts undefined or null Cell (the implementation already managed it)
SelectionCellsHandler: simplify implementation

Notes

Covers #640
Covers #856

Summary by CodeRabbit

Documentation
- Updated example comments to clarify usage and improve type references.
- Enhanced JSDoc comments for clarity and accurate default values.
Refactor
- Simplified conditional checks for label retrieval.
- Improved method overrides for label handling.
- Switched to object-literal arguments for vertex and edge insertion.
- Converted style definitions to use TypeScript types.
New Features
- Broadened label retrieval method to accept optional or null cell arguments.
Style
- Improved explanatory text and code readability in story examples.

This eases the maintenance and detects error earlier. Also use the insert cells methods taken a single parameter, as the legacy methods with several parameters will be deprecated in the future. In addition: - AbstractGraph.getLabel now accepts `undefined` or `null` Cell (the implementation already managed it) - SelectionCellsHandler: simplify implementation

coderabbitai · 2025-07-20T15:00:34Z

Walkthrough

The changes update documentation comments, type signatures, and code style across several files. TypeScript typings are improved, argument handling is made more robust, and documentation is clarified. No core logic or control flow is altered, except for minor refinements in parameter handling and method signatures to increase type safety and clarity.

Changes

File(s)	Change Summary
packages/core/src/view/geometry/Geometry.ts	Updated example usage comments to reference `Point` instead of `mxPoint`.
packages/core/src/view/mixins/EdgeMixin.ts	Refactored to use non-null assertion for `geometry` property; updated comment for clarity.
packages/core/src/view/mixins/LabelMixin.ts	Changed `getLabel` method to use a truthiness check for `cell` instead of explicit `!= null` check.
packages/core/src/view/mixins/LabelMixin.type.ts	Updated `getLabel` method signature to accept an optional `Cell` or `null` parameter.
packages/core/src/view/plugins/SelectionCellsHandler.ts	Removed unused parameters from `refreshHandler`; improved JSDoc comments for properties and methods.
packages/html/stories/Stylesheet.stories.ts	Improved TypeScript typings, refactored argument passing, enhanced null safety, clarified comments, and updated return value of `Template`.

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

npm error Exit handler never called!
npm error This is an error with npm itself. Please report this error at:
npm error https://github.com/npm/cli/issues
npm error A complete log of this run can be found in: /.npm/_logs/2025-07-20T15_02_03_503Z-debug-0.log

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3402cff and fe84846.

📒 Files selected for processing (6)

packages/core/src/view/geometry/Geometry.ts (1 hunks)
packages/core/src/view/mixins/EdgeMixin.ts (1 hunks)
packages/core/src/view/mixins/LabelMixin.ts (1 hunks)
packages/core/src/view/mixins/LabelMixin.type.ts (1 hunks)
packages/core/src/view/plugins/SelectionCellsHandler.ts (4 hunks)
packages/html/stories/Stylesheet.stories.ts (8 hunks)

🧰 Additional context used

🧠 Learnings (6)

📓 Common learnings

Learnt from: tbouffard
PR: maxGraph/maxGraph#776
File: packages/core/src/view/AbstractGraph.ts:492-495
Timestamp: 2025-04-24T12:34:56.726Z
Learning: PR #776 focuses on refactoring to introduce AbstractGraph as a base class and moving code from Graph to AbstractGraph. Type improvements and other refinements should be addressed in future PRs to keep this one focused on the core architectural change.

Learnt from: tbouffard
PR: maxGraph/maxGraph#857
File: packages/core/src/view/plugins/SelectionHandler.ts:1081-1092
Timestamp: 2025-06-26T13:27:19.012Z
Learning: User tbouffard prefers to keep PRs focused on their main objective and defer implementation improvements or logic fixes to future dedicated PRs. For maxGraph, refactoring PRs like Dictionary→Map migration should not include unrelated logic improvements even when valid issues are identified in the migrated code.

Learnt from: tbouffard
PR: maxGraph/maxGraph#835
File: packages/html/stories/Orthogonal.stories.ts:114-117
Timestamp: 2025-05-26T12:34:54.318Z
Learning: The `null!` assertion pattern exists across multiple stories in the TypeScript migration and should be addressed systematically rather than in individual PRs.

Learnt from: tbouffard
PR: maxGraph/maxGraph#740
File: packages/core/src/util/requestUtils.ts:79-106
Timestamp: 2025-03-31T08:32:17.055Z
Learning: PR #740 is focused on reorganizing code into namespaces (guiUtils and requestUtils) without changing any implementations. Implementation improvements should be suggested for future PRs.

Learnt from: tbouffard
PR: maxGraph/maxGraph#593
File: .github/workflows/create-github-release.yml:42-42
Timestamp: 2025-01-28T16:22:25.804Z
Learning: In the GitHub release notes template for maxGraph, TODO comments are intentionally included to mark sections that must be reviewed by maintainers before publishing the release. These TODOs should not be removed or converted to validation steps as they are part of the release review process.

Learnt from: tbouffard
PR: maxGraph/maxGraph#774
File: packages/ts-example-selected-features/package.json:11-13
Timestamp: 2025-04-22T16:34:40.309Z
Learning: In the maxGraph project, TypeScript is defined as a dependency in the root package.json (~5.8.2) and not in individual package.json files for example packages. The build scripts in example packages can use `tsc` commands because the TypeScript dependency is hoisted in the monorepo structure.

Learnt from: tbouffard
PR: maxGraph/maxGraph#715
File: packages/html/stories/ZoomAndFit.stories.ts:0-0
Timestamp: 2025-03-14T06:01:22.326Z
Learning: For future improvements to the Graph component in maxGraph, tbouffard is interested in refactoring the `fit` method to use an object parameter pattern instead of multiple positional parameters, as the current approach makes the API harder to use when many parameters need to be specified with default values.

Learnt from: tbouffard
PR: maxGraph/maxGraph#770
File: packages/core/src/view/cell/CellTracker.ts:99-101
Timestamp: 2025-04-18T07:02:31.931Z
Learning: In TypeScript, when implementing an interface, methods can have fewer parameters than declared in the interface due to TypeScript's structural typing system. Both omitting unused parameters entirely and prefixing them with underscore (e.g., _sender) are valid patterns in the maxGraph codebase.

Learnt from: tbouffard
PR: maxGraph/maxGraph#582
File: packages/core/__tests__/view/handler/config.test.ts:0-0
Timestamp: 2024-11-24T17:28:08.023Z
Learning: In `packages/core/__tests__/view/handler/config.test.ts`, when writing Jest tests in TypeScript for configuration reset functions, small duplication is acceptable if it maintains test clarity and readability. Avoid refactoring tests in ways that could make them harder to understand, especially when future use cases might require specific checks.

Learnt from: tbouffard
PR: maxGraph/maxGraph#849
File: packages/html/stories/DragSource.stories.js:98-101
Timestamp: 2025-06-13T07:48:10.300Z
Learning: User tbouffard prefers answers in English; avoid switching to other languages in future replies.

packages/core/src/view/geometry/Geometry.ts (2)

Learnt from: tbouffard
PR: maxGraph/maxGraph#785
File: packages/core/src/view/style/register.ts:19-19
Timestamp: 2025-04-28T08:24:39.831Z
Learning: In the maxGraph project, the Perimeter namespace is defined in the 'builtin-style-elements.ts' file using the statement 'export * as Perimeter from './perimeter';'. To access the Perimeter namespace in other files, they must import from './builtin-style-elements' instead of directly from './perimeter', as part of the tree-shaking optimization.

Learnt from: tbouffard
PR: maxGraph/maxGraph#785
File: packages/core/src/view/style/register.ts:19-19
Timestamp: 2025-04-28T08:24:39.831Z
Learning: In the maxGraph project, the Perimeter namespace is defined in the './builtin-style-elements' file as a namespace export ('export * as Perimeter'), making this the correct import location for accessing the Perimeter namespace.

packages/core/src/view/mixins/EdgeMixin.ts (1)

Learnt from: tbouffard
PR: maxGraph/maxGraph#791
File: packages/website/docs/usage/edge-styles.md:59-63
Timestamp: 2025-04-29T13:30:18.686Z
Learning: In the EdgeStyles documentation, tbouffard prefers the existing text "A perimeter is a function matching the `EdgeStyleFunction` type" over suggestions that might create redundancy, even if terminology might be technically misaligned.

packages/core/src/view/mixins/LabelMixin.type.ts (1)

Learnt from: tbouffard
PR: maxGraph/maxGraph#770
File: packages/core/src/view/cell/CellTracker.ts:99-101
Timestamp: 2025-04-18T07:02:31.931Z
Learning: In TypeScript, when implementing an interface, methods can have fewer parameters than declared in the interface due to TypeScript's structural typing system. Both omitting unused parameters entirely and prefixing them with underscore (e.g., _sender) are valid patterns in the maxGraph codebase.

packages/core/src/view/plugins/SelectionCellsHandler.ts (1)

Learnt from: tbouffard
PR: maxGraph/maxGraph#770
File: packages/core/src/view/cell/CellTracker.ts:99-101
Timestamp: 2025-04-18T07:02:31.931Z
Learning: In TypeScript, when implementing an interface, methods can have fewer parameters than declared in the interface due to TypeScript's structural typing system. Both omitting unused parameters entirely and prefixing them with underscore (e.g., _sender) are valid patterns in the maxGraph codebase.

packages/html/stories/Stylesheet.stories.ts (14)

Learnt from: tbouffard
PR: maxGraph/maxGraph#791
File: packages/core/src/view/style/edge/Segment.ts:26-35
Timestamp: 2025-04-29T13:29:45.122Z
Learning: In the maxGraph project, JSDoc `{@link}` references to symbols don't need to be imported in the current file to work correctly. TypeDoc can resolve symbols across the entire project namespace, not just within the current file, which is common in modern documentation systems and IDEs.

Learnt from: tbouffard
PR: maxGraph/maxGraph#791
File: packages/website/docs/usage/edge-styles.md:59-63
Timestamp: 2025-04-29T13:30:18.686Z
Learning: In the EdgeStyles documentation, tbouffard prefers the existing text "A perimeter is a function matching the `EdgeStyleFunction` type" over suggestions that might create redundancy, even if terminology might be technically misaligned.

Learnt from: tbouffard
PR: maxGraph/maxGraph#785
File: packages/core/src/view/style/register.ts:19-19
Timestamp: 2025-04-28T08:24:39.831Z
Learning: In the maxGraph project, the Perimeter namespace is defined in the 'builtin-style-elements.ts' file using the statement 'export * as Perimeter from './perimeter';'. To access the Perimeter namespace in other files, they must import from './builtin-style-elements' instead of directly from './perimeter', as part of the tree-shaking optimization.

Learnt from: tbouffard
PR: maxGraph/maxGraph#770
File: packages/core/src/view/cell/CellTracker.ts:99-101
Timestamp: 2025-04-18T07:02:31.931Z
Learning: In TypeScript, when implementing an interface, methods can have fewer parameters than declared in the interface due to TypeScript's structural typing system. Both omitting unused parameters entirely and prefixing them with underscore (e.g., _sender) are valid patterns in the maxGraph codebase.

Learnt from: tbouffard
PR: maxGraph/maxGraph#791
File: packages/website/docs/usage/edge-styles.md:59-63
Timestamp: 2025-04-29T13:32:14.572Z
Learning: In the EdgeStyles documentation, the sentence "A perimeter is a function matching the `EdgeStyleFunction` type" contains an incorrect term - "perimeter" should be replaced with "EdgeStyle" or "custom EdgeStyle" as the document is specifically about EdgeStyles.

Learnt from: tbouffard
PR: maxGraph/maxGraph#720
File: packages/html/stories/Wrapping.stories.ts:37-37
Timestamp: 2025-03-19T06:56:13.709Z
Learning: In Storybook stories for maxGraph, arguments are typed as strings (Record<string, string>) even when they represent boolean values in the argTypes definition. Boolean conversions like Boolean(args.someFlag) are used to convert string representations to actual boolean values.

Learnt from: tbouffard
PR: maxGraph/maxGraph#598
File: packages/website/docs/manual/getting-started.md:70-70
Timestamp: 2024-12-15T18:19:56.236Z
Learning: In code examples within the documentation, such as in `packages/website/docs/manual/getting-started.md`, we assume that the `graph-container` element exists and is an `HTMLElement`, and we avoid adding error handling for its initialization to keep the code simple.

Learnt from: tbouffard
PR: maxGraph/maxGraph#593
File: .github/workflows/create-github-release.yml:42-42
Timestamp: 2025-01-28T16:22:25.804Z
Learning: In the GitHub release notes template for maxGraph, TODO comments are intentionally included to mark sections that must be reviewed by maintainers before publishing the release. These TODOs should not be removed or converted to validation steps as they are part of the release review process.

Learnt from: tbouffard
PR: maxGraph/maxGraph#835
File: packages/html/stories/Orthogonal.stories.ts:114-117
Timestamp: 2025-05-26T12:34:54.318Z
Learning: The `null!` assertion pattern exists across multiple stories in the TypeScript migration and should be addressed systematically rather than in individual PRs.

Learnt from: tbouffard
PR: maxGraph/maxGraph#785
File: packages/core/src/view/style/register.ts:19-19
Timestamp: 2025-04-28T08:24:39.831Z
Learning: In the maxGraph project, the Perimeter namespace is defined in the './builtin-style-elements' file as a namespace export ('export * as Perimeter'), making this the correct import location for accessing the Perimeter namespace.

Learnt from: tbouffard
PR: maxGraph/maxGraph#618
File: packages/website/docs/tutorials/graph.md:56-59
Timestamp: 2025-01-06T09:09:47.410Z
Learning: In the maxGraph library, the classes "Cell" and "GraphDataModel" are valid, while "GraphCell" and "GraphModel" do not exist.

Learnt from: tbouffard
PR: maxGraph/maxGraph#776
File: packages/core/src/view/Graph.ts:85-92
Timestamp: 2025-04-24T12:34:10.366Z
Learning: The Graph class intentionally doesn't use collaborators (cellRenderer, selectionModel, view) from options in initializeCollaborators, as it's designed to replicate the previous implementation using only its factory methods for backward compatibility.

Learnt from: tbouffard
PR: maxGraph/maxGraph#776
File: packages/core/src/view/Graph.ts:77-83
Timestamp: 2025-04-24T12:33:36.243Z
Learning: All register functions in maxGraph (`registerDefaultShapes`, `registerDefaultEdgeStyles`, `registerDefaultPerimeters`, and `registerDefaultEdgeMarkers`) have internal state flags to prevent duplicate registrations, making them inherently idempotent.

Learnt from: tbouffard
PR: maxGraph/maxGraph#826
File: packages/js-example-nodejs/src/index.cjs:64-69
Timestamp: 2025-05-13T12:54:55.231Z
Learning: For example code in the maxGraph repository, maintainers prefer to keep scripts simple without error handling to focus on demonstrating core functionality, especially in demonstration scripts like those in packages/js-example-nodejs.

🧬 Code Graph Analysis (1)

packages/html/stories/Stylesheet.stories.ts (2)

packages/html/stories/shared/configure.js (1)

createMainDiv (53-63)

packages/core/src/types.ts (1)

CellStyle (50-75)

🔇 Additional comments (11)

packages/core/src/view/geometry/Geometry.ts (1)

39-41: Documentation update looks good.

The update from mxPoint to Point in the example code correctly reflects the current class naming in the TypeScript codebase.

packages/core/src/view/mixins/LabelMixin.type.ts (1)

74-74: Type signature improvement aligns with implementation.

Making the cell parameter optional and nullable improves API flexibility and matches the updated implementation in LabelMixin.ts that handles falsy values gracefully.

packages/core/src/view/mixins/EdgeMixin.ts (1)

261-261: Good improvement in type assertion usage.

Replacing the type assertion with a non-null assertion is more appropriate here, and the explanatory comment clearly justifies why the assertion is safe.

packages/core/src/view/plugins/SelectionCellsHandler.ts (2)

60-64: Good simplification of the refreshHandler function.

Removing the unused parameters makes the code cleaner and more readable, which aligns with TypeScript best practices.

83-84: Documentation improvements enhance code clarity.

The addition of @default tags and improved JSDoc formatting makes the API documentation more consistent and informative.

Also applies to: 94-95, 105-105, 112-112

packages/core/src/view/mixins/LabelMixin.ts (1)

47-47: Condition simplification improves robustness.

The change from cell != null to cell makes the method more defensive by handling all falsy values, which aligns with the updated optional parameter type signature.

packages/html/stories/Stylesheet.stories.ts (5)

18-18: LGTM! Import updates support TypeScript migration.

The addition of type CellStyle import enables proper typing of style objects, and the createMainDiv import supports the enhanced template structure with descriptive text.

Also applies to: 20-20

32-39: LGTM! Template function improvements enhance type safety and user experience.

The TypeScript typing for parameters and the restructured template using createMainDiv with descriptive content align well with the migration objectives. The function now provides better type safety and more informative documentation for story viewers.

48-56: LGTM! Safer getLabel method override with improved null handling.

The changes improve type safety by using optional chaining (cell?.isEdge()) and storing the original method reference with a descriptive name. This aligns with the core API changes that now allow the getLabel method to accept optional or null Cell parameters.

72-72: LGTM! Explicit CellStyle typing improves type safety.

Adding explicit CellStyle typing to style objects provides compile-time validation, prevents property assignment errors, and makes the code more self-documenting. This directly supports the PR objective of enabling earlier error detection through TypeScript migration.

Also applies to: 88-88, 101-101, 109-109

124-175: LGTM! API modernization with object parameters improves readability.

The refactoring of insert cell methods to use explicit object parameters instead of multiple positional parameters significantly improves code readability and maintainability. The non-null assertions on geometry access (e1.getGeometry()!.points = ...) are appropriate since geometry is guaranteed to exist after cell insertion. This modernization aligns perfectly with the PR objective to update insert cell method usage in preparation for legacy method deprecation.

✨ Finishing Touches

📝 Generate Docstrings

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

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

sonarqubecloud · 2025-07-20T15:01:25Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

tbouffard added the refactor Code refactoring label Jul 20, 2025

tbouffard mentioned this pull request Jul 20, 2025

refactor!: move tooltip methods to TooltipHandler #640

Merged

13 tasks

tbouffard merged commit dd72497 into main Jul 22, 2025
7 checks passed

tbouffard deleted the refactor/migration_stylesheet_story_to_ts branch July 22, 2025 15:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: migrate the Stylesheet story to TypeScript #878

refactor: migrate the Stylesheet story to TypeScript #878

Uh oh!

tbouffard commented Jul 20, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jul 20, 2025 •

edited

Loading

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

sonarqubecloud bot commented Jul 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

refactor: migrate the Stylesheet story to TypeScript #878

refactor: migrate the Stylesheet story to TypeScript #878

Uh oh!

Conversation

tbouffard commented Jul 20, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notes

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jul 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

sonarqubecloud bot commented Jul 20, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

tbouffard commented Jul 20, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jul 20, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)