From e434beec7afc01d2b6f70d621b69c2f6bd0fd493 Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Mon, 10 Aug 2026 10:01:47 +0100 Subject: [PATCH] fix(web): toasts survive an open dialog; empty-state actions can be gated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both from the final review, and the first one overturns a call I got wrong. The aria-hidden sweep that makes aria-modal true also swallowed the toasts. ToastProvider renders inside the app root, and every modal-raised confirmation is toasted *before* its dialog closes — "Saved …", "Deleted …", "Removed …" — so each one was inserted into a hidden subtree and never announced. Un-hiding a live region afterwards does not replay what it missed. The toast layer is portalled to the body carrying the dialog-layer attribute, which exempts it from the sweep, and sits above the dialog: a toast explaining why a dialog's action failed is no use behind it. EmptyState's action was narrower than the call site it replaced. The old first-workflow button carried loading={isPending}; the new one carried nothing, so a double click created two workflows. The action is a union now — a link takes no pending state, a handler takes loading and disabled. --- web/app/(app)/workflows/page.tsx | 2 +- web/components/ui/Async.tsx | 19 ++++++++++++++++-- web/components/ui/Modal.tsx | 11 ++++++++++- web/components/ui/Toast.tsx | 33 +++++++++++++++++++++++++------- 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/web/app/(app)/workflows/page.tsx b/web/app/(app)/workflows/page.tsx index 828b33d..a028da5 100644 --- a/web/app/(app)/workflows/page.tsx +++ b/web/app/(app)/workflows/page.tsx @@ -73,7 +73,7 @@ export default function WorkflowsPage() { } - action={{ label: "Create your first workflow", onClick: () => create() }} + action={{ label: "Create your first workflow", onClick: () => create(), loading: isPending }} /> } > diff --git a/web/components/ui/Async.tsx b/web/components/ui/Async.tsx index 8850fd1..29cd1d0 100644 --- a/web/components/ui/Async.tsx +++ b/web/components/ui/Async.tsx @@ -63,7 +63,15 @@ export function EmptyState({ title: string; description?: string; icon?: React.ReactNode; - action?: { label: string; href?: string; onClick?: () => void }; + /* + * `loading` matters rather than being decoration: the empty-state button is + * usually the one that creates the first of something, and without it a + * double click creates two. A link action takes neither — there is no + * pending state to show for a navigation. + */ + action?: + | { label: string; href: string; onClick?: never; loading?: never; disabled?: never } + | { label: string; href?: never; onClick: () => void; loading?: boolean; disabled?: boolean }; }) { return (
@@ -80,7 +88,14 @@ export function EmptyState({ {action.label} ) : ( - ))} diff --git a/web/components/ui/Modal.tsx b/web/components/ui/Modal.tsx index e7e768b..cc403f2 100644 --- a/web/components/ui/Modal.tsx +++ b/web/components/ui/Modal.tsx @@ -22,7 +22,16 @@ let lockedOverflow = ""; let lockedPadding = ""; let hidden: HTMLElement[] = []; -const PORTAL_ATTR = "data-vantage-dialog"; +/* + * Marks a body child as belonging to the dialog layer rather than the page, so + * the aria-hidden sweep below skips it. Exported because the toast layer needs + * the same exemption: a confirmation raised by a dialog is raised *before* that + * dialog closes, so a toast rendered inside the app tree would be inserted into + * a hidden subtree and never announced — and un-hiding a live region later does + * not replay what it missed. + */ +export const DIALOG_LAYER_ATTR = "data-vantage-dialog"; +const PORTAL_ATTR = DIALOG_LAYER_ATTR; function lockScroll() { const { body } = document; diff --git a/web/components/ui/Toast.tsx b/web/components/ui/Toast.tsx index 285e3a4..bd725b5 100644 --- a/web/components/ui/Toast.tsx +++ b/web/components/ui/Toast.tsx @@ -1,8 +1,10 @@ "use client"; -import { createContext, useCallback, useContext, useMemo, useRef, useState } from "react"; +import { createContext, useCallback, useContext, useEffect, useMemo, useRef, useState } from "react"; +import { createPortal } from "react-dom"; import { clsx } from "clsx"; import { friendlyMessage } from "./Async"; +import { DIALOG_LAYER_ATTR } from "./Modal"; /* * Mutations succeeded silently. Copying an install one-liner, generating a key, @@ -59,6 +61,10 @@ export function ToastProvider({ children }: { children: React.ReactNode }) { const nextId = useRef(1); const timers = useRef(new Map>()); + // Portals need a DOM, which SSR has not got. + const [mounted, setMounted] = useState(false); + useEffect(() => setMounted(true), []); + const dismiss = useCallback((id: number) => { const timer = timers.current.get(id); if (timer) { @@ -95,6 +101,12 @@ export function ToastProvider({ children }: { children: React.ReactNode }) { {children} {/* + * Portalled to the body and marked as dialog layer, so an open + * modal's aria-hidden sweep leaves it alone. Every modal-raised + * confirmation ("Saved …", "Deleted …", "Removed …") is toasted + * before the dialog closes, and inside the app tree all of them + * would land in a hidden subtree and go unannounced. + * * Two regions, not one polite container holding role="alert" * children: live-region politeness is taken from the nearest * ancestor that declares it, so a single polite wrapper demotes the @@ -102,13 +114,20 @@ export function ToastProvider({ children }: { children: React.ReactNode }) { * the operator asked for did not happen; a confirmation can wait * for a pause in speech. * - * The wrapper is a plain flex column so both regions stack as one - * visual list. + * z-index sits above the dialog layer: a toast reporting why a + * dialog's action failed is no use behind it. */} -
- t.kind !== "error")} politeness="polite" onDismiss={dismiss} /> - t.kind === "error")} politeness="assertive" onDismiss={dismiss} /> -
+ {mounted && + createPortal( +
+ t.kind !== "error")} politeness="polite" onDismiss={dismiss} /> + t.kind === "error")} politeness="assertive" onDismiss={dismiss} /> +
, + document.body, + )} ); }