mirror of
https://github.com/supabase/supabase.git
synced 2026-10-09 03:15:06 +03:00
## 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 <joshenlimek@gmail.com>
154 lines
3.4 KiB
TypeScript
154 lines
3.4 KiB
TypeScript
import { act, renderHook } from '@testing-library/react'
|
|
import { describe, expect, it, vi } from 'vitest'
|
|
|
|
import { useConfirmOnClose } from './useConfirmOnClose'
|
|
|
|
describe('useConfirmOnClose', () => {
|
|
it('closes immediately when the form is clean', () => {
|
|
const onClose = vi.fn()
|
|
|
|
const { result } = renderHook(() =>
|
|
useConfirmOnClose({
|
|
checkIsDirty: () => false,
|
|
onClose,
|
|
})
|
|
)
|
|
|
|
act(() => {
|
|
result.current.confirmOnClose()
|
|
})
|
|
|
|
expect(onClose).toHaveBeenCalledTimes(1)
|
|
expect(result.current.modalProps.visible).toBe(false)
|
|
})
|
|
|
|
it('opens the confirmation modal when the form is dirty', () => {
|
|
const onClose = vi.fn()
|
|
|
|
const { result } = renderHook(() =>
|
|
useConfirmOnClose({
|
|
checkIsDirty: () => true,
|
|
onClose,
|
|
})
|
|
)
|
|
|
|
act(() => {
|
|
result.current.confirmOnClose()
|
|
})
|
|
|
|
expect(onClose).not.toHaveBeenCalled()
|
|
expect(result.current.modalProps.visible).toBe(true)
|
|
})
|
|
|
|
it('ignores open events and handles close events via handleOpenChange', () => {
|
|
const onClose = vi.fn()
|
|
|
|
const { result } = renderHook(() =>
|
|
useConfirmOnClose({
|
|
checkIsDirty: () => true,
|
|
onClose,
|
|
})
|
|
)
|
|
|
|
act(() => {
|
|
result.current.handleOpenChange(true)
|
|
})
|
|
|
|
expect(onClose).not.toHaveBeenCalled()
|
|
expect(result.current.modalProps.visible).toBe(false)
|
|
|
|
act(() => {
|
|
result.current.handleOpenChange(false)
|
|
})
|
|
|
|
expect(onClose).not.toHaveBeenCalled()
|
|
expect(result.current.modalProps.visible).toBe(true)
|
|
})
|
|
|
|
it('confirms and closes after the discard modal is accepted', () => {
|
|
const onClose = vi.fn()
|
|
|
|
const { result } = renderHook(() =>
|
|
useConfirmOnClose({
|
|
checkIsDirty: () => true,
|
|
onClose,
|
|
})
|
|
)
|
|
|
|
act(() => {
|
|
result.current.confirmOnClose()
|
|
})
|
|
|
|
expect(result.current.modalProps.visible).toBe(true)
|
|
|
|
act(() => {
|
|
result.current.modalProps.onClose()
|
|
})
|
|
|
|
expect(onClose).toHaveBeenCalledTimes(1)
|
|
expect(result.current.modalProps.visible).toBe(false)
|
|
})
|
|
|
|
it('cancels and keeps the form open after the discard modal is dismissed', () => {
|
|
const onClose = vi.fn()
|
|
|
|
const { result } = renderHook(() =>
|
|
useConfirmOnClose({
|
|
checkIsDirty: () => true,
|
|
onClose,
|
|
})
|
|
)
|
|
|
|
act(() => {
|
|
result.current.confirmOnClose()
|
|
})
|
|
|
|
expect(result.current.modalProps.visible).toBe(true)
|
|
|
|
act(() => {
|
|
result.current.modalProps.onCancel()
|
|
})
|
|
|
|
expect(onClose).not.toHaveBeenCalled()
|
|
expect(result.current.modalProps.visible).toBe(false)
|
|
})
|
|
|
|
it('uses the latest checkIsDirty and onClose callbacks', () => {
|
|
const onCloseA = vi.fn()
|
|
const onCloseB = vi.fn()
|
|
let isDirty = false
|
|
|
|
const { result, rerender } = renderHook(
|
|
({ onClose }: { onClose: () => void }) =>
|
|
useConfirmOnClose({
|
|
checkIsDirty: () => isDirty,
|
|
onClose,
|
|
}),
|
|
{
|
|
initialProps: { onClose: onCloseA },
|
|
}
|
|
)
|
|
|
|
act(() => {
|
|
result.current.confirmOnClose()
|
|
})
|
|
|
|
expect(onCloseA).toHaveBeenCalledTimes(1)
|
|
|
|
isDirty = true
|
|
rerender({ onClose: onCloseB })
|
|
|
|
act(() => {
|
|
result.current.confirmOnClose()
|
|
})
|
|
|
|
expect(result.current.modalProps.visible).toBe(true)
|
|
|
|
act(() => {
|
|
result.current.modalProps.onClose()
|
|
})
|
|
|
|
expect(onCloseB).toHaveBeenCalledTimes(1)
|
|
})
|
|
})
|