ソースを参照

fix: replace native confirm() dialogs with an in-app ConfirmModal

window.confirm() blocks the JS thread, gives no visual match to the rest
of the app, and (per browser) can suppress the "did this really happen"
feedback a destructive action needs. Replaced all 7 confirm() call sites
across Users, Nodes, Projects, Database and workflow retention settings
with a single reusable ConfirmModal component instead of building five
one-off dialogs.

ConfirmModal keeps the two things a native confirm() gave for free:
Escape cancels without re-triggering whatever opened it, and focus goes
to Cancel (not Confirm) so a double-Enter can't perform a destructive
action by accident; focus returns to the trigger element on close.

No alert() or prompt() calls existed in the audited files - all 7
occurrences were confirm() gating a destructive or disruptive action,
so all became confirmation modals with the action named on the button
rather than a generic OK.
fszontagh 1 ヶ月 前
親
コミット
bdb857aa5f

+ 109 - 0
webui/src/components/workflow/ConfirmModal.tsx

@@ -0,0 +1,109 @@
+import { useEffect, useRef } from 'react'
+import { AlertTriangle } from 'lucide-react'
+
+interface ConfirmModalProps {
+  title: string
+  /** Plain text. Line breaks are respected, so the same "why" a confirm() used
+      to carry over a blank line still reads that way here. */
+  message: string
+  confirmLabel: string
+  cancelLabel?: string
+  /** Red styling for something that removes data; amber for something that is
+      merely disruptive (signs people out, discards unsaved edits). */
+  tone?: 'danger' | 'warning'
+  onConfirm: () => void
+  onCancel: () => void
+}
+
+/**
+ * The one confirmation modal for anything that used to be window.confirm().
+ *
+ * Two things a native confirm() gives for free, kept here on purpose:
+ * - Escape cancels, without re-triggering whatever opened it.
+ * - Focus moves to Cancel (not Confirm) while it is open, and back to
+ *   whatever had focus before once it closes. A destructive action should
+ *   never be one accidental double-Enter away.
+ */
+export function ConfirmModal({
+  title,
+  message,
+  confirmLabel,
+  cancelLabel = 'Cancel',
+  tone = 'danger',
+  onConfirm,
+  onCancel,
+}: ConfirmModalProps) {
+  const cancelRef = useRef<HTMLButtonElement>(null)
+  const previouslyFocused = useRef<HTMLElement | null>(null)
+
+  useEffect(() => {
+    previouslyFocused.current = document.activeElement as HTMLElement | null
+    cancelRef.current?.focus()
+
+    const onKeyDown = (e: KeyboardEvent) => {
+      if (e.key === 'Escape') {
+        e.preventDefault()
+        e.stopPropagation()
+        onCancel()
+      }
+    }
+    // Capture phase, so this wins even if something inside the modal would
+    // otherwise absorb the key first.
+    document.addEventListener('keydown', onKeyDown, true)
+    return () => {
+      document.removeEventListener('keydown', onKeyDown, true)
+      previouslyFocused.current?.focus()
+    }
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [])
+
+  const confirmClasses =
+    tone === 'danger'
+      ? 'bg-red-600 hover:bg-red-700'
+      : 'bg-amber-600 hover:bg-amber-700'
+  const iconWrapClasses =
+    tone === 'danger'
+      ? 'bg-red-100 dark:bg-red-900/30'
+      : 'bg-amber-100 dark:bg-amber-900/30'
+  const iconClasses =
+    tone === 'danger'
+      ? 'text-red-600 dark:text-red-400'
+      : 'text-amber-600 dark:text-amber-400'
+
+  return (
+    <div className="fixed inset-0 bg-black/50 flex items-center justify-center z-50 p-4" onClick={onCancel}>
+      <div
+        className="bg-white dark:bg-slate-800 rounded-xl p-6 w-full max-w-sm"
+        onClick={(e) => e.stopPropagation()}
+        role="alertdialog"
+        aria-modal="true"
+        aria-labelledby="confirm-modal-title"
+      >
+        <div className="flex items-center gap-3 mb-4">
+          <div className={`p-2 rounded-full ${iconWrapClasses}`}>
+            <AlertTriangle className={`w-6 h-6 ${iconClasses}`} />
+          </div>
+          <h2 id="confirm-modal-title" className="text-lg font-semibold text-gray-900 dark:text-gray-100">
+            {title}
+          </h2>
+        </div>
+        <p className="text-gray-600 dark:text-gray-400 mb-6 whitespace-pre-line">{message}</p>
+        <div className="flex justify-end gap-2">
+          <button
+            ref={cancelRef}
+            onClick={onCancel}
+            className="px-4 py-2 text-gray-600 dark:text-gray-400 hover:bg-gray-100 dark:hover:bg-slate-700 rounded-lg"
+          >
+            {cancelLabel}
+          </button>
+          <button
+            onClick={onConfirm}
+            className={`px-4 py-2 text-white rounded-lg ${confirmClasses}`}
+          >
+            {confirmLabel}
+          </button>
+        </div>
+      </div>
+    </div>
+  )
+}

+ 22 - 6
webui/src/components/workflow/WorkflowRetentionSettings.tsx

@@ -2,6 +2,7 @@ import { useEffect, useMemo, useState } from 'react'
 import { useQuery } from '@tanstack/react-query'
 import { AlertTriangle, Infinity as InfinityIcon, Info } from 'lucide-react'
 import { workflowsApi } from '../../api/workflows'
+import { ConfirmModal } from './ConfirmModal'
 
 interface Props {
   workflowId?: string
@@ -49,6 +50,7 @@ export function WorkflowRetentionSettings({ workflowId, ttlSeconds, onChange }:
   // this asked nothing while doing the most.
   const [selected, setSelected] = useState<number | undefined>(ttlSeconds)
   useEffect(() => setSelected(ttlSeconds), [ttlSeconds])
+  const [confirmingApply, setConfirmingApply] = useState(false)
 
   const inherits = selected === undefined
   const dirty = selected !== ttlSeconds
@@ -118,12 +120,8 @@ export function WorkflowRetentionSettings({ workflowId, ttlSeconds, onChange }:
     // Only when data actually goes - agreeing to a change that removes nothing
     // is a dialog that teaches people to dismiss dialogs.
     if (goingNow > 0) {
-      const ok = confirm(
-        `Change this workflow to ${describeChoice()}?\n\n` +
-          `${goingNow.toLocaleString()} of ${held.toLocaleString()} records are already older ` +
-          `than that and are removed as soon as this is saved. This cannot be undone.`
-      )
-      if (!ok) return
+      setConfirmingApply(true)
+      return
     }
     onChange(selected)
   }
@@ -269,6 +267,24 @@ export function WorkflowRetentionSettings({ workflowId, ttlSeconds, onChange }:
           )}
         </div>
       )}
