[Feat] Dialog Refactor (#1140)

* wip

* refactor

* fixes based on manual testing

* Claude Review fixes

* copilot fixes

* fixes

* coderabbit feedback

* low fixes

* feedback

Author: RichardAnderson

.github/instructions/frontend.instructions.md

@@ -12,6 +12,85 @@ description: "React, Inertia, Tailwind v4, and TypeScript frontend standards"
 - Use the `useForm` helper for forms — follow existing patterns in the codebase.
 - Use `<Link>` or `router.visit()` for navigation — never raw `<a>` tags for internal routes.
 
+## Dialogs (centralized registry)
+
+App-level dialogs are **not** mounted inline next to their trigger. They live in a central registry and are opened imperatively. This is the required pattern for any new modal/sheet — do not hand-roll local `open` state with a `<DialogTrigger>`.
+
+**The pieces:**
+- `resources/js/components/dialogs/registry.ts` — maps a typed key to a dialog component.
+- `resources/js/hooks/use-dialog.ts` — `useDialog()` returns `dialog.<key>.open(props)` / `.close()` with full prop typing.
+- `resources/js/stores/dialog-store.ts` — Zustand store holding the single active dialog.
+- `resources/js/components/dialogs/dialog-host.tsx` — renders the active dialog once, app-wide.
+
+**Opening a dialog:**
+```tsx
+const dialog = useDialog();
+// inside a handler / DropdownMenuItem onSelect:
+dialog.firewallForm.open({ serverId: server.id, firewallRule });
+```
+
+**Simple confirm / destructive actions — don't create a component, use `dialog.confirm`:**
+```tsx
+dialog.confirm.open({
+  title: `Delete rule [${rule.name}]`,
+  description: 'Are you sure? This cannot be undone.',
+  variant: 'destructive',
+  confirmLabel: 'Delete',
+  method: 'delete',
+  url: route('firewall.destroy', { server: rule.server_id, firewallRule: rule }),
+});
+```
+
+**Opening from a dropdown — this is the whole point of the pattern:** use a plain `DropdownMenuItem` with the default `onSelect` so the menu closes, then open the dialog. **Never** wrap a `<Dialog>`/`<DialogTrigger>` inside a `DropdownMenuItem` with `onSelect={(e) => e.preventDefault()}` — that leaves the dropdown stuck open behind the dialog.
+```tsx
+<DropdownMenuItem onSelect={() => dialog.editHostedDomain.open({ hostedDomain })}>Edit</DropdownMenuItem>
+```
+
+**Authoring a registered dialog component** — it takes control props, renders `<Dialog>` directly (NO `DialogTrigger`, NO local `open` state), and suppresses Radix close-autofocus (the store restores focus):
+```tsx
+export default function FirewallRuleForm({
+  open,
+  onOpenChange,
+  serverId,
+  firewallRule,
+}: {
+  open: boolean;
+  onOpenChange: (open: boolean) => void;
+  serverId: number;
+  firewallRule?: FirewallRule;
+}) {
+  const form = useForm({ /* seed from props */ });
+  const submit = (e: FormEvent) => {
+    e.preventDefault();
+    form.post(route('firewall.store', { server: serverId }), { onSuccess: () => onOpenChange(false) });
+  };
+  return (
+    <Dialog open={open} onOpenChange={onOpenChange}>
+      <DialogContent onCloseAutoFocus={(e) => e.preventDefault()}>
+        {/* ... */}
+        <Form id="firewall-rule-form" onSubmit={submit}>{/* fields */}</Form>
+        <Button form="firewall-rule-form" type="submit" disabled={form.processing}>Save</Button>
+      </DialogContent>
+    </Dialog>
+  );
+}
+```
+Then register it once in `registry.ts` (`firewallForm: FirewallRuleForm`). The consumer immediately gets `dialog.firewallForm.open(props)` with typed props.
+
+**Where the dialog component file lives:** a dialog belongs to the module it serves — author it in that module's `components/` folder (e.g. the SSL activation dialog is `pages/server-ssls/components/activate-dialog.tsx`, the PHP ini dialog is `pages/php/components/ini-dialog.tsx`), and import it into `registry.ts` via its `@/pages/...` path. The folder already namespaces the file, so keep the filename short (`activate-dialog`, not `activate-server-ssl-dialog`). Only genuinely cross-cutting, generic dialogs (`confirmation-dialog`, `log-viewer-dialog`) and the registry infrastructure itself (`registry.ts`, `dialog-host.tsx`) live under `resources/js/components/dialogs/`.
+
+**Rules & gotchas:**
+- **Form submit buttons** use `form="<form-id>" type="submit"` and the `onSubmit` handler calls `e.preventDefault()`. Do not wire submit via the button's `onClick` (a bare `onClick={submit}` with no `preventDefault` lets Enter fire a native form submission alongside the Inertia request).
+- **Always call `onOpenChange(false)` in `onSuccess`** to close after a successful request.
+- **Lifecycle:** `DialogHost` renders the component with `open` hard-coded `true` and **unmounts it on close** (it never re-renders with `open=false`). Each open is a fresh instance (keyed by `instanceId`), so `useForm` state resets automatically — don't add manual reset-on-open effects. But any `useEffect` that pushes state *outward* based on `open` (e.g. `useInputFocus`'s `setFocused`) **must return a cleanup**, because the component unmounts while `open` is still `true`:
+  ```tsx
+  useEffect(() => {
+    setFocused(open);
+    return () => setFocused(false); // required — unmount happens with open===true
+  }, [open, setFocused]);
+  ```
+- **Authorization:** the registry carries no authz. Every dialog's props must come from server-authorised sources (Inertia page props, API resources) — never URL params or other user-controlled input.
+
 ## Bootstrap Context (`useConfigs`)
 
 - Shared catalogue data (provider lists, site types, GitHub App install state, public key text) is **not** in `page.props` — it lives in the Zustand `useBootstrapStore`, exposed via `useConfigs()` and `usePublicKeyText()` from `@/stores/bootstrap-store`.

