From 3edc45f2f2349fbf7b6baf139e78d34f8bfd1772 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 1 Aug 2026 17:14:04 -0700 Subject: [PATCH] improvement(settings): unify save/discard across editable surfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Save is now always rendered (primary, disabled until there is something to save) and Discard only appears once there is something to discard. Previously sandboxes showed no action at all until dirty — so a new sandbox had no visible Create button — while skills always showed a disabled Save and no Discard. - saveDiscardActions moves to @/components/settings and owns the one rule - adds SettingsActionChip / SettingsActionChips so the settings shell and the ReactNode-slot headers render actions through the same chip path - skills, secrets, connected credentials, custom tools and the secrets manager drop their hand-rolled Save chips for the shared helper - credential-detail drafts seed once per credential instead of re-seeding from every refetch, which was acting as an implicit discard --- .claude/rules/sim-settings-pages.md | 29 ++++- .../hooks/use-credential-detail-form.ts | 28 +++- .../components/invite-modal/invite-modal.tsx | 3 +- .../connected-credential-detail.tsx | 10 +- .../custom-tool-detail/custom-tool-detail.tsx | 42 ++---- .../components/sandboxes/sandboxes.tsx | 3 +- .../save-discard-actions.ts | 33 ----- .../secrets-manager/secrets-manager.tsx | 35 ++--- .../secrets/hooks/use-secret-value.ts | 13 +- .../secrets/[credentialId]/secret-detail.tsx | 10 +- .../skills/[skillId]/skill-detail.tsx | 37 ++++-- .../[workspaceId]/skills/new/skill-create.tsx | 30 +++-- .../settings/save-discard-actions.tsx | 58 +++++++++ .../components/settings/settings-header.tsx | 121 ++++++++++++------ .../components/group-detail.tsx | 2 +- .../components/custom-block-detail.tsx | 2 +- .../components/data-retention-settings.tsx | 4 +- .../components/session-policy-settings.tsx | 2 +- .../ee/sso/components/sso-settings.test.tsx | 9 +- apps/sim/ee/sso/components/sso-settings.tsx | 4 +- .../components/whitelabeling-settings.tsx | 2 +- .../ee/workspace-forking/components/forks.tsx | 4 +- 22 files changed, 297 insertions(+), 184 deletions(-) delete mode 100644 apps/sim/app/workspace/[workspaceId]/settings/components/save-discard-actions/save-discard-actions.ts create mode 100644 apps/sim/components/settings/save-discard-actions.tsx diff --git a/.claude/rules/sim-settings-pages.md b/.claude/rules/sim-settings-pages.md index 154b45a2599..8710553e800 100644 --- a/.claude/rules/sim-settings-pages.md +++ b/.claude/rules/sim-settings-pages.md @@ -167,11 +167,32 @@ Any settings surface with editable state uses **one** shared stack — never hand-roll a Save button, a Discard button, a `beforeunload`, or an "Unsaved changes" modal: -- **`saveDiscardActions(config)`** (`…/components/save-discard-actions/save-discard-actions`) - — returns the canonical dirty-gated **Discard + Save** `SettingsAction[]` (empty - when not dirty). Spread it into a `SettingsPanel` `actions` array, beside any +- **`saveDiscardActions(config)`** (`@/components/settings/save-discard-actions`) + — returns the canonical **Discard + Save** `SettingsAction[]`. **Save is always + rendered** (primary), disabled until there is something to save, so every + editable surface announces its primary action in the same place and a create + form is never a page with no visible way to commit it; **Discard appears only + when dirty**. Spread it into a `SettingsPanel` `actions` array, beside any sibling actions (a detail view's Delete / Remove override). Config: `dirty`, - `saving`, `onSave`, `onDiscard`, `saveDisabled?`, `saveLabel?`, `savingLabel?`. + `saving`, `onSave`, `onDiscard`, `saveDisabled?`, `saveTooltip?`, `creating?`, + `saveLabel?`, `savingLabel?`. Create flows pass `creating` — the + Create / Creating... labels come as a pair and can never drift apart. + `saveLabel`/`savingLabel` are only for genuinely bespoke wording (SSO's + `Update`); never hand-roll the pair to get a create label. +- **``** (same module) — the identical rule + rendered as chips, for surfaces whose header takes a `ReactNode` instead of + action data (`CredentialDetailLayout`: skills, secrets, connected credentials). + Both stacks derive from the one function; never hand-roll a Save chip. + +`CredentialDetailLayout` stays slot-driven for exactly two reasons: its back +control is a real `` (deep-linkable / middle-clickable, which +`SettingsBackAction`'s `onSelect` cannot express), and actions like +`SkillImportButton` own a hidden file input and their own pending state. +**Everything else in one of those headers should be `SettingsAction` data** +rendered through `` from +`@/components/settings/settings-header` — that is the shared chip path, and it +is what keeps tone/icon/variant/tooltip handling from drifting between the two +shells. Reach for it before hand-rolling a `Chip`. - **`useSettingsUnsavedGuard({ isDirty })`** (`…/settings/hooks/use-settings-unsaved-guard`) — syncs the page's local `isDirty` into the shared `useSettingsDirtyStore` (so the sidebar's **section-switch** confirm + the centralized `beforeunload` both diff --git a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts index 7bd0e1375d0..6ebda086901 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts +++ b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts @@ -1,6 +1,6 @@ 'use client' -import { useCallback, useEffect, useState } from 'react' +import { useCallback, useState } from 'react' import { toast } from '@sim/emcn' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' @@ -30,11 +30,21 @@ export function useCredentialDetailForm({ const [displayNameDraft, setDisplayNameDraft] = useState('') const [descriptionDraft, setDescriptionDraft] = useState('') + const [seededCredentialId, setSeededCredentialId] = useState(null) - useEffect(() => { - setDisplayNameDraft(credential?.displayName ?? '') - setDescriptionDraft(credential?.description ?? '') - }, [credential?.id, credential?.displayName, credential?.description]) + // Seed drafts when the credential first resolves (or the route id changes); a + // background refetch of the same credential must not clobber an in-progress + // edit — Discard is the one way to reset. + /** Applies a credential to both drafts — the one definition of "reset to server state". */ + const seedDrafts = useCallback((source: WorkspaceCredential) => { + setDisplayNameDraft(source.displayName) + setDescriptionDraft(source.description ?? '') + }, []) + + if (credential && credential.id !== seededCredentialId) { + setSeededCredentialId(credential.id) + seedDrafts(credential) + } const isDisplayNameDirty = credential ? displayNameDraft !== credential.displayName : false const isDescriptionDirty = credential @@ -68,9 +78,14 @@ export function useCredentialDetailForm({ isDescriptionDirty, displayNameDraft, descriptionDraft, - updateCredential, + updateCredential.mutateAsync, + updateCredential.isPending, ]) + const discard = useCallback(() => { + if (credential) seedDrafts(credential) + }, [credential, seedDrafts]) + return { displayNameDraft, setDisplayNameDraft, @@ -78,6 +93,7 @@ export function useCredentialDetailForm({ setDescriptionDraft, isDirty, save, + discard, isSaving: updateCredential.isPending, handleBackClick: guard.handleBackClick, showUnsavedAlert: guard.showUnsavedAlert, diff --git a/apps/sim/app/workspace/[workspaceId]/components/invite-modal/invite-modal.tsx b/apps/sim/app/workspace/[workspaceId]/components/invite-modal/invite-modal.tsx index e80ebb4033a..c110e1b064e 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/invite-modal/invite-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/components/invite-modal/invite-modal.tsx @@ -40,8 +40,7 @@ const MEMBERSHIP_OPTIONS = [ type Membership = (typeof MEMBERSHIP_OPTIONS)[number]['value'] -const MEMBERSHIP_HINTS: Record = { - member: 'Joins your organization. Adds a seat.', +const MEMBERSHIP_HINTS: Partial> = { admin: 'Joins your organization and can manage it. Adds a seat.', external: 'Access to the selected workspaces only — no seat. Only available for people already on a paid Sim plan.', diff --git a/apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx b/apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx index 13a7378f9b5..fa40014816c 100644 --- a/apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx +++ b/apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx @@ -15,6 +15,7 @@ import { ArrowLeft } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { useRouter } from 'next/navigation' +import { SaveDiscardChips } from '@/components/settings/save-discard-actions' import { writeOAuthReturnContext } from '@/lib/credentials/client-state' import { resolveCredentialDisplay } from '@/lib/integrations' import { @@ -204,9 +205,12 @@ export function ConnectedCredentialDetail({ > Disconnect - - {form.isSaving ? 'Saving...' : 'Save'} - + ) : null diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/custom-tools/components/custom-tool-detail/custom-tool-detail.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/custom-tools/components/custom-tool-detail/custom-tool-detail.tsx index db3b2f47db2..88788ec0ea7 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/custom-tools/components/custom-tool-detail/custom-tool-detail.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/custom-tools/components/custom-tool-detail/custom-tool-detail.tsx @@ -5,6 +5,7 @@ import { ChipConfirmModal, toast } from '@sim/emcn' import { ArrowLeft, Wrench } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' +import { saveDiscardActions } from '@/components/settings/save-discard-actions' import { ResourceTile } from '@/app/workspace/[workspaceId]/components' import { CredentialDetailHeading, @@ -23,8 +24,6 @@ import { useSchemaGeneration, validateCustomToolSchema, } from '@/app/workspace/[workspaceId]/components/custom-tool-editor' -import { saveDiscardActions } from '@/app/workspace/[workspaceId]/settings/components/save-discard-actions/save-discard-actions' -import type { SettingsAction } from '@/app/workspace/[workspaceId]/settings/components/settings-header/settings-header' import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel' import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section' import { useSettingsUnsavedGuard } from '@/app/workspace/[workspaceId]/settings/hooks/use-settings-unsaved-guard' @@ -50,9 +49,9 @@ interface CustomToolDetailProps { /** * Full-page custom tool editor rendered as a settings detail sub-view: a back - * chip, dirty-gated Discard/Save, Delete, and the Schema and Code editors - * stacked (no tabs — the page has room for both). Uses the same fields as the - * canvas modal so the two surfaces never drift. + * chip, Save/Discard, Delete, and the Schema and Code editors stacked (no tabs — + * the page has room for both). Uses the same fields as the canvas modal so the + * two surfaces never drift. */ export function CustomToolDetail({ workspaceId, @@ -200,22 +199,6 @@ export function CustomToolDetail({ } } - /** - * On create, the primary action is always visible so the page announces what - * it is for — disabled until the schema is a valid function definition. - * (`saveDiscardActions` is dirty-gated and would render nothing on an empty - * draft.) Discard still only appears once there is something to discard. - */ - const createToolActions: SettingsAction[] = [ - ...(dirty ? [{ text: 'Discard', onSelect: handleDiscard, disabled: saving }] : []), - { - text: saving ? 'Creating...' : 'Create', - variant: 'primary' as const, - onSelect: handleSave, - disabled: saving || streaming || !isSchemaValid, - }, - ] - return ( <> void - onDiscard: () => void - saveDisabled?: boolean - savingLabel?: string - saveLabel?: string -} - -/** The dirty-gated Discard + Save action pair for settings surfaces — empty when not dirty. */ -export function saveDiscardActions({ - dirty, - saving, - onSave, - onDiscard, - saveDisabled = false, - savingLabel = 'Saving...', - saveLabel = 'Save', -}: SaveDiscardActionsConfig): SettingsAction[] { - if (!dirty) return [] - return [ - { text: 'Discard', onSelect: onDiscard, disabled: saving }, - { - text: saving ? savingLabel : saveLabel, - variant: 'primary', - onSelect: onSave, - disabled: saving || saveDisabled, - }, - ] -} diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx index 3566336a680..4b6d58ee8ab 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx @@ -6,6 +6,7 @@ import { createLogger } from '@sim/logger' import { useQueryClient } from '@tanstack/react-query' import { useParams, useRouter } from 'next/navigation' import { canMutateWorkspaceSettingsSection } from '@/components/settings/navigation' +import { saveDiscardActions } from '@/components/settings/save-discard-actions' import { clearPendingCredentialCreateRequest, PENDING_CREDENTIAL_CREATE_REQUEST_EVENT, @@ -17,7 +18,6 @@ import { UnsavedChangesModal } from '@/app/workspace/[workspaceId]/components/cr import { RowActionsMenu } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu' import { SecretValueField } from '@/app/workspace/[workspaceId]/settings/components/secrets/components/secret-value-field' import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' -import type { SettingsAction } from '@/app/workspace/[workspaceId]/settings/components/settings-header/settings-header' import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel' import { useSettingsSearch } from '@/app/workspace/[workspaceId]/settings/components/use-settings-search' import { isValidEnvVarName } from '@/executor/constants' @@ -981,27 +981,18 @@ export function SecretsManager() { onChange: setSearchTerm, placeholder: 'Search secrets...', }} - actions={[ - ...(hasChanges - ? [ - { - text: 'Discard', - onSelect: handleCancel, - disabled: isListSaving, - } satisfies SettingsAction, - ] - : []), - { - text: isListSaving ? 'Saving...' : 'Save', - onSelect: handleSave, - disabled: hasConflicts || hasInvalidKeys || isLoading || !hasChanges || isListSaving, - tooltip: hasConflicts - ? 'Resolve all conflicts before saving' - : hasInvalidKeys - ? 'Fix invalid variable names before saving' - : undefined, - }, - ]} + actions={saveDiscardActions({ + dirty: hasChanges, + saving: isListSaving, + onSave: handleSave, + onDiscard: handleCancel, + saveDisabled: hasConflicts || hasInvalidKeys || isLoading, + saveTooltip: hasConflicts + ? 'Resolve all conflicts before saving' + : hasInvalidKeys + ? 'Fix invalid variable names before saving' + : undefined, + })} > {!isLoading && (
diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts b/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts index 394aa641cab..8e246fb6de4 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts @@ -97,5 +97,16 @@ export function useSecretValue({ workspaceId, credential }: UseSecretValueParams } } - return { value: draft, setValue: setDraft, canEdit, isConflicted, isDirty, save, isSaving } + const discard = () => setDraft(currentValue) + + return { + value: draft, + setValue: setDraft, + canEdit, + isConflicted, + isDirty, + save, + discard, + isSaving, + } } diff --git a/apps/sim/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail.tsx b/apps/sim/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail.tsx index 7aca00a8c7f..b1412d0d7f2 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail.tsx @@ -3,6 +3,7 @@ import { useState } from 'react' import { Chip, ChipCopyInput, ChipLink, Send } from '@sim/emcn' import { ArrowLeft, Key } from '@sim/emcn/icons' +import { SaveDiscardChips } from '@/components/settings/save-discard-actions' import { AddPeopleModal, CredentialDetailHeading, @@ -51,9 +52,12 @@ export function SecretDetail({ workspaceId, credentialId }: SecretDetailProps) { )} {canEditValue && ( - - {valueField.isSaving ? 'Saving...' : 'Save'} - + )} ) : null diff --git a/apps/sim/app/workspace/[workspaceId]/skills/[skillId]/skill-detail.tsx b/apps/sim/app/workspace/[workspaceId]/skills/[skillId]/skill-detail.tsx index 68597185bd4..9c37ba94fdb 100644 --- a/apps/sim/app/workspace/[workspaceId]/skills/[skillId]/skill-detail.tsx +++ b/apps/sim/app/workspace/[workspaceId]/skills/[skillId]/skill-detail.tsx @@ -7,6 +7,7 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { useRouter } from 'next/navigation' import { AddPeopleModal } from '@/components/permissions' +import { SaveDiscardChips } from '@/components/settings/save-discard-actions' import { SkillTile } from '@/app/workspace/[workspaceId]/components' import { CredentialDetailHeading, @@ -36,7 +37,7 @@ interface SkillDetailProps { /** * Full-page skill detail, mirroring the integration credential detail surface: - * a fixed action bar (Share / Delete / Save), a heading, editable Name / + * a fixed action bar (Share / Delete / Discard / Save), a heading, editable Name / * Description / Content sections, and the Skill Editors roster. Non-editors * and built-in template skills render read-only. */ @@ -71,15 +72,20 @@ export function SkillDetail({ workspaceId, skillId }: SkillDetailProps) { const [showDeleteConfirm, setShowDeleteConfirm] = useState(false) const [prevSkillId, setPrevSkillId] = useState(null) + /** Applies a full skill shape to all three drafts and remounts the Content editor. */ + const seedDrafts = (source: { name: string; description: string; content: string }) => { + setNameDraft(source.name) + setDescriptionDraft(source.description) + setContentDraft(source.content) + setErrors({}) + setContentSeed((seed) => seed + 1) + } + // Seed drafts when the skill first resolves (or the route id changes); a // background refetch of the same skill must not clobber an in-progress edit. if (skill && skill.id !== prevSkillId) { setPrevSkillId(skill.id) - setNameDraft(skill.name) - setDescriptionDraft(skill.description) - setContentDraft(skill.content) - setErrors({}) - setContentSeed((seed) => seed + 1) + seedDrafts(skill) } const isDirty = @@ -155,11 +161,7 @@ export function SkillDetail({ workspaceId, skillId }: SkillDetailProps) { const handleContentPaste = (text: string): boolean => { const parsed = parseSkillMarkdown(text) if (!parsed.nameFromFrontmatter) return false - setNameDraft(parsed.name) - setDescriptionDraft(parsed.description) - setContentDraft(parsed.content) - setErrors({}) - setContentSeed((seed) => seed + 1) + seedDrafts(parsed) return true } @@ -169,6 +171,10 @@ export function SkillDetail({ workspaceId, skillId }: SkillDetailProps) { ) + const handleDiscard = () => { + if (skill) seedDrafts(skill) + } + const actions = skill && canEdit ? ( <> @@ -178,9 +184,12 @@ export function SkillDetail({ workspaceId, skillId }: SkillDetailProps) { setShowDeleteConfirm(true)} disabled={deleteSkill.isPending}> Delete - - {updateSkill.isPending ? 'Saving...' : 'Save'} - + ) : null diff --git a/apps/sim/app/workspace/[workspaceId]/skills/new/skill-create.tsx b/apps/sim/app/workspace/[workspaceId]/skills/new/skill-create.tsx index 5eaddfeee45..873e7aa9df9 100644 --- a/apps/sim/app/workspace/[workspaceId]/skills/new/skill-create.tsx +++ b/apps/sim/app/workspace/[workspaceId]/skills/new/skill-create.tsx @@ -1,11 +1,12 @@ 'use client' import { useState } from 'react' -import { Chip, ChipLink, toast } from '@sim/emcn' +import { ChipLink, toast } from '@sim/emcn' import { ArrowLeft } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { useRouter } from 'next/navigation' +import { SaveDiscardChips } from '@/components/settings/save-discard-actions' import { SkillTile } from '@/app/workspace/[workspaceId]/components' import { CredentialDetailHeading, @@ -34,7 +35,7 @@ interface SkillCreateProps { /** * Full-page skill creation, mirroring the skill detail surface: a fixed action - * bar (Import / Create skill), a heading, and the editable Name / Description / + * bar (Import / Discard / Create), a heading, and the editable Name / Description / * Content sections. Importing a SKILL.md prefills all three fields in place. */ export function SkillCreate({ workspaceId }: SkillCreateProps) { @@ -90,10 +91,11 @@ export function SkillCreate({ workspaceId }: SkillCreateProps) { } } - const applyImportedSkill = (data: ParsedSkill) => { - setNameDraft(data.name) - setDescriptionDraft(data.description) - setContentDraft(data.content) + /** Applies a full skill shape to all three drafts and remounts the Content editor. */ + const seedDrafts = (source: Pick) => { + setNameDraft(source.name) + setDescriptionDraft(source.description) + setContentDraft(source.content) setErrors({}) setContentSeed((seed) => seed + 1) } @@ -106,7 +108,7 @@ export function SkillCreate({ workspaceId }: SkillCreateProps) { const handleContentPaste = (text: string): boolean => { const parsed = parseSkillMarkdown(text) if (!parsed.nameFromFrontmatter) return false - applyImportedSkill(parsed) + seedDrafts(parsed) return true } @@ -116,12 +118,18 @@ export function SkillCreate({ workspaceId }: SkillCreateProps) { ) + const handleDiscard = () => seedDrafts({ name: '', description: '', content: '' }) + const actions = ( <> - - - {createSkill.isPending ? 'Creating...' : 'Create'} - + + ) diff --git a/apps/sim/components/settings/save-discard-actions.tsx b/apps/sim/components/settings/save-discard-actions.tsx new file mode 100644 index 00000000000..339f1ede0ab --- /dev/null +++ b/apps/sim/components/settings/save-discard-actions.tsx @@ -0,0 +1,58 @@ +import { type SettingsAction, SettingsActionChips } from '@/components/settings/settings-header' + +export interface SaveDiscardActionsConfig { + dirty: boolean + saving: boolean + onSave: () => void + onDiscard: () => void + saveDisabled?: boolean + /** Labels the primary action `Create` / `Creating...` instead of `Save` / `Saving...`. */ + creating?: boolean + /** Bespoke wording (e.g. `Update`). Prefer `creating` for the standard create flow. */ + saveLabel?: string + /** Bespoke in-flight wording. Pair it with `saveLabel` so the two can't drift. */ + savingLabel?: string + /** Explains a `saveDisabled` Save on hover — Save is always visible, so a blocked one should say why. */ + saveTooltip?: string +} + +/** + * The canonical Save/Discard header actions for any editable surface. + * + * Save is always rendered — every editable surface announces its primary action + * in the same place, and a create form is never a page with no visible way to + * commit it — and is disabled until there is something to save. Discard only + * appears once there is something to discard. + */ +export function saveDiscardActions({ + dirty, + saving, + onSave, + onDiscard, + saveDisabled = false, + creating = false, + saveLabel = creating ? 'Create' : 'Save', + savingLabel = creating ? 'Creating...' : 'Saving...', + saveTooltip, +}: SaveDiscardActionsConfig): SettingsAction[] { + return [ + ...(dirty ? [{ id: 'discard', text: 'Discard', onSelect: onDiscard, disabled: saving }] : []), + { + id: 'save', + text: saving ? savingLabel : saveLabel, + variant: 'primary', + onSelect: onSave, + disabled: saving || saveDisabled || !dirty, + tooltip: saveTooltip, + }, + ] +} + +/** + * {@link saveDiscardActions} rendered as chips, for surfaces whose header takes + * a `ReactNode` (`CredentialDetailLayout`) instead of the settings shell's + * action data. Both stacks derive their chips from the one rule above. + */ +export function SaveDiscardChips(config: SaveDiscardActionsConfig) { + return +} diff --git a/apps/sim/components/settings/settings-header.tsx b/apps/sim/components/settings/settings-header.tsx index 98d507b3d01..9093b4d70b4 100644 --- a/apps/sim/components/settings/settings-header.tsx +++ b/apps/sim/components/settings/settings-header.tsx @@ -19,6 +19,8 @@ import { PAGE_HEADER_BAR } from '@/components/page-header-bar' const useIsomorphicLayoutEffect = typeof window === 'undefined' ? useEffect : useLayoutEffect export interface SettingsAction { + /** Stable render identity. Falls back to `text`, which remounts the chip whenever the label flips (Save → Saving...). */ + id?: string text: string textTone?: 'error' icon?: ComponentType<{ className?: string }> @@ -116,6 +118,75 @@ export function useSettingsHeader(config: SettingsHeaderConfig) { }, [register]) } +interface SettingsActionChipProps { + /** Presentation fields plus the default `onSelect` / `onPrefetch` handlers. */ + action: SettingsAction + /** Overrides `action.onSelect` — the header shell passes a ref-reading indirection to avoid stale closures. */ + onSelect?: () => void + /** Overrides `action.onPrefetch`, same reason. */ + onPrefetch?: () => void +} + +/** + * The one chip rendering of a {@link SettingsAction}. Both header stacks render + * through this, so an action's chrome — tone, icon, variant, disabled, tooltip — + * is identical wherever it is mounted. Container spacing still belongs to the + * enclosing shell. + */ +export function SettingsActionChip({ + action, + onSelect = action.onSelect, + onPrefetch = action.onPrefetch, +}: SettingsActionChipProps) { + const chip = ( + is not a hit-test target, so the tooltip's wrapping + // span would never see pointerenter and the explanation would never show. + className={cn(action.tooltip && action.disabled && 'pointer-events-none')} + > + {action.textTone === 'error' ? ( + {action.text} + ) : ( + action.text + )} + + ) + if (!action.tooltip) return chip + return ( + + + {chip} + + {action.tooltip} + + ) +} + +/** + * Renders a {@link SettingsAction} list as chips using each action's own + * handlers. This is the data path for headers that take a `ReactNode` + * (`CredentialDetailLayout`) — reach for it instead of hand-rolling `Chip`s, so + * those surfaces keep the same chrome as the settings shell. The shell itself + * maps through {@link SettingsActionChip} directly, since it must route every + * handler through a ref to avoid stale closures. + */ +export function SettingsActionChips({ actions }: { actions: SettingsAction[] }) { + return ( + <> + {actions.map((action) => ( + + ))} + + ) +} + export function SettingsHeaderShell({ children }: { children: ReactNode }) { const read = useContext(ReadContext) const configRef = read?.configRef @@ -138,44 +209,18 @@ export function SettingsHeaderShell({ children }: { children: ReactNode }) { Docs )} - {actions?.map((action, index) => { - const chip = ( - configRef?.current.actions?.[index]?.onSelect()} - onMouseEnter={ - action.onPrefetch - ? () => configRef?.current.actions?.[index]?.onPrefetch?.() - : undefined - } - onFocus={ - action.onPrefetch - ? () => configRef?.current.actions?.[index]?.onPrefetch?.() - : undefined - } - disabled={action.disabled} - > - {action.textTone === 'error' ? ( - {action.text} - ) : ( - action.text - )} - - ) - return action.tooltip ? ( - - - {chip} - - {action.tooltip} - - ) : ( - chip - ) - })} + {actions?.map((action, index) => ( + configRef?.current.actions?.[index]?.onSelect()} + onPrefetch={ + action.onPrefetch + ? () => configRef?.current.actions?.[index]?.onPrefetch?.() + : undefined + } + /> + ))}
({ VerifiedDomainsSection: () =>
, })) -vi.mock( - '@/app/workspace/[workspaceId]/settings/components/save-discard-actions/save-discard-actions', - () => ({ - saveDiscardActions: () => [], - }) -) +vi.mock('@/components/settings/save-discard-actions', () => ({ + saveDiscardActions: () => [], +})) vi.mock('@/app/workspace/[workspaceId]/settings/components/settings-empty-state', () => ({ SettingsEmptyState: ({ children }: { children?: ReactNode }) =>
{children}
, diff --git a/apps/sim/ee/sso/components/sso-settings.tsx b/apps/sim/ee/sso/components/sso-settings.tsx index 959581bd9c8..b7f3df18a85 100644 --- a/apps/sim/ee/sso/components/sso-settings.tsx +++ b/apps/sim/ee/sso/components/sso-settings.tsx @@ -17,14 +17,14 @@ import { import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { ChevronDown, Eye, EyeOff } from 'lucide-react' +import { saveDiscardActions } from '@/components/settings/save-discard-actions' +import type { SettingsAction } from '@/components/settings/settings-header' import type { SsoRegistrationBody } from '@/lib/api/contracts/auth' import { useSession } from '@/lib/auth/auth-client' import { isEnterprise } from '@/lib/billing/plan-helpers' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { getBaseUrl } from '@/lib/core/utils/urls' -import { saveDiscardActions } from '@/app/workspace/[workspaceId]/settings/components/save-discard-actions/save-discard-actions' import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' -import type { SettingsAction } from '@/app/workspace/[workspaceId]/settings/components/settings-header/settings-header' import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel' import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section' import { useSettingsUnsavedGuard } from '@/app/workspace/[workspaceId]/settings/hooks/use-settings-unsaved-guard' diff --git a/apps/sim/ee/whitelabeling/components/whitelabeling-settings.tsx b/apps/sim/ee/whitelabeling/components/whitelabeling-settings.tsx index deecd10d1ce..64dedd8a589 100644 --- a/apps/sim/ee/whitelabeling/components/whitelabeling-settings.tsx +++ b/apps/sim/ee/whitelabeling/components/whitelabeling-settings.tsx @@ -6,6 +6,7 @@ import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { Image as ImageIcon, X } from 'lucide-react' import Image from 'next/image' +import { saveDiscardActions } from '@/components/settings/save-discard-actions' import { isEnterprise } from '@/lib/billing/plan-helpers' import { HEX_COLOR_REGEX } from '@/lib/branding' import { isBillingEnabled } from '@/lib/core/config/env-flags' @@ -13,7 +14,6 @@ import { CHIP_FIELD_INPUT, CHIP_FIELD_SHELL, } from '@/app/workspace/[workspaceId]/components/credential-detail' -import { saveDiscardActions } from '@/app/workspace/[workspaceId]/settings/components/save-discard-actions/save-discard-actions' import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel' import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section' diff --git a/apps/sim/ee/workspace-forking/components/forks.tsx b/apps/sim/ee/workspace-forking/components/forks.tsx index 38df0a0cead..550be464c49 100644 --- a/apps/sim/ee/workspace-forking/components/forks.tsx +++ b/apps/sim/ee/workspace-forking/components/forks.tsx @@ -7,6 +7,8 @@ import { getErrorMessage } from '@sim/utils/errors' import { AlertTriangle, Plus } from 'lucide-react' import { useParams, useRouter } from 'next/navigation' import { useQueryState } from 'nuqs' +import { saveDiscardActions } from '@/components/settings/save-discard-actions' +import type { SettingsAction } from '@/components/settings/settings-header' import type { ForkLineageChildApi, ForkLineageNodeApi } from '@/lib/api/contracts/workspace-fork' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { FloatingOverflowText } from '@/app/workspace/[workspaceId]/components' @@ -24,9 +26,7 @@ import { type RowAction, RowActionsMenu, } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu' -import { saveDiscardActions } from '@/app/workspace/[workspaceId]/settings/components/save-discard-actions/save-discard-actions' import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' -import type { SettingsAction } from '@/app/workspace/[workspaceId]/settings/components/settings-header/settings-header' import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel' import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section' import { useSettingsSearch } from '@/app/workspace/[workspaceId]/settings/components/use-settings-search'