Skip to content

Commit c5d07cc

Browse files
authored
fix(settings): prevent false unsaved prompts and preserve drafts (#8704)
* fix(settings): prevent false custom block unsaved changes * fix(settings): preserve drafts and guard settings navigation * fix(settings): reconcile saved values and pending secret edits * fix(settings): preserve fresh snapshots and same-page history * fix(settings): expire stale history confirmations safely * fix(settings): retain drafts until history traversal occurs
1 parent 2e7082f commit c5d07cc

53 files changed

Lines changed: 4558 additions & 1498 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.agents/skills/add-settings-page/SKILL.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ Each grep lists candidates; review every match against the expected ones named b
6666
- Editable pages: confirm Save/Discard go through `saveDiscardActions()` and
6767
dirty is wired via `useSettingsUnsavedGuard` (called before early-return
6868
gates) — flag any hand-rolled Save button, `beforeunload`, or unsaved modal.
69-
`git grep -n "beforeunload" -- 'apps/sim/**/settings/**' 'apps/sim/ee/'`
70-
should only hit the centralized `use-settings-before-unload.ts`.
69+
`git grep -n "beforeunload" -- 'apps/sim/**/settings/**' 'apps/sim/ee/' 'apps/sim/components/settings/' ':(exclude,glob)**/*.test.*'`
70+
should only hit the centralized `use-settings-browser-navigation.ts`.
7171
5. Fix each finding with the smallest structural change that satisfies the checklist;
7272
do not touch handlers, state, queries, or gate returns. A pixel-size fix swaps
7373
only the size class for its exact-pixel token (`text-[12px]` → `text-caption`).

‎.claude/rules/sim-settings-pages.md‎

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -339,34 +339,33 @@ rendered through `<SettingsActionChips actions={…} />` from
339339
`@/components/settings/settings-header` — that is the shared chip path, and it
340340
is what keeps tone/icon/variant/tooltip handling from drifting between the two
341341
shells. Reach for it before hand-rolling a `Chip`.
342-
- **`useSettingsUnsavedGuard({ isDirty })`** (`…/settings/hooks/use-settings-unsaved-guard`)
343-
— syncs the page's local `isDirty` into the shared `useSettingsDirtyStore` (so
344-
the sidebar's **section-switch** confirm + the centralized `beforeunload` both
345-
apply for free) and returns `{ showUnsavedModal, setShowUnsavedModal, guardBack,
346-
confirmDiscard }` for a detail view's **in-view back** chip.
347-
- **Top-level pages** (whitelabeling, sso): call it **unassigned** —
348-
`useSettingsUnsavedGuard({ isDirty: hasChanges })` — they only need the
349-
store-sync; the sidebar/`beforeunload` do the rest.
350-
- **Detail sub-views** (data-retention, access-control group-detail): route the
351-
back chip through `onClick={() => guard.guardBack(closeFn)}` and render the
352-
shared `<UnsavedChangesModal open={guard.showUnsavedModal}
353-
onOpenChange={guard.setShowUnsavedModal} onDiscard={guard.confirmDiscard} />`
354-
(from `@/app/workspace/[workspaceId]/components/credential-detail`). The
355-
in-view header **Discard** chip (via `saveDiscardActions({ onDiscard })`) is a
356-
*reset to original* — distinct from the back-confirm's discard, which leaves.
357-
- **`useSettingsBeforeUnload`** is mounted by the settings shells
358-
(`settings/layout.tsx` and `components/settings/standalone-settings-shell.tsx`) —
359-
never add a per-page `beforeunload`.
360-
- **Dirty *computation* stays local** (shapes differ: field-compare vs
361-
normalize+stringify) — only how dirty is *consumed* is shared. Derive it (a
362-
`const`/`useMemo`), never store it in `useState`.
363-
- **CRITICAL — rules of hooks:** call `useSettingsUnsavedGuard(...)`
364-
**unconditionally, before every early-return gate** (entitlement / loading /
365-
not-entitled `return <SettingsEmptyState>`). A hook placed after a gate is
366-
skipped on gated renders and crashes.
367-
- The route-based credential detail keeps its own `useUnsavedChangesGuard` (it
368-
guards real `router.push` navigation + browser Back via a history sentinel);
369-
it already shares `UnsavedChangesModal`, so copy stays unified.
342+
- **`useSettingsUnsavedGuard({ isDirty, navigationBlocked, onDiscard })`** from
343+
`@/components/settings/use-settings-unsaved-guard` registers one editor with the
344+
shared settings store. Register every full-page draft, including inline creation
345+
forms. Dirty computation stays local and derived; fetched defaults stay clean.
346+
- **`onDiscard` resets the registered draft.** The root `SettingsNavigationGuard`
347+
owns the discard dialog, internal link interception, browser Back/Forward, and
348+
refresh protection for registered editors. Do not add a per-page dialog or
349+
history listener for an editor using this hook. Skill create/detail pages retain
350+
their existing `useUnsavedChangesGuard` and local dialog until they migrate to
351+
the shared registration.
352+
- **Native history needs entry indexes.** The Navigation API supplies them for
353+
legacy entries. Without it, native traversal to an unindexed entry cannot be
354+
cancelled reliably; never guess a direction or rewrite the history stack.
355+
- **History confirmation authorizes traversal before discarding.** Drafts are
356+
discarded only when the browser reports the confirmed traversal, so a no-op
357+
Back or Forward retains edits. Known cross-document traversal uses native unload
358+
protection. An unknown cross-document target on a classic History browser can
359+
require a second native confirmation after the shared dialog.
360+
- **Detail back controls** call `guard.guardBack(closeFn)`. Other destructive view
361+
transitions, such as switching an editor's direction or environment, also use
362+
the shared `requestLeave` action before resetting their draft.
363+
- **`navigationBlocked` covers pending saves and uploads.** The shared guard blocks
364+
navigation while requests are pending. Attempts are not queued; retry after they
365+
settle. Disable or preserve edits made during requests;
366+
failed saves retain drafts, and successful saves only clear committed values.
367+
- **Call the hook unconditionally, before every early-return gate** (entitlement,
368+
loading, or empty-state return), so gated renders preserve hook order.
370369

