Skip to content

fix: mirror shape rotation and connector geometry in SVG thumbnails/exports - #591

Merged
vibhavkatre merged 5 commits into
frappe:mainfrom
bvnaik05:fix/572-thumbnail-rotation-connectors
Sep 1, 2026
Merged

fix: mirror shape rotation and connector geometry in SVG thumbnails/exports#591
vibhavkatre merged 5 commits into
frappe:mainfrom
bvnaik05:fix/572-thumbnail-rotation-connectors

Conversation

@bvnaik05

@bvnaik05 bvnaik05 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Closes #572

What changed

  • Updated SVG thumbnail / export rendering logic in useThumbnail.js to correctly apply shape rotation and flip transformations to the entire shape group (body + text).
  • Replaced the simple straight <line> fallback for block connectors in useThumbnail.js with a comprehensive SVG <path> builder.
  • Imported anchorPoint from @/diagram/geometry.js to resolve endpoints to their correct anchor positions rather than using shape centers as a cheap approximation.
  • Supported curved, elbow, and straight path data, including correct marker start/end arrowhead rendering and label pills.
  • Added a unit test verifying shape transformations and connector attributes.

Why it matters

This ensures that the minimap, saved thumbnails, and exported images (PNG, PDF, SVG) match the live canvas rendering exactly, correcting issues where shape rotation was ignored and connector paths/arrowheads differed from the live editor.

Verified

  • yarn vitest run src/composables/useThumbnail.test.js
  • yarn eslint src/composables/useThumbnail.js src/composables/useThumbnail.test.js
  • Built SPA assets successfully

Copilot AI lite review requested due to automatic review settings August 31, 2026 09:32

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Reviews (5): Last reviewed commit: "fix: normalize attached endpoint and elb..." | Re-trigger Greptile

Comment thread frontend/src/composables/useThumbnail.js Outdated
Comment thread frontend/src/composables/useThumbnail.js Outdated
Comment thread frontend/src/composables/useThumbnail.js Outdated
Comment thread frontend/src/composables/useThumbnail.js Outdated
Comment thread frontend/src/composables/useThumbnail.js Outdated

@vibhavkatre vibhavkatre left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This closes a genuine gap: the export was drawing every connector as a straight grey line between two shape centres, so a diagram with elbows, arrowheads or a routed flowchart looked nothing like the canvas in a thumbnail, a PNG or a PDF.

I checked the parts that had to match the canvas exactly, and they do:

  • endpointPoint now uses anchorPoint(shape, endpoint.anchor || 'right') — the same expression, including the 'right' default, that ConnectorView.vue's resolve uses. The centre approximation was the source of most of the divergence.
  • The mind-map branch path goes through branchPathPoints(a, b), which is what the canvas draws (ConnectorView.vue:109) despite the name suggesting it returns points rather than a d string.
  • The branch stroke colour mirrors strokeColor — the child's border colour, falling back to #525252.
  • elbowPath reproduces the canvas version exactly: same 14-unit clamp, same r > 0.5 guard, same three-leg construction.
  • escapeAttr/safeColor/num are applied on the new attributes, and the id used for the marker url(#…) is stripped to [A-Za-z0-9_-]. The XSS matrix picking up block shape rotation and connector id is the right instinct — that table is the reason a persisted value cannot close an attribute here.

Two follow-ups rather than blockers:

  1. elbowPath now exists twice — here and ConnectorView.vue:118 — and #598 adds a third inline copy. Once that lands, moving it into src/diagram/ and importing it in both places would stop the three drifting apart. Same argument for the marker shapes, which duplicate ConnectorMarker.vue.
  2. The <defs> block is emitted per connector, so a large diagram repeats identical marker definitions. Correct, just wasteful; a single hoisted <defs> keyed by colour and type would be tidier if export size ever matters.

I have asked #598 to rebase onto this and add midX to the elbow branch here, since this version hardcodes the midpoint.

Merging.

@vibhavkatre
vibhavkatre merged commit c9ff6b6 into frappe:main Sep 1, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Minimap and export/thumbnail rendering diverge from the live canvas: rotation is ignored, mind-map/flowchart branches look different

3 participants