Migrate admin user editor + Review pop-up to in-house Dialog (a11y)

Original task: Make admin pop-ups close with Escape and trap keyboard
focus (#185).

Changes
- artifacts/tx-os/src/pages/admin.tsx
  - UserGroupsEditor and its inner "Review changes" pop-up no longer
    use the hand-rolled fixed-inset overlay. Both now use the in-house
    Radix-based Dialog (Dialog/DialogContent/DialogHeader/DialogTitle/
    DialogDescription/DialogFooter) so Escape closes only the topmost
    dialog, Tab/Shift+Tab is trapped, and the close button + aria-modal
    semantics come for free.
  - Removed the manual `keydown` Escape useEffect that was previously
    needed for the Review pop-up.
  - Preserved all existing data-testids (edit-user-save,
    edit-user-review-dialog, edit-user-review-back, etc.) plus added
    a new edit-user-dialog testid for the editor shell.
  - Kept the original glass-panel look by passing border-0 / shadow-none
    and rounded-3xl through DialogContent's className.
  - Focus return: Radix's automatic restoration is unreliable for
    nested dialogs. Added two explicit hops:
      - The Review dialog uses onCloseAutoFocus to refocus the Save
        button via a saveBtnRef.
      - The editor dialog captures whatever was focused when it first
        rendered (the pencil icon) into triggerElementRef, and
        onCloseAutoFocus refocuses it. Verified the pencil button is
        what the test sees.

Tests
- artifacts/tx-os/tests/admin-user-edit-review.spec.mjs
  - New test "Escape closes topmost dialog; Tab keeps focus inside;
    focus returns to opener" runs in both en and ar:
      - Esc closes only the Review (editor stays open) and focus
        returns to the Save button.
      - Tab x15 and Shift+Tab x5 keep document.activeElement inside
        the editor dialog.
      - Esc on the editor closes it and returns focus to the pencil.
  - All 4 specs in this file pass (~58s).

No deviations from the task; scope intentionally limited to the user
editor + Review pop-up. The same pattern in the Groups/Roles/Apps
editors was filed as follow-up #266.

Replit-Task-Id: fc613d10-06d0-460f-b1a4-0a65ff021d4a

artifacts/tx-os/src/pages/admin.tsx

@@ -143,6 +143,14 @@ import { Input } from "@/components/ui/input";
 import { Label } from "@/components/ui/label";
 import { Switch } from "@/components/ui/switch";
 import { Sheet, SheetContent, SheetTrigger } from "@/components/ui/sheet";
+import {
+  Dialog,
+  DialogContent,
+  DialogDescription,
+  DialogFooter,
+  DialogHeader,
+  DialogTitle,
+} from "@/components/ui/dialog";
 import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip";
 import { useToast } from "@/hooks/use-toast";
 import { cn } from "@/lib/utils";
@@ -3821,15 +3829,23 @@ function UserGroupsEditor({
   const [isActive, setIsActive] = useState(original.isActive);
   const [groupSearch, setGroupSearch] = useState("");
   const [reviewOpen, setReviewOpen] = useState(false);
-
-  useEffect(() => {
-    if (!reviewOpen) return;
-    const onKey = (e: KeyboardEvent) => {
-      if (e.key === "Escape") setReviewOpen(false);
-    };
-    window.addEventListener("keydown", onKey);
-    return () => window.removeEventListener("keydown", onKey);
-  }, [reviewOpen]);
+  // Used to send focus back to the Save button when the inner Review
+  // dialog closes. Radix's automatic focus restoration is unreliable
+  // for nested dialogs, so we steer it explicitly via onCloseAutoFocus.
+  const saveBtnRef = useRef<HTMLButtonElement>(null);
+  // Capture whatever was focused (typically the pencil edit button) at
+  // the moment this editor first renders, so we can return focus there
+  // when the editor closes. We must read this *during* render — by the
+  // time a useEffect runs, Radix's FocusScope has already moved focus
+  // inside the dialog. useState's lazy initializer gives us a stable,
+  // mount-time snapshot.
+  const [triggerElement] = useState<HTMLElement | null>(() => {
+    if (typeof document === "undefined") return null;
+    const active = document.activeElement;
+    return active instanceof HTMLElement && active !== document.body
+      ? active
+      : null;
+  });
 
   const toggle = (id: number) => {
     const next = new Set(selected);
@@ -3930,9 +3946,30 @@ function UserGroupsEditor({
   };
 
   return (
-    <div className="fixed inset-0 bg-slate-900/30 backdrop-blur-sm z-50 flex items-center justify-center p-4">
-      <div className="glass-panel rounded-3xl p-6 w-full max-w-md space-y-3 max-h-[85vh] overflow-y-auto">
-        <h2 className="font-semibold text-foreground">{t("admin.users.editUserGroups", { username: user.username })}</h2>
+    <Dialog
+      open={true}
+      onOpenChange={(o) => {
+        if (!o) onClose();
+      }}
+    >
+      <DialogContent
+        data-testid="edit-user-dialog"
+        className="glass-panel border-0 shadow-none rounded-3xl sm:rounded-3xl p-6 w-full max-w-md max-h-[85vh] overflow-y-auto gap-3"
+        onCloseAutoFocus={(e) => {
+          // Return focus to the trigger that opened the editor (e.g. the
+          // pencil button). Radix's default restoration is unreliable
+          // because the parent re-renders on close.
+          if (triggerElement && document.body.contains(triggerElement)) {
+            e.preventDefault();
+            triggerElement.focus();
+          }
+        }}
+      >
+        <DialogHeader>
+          <DialogTitle className="text-base font-semibold text-foreground">
+            {t("admin.users.editUserGroups", { username: user.username })}
+          </DialogTitle>
+        </DialogHeader>
         <div className="grid grid-cols-2 gap-2">
           <div className="space-y-1">
             <Label className="text-xs">{t("admin.users.col.displayNameAr")}</Label>
@@ -4010,9 +4047,10 @@ function UserGroupsEditor({
             return r ? r.name : `#${id}`;
           }}
         />
-        <div className="flex gap-2 pt-2">
+        <DialogFooter className="flex flex-row gap-2 pt-2 sm:justify-stretch sm:space-x-0">
           <Button variant="outline" className="flex-1" onClick={onClose}>{t("common.cancel")}</Button>
           <Button
+            ref={saveBtnRef}
             className="flex-1"
             disabled={!hasChanges}
             title={hasChanges ? undefined : t("admin.users.review.noChanges")}
@@ -4021,23 +4059,28 @@ function UserGroupsEditor({
           >
             {t("common.save")}
           </Button>
-        </div>
-      </div>
+        </DialogFooter>
 
-      {reviewOpen && (
-        <div
-          className="fixed inset-0 bg-slate-900/50 backdrop-blur-sm z-[60] flex items-center justify-center p-4"
-          data-testid="edit-user-review-dialog"
-          role="dialog"
-          aria-modal="true"
-        >
-          <div className="glass-panel rounded-3xl p-6 w-full max-w-md space-y-4 max-h-[85vh] overflow-y-auto">
-            <div className="space-y-1">
-              <h3 className="font-semibold text-foreground">{t("admin.users.review.title")}</h3>
-              <p className="text-xs text-muted-foreground">
+        <Dialog open={reviewOpen} onOpenChange={setReviewOpen}>
+          <DialogContent
+            data-testid="edit-user-review-dialog"
+            className="glass-panel border-0 shadow-none rounded-3xl sm:rounded-3xl p-6 w-full max-w-md max-h-[85vh] overflow-y-auto gap-4"
+            onCloseAutoFocus={(e) => {
+              // Nested-dialog focus restoration is unreliable in Radix:
+              // the outer Dialog's FocusScope can swallow the restore.
+              // Steer focus back to the Save button that opened us.
+              e.preventDefault();
+              saveBtnRef.current?.focus();
+            }}
+          >
+            <DialogHeader className="space-y-1">
+              <DialogTitle className="text-base font-semibold text-foreground">
+                {t("admin.users.review.title")}
+              </DialogTitle>
+              <DialogDescription className="text-xs text-muted-foreground">
                 {t("admin.users.review.subtitle", { username: user.username })}
-              </p>
-            </div>
+              </DialogDescription>
+            </DialogHeader>
             <ul className="space-y-2 text-sm">
               {fieldChanges.map((c) => (
                 <li
@@ -4090,7 +4133,7 @@ function UserGroupsEditor({
                 </div>
               </div>
             )}
-            <div className="flex gap-2 pt-1">
+            <DialogFooter className="flex flex-row gap-2 pt-1 sm:justify-stretch sm:space-x-0">
               <Button
                 variant="outline"
                 className="flex-1"
@@ -4112,11 +4155,11 @@ function UserGroupsEditor({
                   t("admin.users.review.confirm")
                 )}
               </Button>
-            </div>
-          </div>
-        </div>
-      )}
-    </div>
+            </DialogFooter>
+          </DialogContent>
+        </Dialog>
+      </DialogContent>
+    </Dialog>
   );
 }
 

artifacts/tx-os/tests/admin-user-edit-review.spec.mjs

@@ -268,5 +268,83 @@ for (const lang of ["en", "ar"]) {
       expect(ids).toContain(groupB.id);
       expect(ids).not.toContain(groupA.id);
     });
+
+    // Keyboard accessibility: now that both dialogs use the in-house
+    // Radix-based Dialog primitive, Escape should close just the topmost
+    // dialog and Tab should stay trapped inside the open dialog.
+    test("Escape closes topmost dialog; Tab keeps focus inside; focus returns to opener", async ({
+      page,
+    }) => {
+      const admin = await createUser("kbd_admin", {
+        admin: true,
+        displayNameAr: "لوحة المفاتيح",
+        displayNameEn: "Keyboard",
+        preferredLanguage: lang,
+      });
+      const target = await createUser("kbd_target", {
+        displayNameAr: "هدف لوحة",
+        displayNameEn: "Kbd Target",
+      });
+
+      await setLanguage(page, lang);
+      await login(page, admin.username, TEST_PASSWORD);
+      await page.goto("/admin#section=users");
+
+      const editBtn = page.locator(`[data-testid="user-edit-${target.id}"]`);
+      await expect(editBtn).toBeVisible({ timeout: 10_000 });
+      await editBtn.click();
+
+      const editorDialog = page.locator(
+        '[data-testid="edit-user-dialog"]',
+      );
+      await expect(editorDialog).toBeVisible();
+
+      // Make a change so Save is enabled, then open the Review dialog.
+      const arInput = page.locator('[data-testid="edit-user-display-ar"]');
+      await arInput.fill("هدف لوحة 2");
+      const saveBtn = page.locator('[data-testid="edit-user-save"]');
+      await expect(saveBtn).toBeEnabled();
+      await saveBtn.click();
+
+      const reviewDialog = page.locator(
+        '[data-testid="edit-user-review-dialog"]',
+      );
+      await expect(reviewDialog).toBeVisible();
+
+      // Escape should close ONLY the topmost (review) dialog, leaving the
+      // editor open underneath.
+      await page.keyboard.press("Escape");
+      await expect(reviewDialog).toBeHidden();
+      await expect(editorDialog).toBeVisible();
+
+      // After the review closes, focus returns to the Save button that
+      // opened it.
+      await expect(saveBtn).toBeFocused();
+
+      // Tab should stay trapped inside the editor dialog: after several
+      // Tab presses the active element must still be a descendant of the
+      // editor dialog (never something on the page behind it).
+      for (let i = 0; i < 15; i++) {
+        await page.keyboard.press("Tab");
+        const stillInside = await editorDialog.evaluate((el) =>
+          el.contains(document.activeElement),
+        );
+        expect(stillInside).toBe(true);
+      }
+
+      // Shift+Tab also stays inside.
+      for (let i = 0; i < 5; i++) {
+        await page.keyboard.press("Shift+Tab");
+        const stillInside = await editorDialog.evaluate((el) =>
+          el.contains(document.activeElement),
+        );
+        expect(stillInside).toBe(true);
+      }
+
+      // Escape on the editor closes it and returns focus to the pencil.
+      await page.keyboard.press("Escape");
+      await expect(editorDialog).toBeHidden();
+      await expect(editBtn).toBeFocused();
+    });
   });
 }