improvement(settings): unify save/discard across editable surfaces - #6173
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview
Credential-detail editors (connected integration, skill detail/create, secret value, Invite modal: removes the “Joins your organization. Adds a seat.” hint for Member membership. Reviewed by Cursor Bugbot for commit 3edc45f. Configure here. |
Greptile SummaryThe PR centralizes editable-surface actions and makes Save/Create consistently visible while adding dirty-only Discard behavior.
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code regression established across the migrated editable surfaces. The shared helper preserves existing save, validation, and permission gates while standardizing action visibility, and the draft-seeding changes retain explicit discard paths without an established cross-resource data-loss path.
|
| Filename | Overview |
|---|---|
| apps/sim/components/settings/save-discard-actions.tsx | Introduces the canonical dirty-aware Save/Discard action configuration and ReactNode-slot chip renderer. |
| apps/sim/components/settings/settings-header.tsx | Consolidates SettingsAction chip rendering and adds stable action identities without an established behavioral regression. |
| apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts | Changes credential draft seeding to preserve edits across same-ID refetches and exposes an explicit discard reset. |
| apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts | Adds an explicit reset-to-current-value operation for the shared Discard action. |
| apps/sim/app/workspace/[workspaceId]/skills/[skillId]/skill-detail.tsx | Migrates skill detail actions to the shared Save/Discard chip path and centralizes draft resetting. |
| apps/sim/app/workspace/[workspaceId]/settings/components/custom-tools/components/custom-tool-detail/custom-tool-detail.tsx | Replaces separate create/edit action construction with the shared helper and standardized create labels. |
| apps/sim/app/workspace/[workspaceId]/settings/components/sandboxes/sandboxes.tsx | Uses the shared helper's create mode so sandbox creation consistently exposes a disabled Create action. |
| apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx | Replaces hand-built Save/Discard actions with the shared helper while retaining validation tooltips and disabled conditions. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
E[Editable settings surface] --> H[saveDiscardActions]
H --> D{Dirty?}
D -->|No| S[Disabled Save or Create]
D -->|Yes| DS[Discard plus enabled Save or Create]
H --> P[SettingsPanel actions]
H --> C[SaveDiscardChips]
P --> R[SettingsActionChip]
C --> R
Reviews (1): Last reviewed commit: "improvement(settings): unify save/discar..." | Re-trigger Greptile
Summary
saveDiscardActionsmoves to@/components/settingsand owns the single rule; create flows passcreatingso the Create/Creating… labels can't driftSettingsActionChip/SettingsActionChipsso the settings shell and the ReactNode-slot headers (CredentialDetailLayout) render actions through one chip path instead of two copiesType of Change
Testing
Tested manually.
tsc,bun run lint:checkand the sso-settings suite pass.Not yet verified in a browser: the always-present Save changes the resting header on ~10 surfaces, and the
saveTooltipon a disabled Save relies on a newpointer-events-none(a disabled button isn't a hit-test target, so the tooltip never fired before). Both want a look on staging.Checklist