371370
## Detail sub-views
372371

‎.cursor/rules/sim-settings-pages.mdc‎

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -336,34 +336,33 @@ rendered through `<SettingsActionChips actions={…} />` from
336336
`@/components/settings/settings-header` — that is the shared chip path, and it
337337
is what keeps tone/icon/variant/tooltip handling from drifting between the two
338338
shells. Reach for it before hand-rolling a `Chip`.
339-
- **`useSettingsUnsavedGuard({ isDirty })`** (`…/settings/hooks/use-settings-unsaved-guard`)
340-
— syncs the page's local `isDirty` into the shared `useSettingsDirtyStore` (so
341-
the sidebar's **section-switch** confirm + the centralized `beforeunload` both
342-
apply for free) and returns `{ showUnsavedModal, setShowUnsavedModal, guardBack,
343-
confirmDiscard }` for a detail view's **in-view back** chip.
344-
- **Top-level pages** (whitelabeling, sso): call it **unassigned** —
345-
`useSettingsUnsavedGuard({ isDirty: hasChanges })` — they only need the
346-
store-sync; the sidebar/`beforeunload` do the rest.
347-
- **Detail sub-views** (data-retention, access-control group-detail): route the
348-
back chip through `onClick={() => guard.guardBack(closeFn)}` and render the
349-
shared `<UnsavedChangesModal open={guard.showUnsavedModal}
350-
onOpenChange={guard.setShowUnsavedModal} onDiscard={guard.confirmDiscard} />`
351-
(from `@/app/workspace/[workspaceId]/components/credential-detail`). The
352-
in-view header **Discard** chip (via `saveDiscardActions({ onDiscard })`) is a
353-
*reset to original* — distinct from the back-confirm's discard, which leaves.
354-
- **`useSettingsBeforeUnload`** is mounted by the settings shells
355-
(`settings/layout.tsx` and `components/settings/standalone-settings-shell.tsx`) —
356-
never add a per-page `beforeunload`.
357-
- **Dirty *computation* stays local** (shapes differ: field-compare vs
358-
normalize+stringify) — only how dirty is *consumed* is shared. Derive it (a
359-
`const`/`useMemo`), never store it in `useState`.
360-
- **CRITICAL — rules of hooks:** call `useSettingsUnsavedGuard(...)`
361-
**unconditionally, before every early-return gate** (entitlement / loading /
362-
not-entitled `return <SettingsEmptyState>`). A hook placed after a gate is
363-
skipped on gated renders and crashes.
364-
- The route-based credential detail keeps its own `useUnsavedChangesGuard` (it
365-
guards real `router.push` navigation + browser Back via a history sentinel);
366-
it already shares `UnsavedChangesModal`, so copy stays unified.
339+
- **`useSettingsUnsavedGuard({ isDirty, navigationBlocked, onDiscard })`** from
340+
`@/components/settings/use-settings-unsaved-guard` registers one editor with the
341+
shared settings store. Register every full-page draft, including inline creation
342+
forms. Dirty computation stays local and derived; fetched defaults stay clean.
343+
- **`onDiscard` resets the registered draft.** The root `SettingsNavigationGuard`
344+
owns the discard dialog, internal link interception, browser Back/Forward, and
345+
refresh protection for registered editors. Do not add a per-page dialog or
346+
history listener for an editor using this hook. Skill create/detail pages retain
347+
their existing `useUnsavedChangesGuard` and local dialog until they migrate to
348+
the shared registration.
349+
- **Native history needs entry indexes.** The Navigation API supplies them for
350+
legacy entries. Without it, native traversal to an unindexed entry cannot be
351+
cancelled reliably; never guess a direction or rewrite the history stack.
352+
- **History confirmation authorizes traversal before discarding.** Drafts are
353+
discarded only when the browser reports the confirmed traversal, so a no-op
354+
Back or Forward retains edits. Known cross-document traversal uses native unload
355+
protection. An unknown cross-document target on a classic History browser can
356+
require a second native confirmation after the shared dialog.
357+
- **Detail back controls** call `guard.guardBack(closeFn)`. Other destructive view
358+
transitions, such as switching an editor's direction or environment, also use
359+
the shared `requestLeave` action before resetting their draft.
360+
- **`navigationBlocked` covers pending saves and uploads.** The shared guard blocks
361+
navigation while requests are pending. Attempts are not queued; retry after they
362+
settle. Disable or preserve edits made during requests;
363+
failed saves retain drafts, and successful saves only clear committed values.
364+
- **Call the hook unconditionally, before every early-return gate** (entitlement,
365+
loading, or empty-state return), so gated renders preserve hook order.
367366

