feat(app): auto-save tile edits when closing the editor modal

[brone1323]

Summary

Closes #1668.

Currently, closing the tile editor with unsaved changes shows a "Discard" confirmation dialog. This PR implements auto-save on close instead, so edits are never accidentally lost.

Changes

• EditTimeChartForm: Adds a saveRef prop (parallel to the existing submitRef) that the parent can hold to programmatically trigger handleSubmit(handleSave) — the same validated-save path the Save button uses.
• EditTileModal: Drops the useConfirm discard dialog. On close with unsaved changes, calls saveRef.current() and shows a green toast confirming the save. The Cancel button in the form still closes without saving, preserving an explicit escape hatch.

Behaviour after this PR

Scenario	Before	After
Close modal with unsaved changes	"Discard?" confirm dialog	Auto-saves + toast
Click Cancel button	Closes immediately	Closes immediately (unchanged)
Click Save button	Saves + closes	Saves + closes (unchanged)
Invalid form on close	Discard dialog	handleSave shows "Invalid Chart" error; modal stays open

If form validation fails, handleSave shows its own error notification and the save is skipped, keeping the modal open for the user to fix the issue.

[vercel]

@brone1323 is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 34c6d50

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[github-actions]

Deep Review

The auto-save-on-close flow has an async/sync mismatch in handleClose: it fires the success toast and unmounts the form synchronously while handleSubmit(handleSave)() is still pending in a microtask. This produces concrete data-loss paths the PR was meant to eliminate, plus a Cancel-button semantic change that contradicts the PR's own behaviour table.

🔴 P0/P1 -- must fix

• packages/app/src/DBDashboardPage.tsx:990 -- handleClose calls saveRef.current() then unconditionally runs setHasUnsavedChanges(false); onClose();, so when the form is invalid handleSave short-circuits with the red Invalid Chart toast yet the modal still unmounts and the green Tile changes saved toast fires alongside it, losing the user's edits on the very path the PR was meant to protect.
  • Fix: Have saveRef resolve a Promise<boolean> reporting whether onSave actually fired, then in handleClose await it and gate the green notification plus onClose() on a true result; on false keep the modal open.
  • _{correctness, julik-frontend-races, kieran-typescript, testing}
• packages/app/src/DBDashboardPage.tsx:1027 -- the form is rendered with onClose={handleClose}, so the Cancel button in ChartActionBar (whose onClick={onClose}) now flows through the auto-save branch instead of dismissing without persisting, which both contradicts the PR description's "Closes immediately (unchanged)" row and breaks the conventional Cancel-as-discard semantics.
  • Fix: Pass a distinct onCancel (or onDiscard) callback to EditTimeChartForm that bypasses the auto-save branch and only resets dirty state before invoking the parent onClose, and wire ChartActionBar's Cancel button to that callback.
  • _correctness

🟡 P2 -- recommended

• packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx:383 -- saveRef.current = () => handleSubmit(handleSave)() discards the Promise<void> returned by react-hook-form, and the ref type MutableRefObject<(() => void) | undefined> hides the async-ness, so callers cannot await success even on the happy path -- a server-side save error after onClose() would still leave the green toast as the last user-visible signal.
  • Fix: Type saveRef as MutableRefObject<(() => Promise<boolean>) | undefined> and have the wiring return handleSubmit(handleSave)().then(() => savedSuccessfullyFlag), then await it at every call site.
  • _{correctness, julik-frontend-races, kieran-typescript}
• packages/app/src/DBDashboardPage.tsx:988 -- the diff introduces four new behaviours (auto-save on close, green toast, dirty-flag reset, isSaving guard) across X / Esc / click-outside / Cancel, and no tests in packages/app exercise any of them; the headline PR claim that the modal stays open on invalid form has no assertion to keep it honest.
  • Fix: Add a DBDashboardPage (or EditTileModal) test covering: dirty + valid close -> onSave called once + green toast; dirty + invalid close -> modal stays open + red toast + no green toast; isSaving=true close -> no-op; pristine close -> onClose once with no toast.
  • _{testing, maintainability}

🔵 P3 nitpicks (4)

• packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx:381 -- the new useEffect reads handleSave before its const declaration at line 395; it works at runtime because effects fire post-render, but a future refactor moving any of this into the render body would trip the TDZ.
  • Fix: Move the new useEffect block to after the handleSave declaration so the file reads top-to-bottom without forward references.
  • _{kieran-typescript, maintainability}
• packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx:385 -- the effect assigns saveRef.current but never clears it on unmount, mirroring submitRef but leaving a stale closure pinned to the previous mount if the ref ever outlives its current consumer.
  • Fix: Return () => { if (saveRef) saveRef.current = undefined; } from the effect.
  • _{julik-frontend-races, kieran-typescript}
• packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx:103 -- exposing one MutableRefObject per imperative action (submitRef, now saveRef) duplicates the "let the parent poke the form" concept; from the prop types alone the two refs look interchangeable.
  • Fix: Consolidate into a single formApiRef: MutableRefObject<{ submit(): void; save(): Promise<boolean> } | undefined> (or use useImperativeHandle) before a third action lands.
  • _{kieran-typescript, maintainability}
• packages/app/src/DBDashboardPage.tsx:990 -- the guard hasUnsavedChanges && saveRef.current silently falls through to setHasUnsavedChanges(false); onClose() when the ref has not yet been assigned by the child's post-commit effect, so the very-first-render close path loses edits with neither toast nor save.
  • Fix: Either assign saveRef.current synchronously via a callback ref, or log/warn (and fall back to the prior confirm flow) when hasUnsavedChanges is true but saveRef.current is undefined.
  • _{correctness, maintainability}

---

Reviewers (5): correctness, julik-frontend-races, kieran-typescript, testing, maintainability.

Testing gaps:

• No assertion that the modal stays open and skips the green toast when handleSave rejects with Invalid Chart.
• No coverage of the Cancel button vs X/Esc/click-outside divergence introduced by routing all four through handleClose.
• No coverage of the saveRef.current === undefined race window between mount and the first effect commit.

[github-actions]

PR Review

• 🐛 Critical / runtime crash — TDZ ReferenceError. In packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx:381-385, the new useEffect(..., [handleSave, handleSubmit, saveRef]) references handleSave in its dependency array, but handleSave is declared with const at line 395 (below). Evaluating the deps array during render before the declaration throws ReferenceError: Cannot access 'handleSave' before initialization, which will crash the editor on mount. → Fix: move the useEffect (lines 381-385) below the const handleSave = useCallback(...) definition (after line 413). Same fix needed for the closure at line 383 referencing handleSave.

• 🐛 PR description contradicts the implementation — invalid-form path still closes the modal and "loses" edits. PR description says: "handleSave shows 'Invalid Chart' error; modal stays open." But in DBDashboardPage.tsx:988-1002, handleClose calls saveRef.current() (fire-and-forget) and then unconditionally calls setHasUnsavedChanges(false); onClose();. So when validation fails: the red "Invalid Chart" toast fires, the green "Tile changes saved" toast also fires, and the modal closes anyway — the exact data-loss this PR is supposed to prevent. → Fix: have saveRef.current return a success boolean (or a Promise) so handleClose can skip onClose() (and the green toast) when save was rejected; only show the success notification on confirmed save.

• ⚠️ Premature success toast. The "Your edits were automatically saved" notification at DBDashboardPage.tsx:993-998 fires synchronously before handleSubmit(handleSave)() (an async react-hook-form pipeline) resolves. Combined with the issue above, the toast lies to the user on validation failures. → Fix: show the toast inside the resolved/success branch of the save (via the same return-value/Promise fix as above).