From d60106d09a930fca6cafbc1250ba29d15fddcde6 Mon Sep 17 00:00:00 2001
From: Dotta <bippadotta@protonmail.com>
Date: Sun, 7 Jun 2026 02:35:15 +0000
Subject: [PATCH] Polish Done-reviewing handoff dialog UX (PAP-10575)

Address non-blocking UX-review nits on the Done-reviewing handoff:

- a11y: the disabled "Done reviewing" rail CTA used a non-focusable
  <span> as the tooltip trigger, so keyboard/SR users couldn't learn
  why it was disabled. Make the Button itself the trigger via
  aria-disabled (kept in tab order) instead of the `disabled` attribute,
  guard onClick, and keep the dimmed visual via aria-disabled: classes.
- copy: replace the agent-jargon "a real wake" dialog subhead with plain
  English ("We'll notify them so they pick the review back up").
- copy: add a clarifying title to the "Just close" ghost button
  ("Close this dialog without notifying the owner").

Update DocumentReviewRail tests to assert the aria-disabled/inert
behavior and that the handler fires when review can be finished.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 .../documents/DocumentReviewRail.test.tsx     | 33 +++++++++++++++--
 .../documents/DocumentReviewRail.tsx          | 36 ++++++++++++-------
 .../documents/DoneReviewingDialog.tsx         |  3 +-
 3 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/ui/src/components/documents/DocumentReviewRail.test.tsx b/ui/src/components/documents/DocumentReviewRail.test.tsx
index 96b4ce0e356..fb27531612b 100644
--- a/ui/src/components/documents/DocumentReviewRail.test.tsx
+++ b/ui/src/components/documents/DocumentReviewRail.test.tsx
@@ -246,7 +246,8 @@ describe("DocumentReviewRail", () => {
     expect(container.textContent).toContain("Orphaned feedback (1)");
   });
 
-  it("disables Done reviewing for read-only viewers", async () => {
+  it("keeps Done reviewing keyboard-focusable but inert for read-only viewers", async () => {
+    const onDoneReviewing = vi.fn();
     await act(() =>
       root.render(
         <DocumentReviewRail
@@ -254,11 +255,39 @@ describe("DocumentReviewRail", () => {
           canReview={false}
           canFinishReview={false}
           latestRevisionId="rev-1"
+          doneReviewingDisabledReason="You don't have edit access"
+          onDoneReviewing={onDoneReviewing}
           {...baseHandlers}
         />,
       ),
     );
     const done = container.querySelector<HTMLButtonElement>('[data-testid="rail-done-reviewing"]');
-    expect(done?.disabled).toBe(true);
+    // aria-disabled (not the `disabled` attribute) so it stays in the tab order and can
+    // surface the tooltip explaining *why* it's disabled to keyboard / screen-reader users.
+    expect(done?.getAttribute("aria-disabled")).toBe("true");
+    expect(done?.hasAttribute("disabled")).toBe(false);
+    // ...but clicking it must not fire the handoff.
+    await act(() => done!.click());
+    expect(onDoneReviewing).not.toHaveBeenCalled();
+  });
+
+  it("fires the Done reviewing handler when the viewer can finish review", async () => {
+    const onDoneReviewing = vi.fn();
+    await act(() =>
+      root.render(
+        <DocumentReviewRail
+          reviewIndex={makeIndex()}
+          canReview
+          canFinishReview
+          latestRevisionId="rev-1"
+          onDoneReviewing={onDoneReviewing}
+          {...baseHandlers}
+        />,
+      ),
+    );
+    const done = container.querySelector<HTMLButtonElement>('[data-testid="rail-done-reviewing"]');
+    expect(done?.getAttribute("aria-disabled")).toBeNull();
+    await act(() => done!.click());
+    expect(onDoneReviewing).toHaveBeenCalledTimes(1);
   });
 });
diff --git a/ui/src/components/documents/DocumentReviewRail.tsx b/ui/src/components/documents/DocumentReviewRail.tsx
index e1a0449485c..ecf118e2869 100644
--- a/ui/src/components/documents/DocumentReviewRail.tsx
+++ b/ui/src/components/documents/DocumentReviewRail.tsx
@@ -388,19 +388,29 @@ function ReviewRailBody({
         <div className="border-t border-border p-2">
           <Tooltip>
             <TooltipTrigger asChild>
-              <span className="block">
-                <Button
-                  type="button"
-                  className="w-full"
-                  size="sm"
-                  disabled={!canFinishReview}
-                  onClick={onDoneReviewing}
-                  data-testid="rail-done-reviewing"
-                >
-                  <CheckCircle2 className="mr-1.5 h-4 w-4" aria-hidden="true" />
-                  Done reviewing
-                </Button>
-              </span>
+              {/*
+                Use aria-disabled (not the `disabled` attribute) so the button stays
+                in the tab order and remains the tooltip trigger: keyboard and screen-reader
+                users can focus it to learn *why* it's disabled, instead of hitting a dead
+                control. onClick is guarded so the action can't fire while disabled.
+              */}
+              <Button
+                type="button"
+                className="w-full aria-disabled:cursor-not-allowed aria-disabled:opacity-50"
+                size="sm"
+                aria-disabled={!canFinishReview || undefined}
+                onClick={(event) => {
+                  if (!canFinishReview) {
+                    event.preventDefault();
+                    return;
+                  }
+                  onDoneReviewing?.();
+                }}
+                data-testid="rail-done-reviewing"
+              >
+                <CheckCircle2 className="mr-1.5 h-4 w-4" aria-hidden="true" />
+                Done reviewing
+              </Button>
             </TooltipTrigger>
             {!canFinishReview ? (
               <TooltipContent>
diff --git a/ui/src/components/documents/DoneReviewingDialog.tsx b/ui/src/components/documents/DoneReviewingDialog.tsx
index 5dff1da0953..90f66df50c1 100644
--- a/ui/src/components/documents/DoneReviewingDialog.tsx
+++ b/ui/src/components/documents/DoneReviewingDialog.tsx
@@ -93,7 +93,7 @@ export function DoneReviewingDialog({
         <DialogHeader>
           <DialogTitle>Done reviewing</DialogTitle>
           <DialogDescription>
-            Hand the document back to its owner with an optional note and a real wake.
+            Hand the document back to its owner with an optional note. We'll notify them so they pick the review back up.
           </DialogDescription>
         </DialogHeader>
 
@@ -171,6 +171,7 @@ export function DoneReviewingDialog({
             variant="ghost"
             onClick={() => onOpenChange(false)}
             disabled={submitting}
+            title="Close this dialog without notifying the owner"
             data-testid="done-reviewing-just-close"
           >
             Just close