CLAUDE.md

@@ -67,6 +67,7 @@ Vito has a specific architecture. Match these patterns exactly:
 
 - Inertia pages in `resources/js/pages/`. React components in `resources/js/components/`. Functional components + hooks.
 - Forms: use the `useForm` helper, follow existing patterns. Navigation: `<Link>` or `router.visit()` — never raw `<a>` for internal routes.
+- **Dialogs**: all modals/sheets use the centralized registry (`resources/js/components/dialogs/registry.ts`) opened via `useDialog()` — `dialog.<key>.open(props)`, or `dialog.confirm.open({...})` for simple confirms. Never nest `<Dialog>`/`<DialogTrigger>` inside a `DropdownMenuItem`. **Read the "Dialogs" section of `.github/instructions/frontend.instructions.md` before adding or changing any dialog.**
 - Tailwind v4: `@import "tailwindcss"`, `@theme` for config, **`gap-*` over margins** for sibling spacing.
 - Use Shadcn components and semantic tokens (`text-foreground`, `bg-background`, `text-muted-foreground`). Avoid hard-coded colors and custom CSS.
 - **React hooks**: include ALL dependencies in `useEffect` / `useMemo` / `useCallback` arrays. Return cleanup for timers/subscriptions. Stale closures from missing deps are a recurring bug here.

app/Enums/DeploymentStatus.php

@@ -3,8 +3,9 @@
 namespace App\Enums;
 
 use App\Contracts\VitoEnum;
+use Forjed\InertiaTable\Contracts\HasTableDisplay;
 
