fix(toast,overlay): review round 1 — close toast-timer edge, document Escape limit
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.
This commit is contained in:
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user