Skip to content

fix: normalize exported polygon points against the rendered canvas - #213

Open
burakeyler wants to merge 1 commit into
meshery-extensions:masterfrom
burakeyler:fix/212-normalize-against-rendered-canvas
Open

burakeyler wants to merge 1 commit into
meshery-extensions:masterfrom
burakeyler:fix/212-normalize-against-rendered-canvas

Conversation

@burakeyler

@burakeyler burakeyler commented Sep 13, 2026 •

Copy link
Copy Markdown

Fixes #212

Symptom

Exported polygon coordinates are meant to be in Cytoscape's -1..1 range, but showCytoArray normalized with a hardcoded 260 px half-extent while the SVG (no viewBox, width: 100%) is typically ~834×518. Points near the right edge exported ~2.0, and the visible centre didn't export 0 — and the numbers depended on window width.

Fix

Takes the first option from the issue (not the viewBox design change): showCytoArray now calls the existing normalizeToCanvas(x, y, rect) with boardRef.current.getBoundingClientRect(), the same helper handlePointerMove uses. So there's one normalizer for both the export and the hover readout, and they can't disagree. The comment on normalizeToCanvas that documented the divergence is updated.

If the canvas has no rendered size (e.g. hidden), it now reports the existing "Failed to extract and normalize polygon points." error instead of dividing by zero.

This intentionally changes exported values for existing users, as discussed in the issue.

Verification

  • Math check with the helper: on an 834×518 canvas the centre exports [0, 0] and the bottom-right corner [1, 1]; the issue's worked example (800 px wide, x = 400) now exports 0 instead of 0.538.
  • The component parses/compiles with esbuild (JSX).
  • I did not run the Gatsby dev server or click through the UI in a browser; a quick manual check (draw near the edges at two window widths, confirm the output stays within -1..1) would be worth doing in review.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved polygon rendering so shapes remain accurately positioned and scaled across different canvas sizes.
    • Added validation to prevent shape processing when the canvas has no measurable display area.

showCytoArray divided by a hardcoded 260px half-extent, but the SVG has no
viewBox and fills its container, so at most viewport widths the exported
points left the -1..1 range and the visible centre did not export 0. Use the
same rect-derived normalizeToCanvas the hover readout uses, so the export
and the readout agree.

Fixes meshery-extensions#212

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Burak Eyler <burakeyler@gmail.com>
@coderabbitai

coderabbitai Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: b05e06d1-50ea-4ab4-b080-22c3c932dcbf

📥 Commits

Reviewing files that changed from the base of the PR and between 5cea3b5 and 84c3b44.

📒 Files selected for processing (1)
  • site/src/components/ShapeBuilder/index.js

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

showCytoArray now normalizes polygon points using the rendered canvas bounds from boardRef. It throws an error when the canvas has no rendered size. The related comments now document the rendered-coordinate basis.

Changes

Canvas normalization

Layer / File(s) Summary
Export normalization
site/src/components/ShapeBuilder/index.js
showCytoArray obtains the canvas bounding rectangle, validates its width and height, and passes the rectangle to normalizeToCanvas instead of using fixed 260px half-extents. The comments describe the rendered SVG coordinate space.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: jeetburman

Merge Risk: ⚪ Minimal · up to 84c3b

Exported coordinates now use the rendered canvas dimensions consistently with hover readouts and avoid division by zero when no rendered size exists.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: polygon export normalization now uses the rendered canvas instead of a hardcoded extent.
Linked Issues check ✅ Passed Issue #212 requires rendered SVG dimensions, shared export/readout normalization, centered -1..1 coordinates, and protection against zero dimensions. In site/src/components/ShapeBuilder/index.js, `s…
Out of Scope Changes check ✅ Passed The reviewed change is limited to the normalization comment and export path in site/src/components/ShapeBuilder/index.js. These changes directly implement issue #212. No unrelated behavior or files …
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exported polygon coordinates exceed the -1..1 range: showCytoArray normalizes against a hardcoded 260px half-extent

1 participant