From 4f7153bb7de862aa7f1d72b7c795b2eefd25d885 Mon Sep 17 00:00:00 2001
From: npeter83 <npeter83+claude@gmail.com>
Date: Tue, 21 Jul 2026 22:37:42 +0200
Subject: [PATCH] =?UTF-8?q?fix(toast,overlay):=20review=20round=201=20?=
 =?UTF-8?q?=E2=80=94=20close=20toast-timer=20edge,=20document=20Escape=20l?=
 =?UTF-8?q?imit?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review found two low-severity items:
- toast: armDismiss now always clears the prior timer (and arms only when duration is
  truthy), so a coalesced repeat that upgrades a toast to requiresInteraction no longer
  lets the original timer fire and dismiss it. Was a pre-existing gap the C-3.25 change
  hadn't closed.
- overlay: documented the known limitation of useDismiss's blanket stopPropagation — a
  PlexPlayer menu open during an auto-skip countdown swallows the Escape that would also
  cancel the skip. Proper fix needs R8's shared layer-registry (topmost-only dispatch),
  not a broad stopPropagation; accepted until then.
---
 frontend/src/lib/notifications.ts | 23 +++++++++++++++--------
 frontend/src/lib/useDismiss.ts    |  6 ++++++
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/frontend/src/lib/notifications.ts b/frontend/src/lib/notifications.ts
index 0c318726..b57cc7c3 100644
--- a/frontend/src/lib/notifications.ts
+++ b/frontend/src/lib/notifications.ts
@@ -134,13 +134,20 @@ let counter = items.reduce((m, n) => Math.max(m, n.id), 0) + 1;
 // deadline and dismisses the re-surfaced toast early (C-3.25).
 const dismissTimers = new Map<number, ReturnType<typeof setTimeout>>();
 
-function armDismiss(id: number, duration: number): void {
+function armDismiss(id: number, duration: number | undefined): void {
+  // Always clear any existing timer first. A falsy duration then arms nothing — so a toast that a
+  // coalesced repeat upgraded to requiresInteraction (duration undefined) correctly STAYS, instead
+  // of the original timer firing and dismissing the now-must-interact toast.
   const existing = dismissTimers.get(id);
-  if (existing) clearTimeout(existing);
-  dismissTimers.set(
-    id,
-    setTimeout(() => dismiss(id), duration),
-  );
+  if (existing) {
+    clearTimeout(existing);
+    dismissTimers.delete(id);
+  }
+  if (duration)
+    dismissTimers.set(
+      id,
+      setTimeout(() => dismiss(id), duration),
+    );
 }
 
 // Cached derived snapshots — useSyncExternalStore needs stable references between
@@ -257,7 +264,7 @@ export function notify(input: NotifyInput): number {
           : n,
       );
       emit();
-      if (duration) armDismiss(prior.id, duration);
+      armDismiss(prior.id, duration); // clears the prior timer even when the repeat is now must-interact
       // Deliberately no re-beep: the first occurrence already sounded; a loop must stay quiet.
       return prior.id;
     }
@@ -289,7 +296,7 @@ export function notify(input: NotifyInput): number {
   ) {
     beep();
   }
-  if (duration) armDismiss(id, duration);
+  armDismiss(id, duration);
   return id;
 }
 
diff --git a/frontend/src/lib/useDismiss.ts b/frontend/src/lib/useDismiss.ts
index 9845d2f7..94b365bb 100644
--- a/frontend/src/lib/useDismiss.ts
+++ b/frontend/src/lib/useDismiss.ts
@@ -21,6 +21,12 @@ export function useDismiss(
         // Stop the same Escape from also reaching a player's window-level listener underneath —
         // this document listener bubbles first, so one Escape closes only this popover, not the
         // popover AND the player behind it. (U-C quick-fix.)
+        //
+        // KNOWN LIMITATION (accepted, → R8 popover-platform): because this is a blanket
+        // stopPropagation, a PlexPlayer settings/tracks menu open DURING an auto-skip countdown
+        // swallows the Escape that would otherwise ALSO cancel the skip — Escape closes the menu,
+        // and the skip proceeds. Properly fixing this needs the shared layer-registry R8 builds
+        // (topmost-only dispatch), not a broad stopPropagation.
         e.stopPropagation();
         onClose();
       }