Skip to content

fix(studio): surface save failures when deleting all animations for an element#1411

Closed
calcarazgre646 wants to merge 1 commit into
heygen-com:mainfrom
calcarazgre646:fix/studio-delete-all-animations-silent-failure
Closed

fix(studio): surface save failures when deleting all animations for an element#1411
calcarazgre646 wants to merge 1 commit into
heygen-com:mainfrom
calcarazgre646:fix/studio-delete-all-animations-silent-failure

Conversation

@calcarazgre646

Copy link
Copy Markdown
Contributor

Problem

deleteAllForSelector in useGsapScriptCommits fired its mutation with void commitMutation(...):

void commitMutation(
  selection,
  { type: "delete-all-for-selector", targetSelector },
  { label: "Delete all animations for element" },
);

A failed "Delete all animations for element" was therefore swallowed — no error toast and no trackGsapSaveFailure telemetry — leaving the user believing the operation succeeded. Every sibling mutation in the hook (deleteGsapAnimation, removeGsapProperty, removeAllKeyframes, updateGsapProperty, …) routes through commitMutationSafely, which .catches the rejection, records trackGsapSaveFailure, and shows an error toast. This one handler was the only mutation that did not.

Fix

Route deleteAllForSelector through commitMutationSafely, matching its siblings. It is a single user action (one context-menu invocation per element), so the failure toast fires at most once — no spam.

Notes

  • Swept the hook and the rest of studio/src for the same fire-and-forget pattern: this was the only user-facing mutation that surfaced nothing. The remaining bare commitMutation calls either return the promise for the caller to handle or have their own .catch(trackGsapSaveFailure).
  • No test added: there is no existing render harness for this hook, and the change is a one-line routing fix to the already-used commitMutationSafely wrapper that ~10 sibling handlers rely on.

…n element

`deleteAllForSelector` fired its mutation with `void commitMutation(...)`, so a
failed "Delete all animations for element" was swallowed: no error toast and no
`trackGsapSaveFailure` telemetry, leaving the user believing it worked. Every
sibling mutation in the hook (`deleteGsapAnimation`, `removeGsapProperty`,
`removeAllKeyframes`, etc.) routes through `commitMutationSafely`, which catches
the rejection and surfaces it. Route this one through the same wrapper.

It is a single user action (one context-menu invocation per element), so the
failure toast fires at most once, with no spam.
@miguel-heygen

Copy link
Copy Markdown
Collaborator

Closing — the function this PR claims to fix (deleteAllForSelector), the wrapper it routes through (commitMutationSafely), and the telemetry it references (trackGsapSaveFailure) do not exist anywhere in the codebase and never have (confirmed via git history search). Every sibling mutation uses void commitMutation(...) — the exact pattern the PR describes as the bug. This would cause a build failure if merged.

Happy to reopen if evidence of the issue is provided.

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.

2 participants