Skip to content

Add SVG component support with runtime fixes - #390

Open
bpc-oss wants to merge 1 commit into
nodegui:masterfrom
bpc-oss:feat/svg-support-pr-ready
Open

bpc-oss wants to merge 1 commit into
nodegui:masterfrom
bpc-oss:feat/svg-support-pr-ready

Conversation

@bpc-oss

@bpc-oss bpc-oss commented Jun 27, 2026

Copy link
Copy Markdown

Summary

This PR adds SVG component support to react-nodegui using QSvgWidget, and includes the runtime fixes needed to make the feature work reliably.

It covers:

  • Svg root component plus exported helpers like Rect, Circle, Path, and SvgText
  • inline SVG tree serialization
  • src, content, and buffer loading paths
  • mixed text / tspan handling inside SVG text nodes
  • safe child move/reorder behavior in SVG trees
  • a repo-local acceptance command: npm run verify:svg

Fixes #31.

Why this PR

There are already related attempts in #387, #388, and #389. Compared with those branches, this version also fixes the runtime issues that blocked a clean handoff:

  • fixes unbound createInstance behavior for nested SVG elements
  • removes the private @nodegui/nodegui/dist/... dependency
  • adds SVG-specific text update handling in the reconciler
  • fixes reorder semantics so moved SVG children are not duplicated
  • codifies acceptance coverage in-repo instead of relying only on ad hoc smoke commands

Verification

Ran locally:

  • npm run build
  • npm run verify:svg

npm run verify:svg covers:

  • root import smoke
  • README helper shapes
  • lowercase rect
  • SvgText
  • content / buffer / src
  • adjacent text serialization
  • mixed tspan serialization
  • mixed text update after mount
  • reorder / move behavior

Notes

  • SVG text should use SvgText rather than the existing host Text component.
  • Verification is now repo-local and repeatable, though it is still an acceptance harness rather than a full unit/integration test suite.

@woahwhattheheck woahwhattheheck 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.

A focused serialization correction from our preserved source packet: true-valued JSX attributes currently serialize as bare names, which is valid HTML boolean syntax but invalid XML attribute syntax for inline SVG. The existing escaped string path already converts true to "true". The suggestion keeps false/null omission and the rest of this implementation unchanged, preserving @bpc-oss's feature authorship. Source inspection only; I did not run Qt, TypeScript, or verify:svg. AI-assisted review.

Packet: https://github.com/woahwhattheheck/commons/tree/main/work/bounty-intake/nodegui31-svg-boolean-20261005-7ca6

return result;
}

result.push(value === true ? attributeName : `${attributeName}="${escapeAttribute(attributeValue)}"`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use the existing quoted/escaped value path for true as well, so the output remains well-formed XML:

Suggested change
result.push(value === true ? attributeName : `${attributeName}="${escapeAttribute(attributeValue)}"`);
result.push(`${attributeName}="${escapeAttribute(attributeValue)}"`);

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for svg in React code.

2 participants