fix(VertexHandle): add missing setPosition call in processEvent by HalilFocic · Pull Request #583 · maxGraph/maxGraph · GitHub
Skip to content

fix(VertexHandle): add missing setPosition call in processEvent#583

Merged
tbouffard merged 1 commit into
maxGraph:mainfrom
HalilFocic:fix/resize-inner-rectangle-481
Nov 24, 2024
Merged

fix(VertexHandle): add missing setPosition call in processEvent#583
tbouffard merged 1 commit into
maxGraph:mainfrom
HalilFocic:fix/resize-inner-rectangle-481

Conversation

@HalilFocic

@HalilFocic HalilFocic commented Nov 23, 2024

Copy link
Copy Markdown

Body of processEvent funciton inside VertexHandle was missing setPosition call that will properly update the values inside CellStyle This ensures the shape's internal lines are redrawn correctly during hnadle movements.

Before this change, dragging the custom handles did not update the shape's divider positions due to the missing setPosition call.

PR Checklist

  • Addresses an existing open issue: Fixes [BUG] Not possible to resize the inner rectangle in the Handles story #481 .
  • You have discussed this issue with the maintainers of maxGraph, and you are assigned to the issue.
  • The scope of the PR is sufficiently narrow to be examined in a single session. A PR covering several issues must be split into separate PRs. Do not create a large PR, otherwise it cannot be reviewed and you will be asked to split it later or the PR will be closed.
  • I have added tests to prove my fix is effective or my feature works. This can be done in the form of automatic tests in packages/core/_tests_ or a new or altered Storybook story in packages/html/stories (an existing story may also demonstrate the change).
  • I have provided screenshot/videos to demonstrate the change. If no releavant, explain why.
  • I have added or edited necessary documentation, or no docs changes are needed.
  • The PR title follows the "Conventional Commits" guidelines.

Overview

Main issue was missing setPosition function call that was missing from processChange event. This line was present inside mxGraph code, it probably got overlooked while migrating the code. Adding this line back caused the custom setPosition function written inside story to be triggered and position the label lines correctly.

After change:

inner-rectangle.mov

Notes

ℹ️ This change only reflects the inner rectangle issue,not rotating handle being misaligned, that was fixed in other PR.

Summary by CodeRabbit

  • New Features

    • Enhanced the Vertex Handle functionality to update its position based on mouse interactions and current state during event processing.
  • Bug Fixes

    • Improved responsiveness of the Vertex Handle during user interactions.

Body of processEvent funciton inside VertexHandle was missing setPosition call that will properly update the values inside CellStyle
This ensures the shape's internal lines are redrawn correctly during hnadle movements.

Before this change, dragging the custom handles did not update the shape's divider positions due to the missing setPosition call.
@coderabbitai

coderabbitai Bot commented Nov 23, 2024

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
packages/core/src/view/cell/VertexHandle.ts (1)

132-133: Consider adding tests to prevent regression.

Since this was a bug fix for handle movement behavior, it would be valuable to add tests that verify:

  1. The setPosition is called during handle movement
  2. The shape's internal lines update correctly
  3. The divider positions update properly during drag operations

Would you like me to help create test cases for these scenarios?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 988ff48 and a0f858b.

📒 Files selected for processing (1)
  • packages/core/src/view/cell/VertexHandle.ts (1 hunks)
🔇 Additional comments (1)
packages/core/src/view/cell/VertexHandle.ts (1)

132-132: LGTM! This fixes the handle position update issue.

The addition of setPosition call correctly addresses the bug where handle movements weren't properly updating the shape's internal lines and divider positions.

Let's verify this fixes issue #481 by checking for similar patterns in the codebase:

✅ Verification successful

The setPosition call addition follows the correct pattern and fixes the issue

The verification confirms:

  • VertexHandle is the base class implementing the setPosition method
  • The method is properly called within processEvent after calculating the new point position
  • The pattern is consistent with how custom handles are processed in both VertexHandler and EdgeHandler
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for similar handle implementations to confirm the pattern
# Look for other vertex handle implementations that use setPosition
ast-grep --pattern 'class $_ extends VertexHandle {
  $$$
  setPosition($_, $_, $_) {
    $$$
  }
}'

# Look for other handle processEvent implementations
ast-grep --pattern 'processEvent($_) {
  $$$
  setPosition($_, $_, $_)
  $$$
}'

Length of output: 184


Script:

#!/bin/bash
# Let's try a different approach to verify the pattern

# First, let's find all handle-related files
fd "Handle\.ts$" --type f

# Then, let's look for setPosition usage patterns in handle files
rg "setPosition\(" -A 3 --type ts

# Also check for processEvent implementations
rg "processEvent\(" -A 5 --type ts

# Look for the base VertexHandle class definition
rg "class VertexHandle" -A 10 --type ts

Length of output: 4069

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

praise: Good catch. This seems to have been removed by mistake in a3102af.
This line existed prior this commit and in the original mxGraph code

https://github.com/jgraph/mxgraph/blob/ff141aab158417bd866e2dfebd06c61d40773cd2/javascript/src/js/handler/mxHandle.js#L101-L105

@tbouffard tbouffard changed the title fix(vertex-handler): add missing setPosition call inside processEvent fix(VertexHandler): add missing setPosition call in processEvent Nov 24, 2024
@tbouffard tbouffard changed the title fix(VertexHandler): add missing setPosition call in processEvent fix(VertexHandle): add missing setPosition call in processEvent Nov 24, 2024

@tbouffard tbouffard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for this contribution.
✔️ Tested locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Not possible to resize the inner rectangle in the Handles story

2 participants