-enum DeploymentStatus: string implements VitoEnum
+enum DeploymentStatus: string implements HasTableDisplay, VitoEnum
 {
     case DEPLOYING = 'deploying';
     case FINISHED = 'finished';

app/Http/Controllers/ApplicationController.php

@@ -12,7 +12,6 @@
 use App\Exceptions\SourceControlIsNotConnected;
 use App\Exceptions\SSHError;
 use App\Helpers\EnvParser;
-use App\Http\Resources\DeploymentResource;
 use App\Http\Resources\DeploymentScriptResource;
 use App\Http\Resources\LoadBalancerServerResource;
 use App\Http\Resources\WorkerResource;
@@ -21,6 +20,7 @@
 use App\Models\Server;
 use App\Models\Site;
 use App\SiteTypes\AbstractProxiedSiteType;
+use App\Tables\DeploymentTable;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
@@ -52,7 +52,7 @@ public function index(Server $server, Site $site): Response
         $bootstrapWorker = $type instanceof AbstractProxiedSiteType ? $type->bootstrapWorker() : null;
 
         return Inertia::render('application/index', [
-            'deployments' => DeploymentResource::collection($site->deployments()->latest()->simplePaginate(config('web.pagination_size'))),
+            'deployments' => DeploymentTable::make($site->deployments())->paginate(),
             'deploymentScript' => new DeploymentScriptResource($deploymentScript),
             'buildScript' => $buildScript ? new DeploymentScriptResource($buildScript) : null,
             'preFlightScript' => $preFlightScript ? new DeploymentScriptResource($preFlightScript) : null,

app/Tables/DeploymentTable.php

@@ -0,0 +1,42 @@
+<?php
+
+namespace App\Tables;
+
+use App\Http\Resources\ServerLogResource;
+use App\Models\Deployment;
+use Forjed\InertiaTable\Column;
+use Forjed\InertiaTable\Columns\ActionsColumn;
+use Forjed\InertiaTable\Columns\DateTimeColumn;
+use Forjed\InertiaTable\Columns\EnumColumn;
+use Forjed\InertiaTable\Columns\TextColumn;
+use Forjed\InertiaTable\Table;
+
+class DeploymentTable extends Table
+{
+    protected array $tableSettings = ['realtime' => 'deployment'];
+
+    protected string $defaultSort = '-created_at';
+
+    protected function query(): void
+    {
+        $this->perPage = config('web.pagination_size');
+        $this->query->with('log', 'site');
+    }
+
+    protected function columns(): array
+    {
+        return [
+            TextColumn::make('id', 'ID')->sortable(),
+            Column::make('commit', 'Commit'),
+            DateTimeColumn::make('created_at', 'Deployed At')->sortable()->toLocal(),
+            EnumColumn::make('status', 'Status')->sortable(),
+            Column::make('release', 'Release'),
+            Column::data('site_id'),
+            Column::data('server_id', fn (Deployment $deployment) => $deployment->site->server_id),
+            Column::data('active'),
+            Column::data('commit_data'),
+            Column::data('log', fn (Deployment $deployment) => $deployment->log ? ServerLogResource::make($deployment->log) : null),
+            ActionsColumn::make(),
+        ];
+    }
+}

app/Tables/Servers/FirewallRuleTable.php

@@ -22,9 +22,9 @@ protected function columns(): array
     {
         return [
             TextColumn::make('name', 'Name')->sortable(),
-            TextColumn::make('type', 'Type')->uppercase()->sortable(),
+            TextColumn::make('type', 'Type')->sortable(),
             TextColumn::make('source', 'Source')->fallback('any')->sortable(),
-            TextColumn::make('protocol', 'Protocol')->uppercase()->sortable(),
+            TextColumn::make('protocol', 'Protocol')->sortable(),
             TextColumn::make('port', 'Port')->sortable(),
             EnumColumn::make('status', 'Status')->sortable(),
             Column::data('id'),

resources/js/components/dialogs/confirmation-dialog.tsx

@@ -0,0 +1,86 @@
+import { Button } from '@/components/ui/button';
+import { Dialog, DialogClose, DialogContent, DialogDescription, DialogFooter, DialogHeader, DialogTitle } from '@/components/ui/dialog';
+import { useForm } from '@inertiajs/react';
+import type { VisitOptions } from '@inertiajs/core';
+import { LoaderCircleIcon } from 'lucide-react';
+import FormSuccessful from '@/components/form-successful';
+import InputError from '@/components/ui/input-error';
+
+export type ConfirmationDialogProps = {
+  open: boolean;
+  onOpenChange: (open: boolean) => void;
+  title: string;
+  description?: string;
+  confirmLabel?: string;
+  variant?: 'default' | 'destructive';
+  method: 'delete' | 'post' | 'patch' | 'put';
+  url: string;
+  data?: Record<string, string | number | boolean | null>;
+  options?: VisitOptions;
+  onSuccess?: () => void;
+};
+
+export default function ConfirmationDialog({
+  open,
+  onOpenChange,
+  title,
+  description,
+  confirmLabel = 'Confirm',
+  variant = 'default',
+  method,
+  url,
+  data,
+  options,
+  onSuccess,
+}: ConfirmationDialogProps) {
+  const form = useForm<Record<string, string | number | boolean | null>>(data ?? {});
+
+  const submit = () => {
+    const visitOptions: VisitOptions = {
+      preserveScroll: true,
+      ...options,
+      onSuccess: () => {
+        onOpenChange(false);
+        onSuccess?.();
+      },
+    };
+    if (method === 'delete') {
+      form.delete(url, visitOptions);
+    } else if (method === 'post') {
+      form.post(url, visitOptions);
+    } else if (method === 'patch') {
+      form.patch(url, visitOptions);
+    } else {
+      form.put(url, visitOptions);
+    }
+  };
+
+  const errors = Object.values(form.errors) as string[];
+
+  return (
+    <Dialog open={open} onOpenChange={onOpenChange}>
+      <DialogContent onCloseAutoFocus={(e) => e.preventDefault()}>
+        <DialogHeader>
+          <DialogTitle>{title}</DialogTitle>
+          <DialogDescription className="sr-only">{description ?? title}</DialogDescription>
+        </DialogHeader>
+        <div className="space-y-2 p-4">
+          {description && <p>{description}</p>}
+          {errors.map((error) => (
+            <InputError key={error} message={error} />
+          ))}
+        </div>
+        <DialogFooter>
+          <DialogClose asChild>
+            <Button variant="outline">Cancel</Button>
+          </DialogClose>
+          <Button variant={variant} disabled={form.processing} onClick={submit}>
+            {form.processing && <LoaderCircleIcon className="animate-spin" />}
+            <FormSuccessful successful={form.recentlySuccessful} />
+            {confirmLabel}
+          </Button>
+        </DialogFooter>
+      </DialogContent>
+    </Dialog>
+  );
+}

resources/js/components/dialogs/dialog-host.tsx

@@ -0,0 +1,28 @@
+import { useEffect } from 'react';
+import type { ComponentType } from 'react';
+import { router } from '@inertiajs/react';
+import { useDialogStore } from '@/stores/dialog-store';
+import { dialogs, type DialogControlProps } from './registry';
+
+export default function DialogHost() {
+  const active = useDialogStore((s) => s.active);
+  const instanceId = useDialogStore((s) => s.instanceId);
+
+  useEffect(() => {
+    return router.on('navigate', () => useDialogStore.getState().close());
+  }, []);
+
+  if (!active) {
+    return null;
+  }
+
+  const Component = dialogs[active.key] as ComponentType<typeof active.props & DialogControlProps> | undefined;
+
+  if (!Component) {
+    return null;
+  }
+
+  return (
+    <Component key={`${active.key}:${instanceId}`} open onOpenChange={(o: boolean) => !o && useDialogStore.getState().close()} {...active.props} />
+  );
+}

resources/js/components/dialogs/log-viewer-dialog.tsx

@@ -0,0 +1,39 @@
+import { Button } from '@/components/ui/button';
+import { Dialog, DialogContent, DialogDescription, DialogFooter, DialogHeader, DialogTitle } from '@/components/ui/dialog';
+import LogOutput from '@/components/log-output';
+import { useLogContent } from '@/hooks/use-log-content';
+
+type LogViewerDialogProps = {
+  open: boolean;
+  onOpenChange: (open: boolean) => void;
+  serverId: number;
+  logId: number;
+  title: string;
+};
+
+export default function LogViewerDialog({ open, onOpenChange, serverId, logId, title }: LogViewerDialogProps) {
+  const { content, isLoading, error } = useLogContent({ serverId, logId, enabled: open });
+
+  return (
+    <Dialog open={open} onOpenChange={onOpenChange}>
+      <DialogContent className="sm:max-w-5xl" onCloseAutoFocus={(e) => e.preventDefault()}>
+        <DialogHeader>
+          <DialogTitle>{title}</DialogTitle>
+          <DialogDescription className="sr-only">Log contents</DialogDescription>
+        </DialogHeader>
+        <LogOutput>
+          <>
+            {isLoading && 'Loading...'}
+            {error && <div className="text-destructive">Error: {error}</div>}
+            {content && !error && content}
+          </>
+        </LogOutput>
+        <DialogFooter>
+          <a href={route('logs.download', { server: serverId, log: logId })} target="_blank" rel="noopener noreferrer">
+            <Button variant="outline">Download</Button>
+          </a>
+        </DialogFooter>
+      </DialogContent>
+    </Dialog>
+  );
+}