368367
## Detail sub-views
369368

‎apps/sim/app/layout.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { Metadata, Viewport } from 'next'
33
import Script from 'next/script'
44
import { NuqsAdapter } from 'nuqs/adapters/next/app'
55
import { BrandedLayout } from '@/components/branded-layout'
6+
import { SettingsNavigationGuard } from '@/components/settings/settings-navigation-guard'
67
import { PasteAdmissionGuard } from '@/app/_shell/paste-admission-guard'
78
import { BrowserTelemetry } from '@/app/_shell/providers/browser-telemetry'
89
import { PostHogProvider } from '@/app/_shell/providers/posthog-provider'
@@ -45,6 +46,7 @@ export default function RootLayout({ children }: { children: React.ReactNode })
4546
const themeCSS = generateThemeCSS()
4647
const application = (
4748
<ToastProvider>
49+
<SettingsNavigationGuard />
4850
<DesktopUpdateNotification />
4951
<PasteAdmissionGuard />
5052
<PostHogProvider consentRequired={isHosted}>

‎apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.tsx‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import type { ResourceScope } from '@/lib/core/resource-scope'
1414
import { organizationRoutes } from '@/lib/navigation/paths'
1515
import { describeSearchSource } from '@/lib/sim-search/source-identity'
1616
import { useOrganizationContext } from '@/app/o/[organizationId]/providers/organization-provider'
17-
import { UnsavedChangesModal } from '@/app/workspace/[workspaceId]/components/credential-detail/components/unsaved-changes-modal'
1817
import { ConnectorActionFeedback } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connector-actions'
1918
import { getConnectorSyncState } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/connector-sync-state'
2019
import { useConnectorActions } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/connectors-section/use-connector-actions'
@@ -341,7 +340,11 @@ function SourceSettingsForm({
341340
isSearchIndex: true,
342341
onSaved,
343342
})
344-
const guard = useSettingsUnsavedGuard({ isDirty: form.dirty, navigationBlocked: form.saving })
343+
const guard = useSettingsUnsavedGuard({
344+
isDirty: form.dirty,
345+
navigationBlocked: form.saving,
346+
onDiscard,
347+
})
345348
return (
346349
<SourcePanel
347350
connector={connector}
@@ -370,11 +373,6 @@ function SourceSettingsForm({
370373
<div className='-mx-2 flex flex-col gap-4'>
371374
<ConnectorSettingsFields {...form.fieldsProps} />
372375
</div>
373-
<UnsavedChangesModal
374-
open={guard.showUnsavedModal}
375-
onOpenChange={guard.setShowUnsavedModal}
376-
onDiscard={guard.confirmDiscard}
377-
/>
378376
</SourcePanel>
379377
)
380378
}

‎apps/sim/app/o/[organizationId]/settings/layout.tsx‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,13 @@
22

33
import type { ReactNode } from 'react'
44
import { SettingsPendingSection } from '@/components/settings/settings-pending-section'
5-
import { useSettingsBeforeUnload } from '@/components/settings/use-settings-before-unload'
65
import { resolveOrganizationSurfaceHeaderMeta } from '@/app/o/[organizationId]/settings/navigation'
76

87
interface OrganizationSettingsLayoutProps {
98
children: ReactNode
109
}
1110

1211
export default function OrganizationSettingsLayout({ children }: OrganizationSettingsLayoutProps) {
13-
useSettingsBeforeUnload()
1412
return (
1513
<div className='flex h-full flex-col bg-[var(--bg)]'>
1614
<SettingsPendingSection resolveMeta={resolveOrganizationSurfaceHeaderMeta}>

0 commit comments

Comments
 (0)