Skip to content

fix(core): apply position edits to SVG elements, not just HTML#2724

Merged
hblee-hg merged 1 commit into
heygen-com:mainfrom
hblee12294:07-21-fix_svg_position_edits
Jul 21, 2026
Merged

fix(core): apply position edits to SVG elements, not just HTML#2724
hblee-hg merged 1 commit into
heygen-com:mainfrom
hblee12294:07-21-fix_svg_position_edits

Conversation

@hblee12294

Copy link
Copy Markdown
Contributor

What

Position edits (the SDK moveElement convention — data-x/data-y + captured
baseline) now apply to SVG elements (authored <text>, shapes, groups),
not only HTML. Previously the runtime silently dropped them on SVG.

Why

applyPositionEdits in runtime/positionEdits.ts gated each candidate with
el instanceof HTMLElement, so SVGElement nodes were skipped in both the
apply and orphan-reset loops — a committed move on an SVG element was never
realized. This surfaced in downstream editors (e.g. HeyGen AI Studio) where a
user moves an inner SVG <text> label: the move persisted in the model but
produced no visual offset. It was also missing in the rendered video, since the
render injects the generated position-edits-render-inline bundle that is built
from this same module.

How

  • runtime/positionEdits.ts:
    • Added type StylableElement = HTMLElement | SVGElement and broadened
      applyPositionEditToElement, readCurrentTranslate,
      primeGsapTransformCache, and the lastAppliedTranslate map to it — every
      DOM API used (.style, attribute get/set, ownerDocument) exists on both.
    • isStylable now admits SVGElement resolved from the document's own realm
      (doc.defaultView.SVGElement), preserving the existing cross-realm safety
      and the .style.setProperty duck-type fallback.
    • SVG uses the same independent CSS translate longhand as HTML: it composes
      additively with GSAP's transform and survives seeks (same rationale as the
      HTML path), and Chrome honors translate on SVG (px = user units).
  • Regenerated src/generated/positio (bun run build:position-edits-render) so the render-time script carries the fix and check:position-edits-rend

Additive only — no behavior change f

Test plan

How was this tested?

  • Unit tests added/updated — newonEdits.test.ts(a marked SVG element is counted and receives the composedtranslate); full suite green (23/23). oxllean.
  • Manual testing performed — verified in a downstream editor: moving an inner SVG `n the
    composition preview.
  • Documentation updated (if applor fix; no doc surface).

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

R1: APPROVE — clean, focused fix. Widening the element predicate from HTMLElement to HTMLElement | SVGElement is strictly additive across all four threading sites (applyPositionEditToElement, readCurrentTranslate, primeGsapTransformCache, lastAppliedTranslate WeakMap key), and isStylable is broadened symmetrically to admit RealmSVGElement alongside RealmHTMLElement from the document's own realm. Cross-realm safety preserved; .style.setProperty duck-type fallback still catches both.

Correctness spot-checks

  • Every DOM API used on the widened type — .style, getAttribute / hasAttribute / setAttribute / removeAttribute, .ownerDocument.defaultView, getComputedStyle — is inherited from Element and works on SVGElement. ✓
  • Modern Chrome honors the independent CSS translate longhand on SVG elements (px = user units, composes with existing SVG transform) — same rationale as the HTML path, PR body cites correctly. ✓
  • gsap.getProperty(el, "x") accepts SVG targets (GSAP 3.x has SVG-native handling); priming is best-effort and the try/catch swallows any provider-specific issue anyway. ✓
  • Widening a parameter type is strictly compatible with all prior callers (SDK's iframe.ts, runtime init.ts, studio manualEdits.ts / useDomEditPreviewSync.ts / etc.) — every existing HTMLElement call continues typing. ✓

Regression test

The new SVG <text> test in positionEdits.test.ts is the right shape — creates via createElementNS, sets data-x/data-y + baseline, asserts both count-returned-1 AND translate === "145px 1px". That's the minimum bar for a "silently dropped → now applies" fix.

Minor nit (non-blocking, optional follow-up)

The existing cross-realm regression test only covers HTML (an iframe-hosted <div>). The new RealmSVGElement !== undefined && el instanceof RealmSVGElement branch is exercised only same-realm. Symmetry with the HTML path gives strong prior that cross-realm SVG works too, but a parallel iframe-hosted SVG test (create <text> inside iframeDoc.body, verify applyPositionEdits(iframeDoc) returns 1 + writes the translate) would close the completeness gap. Truly optional — not a blocker.

— Via

@hblee12294
hblee12294 force-pushed the 07-21-fix_svg_position_edits branch from 86fa188 to 10220b9 Compare July 21, 2026 22:52
@hblee-hg
hblee-hg merged commit 63539a0 into heygen-com:main Jul 21, 2026
53 checks passed
@hblee12294
hblee12294 deleted the 07-21-fix_svg_position_edits branch July 21, 2026 23:55
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.

3 participants