fix(ConnectorShape): ignore unset arrows in edge bounding box by tbouffard · Pull Request #1094 · maxGraph/maxGraph · GitHub
Skip to content

fix(ConnectorShape): ignore unset arrows in edge bounding box#1094

Merged
tbouffard merged 1 commit into
mainfrom
fix/ConnectorShape.augmentBoundingBox_use_correct_default_style_value
Jun 22, 2026
Merged

fix(ConnectorShape): ignore unset arrows in edge bounding box#1094
tbouffard merged 1 commit into
mainfrom
fix/ConnectorShape.augmentBoundingBox_use_correct_default_style_value

Conversation

@tbouffard

@tbouffard tbouffard commented Jun 22, 2026

Copy link
Copy Markdown
Member

PR Checklist

  • Addresses an existing open issue: closes #<the_issue_number_here>. If not, explain why (minor changes, etc.).
  • 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.
  • I have added tests to prove my fix is effective or my feature works.
  • I have provided screenshot/videos to demonstrate the change. Not relevant: the change only affects the computed bounding box dimensions, it has no visual rendering output of its own; it is covered by automatic tests.
  • I have added or edited necessary documentation, or no docs changes are needed.
  • The PR title follows the "Conventional Commits" guidelines.

Overview

ConnectorShape.augmentBoundingBox treated an unset startArrow/endArrow as present, so an edge with no arrow on a side still grew its bounding box by an extra marker size on that side.

The check was a strict comparison against NONE:

if (this.style.startArrow !== NONE) { ... }

When the style does not define the arrow, this.style.startArrow is undefined, and undefined !== 'none' is true, so the branch runs for arrows that were never set. This grows the bounding box (and therefore the graph bounds and the computed fit scale) more than it should.

This is a porting regression from mxGraph. mxConnector.augmentBoundingBox reads the arrow with getValue(style, ARROW, NONE), which defaults an absent key to NONE, so the branch is correctly skipped.

The fix defaults the value before comparing, mirroring mxGraph and aligning with createMarker in the same file which already uses ... || NONE:

if ((this.style.startArrow ?? NONE) !== NONE) { ... }
if ((this.style.endArrow ?? NONE) !== NONE) { ... }

Tests: data-driven coverage was added on augmentBoundingBox for the cases only end arrow set, only start arrow set, no arrow set, explicit none on both sides, and both arrows set. The first three were red before the fix.

Notes

While investigating this, it became clear that the mxGraph to maxGraph migration introduced regressions in several places where a style default value is not handled the way mxGraph handled it (mxGraph systematically went through getValue(style, KEY, default), whereas maxGraph sometimes reads the property directly and compares it without applying the default). Several such cases have already been fixed individually, but we will need to conduct a general investigation to systematically compare the default values used in mxGraph and maxGraph, to catch the remaining occurrences rather than fixing them one by one as they are discovered.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed incorrect bounding box calculations for connector shapes when arrow styles are undefined or set to none.
  • Tests

    • Added test coverage for connector shape bounding box behavior with various arrow configurations.

…ingBox

Default an absent startArrow/endArrow to NONE before comparing, so an unset arrow no longer enlarges the bounding
box. The strict `this.style.startArrow !== NONE` check evaluated `undefined !== 'none'` as true, running the branch
for arrows that were never set and growing the box by an extra marker size per side. This shrank the computed graph
bounds and the fit scale slightly compared to mxGraph.

mxGraph mxConnector.augmentBoundingBox reads arrows via getValue(style, ARROW, NONE), which defaults absent keys to
NONE. The fix mirrors that with `(this.style.startArrow ?? NONE) !== NONE`, also aligning with createMarker in the
same file which already defaults the arrow type to NONE.

Add data-driven tests on augmentBoundingBox covering: only end arrow set, only start arrow set, no arrow set,
explicit none on both sides, and both arrows set.
@tbouffard tbouffard added the bug Something isn't working label Jun 22, 2026
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

@tbouffard tbouffard changed the title fix(shape): treat absent arrow as none in ConnectorShape.augmentBoundingBox fix(ConnectorShape): ignore unset arrows in edge bounding box Jun 22, 2026
@tbouffard tbouffard merged commit b4ebeb4 into main Jun 22, 2026
14 checks passed
@tbouffard tbouffard deleted the fix/ConnectorShape.augmentBoundingBox_use_correct_default_style_value branch June 22, 2026 08:22
@sonarqubecloud

Copy link
Copy Markdown

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.

2 participants