Skip to content

refactor: simplify Editor installDblClickHandler #938

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.

Sign up for GitHub

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

Jump to bottom
Merged
tbouffard merged 1 commit into from
Nov 5, 2025
Merged

refactor: simplify Editor installDblClickHandler #938

tbouffard merged 1 commit into from
Nov 5, 2025

Conversation

@tbouffard
Copy link
Member

@tbouffard tbouffard commented Nov 5, 2025 •
edited
Loading

Remove usage of variable holding this.

The callback passed to graph.addListener is an arrow function.
Arrow functions do not have their own this binding; they inherit this from the enclosing scope, which in this case is the Editor instance.

These rollbacks 1f31f7f.

Notes

Covers #360
Rollback #934

Summary by CodeRabbit

  • Refactor
    • Simplified internal code structure in the editor's double-click handler for improved maintainability. No functional changes to end-user experience.

@tbouffard
refactor: simplify Editor installDblClickHandler
a066dd4 
Remove usage of variable holding this.

The callback passed to graph.addListener is an arrow function.
Arrow functions do not have their own this binding; they inherit this from the enclosing scope, which in this case is
the Editor instance.
@tbouffard tbouffard added the refactor Code refactoring label Nov 5, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 5, 2025 •
edited
Loading

Walkthrough

Refactoring to replace a local variable alias for the Editor instance with direct this binding in the double-click handler, maintaining identical control flow and behavior.

Changes

Cohort / File(s) Change Summary
Editor double-click handler refactoring
packages/core/src/editor/Editor.ts		 Replaced local editor variable with direct this reference in installDblClickHandler. Renamed unused sender parameter to _sender. Updated handler condition to check this.cell, this.graph.enabled, and this.dblClickAction instead of the aliased variable.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • No logic changes; purely mechanical refactoring of variable reference
  • Single file affected with straightforward context binding updates

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete. Missing required sections from template: PR Checklist items (unchecked), explicit issue reference format, information about tests, screenshots/videos, and documentation changes. Fill out the complete PR template including the checklist items, explicit issue closure statement (e.g., 'closes #360'), test details, and documentation notes.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: refactoring to simplify the Editor's installDblClickHandler method.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/editor_install_dbclick_handler

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 5, 2025

Quality Gate Passed 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 tbouffard mentioned this pull request Nov 5, 2025
Closed
@tbouffard tbouffard merged commit 07e2fd3 into main Nov 5, 2025
7 checks passed
@tbouffard tbouffard deleted the refactor/editor_install_dbclick_handler branch November 5, 2025 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

refactor Code refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tbouffard