From 0442bd29b34a865a370e5881343a0869be6c93ce Mon Sep 17 00:00:00 2001 From: Anso Date: Wed, 6 May 2026 20:18:13 -0400 Subject: [PATCH] refactor(frontend): migrate NodeManager dialogs to Modal chrome (#949) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(frontend): migrate NodeManager dialogs to Modal chrome Bring all four NodeManager dialogs onto §10 Modal primitives: - Add node form -> Modal at lg, kicker NODES · ADD LOCAL or NODES · ADD REMOTE depending on the form's type radio. The Add button is now a normal button that calls setCreateOpen(true) rather than a DialogTrigger - Edit node form -> Modal at lg, kicker NODES · EDIT - Pilot enrollment dialog -> Modal at xl, kicker NODES · PILOT ENROLLMENT - Delete confirm -> destructive ConfirmModal, kicker NODES · DELETE · IRREVERSIBLE handleDelete now closes from finally so the dialog clears on errors too, matching the ConfirmModal Promise-aware contract. * fix(frontend): make ModalBody scrollable, scope nodes test locator to dialog Two related fixes for the Add Node modal regression on small viewports: 1. ModalBody now caps at max-h-[calc(85vh-12rem)] with overflow-y-auto. Tall forms that would push the footer offscreen on a 720px viewport (Add Node has 6 form sections plus header and footer) now scroll inside the body while the cyan-rail header and Cancel/Submit footer stay anchored. Benefits every form modal, not just NodeManager. 2. e2e/nodes.spec.ts now scopes the submit-button locator to getByRole('dialog'), removing the .last() pattern. The previous approach worked when the modal was guaranteed to render after the trigger in DOM order, but it doesn't survive an offscreen footer. --- e2e/nodes.spec.ts | 6 +- frontend/src/components/NodeManager.tsx | 138 ++++++++++++++---------- frontend/src/components/ui/modal.tsx | 2 +- 3 files changed, 83 insertions(+), 63 deletions(-) diff --git a/e2e/nodes.spec.ts b/e2e/nodes.spec.ts index 80a1cfe6..4839b617 100644 --- a/e2e/nodes.spec.ts +++ b/e2e/nodes.spec.ts @@ -46,8 +46,8 @@ test.describe('Node management', () => { await page.locator('#node-api-url').fill('http://localhost:6379'); // api_token is required to enable the submit button; use a dummy value since we're testing URL validation await page.locator('#node-api-token').fill('dummy-token'); - // Use .last() to target the dialog submit button, not the trigger - await page.getByRole('button', { name: /add node/i }).last().click(); + // Scope to the dialog so we target the submit button, not the trigger + await page.getByRole('dialog').getByRole('button', { name: /add node/i }).click(); await expect(page.getByText(/loopback|localhost/i)).toBeVisible({ timeout: 5_000 }); }); @@ -59,7 +59,7 @@ test.describe('Node management', () => { await page.locator('#node-api-url').fill('not-a-url-at-all'); // api_token is required to enable the submit button; use a dummy value since we're testing URL validation await page.locator('#node-api-token').fill('dummy-token'); - await page.getByRole('button', { name: /add node/i }).last().click(); + await page.getByRole('dialog').getByRole('button', { name: /add node/i }).click(); await expect(page.getByText(/valid url|invalid url/i)).toBeVisible({ timeout: 5_000 }); }); diff --git a/frontend/src/components/NodeManager.tsx b/frontend/src/components/NodeManager.tsx index e3fd9577..d2344501 100644 --- a/frontend/src/components/NodeManager.tsx +++ b/frontend/src/components/NodeManager.tsx @@ -4,8 +4,7 @@ import type { Node, NodeMode } from '@/context/NodeContext'; import { apiFetch } from '@/lib/api'; import { copyToClipboard } from '@/lib/clipboard'; import { toast } from '@/components/ui/toast-store'; -import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogFooter, DialogTrigger } from './ui/dialog'; -import { AlertDialog, AlertDialogAction, AlertDialogCancel, AlertDialogContent, AlertDialogDescription, AlertDialogFooter, AlertDialogHeader, AlertDialogTitle } from './ui/alert-dialog'; +import { Modal, ModalHeader, ModalBody, ModalFooter, ConfirmModal } from './ui/modal'; import { Button } from './ui/button'; import { Input } from './ui/input'; import { Label } from './ui/label'; @@ -232,11 +231,12 @@ export function NodeManager() { throw new Error(err.error || 'Failed to delete node'); } toast.success(`Node "${deletingNode.name}" deleted`); - setDeleteOpen(false); - setDeletingNode(null); await refreshNodes(); } catch (error) { toast.error((error as Error).message || 'Failed to delete node'); + } finally { + setDeleteOpen(false); + setDeletingNode(null); } }; @@ -422,28 +422,40 @@ export function NodeManager() {
{/* Actions */}
- { + setFormData(defaultFormData); + setCreateOpen(true); + }} + > + + Add node + + { setCreateOpen(open); - // Reset form to defaults every time the dialog opens if (open) setFormData(defaultFormData); }} + size="lg" > - - - - Add node - - - - - Add {formData.type === 'local' ? 'Local' : 'Remote'} Node - + + {renderFormFields()} - - + + setCreateOpen(false)}>Cancel + } + primary={ Add node - - - + } + /> +
@@ -723,12 +735,10 @@ export function NodeManager() {
)} - {/* Edit Dialog */} - - - - Edit Node - + {/* Edit Modal */} + + + {renderFormFields()} {formData.type === 'remote' && formData.mode === 'pilot_agent' && editingNodeId !== null && (
@@ -749,23 +759,28 @@ export function NodeManager() {
)} - - + + { setEditOpen(false); setEditingNodeId(null); }}>Cancel + } + primary={ - - - + } + /> + - {/* Pilot enrollment dialog (create + regenerate flows both open this) */} - { if (!open) { @@ -775,13 +790,16 @@ export function NodeManager() { setFormData(defaultFormData); } }} + size="xl" > - - - Enroll the pilot agent - + + {activeEnrollment && ( -
+ <>

Run this command on {activeEnrollment.nodeName} to start the pilot agent. The token below is valid for 15 minutes and can only be used once.

@@ -797,10 +815,13 @@ export function NodeManager() { {enrollmentCopied ? 'Copied' : 'Copy command'}
- + )} - + + { setActiveEnrollment(null); setEnrollmentCopied(false); @@ -811,25 +832,24 @@ export function NodeManager() { > Done - - - + } + /> + {/* Delete Confirmation */} - - - - Delete Node - - Are you sure you want to remove {deletingNode?.name}? This will only remove the node from Sencho - it will not affect the remote instance or any running containers. - - - - Cancel - Delete - - - + +

+ Removes {deletingNode?.name} from this console. The remote instance and its containers are not affected. +

+ ); } diff --git a/frontend/src/components/ui/modal.tsx b/frontend/src/components/ui/modal.tsx index 8928184d..dee9ab42 100644 --- a/frontend/src/components/ui/modal.tsx +++ b/frontend/src/components/ui/modal.tsx @@ -141,7 +141,7 @@ function ConfirmDestructiveHeader(props: ModalHeaderBaseProps) { } export function ModalBody({ className, ...props }: React.HTMLAttributes) { - return
; + return
; } interface ModalFooterProps {