-
Notifications
You must be signed in to change notification settings - Fork 199
docs: update references to GraphDataModel #641
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Some JSDoc comments previously mention the `Transactions` class which doesn't exist. Use `GraphDataModel` instead. The `Transactions` class had been temporarily introduced during the mxGraph migration, but had been removed prior the release of version 0.1.0.
WalkthroughThis pull request focuses on updating documentation comments across multiple files in the Changes
Possibly related PRs
Suggested Labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/core/src/view/undoable_changes/GeometryChange.ts (1)
45-45: Improve JSDoc formatting while keeping the correct reference.The update to reference
GraphDataModelis correct, but the JSDoc formatting could be improved for better readability.- * Changes the geometry of {@link cell} to {@link previous} using{@link GraphDataModel.geometryForCellChanged}. + * Changes the geometry of {@link cell} to {@link previous} using {@link GraphDataModel.geometryForCellChanged}.packages/core/src/view/undoable_changes/StyleChange.ts (1)
41-44: Consider enhancing the method documentation.While the documentation is technically correct, it could be more helpful to explain the style swapping behavior and its purpose in the undo/redo context.
Consider expanding the documentation like this:
/** - * Changes the style of {@link cell} to {@link previous} using {@link GraphDataModel.styleForCellChanged}. + * Executes the style change by swapping the current and previous styles, enabling undo/redo functionality. + * Uses {@link GraphDataModel.styleForCellChanged} to update the cell's style and returns the previous style + * for future undo operations. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
packages/core/src/serialization/CodecRegistry.ts(1 hunks)packages/core/src/util/Constants.ts(1 hunks)packages/core/src/util/ObjectIdentity.ts(1 hunks)packages/core/src/util/styleUtils.ts(2 hunks)packages/core/src/view/event/EventSource.ts(1 hunks)packages/core/src/view/handler/ConnectionHandler.ts(1 hunks)packages/core/src/view/image/ImageBox.ts(0 hunks)packages/core/src/view/undoable_changes/ChildChange.ts(1 hunks)packages/core/src/view/undoable_changes/CollapseChange.ts(1 hunks)packages/core/src/view/undoable_changes/GeometryChange.ts(1 hunks)packages/core/src/view/undoable_changes/RootChange.ts(1 hunks)packages/core/src/view/undoable_changes/StyleChange.ts(1 hunks)packages/core/src/view/undoable_changes/TerminalChange.ts(1 hunks)packages/core/src/view/undoable_changes/ValueChange.ts(1 hunks)packages/core/src/view/undoable_changes/VisibleChange.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/core/src/view/image/ImageBox.ts
✅ Files skipped from review due to trivial changes (7)
- packages/core/src/view/undoable_changes/ValueChange.ts
- packages/core/src/view/undoable_changes/CollapseChange.ts
- packages/core/src/view/undoable_changes/ChildChange.ts
- packages/core/src/view/undoable_changes/TerminalChange.ts
- packages/core/src/view/undoable_changes/VisibleChange.ts
- packages/core/src/view/undoable_changes/RootChange.ts
- packages/core/src/view/handler/ConnectionHandler.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (windows-2022)
- GitHub Check: build (ubuntu-22.04)
🔇 Additional comments (8)
packages/core/src/view/undoable_changes/StyleChange.ts (1)
41-41: Documentation update looks good!The documentation change correctly references
GraphDataModel.styleForCellChanged, aligning with the current codebase structure and removing the outdated reference to theTransactionsclass.packages/core/src/view/event/EventSource.ts (1)
38-44: Documentation improvements look good!The updated list of known subclasses is well-structured and correctly includes
GraphDataModel, improving documentation clarity.packages/core/src/serialization/CodecRegistry.ts (1)
29-29: Example code update looks good!The example correctly uses
GraphDataModelinstead ofTransactions, maintaining consistency with the current codebase structure.packages/core/src/util/styleUtils.ts (1)
322-322: Parameter documentation updates look good!The @param documentation correctly uses
{@link GraphDataModel}with proper JSDoc syntax, improving type reference clarity.Also applies to: 364-364
packages/core/src/util/Constants.ts (2)
55-57: LGTM! Documentation improvementThe simplified documentation better reflects the constant's role as the source of truth for the object ID field name.
Line range hint
201-202: Consider removing unused constantsThese constants are marked as unused (both in maxGraph and mxGraph). Consider creating a separate PR to remove them and their documentation references.
Let's verify their usage across the codebase:
Also applies to: 209-210
packages/core/src/util/ObjectIdentity.ts (2)
34-35: LGTM! Documentation improvementThe updated documentation now correctly references
IDENTITY_FIELD_NAMEas the source of truth for the default value, which improves maintainability.
Line range hint
1-1: Verify PR objective alignmentThe changes in these files focus on improving documentation consistency for object ID field names, which appears to be different from the PR's stated objective of updating references from
TransactionstoGraphDataModel. Could you clarify if these changes are intentional additions to the PR's scope?Let's check for any remaining references to
Transactionsin these files:🧰 Tools
🪛 Biome (1.9.4)
[error] 31-68: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
Some JSDoc comments previously mention the
Transactionsclass which doesn't exist. UseGraphDataModelinstead.The
Transactionsclass had been temporarily introduced during the mxGraph migration, but had been removed prior the release of version 0.1.0.Summary by CodeRabbit
Documentation
TransactionstoGraphDataModelacross multiple files.Chores
HIGHLIGHT_COLORandCONNECT_TARGET_COLORfor potential future removal.These changes primarily enhance the clarity and accuracy of the project's documentation.