From aef1d70351311e500a4565f1d6dac7365aa4bb55 Mon Sep 17 00:00:00 2001 From: Danny White <3104761+dnywh@users.noreply.github.com> Date: Thu, 5 Mar 2026 11:32:39 +1100 Subject: [PATCH] chore(studio): standardise discard changes behaviour (#43201) ## What kind of change does this PR introduce? UX consistency improvement. Updates DEPR-355. ## What is the current behavior? Discard-confirm close behavioir is implemented inconsistently across Studio forms: - some sheets/dialogs used `useConfirmOnClose` - some duplicated local `CloseConfirmationModal` components - some (e.g. `CreateHookSheet`) closed unconditionally and could lose unsaved changes ## What is the new behavior? Extracts and validates a reusable discard-close pattern for dialogs/sheets - enhances `useConfirmOnClose` with `handleOpenChange(open)` for `Dialog`/`Sheet` `onOpenChange` - adds shared `DiscardChangesConfirmationDialog` (`AlertDialog`-based, override-able copy) - migrates: - `InviteMemberButton` - `CreateHookSheet` - `EditSecretSheet` This standardizes close-guard behavior for backdrop/escape/close-button/cancel-button flows without trying to block route changes or arbitrary unmounts. ## Additional context `CreateHookSheet` now also marks the generated secret action as dirty (`setValue(..., { shouldDirty: true })`) so the discard guard behaves correctly. - Added tests for `useConfirmOnClose` covering: - clean vs dirty close - handleOpenChange(true|false) - confirm/cancel behavior - latest callback ref behavior A follow-up PR is needed to migrate remaining duplicated `CloseConfirmationModal` usages and older `useConfirmOnClose` call sites to the shared `DiscardChangesConfirmationDialog` + `handleOpenChange` pattern. --------- Co-authored-by: Joshen Lim --- .../content/docs/components/alert-dialog.mdx | 1 + .../content/docs/components/dialog.mdx | 1 + .../docs/fragments/confirmation-modal.mdx | 3 + .../content/docs/ui-patterns/modality.mdx | 28 ++++ .../interfaces/Auth/Hooks/CreateHookSheet.tsx | 45 ++++-- .../interfaces/Auth/Hooks/HooksListing.tsx | 9 +- .../EdgeFunctionSecrets/EditSecretSheet.tsx | 38 ++--- .../TeamSettings/InviteMemberButton.tsx | 152 ++++++----------- .../DiscardChangesConfirmationDialog.tsx | 78 +++++++++ .../hooks/ui/useConfirmOnClose.test.tsx | 153 ++++++++++++++++++ apps/studio/hooks/ui/useConfirmOnClose.tsx | 12 +- 11 files changed, 374 insertions(+), 146 deletions(-) create mode 100644 apps/studio/components/ui-patterns/Dialogs/DiscardChangesConfirmationDialog.tsx create mode 100644 apps/studio/hooks/ui/useConfirmOnClose.test.tsx diff --git a/apps/design-system/content/docs/components/alert-dialog.mdx b/apps/design-system/content/docs/components/alert-dialog.mdx index 0968428ea14..7c35e586b3c 100644 --- a/apps/design-system/content/docs/components/alert-dialog.mdx +++ b/apps/design-system/content/docs/components/alert-dialog.mdx @@ -99,6 +99,7 @@ This enforced decision helps prevent accidental dismissal of critical warnings o - **Keep content concise:** AlertDialogDescription renders as a single paragraph and must not contain block-level elements such as lists, multiple paragraphs, or complex layouts. - **Use for critical decisions only:** Reserve Alert Dialog for destructive or irreversible actions, or for warnings that require explicit acknowledgement. +- **Use for dirty-form discard confirmation:** A short discard-confirmation step after a dirty form dismissal attempt (backdrop, Escape, or `Cancel`) is a valid Alert Dialog pattern. In Studio, prefer `DiscardChangesConfirmationDialog` for this flow. - **Always provide a cancel action:** Include AlertDialogCancel so users can safely back out, in addition to supporting the Escape key. - **Avoid rich content:** If the dialog requires detailed explanations, callouts, or form inputs, use [Confirmation Modal](../fragments/confirmation-modal) or [Dialog](../components/dialog) instead. diff --git a/apps/design-system/content/docs/components/dialog.mdx b/apps/design-system/content/docs/components/dialog.mdx index 108d9852e05..3dffe213230 100644 --- a/apps/design-system/content/docs/components/dialog.mdx +++ b/apps/design-system/content/docs/components/dialog.mdx @@ -58,6 +58,7 @@ import { - **Use for non-critical interactions:** Dialog is appropriate when dismissal has no serious consequences. - **Design for cancellation**: Assume users may close the dialog without completing the action. +- **Guard dirty forms on dismissal:** For form dialogs, keep `Dialog` dismissible but intercept close attempts when the form is dirty and show a discard-confirmation dialog (for example, Studio's `DiscardChangesConfirmationDialog` pattern via `useConfirmOnClose`). - **Keep focus contained**: Dialog content should remain scoped to a single task or flow. - **Avoid destructive confirmations**: If the dialog’s primary purpose is to confirm a risky action, use a confirmation-focused pattern instead. - **Compose freely**: Dialog is intentionally unopinionated. Build custom layouts, forms, or step-based flows as needed. diff --git a/apps/design-system/content/docs/fragments/confirmation-modal.mdx b/apps/design-system/content/docs/fragments/confirmation-modal.mdx index dfe727fa171..e613c83444f 100644 --- a/apps/design-system/content/docs/fragments/confirmation-modal.mdx +++ b/apps/design-system/content/docs/fragments/confirmation-modal.mdx @@ -10,6 +10,8 @@ Use Confirmation Modal when the user needs extra context to make a decision, suc If the confirmation can be expressed as a single short paragraph, use [Alert Dialog](../components/alert-dialog). If the action is highly destructive and requires explicit typed intent, use [Text Confirm Dialog](../fragments/text-confirm-dialog). See [Modality](../ui-patterns/modality) for broader guidance on choosing the appropriate pattern. +For dirty-form dismissal in dialogs/sheets, prefer the dedicated discard-confirmation pattern (`DiscardChangesConfirmationDialog` + `useConfirmOnClose`) rather than creating new local `CloseConfirmationModal` wrappers. The ad-hoc `CloseConfirmationModal` wrapper pattern is deprecated for new implementations. + ## Usage @@ -54,6 +56,7 @@ export default function ConfirmationModalDemo() { ## Guidelines - **Use for moderate complexity:** Suitable when the confirmation requires more than a single sentence but does not need typed intent. +- **Do not use for standard dirty-form dismissal:** Prefer `DiscardChangesConfirmationDialog` for unsaved-changes prompts so copy, behavior, and wiring stay consistent across dialogs/sheets. - **Avoid critical destruction:** Do not use for irreversible or high-risk actions that could benefit from stronger safeguards. - **Keep content focused:** Include only the context needed to make the decision. If the dialog becomes a full flow, use a custom [Dialog](../components/dialog) instead. - **Provide clear actions:** Ensure confirm and cancel labels clearly describe the outcome of each choice. diff --git a/apps/design-system/content/docs/ui-patterns/modality.mdx b/apps/design-system/content/docs/ui-patterns/modality.mdx index 3f593a0eb8e..83683b10874 100644 --- a/apps/design-system/content/docs/ui-patterns/modality.mdx +++ b/apps/design-system/content/docs/ui-patterns/modality.mdx @@ -18,6 +18,18 @@ We have two main ways of handling modality: As a general rule: use dialogs for short, focused tasks and use sheets for longer forms or more detailed views. +### Dirty form dismissal pattern + +When a dialog or sheet contains a form, users should generally be allowed to attempt dismissal via all normal affordances (backdrop click, Escape key, close icon, and footer `Cancel` button). + +If the form is clean, close immediately. If the form has unsaved changes, show a discard-confirmation dialog instead of closing immediately. + +This pattern is implemented in Studio with `useConfirmOnClose` plus `DiscardChangesConfirmationDialog`. + +- **Prompt on footer `Cancel` too:** If a button is labeled `Cancel`, users expect it to stop the current action, not silently discard edits. Prompting keeps behavior consistent with backdrop/Escape dismissal and prevents accidental loss. +- **Use explicit labels for no-prompt discard:** If you intentionally want a one-click destructive exit, label the action `Discard` (or `Discard changes`) rather than `Cancel`. +- **Guard close attempts, not unmounts:** This pattern should intercept controlled modal/sheet close attempts (`onOpenChange`, close buttons, footer actions). It should not attempt to block route changes or arbitrary component unmounts. + ## Dialogs Dialogs are centered overlays used for short, focused tasks. All dialogs should follow these best practices: @@ -41,6 +53,22 @@ There are quite a few dialog components, each suited to a different task or cont +#### Discard changes confirmation dialog (pattern) + +For dirty form dismissal, use a short discard-confirmation dialog after a close attempt instead of disabling dismissal entirely. + +- The primary form remains in a `Dialog` or `Sheet` (dismissible). +- Closing is intercepted only when the form is dirty. +- The follow-up confirmation is an `AlertDialog` pattern (`DiscardChangesConfirmationDialog` in Studio). + +Typical flow: + +1. User attempts to close the dialog/sheet (backdrop, Escape, close icon, or `Cancel`) +2. If the form is clean, close immediately +3. If the form is dirty, show discard confirmation +4. `Keep editing` returns to the form +5. `Discard changes` closes and resets the form + #### Text Confirm Dialog [Text Confirm Dialog](../fragments/text-confirm-dialog) adds a deliberate speed bump for highly destructive actions by requiring the user to type an exact confirmation string before proceeding. The confirm action remains disabled until the input matches. diff --git a/apps/studio/components/interfaces/Auth/Hooks/CreateHookSheet.tsx b/apps/studio/components/interfaces/Auth/Hooks/CreateHookSheet.tsx index 3598ebd0f2d..8419af74e3a 100644 --- a/apps/studio/components/interfaces/Auth/Hooks/CreateHookSheet.tsx +++ b/apps/studio/components/interfaces/Auth/Hooks/CreateHookSheet.tsx @@ -1,6 +1,7 @@ import { zodResolver } from '@hookform/resolvers/zod' import { useParams } from 'common' import { convertArgumentTypes } from 'components/interfaces/Database/Functions/Functions.utils' +import { DiscardChangesConfirmationDialog } from 'components/ui-patterns/Dialogs/DiscardChangesConfirmationDialog' import CodeEditor from 'components/ui/CodeEditor/CodeEditor' import { DocsButton } from 'components/ui/DocsButton' import FunctionSelector from 'components/ui/FunctionSelector' @@ -9,6 +10,7 @@ import { AuthConfigResponse } from 'data/auth/auth-config-query' import { useAuthHooksUpdateMutation } from 'data/auth/auth-hooks-update-mutation' import { executeSql } from 'data/sql/execute-sql-query' import { useSelectedProjectQuery } from 'hooks/misc/useSelectedProject' +import { useConfirmOnClose } from 'hooks/ui/useConfirmOnClose' import { DOCS_URL } from 'lib/constants' import randomBytes from 'randombytes' import { useEffect, useMemo } from 'react' @@ -156,7 +158,16 @@ export const CreateHookSheet = ({ }, }) + const isDirty = form.formState.isDirty const values = form.watch() + const { + confirmOnClose, + handleOpenChange, + modalProps: discardChangesModalProps, + } = useConfirmOnClose({ + checkIsDirty: () => isDirty, + onClose, + }) const statements = useMemo(() => { let permissionChanges: string[] = [] @@ -270,7 +281,7 @@ export const CreateHookSheet = ({ }, [authConfig, title, visible, definition]) return ( - onClose()}> + -
-

- The following statements will be executed on the selected function: -

- -
+ + {statements.length > 0 && ( +
+

+ The following statements will be executed on the selected function: +

+ +
+ )} ) : (
@@ -470,7 +484,9 @@ export const CreateHookSheet = ({ className="rounded-l-none text-xs" onClick={() => { const authHookSecret = generateAuthHookSecret() - form.setValue('httpsValues.secret', authHookSecret) + form.setValue('httpsValues.secret', authHookSecret, { + shouldDirty: true, + }) }} > Generate secret @@ -494,7 +510,7 @@ export const CreateHookSheet = ({
)} - - {!hasAccessToSso ? ( + + {!hasAccessToSso && ( - ) : null} + )} } /> @@ -341,10 +311,7 @@ export const InviteMemberButton = () => { render={({ field }) => ( - form.setValue('role', value)} - > + {orgScopedRoles.find((role) => role.id === Number(field.value))?.name ?? 'Unknown'} @@ -382,10 +349,7 @@ export const InviteMemberButton = () => { render={({ field }) => ( - form.setValue('applyToOrg', value)} - /> + )} @@ -441,7 +405,7 @@ export const InviteMemberButton = () => { /> -