+
+      {confirmingApply && (
+        <ConfirmModal
+          title="Change retention?"
+          message={
+            `Change this workflow to ${describeChoice()}?\n\n` +
+            `${goingNow.toLocaleString()} of ${held.toLocaleString()} records are already older ` +
+            `than that and are removed as soon as this is saved. This cannot be undone.`
+          }
+          confirmLabel="Apply and remove records"
+          tone="danger"
+          onConfirm={() => {
+            setConfirmingApply(false)
+            onChange(selected)
+          }}
+          onCancel={() => setConfirmingApply(false)}
+        />
+      )}
     </div>
   )
 }

+ 17 - 2
webui/src/pages/DatabasePage.tsx

@@ -25,6 +25,7 @@ import {
 } from 'lucide-react'
 import { format } from 'date-fns'
 import { useTheme } from '../contexts/ThemeContext'
+import { ConfirmModal } from '../components/workflow/ConfirmModal'
 
 interface Document {
   _id: string
@@ -91,6 +92,7 @@ export default function DatabasePage() {
   const [justCreated, setJustCreated] = useState<string[]>([])
   const [showCollectionSettings, setShowCollectionSettings] = useState(false)
   const [dropConfirmText, setDropConfirmText] = useState('')
+  const [confirmingDrop, setConfirmingDrop] = useState(false)
 
   useEffect(() => {
     if (!showCollectionSettings) return
@@ -645,8 +647,7 @@ export default function DatabasePage() {
               <button
                 onClick={() => {
                   if (!selectedCollection) return
-                  if (!confirm(`Delete the collection "${selectedCollection}"?\n\nIt is empty, so nothing is lost, but it cannot be undone.`)) return
-                  dropCollectionMutation.mutate(selectedCollection)
+                  setConfirmingDrop(true)
                 }}
                 disabled={
                   !collectionInfo ||
@@ -1063,6 +1064,20 @@ export default function DatabasePage() {
         </div>
       )}
 
+      {confirmingDrop && selectedCollection && (
+        <ConfirmModal
+          title="Delete collection?"
+          message={`Delete the collection "${selectedCollection}"?\n\nIt is empty, so nothing is lost, but it cannot be undone.`}
+          confirmLabel="Delete collection"
+          tone="danger"
+          onConfirm={() => {
+            dropCollectionMutation.mutate(selectedCollection)
+            setConfirmingDrop(false)
+          }}
+          onCancel={() => setConfirmingDrop(false)}
+        />
+      )}
+
       {/* Collection settings: what the database knows about the collection
           itself, and the two things about it that can be changed. */}
       {showCollectionSettings && selectedCollection && (

+ 17 - 5
webui/src/pages/NodesPage.tsx

@@ -6,6 +6,7 @@ import { useTheme } from '../contexts/ThemeContext'
 import Editor from '@monaco-editor/react'
 import { getCategoryConfig, getSortedCategories } from '../config/nodeCategories'
 import { NodeIcon } from '../components/workflow/NodeIcon'
+import { ConfirmModal } from '../components/workflow/ConfirmModal'
 
 const NODE_TEMPLATE = `/**
  * @node my-node
@@ -60,6 +61,7 @@ export default function NodesPage() {
   const [editingCode, setEditingCode] = useState('')
   const [showEditor, setShowEditor] = useState(false)
   const [showNewNode, setShowNewNode] = useState(false)
+  const [nodeToDelete, setNodeToDelete] = useState<NodeDefinition | null>(null)
   const [newNodeData, setNewNodeData] = useState({
     id: '',
     name: '',
@@ -351,11 +353,7 @@ export default function NodesPage() {
                         <RefreshCw className="w-4 h-4" />
                       </button>
                       <button
-                        onClick={() => {
-                          if (confirm(`Delete node "${node.name}"?`)) {
-                            deleteNodeMutation.mutate(node.id)
-                          }
-                        }}
+                        onClick={() => setNodeToDelete(node)}
                         className="p-1.5 text-gray-500 dark:text-gray-400 hover:text-red-600 dark:hover:text-red-400 hover:bg-red-50 dark:hover:bg-red-900/30 rounded"
                         title="Delete"
                       >
@@ -524,6 +522,20 @@ export default function NodesPage() {
           </div>
         </div>
       )}
+
+      {nodeToDelete && (
+        <ConfirmModal
+          title="Delete node?"
+          message={`Delete node "${nodeToDelete.name}"? This action cannot be undone.`}
+          confirmLabel="Delete node"
+          tone="danger"
+          onConfirm={() => {
+            deleteNodeMutation.mutate(nodeToDelete.id)
+            setNodeToDelete(null)
+          }}
+          onCancel={() => setNodeToDelete(null)}
+        />
+      )}
     </div>
   )
 }

+ 37 - 12
webui/src/pages/ProjectsPage.tsx

@@ -4,6 +4,7 @@ import { FolderKanban, Plus, Trash2, UserPlus, X, Lock, Users } from 'lucide-rea
 import { projectsApi, usersApi, Project } from '../api/users'
 import { useAuthStore } from '../stores/authStore'
 import { describeDuration } from '../components/workflow/WorkflowRetentionSettings'
+import { ConfirmModal } from '../components/workflow/ConfirmModal'
 
 /**
  * Projects: who work belongs to, and who may touch it.
@@ -39,6 +40,8 @@ export default function ProjectsPage() {
   const [addUserId, setAddUserId] = useState('')
   const [addRole, setAddRole] = useState('editor')
   const [error, setError] = useState<string | null>(null)
+  const [deleteTarget, setDeleteTarget] = useState<Project | null>(null)
+  const [pendingRetention, setPendingRetention] = useState<{ project: Project; seconds: number } | null>(null)
 
   const { data: projects = [], isLoading } = useQuery({
     queryKey: ['projects'],
@@ -144,13 +147,7 @@ export default function ProjectsPage() {
             disabled={!canManage || retentionMutation.isPending}
             onChange={(e) => {
               const seconds = Number(e.target.value)
-              const count = project.workflowCount
-              const warning =
-                seconds === 0
-                  ? `Keep the data of "${project.name}" for ever?\n\nNothing is ever removed, and it outlives the workflows themselves. ${count} workflow${count === 1 ? '' : 's'} inherit this.`
-                  : `Keep the data of "${project.name}" for ${describeDuration(seconds)}?\n\nThis runs from when each record was written, so anything already older goes as soon as it is applied. ${count} workflow${count === 1 ? '' : 's'} inherit this - the ones that set their own are not touched.\n\nEach workflow's settings show exactly how much of its data this removes.`
-              if (!confirm(warning)) return
-              retentionMutation.mutate({ id: project._id, ttlSeconds: seconds })
+              setPendingRetention({ project, seconds })
             }}
             className="w-full px-2 py-1.5 text-xs border border-gray-300 dark:border-slate-600 rounded bg-white dark:bg-slate-900 text-gray-900 dark:text-gray-100 disabled:opacity-60"
           >
@@ -175,11 +172,7 @@ export default function ProjectsPage() {
               </button>
               {canManage && (
                 <button
-                  onClick={() => {
-                    if (confirm(`Delete "${project.name}"? It has to be empty first.`)) {
-                      deleteMutation.mutate(project._id)
-                    }
-                  }}
+                  onClick={() => setDeleteTarget(project)}
                   className="p-1 text-gray-500 hover:text-red-600 dark:hover:text-red-400 rounded"
                   title="Delete this project"
                 >
@@ -371,6 +364,38 @@ export default function ProjectsPage() {
           </div>
         </div>
       )}
+
+      {deleteTarget && (
+        <ConfirmModal
+          title="Delete project?"
+          message={`Delete "${deleteTarget.name}"? It has to be empty first.`}
+          confirmLabel="Delete project"
+          tone="danger"
+          onConfirm={() => {
+            deleteMutation.mutate(deleteTarget._id)
+            setDeleteTarget(null)
+          }}
+          onCancel={() => setDeleteTarget(null)}
+        />
+      )}
+
+      {pendingRetention && (
+        <ConfirmModal
+          title="Change retention?"
+          message={
+            pendingRetention.seconds === 0
+              ? `Keep the data of "${pendingRetention.project.name}" for ever?\n\nNothing is ever removed, and it outlives the workflows themselves. ${pendingRetention.project.workflowCount} workflow${pendingRetention.project.workflowCount === 1 ? '' : 's'} inherit this.`
+              : `Keep the data of "${pendingRetention.project.name}" for ${describeDuration(pendingRetention.seconds)}?\n\nThis runs from when each record was written, so anything already older goes as soon as it is applied. ${pendingRetention.project.workflowCount} workflow${pendingRetention.project.workflowCount === 1 ? '' : 's'} inherit this - the ones that set their own are not touched.\n\nEach workflow's settings show exactly how much of its data this removes.`
+          }
+          confirmLabel="Change retention"
+          tone="warning"
+          onConfirm={() => {
+            retentionMutation.mutate({ id: pendingRetention.project._id, ttlSeconds: pendingRetention.seconds })
+            setPendingRetention(null)
+          }}
+          onCancel={() => setPendingRetention(null)}
+        />
+      )}
     </div>
   )
 }

+ 37 - 16
webui/src/pages/UsersPage.tsx

@@ -3,6 +3,7 @@ import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'
 import { UserPlus, Trash2, ShieldCheck, X, KeyRound } from 'lucide-react'
 import { usersApi, User } from '../api/users'
 import { useAuthStore } from '../stores/authStore'
+import { ConfirmModal } from '../components/workflow/ConfirmModal'
 
 /**
  * The accounts on this installation.
@@ -43,6 +44,8 @@ export default function UsersPage() {
   const [resetTarget, setResetTarget] = useState<User | null>(null)
   const [resetPasswordValue, setResetPasswordValue] = useState('')
   const [resetError, setResetError] = useState<string | null>(null)
+  const [deleteTarget, setDeleteTarget] = useState<User | null>(null)
+  const [confirmingReset, setConfirmingReset] = useState(false)
 
   const { data: users = [], isLoading, isError } = useQuery({
     queryKey: ['users'],
@@ -188,15 +191,7 @@ export default function UsersPage() {
                 </button>
 
                 <button
-                  onClick={() => {
-                    if (confirm(
-                      `Delete ${user.username}? Their personal project stays if it holds any workflows or credentials - ` +
-                      `there is no owner left to delete it, and only an admin will be able to reach it. ` +
-                      `If it is empty, it is removed along with the account.`
-                    )) {
-                      deleteMutation.mutate(user.id)
-                    }
-                  }}
+                  onClick={() => setDeleteTarget(user)}
                   disabled={isOwner(user) || user.id === me?.id || deleteMutation.isPending}
                   className="p-1.5 text-gray-500 dark:text-gray-400 hover:text-red-600 dark:hover:text-red-400 hover:bg-red-50 dark:hover:bg-red-900/30 rounded disabled:opacity-40"
                   title={user.id === me?.id ? 'You cannot delete your own account' : 'Delete this account'}
@@ -308,13 +303,7 @@ export default function UsersPage() {
                 Cancel
               </button>
               <button
-                onClick={() => {
-                  if (confirm(
-                    `Reset ${resetTarget.username}'s password? They will be signed out of every session immediately.`
-                  )) {
-                    resetMutation.mutate({ id: resetTarget.id, newPassword: resetPasswordValue })
-                  }
-                }}
+                onClick={() => setConfirmingReset(true)}
                 disabled={!resetPasswordValue || resetMutation.isPending}
                 className="px-3 py-1.5 text-sm bg-primary-600 text-white rounded hover:bg-primary-700 disabled:opacity-40"
               >
@@ -324,6 +313,38 @@ export default function UsersPage() {
           </div>
         </div>
       )}
+
+      {deleteTarget && (
+        <ConfirmModal
+          title="Delete account?"
+          message={
+            `Delete ${deleteTarget.username}? Their personal project stays if it holds any workflows or credentials - ` +
+            `there is no owner left to delete it, and only an admin will be able to reach it. ` +
+            `If it is empty, it is removed along with the account.`
+          }
+          confirmLabel="Delete account"
+          tone="danger"
+          onConfirm={() => {
+            deleteMutation.mutate(deleteTarget.id)
+            setDeleteTarget(null)
+          }}
+          onCancel={() => setDeleteTarget(null)}
+        />
+      )}
+
+      {confirmingReset && resetTarget && (
+        <ConfirmModal
+          title="Reset password?"
+          message={`Reset ${resetTarget.username}'s password? They will be signed out of every session immediately.`}
+          confirmLabel="Reset password"
+          tone="warning"
+          onConfirm={() => {
+            resetMutation.mutate({ id: resetTarget.id, newPassword: resetPasswordValue })
+            setConfirmingReset(false)
+          }}
+          onCancel={() => setConfirmingReset(false)}
+        />
+      )}
     </div>
   )
 }