Skip to content

Conversation

tbouffard
Copy link
Member

@tbouffard tbouffard commented Apr 18, 2025

Declare it on classes that must implement it.
This avoids implementing the wrong method signature or omitting a method.

Summary by CodeRabbit

  • Refactor
    • Standardized mouse event handler signatures by marking unused parameters and aligning interface implementations across multiple components.
    • Updated type annotations for improved consistency and interface adherence.
    • Enhanced documentation clarity with updated terminology and formatting.
  • Style
    • Simplified parameter usage and refined comments for better readability.

Declare it on classes that must implement it. This prevents to implement wrong method signature or to miss a method.
@tbouffard tbouffard added the refactor Code refactoring label Apr 18, 2025
Copy link

coderabbitai bot commented Apr 18, 2025

Walkthrough

This update standardizes the implementation of mouse event handler interfaces across multiple classes in the codebase. Several classes now explicitly implement the MouseListenerSet interface, and the method signatures for mouse event handlers (mouseDown, mouseMove, mouseUp) have been updated to clarify parameter usage. Unused parameters are renamed or removed, and type annotations are refined for consistency. Additionally, minor documentation and comment improvements were made, and some import statements were adjusted to use type-only imports. There are no changes to the internal logic, control flow, or exported entity behavior beyond interface and signature clarifications.

Changes

File(s) Change Summary
packages/core/src/editor/Editor.ts Updated type annotations for the _sender parameter from any to EventSource in mouseDown, mouseMove, and mouseUp methods within the insertHandler object. No logic changes.
packages/core/src/view/GraphView.ts Corrected a comment and simplified the mouseDown handler in graph.addMouseListener by removing unused parameters.
packages/core/src/view/cell/CellTracker.ts CellTracker now implements MouseListenerSet. mouseDown and mouseUp methods have no parameters; mouseMove renames the first parameter to _sender.
packages/core/src/view/handler/EdgeHandler.ts
packages/core/src/view/handler/VertexHandler.ts
Both handler classes now implement MouseListenerSet. In mouse event methods, the first parameter is renamed to _sender to indicate it is unused.
packages/core/src/view/other/DragSource.ts Updated documentation comments and type references for clarity and consistency. No logic changes.
packages/core/src/view/other/Outline.ts Outline implements MouseListenerSet. Constructor's second parameter is now optional. Mouse event method parameters updated to _sender. Simplified container check.
packages/core/src/view/other/PanningManager.ts Removed unused parameters from mouseDown, mouseMove, and mouseUp methods in the mouseListener object. No logic changes.
packages/core/src/view/plugins/ConnectionHandler.ts
packages/core/src/view/plugins/RubberBandHandler.ts
packages/core/src/view/plugins/TooltipHandler.ts
These plugin handler classes now implement MouseListenerSet in addition to GraphPlugin. Mouse event handler parameters are renamed to _sender.
packages/core/src/view/plugins/SelectionCellsHandler.ts SelectionCellsHandler now implements both GraphPlugin and MouseListenerSet.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Graph
    participant MouseListenerSet (Handler)
    User->>Graph: Triggers mouse event (down/move/up)
    Graph->>MouseListenerSet: Calls mouseDown/_Move/_Up(_sender, event)
    MouseListenerSet-->>Graph: Handles event (parameter _sender may be unused)
    Graph-->>User: Event processed
Loading

This diagram represents the standardized flow of mouse events from the user through the graph to the handlers implementing the MouseListenerSet interface, reflecting the updated method signatures and interface implementations.

Suggested labels

javascript

Tip

⚡💬 Agentic Chat (Pro Plan, General Availability)
  • We're introducing multi-step agentic chat in review comments and issue comments, within and outside of PR's. This feature enhances review and issue discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments and add commits to existing pull requests.

📜 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 8e8775a and f31dbbd.

📒 Files selected for processing (1)
  • packages/core/src/editor/Editor.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/core/src/editor/Editor.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: build (ubuntu-22.04)
  • GitHub Check: build (windows-2022)

🪧 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.
    • Generate unit testing code for this file.
    • 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 generate unit testing code for this file.
    • @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 generate unit testing code.
    • @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.

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 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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 7cac321 and 35e386f.

