fix(editor): ensure editor context in installDblClickHandler callback #934

tbouffard · 2025-10-31T15:17:23Z

Fix issue where the this reference in the double-click handler
callback was incorrectly bound to the graph instance instead of the
editor instance, breaking compatibility with mxGraph behavior.

Changes:

Store editor reference explicitly using const editor = this
Use editor instead of this in the callback to make intent clear
Add test to verify correct editor context in double-click handler

This matches the mxGraph pattern where mxUtils.bind(this, ...) was
used to ensure the callback had the correct editor context.

Notes

Closes #360

Summary by CodeRabbit

Tests
- Added test coverage for double-click event handling on graph cells.
Bug Fixes
- Improved double-click handler to reliably execute configured actions when cells are double-clicked.
Refactor
- Internal callback binding for the double-click handler improved to ensure consistent execution context.

Fix issue where the `this` reference in the double-click handler callback was incorrectly bound to the graph instance instead of the editor instance, breaking compatibility with mxGraph behavior. Changes: - Store editor reference explicitly using `const editor = this` - Use `editor` instead of `this` in the callback to make intent clear - Add test to verify correct editor context in double-click handler This matches the mxGraph pattern where `mxUtils.bind(this, ...)` was used to ensure the callback had the correct editor context.

coderabbitai · 2025-10-31T15:18:10Z

Walkthrough

Bind the editor instance into the double-click callback so the handler uses the editor (not the graph) and add a test that fires a DOUBLE_CLICK event to ensure the configured dblClickAction is executed with the target cell.

Changes

Cohort / File(s)	Summary
Test Suite Addition `packages/core/__tests__/editor/Editor.test.ts`	Adds imports for `EventObject` and `InternalEvent`. Introduces `installDblClickHandler` test that creates an Editor, stubs `execute`, sets `dblClickAction`, attaches a DOM container with a test vertex, fires `EventObject(DOUBLE_CLICK)` containing the cell, asserts `editor.execute` called with the action and cell, and cleans up.
Reference Binding Fix `packages/core/src/editor/Editor.ts`	In `Editor.installDblClickHandler`, introduce `const editor = this` and replace direct `this` uses inside the callback with `editor` references so callbacks access the editor instance rather than the graph instance.

Sequence Diagram(s)

sequenceDiagram
  participant DOM as DOM
  participant Graph as Graph
  participant Editor as Editor
  participant Exec as editor.execute(action, cell)

  rect #f0f9ff
    DOM->>Graph: user double-clicks cell
    Graph->>Editor: emit DOUBLE_CLICK EventObject(cell)
  end

  rect #e8f5e9
    Editor->>Exec: call execute(dblClickAction, cell)
    Exec-->>Editor: action executed
  end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Focus areas:
- Verify const editor = this is correctly used and no remaining this scope bugs exist in the callback.
- Confirm the new test reliably isolates the handler and asserts the correct execute invocation and parameters.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The PR title "fix(editor): ensure editor context in installDblClickHandler callback" follows the Conventional Commits format with the type "fix" and a specific scope "(editor)". It clearly and concisely describes the main change: fixing the context binding issue in the double-click handler. The title directly reflects the primary objective of ensuring the callback's `this` reference points to the editor instance rather than the graph instance.
Linked Issues Check	✅ Passed	The code changes directly address issue #360's requirement to ensure the double-click handler callback's `this` reference refers to the editor instance, matching mxGraph behavior. The modification to Editor.ts explicitly stores the editor reference with `const editor = this` and uses this reference in the callback instead of direct `this` usage. The added test in Editor.test.ts validates that the editor context is correctly preserved when a DOUBLE_CLICK event fires, verifying the fix is effective.
Out of Scope Changes Check	✅ Passed	All changes in the pull request are directly related to fixing issue #360. The modifications to Editor.ts focus solely on correcting the `this` binding in installDblClickHandler, and the test additions in Editor.test.ts serve to verify that the fix works correctly. No unrelated refactoring, documentation updates, or other out-of-scope changes are present in the changeset.
Description Check	✅ Passed	The PR description provides clear context about the problem being solved: the incorrect binding of `this` to the graph instance instead of the editor instance. It lists the specific implementation changes, mentions the added test to verify the fix, and references the linked issue with "Closes #360". While the description doesn't explicitly check all template boxes in a structured checklist format, it covers all essential information required: problem statement, changes made, test coverage, and issue linkage.

✨ 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 fix/editor_installDbClickHandler

📜 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 421b038 and 41b1e48.

📒 Files selected for processing (1)

packages/core/__tests__/editor/Editor.test.ts (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

packages/core/tests/editor/Editor.test.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

GitHub Check: build
GitHub Check: build (windows-2022)
GitHub Check: build (macos-14)
GitHub Check: build (ubuntu-24.04)

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 · 2025-10-31T15:29:23Z

Quality Gate passed

Issues
0 New issues
1 Accepted issue

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 bug Something isn't working label Oct 31, 2025

simplify test code

41b1e48

tbouffard merged commit 1f31f7f into main Oct 31, 2025
7 checks passed

tbouffard deleted the fix/editor_installDbClickHandler branch October 31, 2025 16:42

coderabbitai bot mentioned this pull request Nov 5, 2025

refactor: simplify Editor installDblClickHandler #938

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(editor): ensure editor context in installDblClickHandler callback #934

fix(editor): ensure editor context in installDblClickHandler callback #934

Uh oh!

tbouffard commented Oct 31, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Oct 31, 2025 •

edited

Loading

Uh oh!

sonarqubecloud bot commented Oct 31, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

fix(editor): ensure editor context in installDblClickHandler callback #934

fix(editor): ensure editor context in installDblClickHandler callback #934

Uh oh!

Conversation

tbouffard commented Oct 31, 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 Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

sonarqubecloud bot commented Oct 31, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

tbouffard commented Oct 31, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 31, 2025 •

edited

Loading