From 089fafcf8da1e9ada069d9d98c37c42f7bfe9ce8 Mon Sep 17 00:00:00 2001 From: Celal Zamanoglu <95054566+celalzamanoglu@users.noreply.github.com> Date: Sat, 17 Jan 2026 00:29:27 +0300 Subject: [PATCH] improve credential ui/ux (#4474) --- .../routes/credentials/CredentialsModal.tsx | 20 +++- .../CredentialParameterSourceSelector.tsx | 15 ++- .../LoginBlockCredentialSelector.tsx | 113 +++++++++++------- .../nodes/TaskNode/ParametersMultiSelect.tsx | 43 ++++++- 4 files changed, 134 insertions(+), 57 deletions(-) diff --git a/skyvern-frontend/src/routes/credentials/CredentialsModal.tsx b/skyvern-frontend/src/routes/credentials/CredentialsModal.tsx index ad9d09dd..625eeb15 100644 --- a/skyvern-frontend/src/routes/credentials/CredentialsModal.tsx +++ b/skyvern-frontend/src/routes/credentials/CredentialsModal.tsx @@ -67,12 +67,27 @@ function generateDefaultCredentialName(existingNames: string[]): string { type Props = { onCredentialCreated?: (id: string) => void; + /** Optional controlled mode: pass isOpen and onOpenChange to control modal state locally */ + isOpen?: boolean; + onOpenChange?: (open: boolean) => void; }; -function CredentialsModal({ onCredentialCreated }: Props) { +function CredentialsModal({ + onCredentialCreated, + isOpen: controlledIsOpen, + onOpenChange: controlledOnOpenChange, +}: Props) { const credentialGetter = useCredentialGetter(); const queryClient = useQueryClient(); - const { isOpen, type, setIsOpen } = useCredentialModalState(); + const { + isOpen: urlIsOpen, + type, + setIsOpen: setUrlIsOpen, + } = useCredentialModalState(); + + // Use controlled props if provided, otherwise fall back to URL-based state + const isOpen = controlledIsOpen ?? urlIsOpen; + const setIsOpen = controlledOnOpenChange ?? setUrlIsOpen; const { data: credentials } = useCredentialsQuery({ page_size: 100, }); @@ -120,6 +135,7 @@ function CredentialsModal({ onCredentialCreated }: Props) { return response.data; }, onSuccess: (data) => { + reset(); setIsOpen(false); queryClient.invalidateQueries({ queryKey: ["credentials"], diff --git a/skyvern-frontend/src/routes/workflows/components/CredentialParameterSourceSelector.tsx b/skyvern-frontend/src/routes/workflows/components/CredentialParameterSourceSelector.tsx index a14892ce..0c40abdd 100644 --- a/skyvern-frontend/src/routes/workflows/components/CredentialParameterSourceSelector.tsx +++ b/skyvern-frontend/src/routes/workflows/components/CredentialParameterSourceSelector.tsx @@ -10,11 +10,8 @@ import { useCredentialsQuery } from "../hooks/useCredentialsQuery"; import { useWorkflowParametersStore } from "@/store/WorkflowParametersStore"; import { WorkflowParameterValueType } from "../types/workflowTypes"; import { PlusIcon } from "@radix-ui/react-icons"; -import { - CredentialModalTypes, - useCredentialModalState, -} from "@/routes/credentials/useCredentialModalState"; import { CredentialsModal } from "@/routes/credentials/CredentialsModal"; +import { useState } from "react"; type Props = { value: string; @@ -25,7 +22,8 @@ function CredentialParameterSourceSelector({ value, onChange }: Props) { const { data: credentials, isFetching } = useCredentialsQuery({ page_size: 100, // Reasonable limit for dropdown selector }); - const { setIsOpen, setType } = useCredentialModalState(); + // Use local state for modal to avoid conflicts with other CredentialsModal instances + const [isModalOpen, setIsModalOpen] = useState(false); const { parameters: workflowParameters } = useWorkflowParametersStore(); const workflowParametersOfTypeCredentialId = workflowParameters.filter( (parameter) => @@ -65,8 +63,7 @@ function CredentialParameterSourceSelector({ value, onChange }: Props) { value={value} onValueChange={(value) => { if (value === "new") { - setIsOpen(true); - setType(CredentialModalTypes.PASSWORD); + setIsModalOpen(true); return; } onChange(value); @@ -90,9 +87,11 @@ function CredentialParameterSourceSelector({ value, onChange }: Props) { { onChange(id); - setIsOpen(false); + setIsModalOpen(false); }} /> diff --git a/skyvern-frontend/src/routes/workflows/editor/nodes/LoginNode/LoginBlockCredentialSelector.tsx b/skyvern-frontend/src/routes/workflows/editor/nodes/LoginNode/LoginBlockCredentialSelector.tsx index ee367c89..7545878d 100644 --- a/skyvern-frontend/src/routes/workflows/editor/nodes/LoginNode/LoginBlockCredentialSelector.tsx +++ b/skyvern-frontend/src/routes/workflows/editor/nodes/LoginNode/LoginBlockCredentialSelector.tsx @@ -11,7 +11,7 @@ import CloudContext from "@/store/CloudContext"; import { useContext, useMemo } from "react"; import { useWorkflowParametersStore } from "@/store/WorkflowParametersStore"; import { CredentialsModal } from "@/routes/credentials/CredentialsModal"; -import { PlusIcon } from "@radix-ui/react-icons"; +import { ExclamationTriangleIcon, PlusIcon } from "@radix-ui/react-icons"; import { CredentialModalTypes, useCredentialModalState, @@ -19,7 +19,12 @@ import { import { useNodes } from "@xyflow/react"; import { AppNode } from ".."; import { isLoginNode } from "./types"; -import { parameterIsSkyvernCredential } from "../../types"; +import { + parameterIsSkyvernCredential, + parameterIsBitwardenCredential, + parameterIsOnePasswordCredential, + parameterIsAzureVaultCredential, +} from "../../types"; type Props = { nodeId: string; @@ -57,27 +62,52 @@ function LoginBlockCredentialSelector({ nodeId, value, onChange }: Props) { parameter.parameterType === "credential" || parameter.parameterType === "onepassword", ); - const credentialInputParameters = workflowParameters.filter( - (parameter) => - parameter.parameterType === "workflow" && - parameter.dataType === "credential_id", - ); const isCloud = useContext(CloudContext); const { data: credentials = [], isFetching } = useCredentialsQuery({ enabled: isCloud, page_size: 100, }); + // Get the set of credential IDs that are in the vault + const credentialIdsInVault = useMemo( + () => new Set(credentials.map((c) => c.credential_id)), + [credentials], + ); + // Determine which credential is currently selected (by credential_id) - // This must be before the early return to comply with React hooks rules + // This handles multiple cases: + // 1. Skyvern credential parameters (have credentialId) + // 2. Workflow input parameters with credential_id type and default value const selectedCredentialId = useMemo(() => { if (!value) return undefined; - const parameter = credentialParameters.find((p) => p.key === value); - if (parameter && parameterIsSkyvernCredential(parameter)) { - return parameter.credentialId; + + // Check if it's a credential parameter + const credentialParam = credentialParameters.find((p) => p.key === value); + if (credentialParam && parameterIsSkyvernCredential(credentialParam)) { + return credentialParam.credentialId; } + + // Check if it's a workflow input parameter with credential_id type and default value + const workflowParam = workflowParameters.find( + (p) => + p.parameterType === "workflow" && + p.key === value && + p.dataType === "credential_id" && + typeof p.defaultValue === "string" && + p.defaultValue, + ); + if (workflowParam && workflowParam.parameterType === "workflow") { + return workflowParam.defaultValue as string; + } + return undefined; - }, [value, credentialParameters]); + }, [value, credentialParameters, workflowParameters]); + + // Check if the selected credential is missing (deleted) + const isCredentialMissing = useMemo(() => { + if (!selectedCredentialId) return false; + return !credentialIdsInVault.has(selectedCredentialId); + }, [selectedCredentialId, credentialIdsInVault]); if (isCloud && isFetching) { return ; @@ -86,48 +116,36 @@ function LoginBlockCredentialSelector({ nodeId, value, onChange }: Props) { const credentialOptions = credentials.map((credential) => ({ label: credential.name, value: credential.credential_id, - type: "credential", + type: "credential" as const, })); - // Get the set of credential IDs that are in the vault - const credentialIdsInVault = new Set(credentials.map((c) => c.credential_id)); - - // Filter credential parameters to only show those that reference credentials - // NOT in the vault (e.g., Bitwarden, 1Password, Azure Vault credentials) - // Skyvern credential parameters are excluded because the actual credential is already shown - const filteredCredentialParameterOptions = credentialParameters + // Only show non-Skyvern credential parameters (Bitwarden, 1Password, Azure Vault) + // Skyvern credential parameters should never be shown - the actual credential is displayed directly + const externalVaultParameterOptions = credentialParameters .filter((parameter) => { + // Never show Skyvern credential parameters if (parameterIsSkyvernCredential(parameter)) { - // Don't show Skyvern credential parameters if the credential is in the vault - return !credentialIdsInVault.has(parameter.credentialId); + return false; } - // Show non-Skyvern credential parameters (Bitwarden, 1Password, etc.) - return true; + // Show Bitwarden, 1Password, Azure Vault credential parameters + return ( + parameterIsBitwardenCredential(parameter) || + parameterIsOnePasswordCredential(parameter) || + parameterIsAzureVaultCredential(parameter) + ); }) .map((parameter) => ({ label: parameter.key, value: parameter.key, - type: "parameter", + type: "parameter" as const, })); - const credentialInputParameterOptions = credentialInputParameters.map( - (parameter) => ({ - label: parameter.key, - value: parameter.key, - type: "parameter", - }), - ); - - const options = [ - ...credentialOptions, - ...filteredCredentialParameterOptions, - ...credentialInputParameterOptions, - ]; + const options = [...credentialOptions, ...externalVaultParameterOptions]; return ( <>