📒 Files selected for processing (12)
  • packages/core/src/editor/Editor.ts (2 hunks)
  • packages/core/src/view/GraphView.ts (1 hunks)
  • packages/core/src/view/cell/CellTracker.ts (4 hunks)
  • packages/core/src/view/handler/EdgeHandler.ts (5 hunks)
  • packages/core/src/view/handler/VertexHandler.ts (5 hunks)
  • packages/core/src/view/other/DragSource.ts (7 hunks)
  • packages/core/src/view/other/Outline.ts (5 hunks)
  • packages/core/src/view/other/PanningManager.ts (1 hunks)
  • packages/core/src/view/plugins/ConnectionHandler.ts (5 hunks)
  • packages/core/src/view/plugins/RubberBandHandler.ts (5 hunks)
  • packages/core/src/view/plugins/SelectionCellsHandler.ts (2 hunks)
  • packages/core/src/view/plugins/TooltipHandler.ts (4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
packages/core/src/view/plugins/SelectionCellsHandler.ts (1)
packages/core/src/types.ts (2)
  • GraphPlugin (1109-1111)
  • MouseListenerSet (1135-1139)
packages/core/src/view/handler/EdgeHandler.ts (1)
packages/core/src/types.ts (1)
  • MouseListenerSet (1135-1139)
packages/core/src/view/plugins/RubberBandHandler.ts (1)
packages/core/src/types.ts (2)
  • GraphPlugin (1109-1111)
  • MouseListenerSet (1135-1139)
packages/core/src/view/cell/CellTracker.ts (1)
packages/core/src/types.ts (1)
  • MouseListenerSet (1135-1139)
packages/core/src/view/other/Outline.ts (1)
packages/core/src/types.ts (1)
  • MouseListenerSet (1135-1139)
packages/core/src/view/handler/VertexHandler.ts (1)
packages/core/src/types.ts (1)
  • MouseListenerSet (1135-1139)
packages/core/src/view/plugins/ConnectionHandler.ts (1)
packages/core/src/types.ts (2)
  • GraphPlugin (1109-1111)
  • MouseListenerSet (1135-1139)
packages/core/src/view/plugins/TooltipHandler.ts (1)
packages/core/src/types.ts (2)
  • GraphPlugin (1109-1111)
  • MouseListenerSet (1135-1139)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: build (windows-2022)
  • GitHub Check: build (ubuntu-22.04)
🔇 Additional comments (37)
packages/core/src/view/plugins/SelectionCellsHandler.ts (2)

27-27: Good addition of type-safe interface import.

The explicit type import of MouseListenerSet prepares for the interface implementation, improving code organization and type safety.


51-51: Excellent enhancement to class declaration.

Explicitly implementing the MouseListenerSet interface alongside GraphPlugin provides stronger type checking and better documents the class's capabilities. This ensures the class correctly implements all required mouse handler methods with proper signatures, preventing potential runtime errors.

packages/core/src/view/other/PanningManager.ts (1)

44-56: Clean and effective parameter simplification.

Removing unused parameters from the mouse event handlers improves code clarity while maintaining interface compatibility. This change makes it immediately clear that these methods don't use any event information. The implementation remains unchanged functionally but is now more maintainable and easier to understand.

packages/core/src/view/GraphView.ts (1)

2173-2177: Simplified method signature improves clarity.

Removing unused parameters from the mouseDown handler creates cleaner, more maintainable code by making it immediately obvious the method doesn't use any event parameters. This refactoring maintains interface compatibility while improving code readability.

packages/core/src/editor/Editor.ts (1)

1628-1628: Improved type safety and parameter naming convention.

Changing the parameter type from any to EventSource and renaming from sender to _sender follows best practices for TypeScript interfaces. The underscore prefix clearly indicates the parameter is intentionally unused, while the explicit type ensures compatibility with the MouseListenerSet interface and helps catch potential type errors.

Also applies to: 1643-1643, 1649-1649

packages/core/src/view/other/DragSource.ts (4)

55-57: Documentation improvements look good.

The class documentation has been reformatted for better readability while maintaining the original content.


126-127: Updated reference from mxGraph to Graph in documentation.

The comment has been updated to use the current class name Graph instead of legacy mxGraph, which improves consistency.


141-142: Updated references from mxGuide to Guide in comments.

The comments have been updated to reference the current class name Guide instead of legacy mxGuide, maintaining consistency with the codebase.

Also applies to: 146-147


294-299: Documentation example code has been modernized.

The code example in the comment has been improved:

  • Changed variable declaration from var to const
  • Updated event utility references from mxEvent to EventUtils

This aligns with modern JavaScript practices and current naming in the codebase.

packages/core/src/view/plugins/ConnectionHandler.ts (3)

60-66: Updated imports to include MouseListenerSet as a type import.

The import statement now correctly includes the MouseListenerSet type, which is necessary for the interface implementation.


207-207: Explicitly implementing MouseListenerSet interface.

The ConnectionHandler class now explicitly implements the MouseListenerSet interface alongside GraphPlugin. This guarantees that the class implements all required mouse event handler methods with the correct signatures, improving type safety.


754-754: Updated mouse event handler parameters to indicate unused parameters.

The first parameter in mouse event handlers has been renamed from sender to _sender to clearly indicate that it's unused within these methods. This follows TypeScript conventions for marking unused parameters.

Also applies to: 1026-1026, 1491-1491

packages/core/src/view/plugins/TooltipHandler.ts (3)

28-28: Added MouseListenerSet to import statement.

The import statement now properly includes the MouseListenerSet type, which is necessary for the interface implementation.


40-40: TooltipHandler now explicitly implements MouseListenerSet interface.

Explicitly implementing the MouseListenerSet interface ensures that the class implements all required mouse event handler methods with the correct signatures, improving type safety and preventing potential errors.


177-178: Updated mouse event handler parameters and documentation.

The parameter naming has been updated from sender to _sender to indicate it's unused in the method implementations, and some comment text has been improved. This follows TypeScript conventions for marking unused parameters.

Also applies to: 185-185, 210-210

packages/core/src/view/handler/EdgeHandler.ts (3)

59-59: Updated imports to include MouseListenerSet.

The import statement now properly includes the MouseListenerSet type, which is necessary for the interface implementation.


77-77: EdgeHandler now explicitly implements MouseListenerSet.

The class now explicitly implements the MouseListenerSet interface, ensuring that it provides all required mouse event handler methods with the correct signatures. This improves type safety and enforces the interface contract.


787-787: Updated mouse event handler parameters to indicate unused parameters.

The first parameter in mouse event handlers has been renamed from sender to _sender to clearly indicate that it's unused within these methods. This is a good practice in TypeScript to mark parameters that are part of an interface but not used in the implementation.

Also applies to: 1311-1311, 1419-1419

packages/core/src/view/cell/CellTracker.ts (3)

24-24: Looks good: Import of MouseListenerSet as type.

The import has been correctly updated to import MouseListenerSet as a type from the types file.


79-79: Good: Explicit implementation of MouseListenerSet interface.

Explicitly implementing the MouseListenerSet interface helps ensure type safety and enforces the correct method signatures.


106-106: Good: Renamed unused parameter to indicate it's not used.

Renaming the sender parameter to _sender clearly indicates that this parameter is not used in the method implementation, which is a good practice.

packages/core/src/view/handler/VertexHandler.ts (5)

33-34: Looks good: Import of MouseListenerSet as type.

The import has been correctly updated to import MouseListenerSet as a type from the types file.


49-49: Good: Explicit implementation of MouseListenerSet interface.

Explicitly implementing the MouseListenerSet interface helps ensure type safety and enforces the correct method signatures.


635-635: Good: Renamed unused parameter to indicate it's not used.

Renaming the sender parameter to _sender clearly indicates that this parameter is not used in the method implementation, which is a good practice.


834-834: Good: Renamed unused parameter to indicate it's not used.

Renaming the sender parameter to _sender clearly indicates that this parameter is not used in the method implementation, which is a good practice.


1210-1210: Good: Renamed unused parameter to indicate it's not used.

Renaming the sender parameter to _sender clearly indicates that this parameter is not used in the method implementation, which is a good practice.

packages/core/src/view/plugins/RubberBandHandler.ts (5)

34-35: Looks good: Import of MouseListenerSet and GraphPlugin as types.

The imports have been correctly updated to import both MouseListenerSet and GraphPlugin as types from the types file.


60-60: Good: Explicit implementation of MouseListenerSet and GraphPlugin interfaces.

Explicitly implementing both the MouseListenerSet and GraphPlugin interfaces helps ensure type safety and enforces the correct method signatures.


180-180: Good: Renamed unused parameter to indicate it's not used.

Renaming the sender parameter to _sender clearly indicates that this parameter is not used in the method implementation, which is a good practice.


243-243: Good: Renamed unused parameter to indicate it's not used.

Renaming the sender parameter to _sender clearly indicates that this parameter is not used in the method implementation, which is a good practice.


300-300: Good: Renamed unused parameter to indicate it's not used.

Renaming the sender parameter to _sender clearly indicates that this parameter is not used in the method implementation, which is a good practice.

packages/core/src/view/other/Outline.ts (6)

38-38: Looks good: Import of MouseListenerSet as type and updating Listenable import.

The imports have been correctly updated to import MouseListenerSet as a type and change Listenable to a type import as well.


81-81: Good: Explicit implementation of MouseListenerSet interface.

Explicitly implementing the MouseListenerSet interface helps ensure type safety and enforces the correct method signatures.


82-87: Clean constructor parameter simplification.

Changed the constructor parameter container to be optional without a default value, which is cleaner and makes the optional nature of the parameter more explicit in the signature.


575-575: Good: Renamed unused parameter to indicate it's not used.

Renaming the sender parameter to _sender clearly indicates that this parameter is not used in the method implementation, which is a good practice.


607-607: Good: Renamed unused parameter to indicate it's not used.

Renaming the sender parameter to _sender clearly indicates that this parameter is not used in the method implementation, which is a good practice.


702-702: Good: Renamed unused parameter to indicate it's not used.

Renaming the sender parameter to _sender clearly indicates that this parameter is not used in the method implementation, which is a good practice.

Copy link

@tbouffard tbouffard merged commit 2fb660c into main Apr 18, 2025
7 checks passed
@tbouffard tbouffard deleted the refactor/type_more_usage_of_MouseListenerSet_interface branch April 18, 2025 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant