feat: add client-side toast directive

[adnaan]

Summary

• Add handleToastDirectives() in dom/directives.ts — reads data-pending JSON from server trigger elements and creates fully client-managed toast DOM
• Add setupToastClickOutside() — dismisses all toasts on outside click (called once at connect)
• Wire both into livetemplate-client.ts (updateDOM and connect)
• CSS lives in the component template (server-rendered), not JS-injected — avoids morphdom removing dynamically injected styles

Test plan

• todos/ E2E tests pass with toast position:fixed assertion
• Toast auto-dismisses after 5s, click-outside dismisses all

🤖 Generated with Claude Code

[claude]

Review

Overall looks solid — XSS is safe (textContent throughout), the dedup guard via PENDING_PROCESSED_KEY is clean, and the architecture is sensible.

Bugs / Issues

1. Unguarded .forEach on parsed JSON (directives.ts ~line 203)
   JSON.parse is caught, but if the server sends non-array valid JSON (object, string, etc.), messages.forEach will throw uncaught. Add a guard after the try/catch:

   if (!Array.isArray(messages) || !messages.length) return;

2. Click-outside listener leaked on disconnect() (livetemplate-client.ts ~line 388)
   setupToastClickOutside replaces the listener on reconnect, but disconnect() never removes it. If the client is torn down (e.g. in a SPA route change), the stale document-level listener remains. Consider returning the listener from setupToastClickOutside and calling removeEventListener in disconnect().

3. Accessibility: aria-live="polite" for errors (directives.ts getOrCreateToastStack)
   Error toasts should use aria-live="assertive" so screen readers announce them immediately. Either create two stacks (one per urgency level) or set the attribute dynamically per message type.

Minor

• If msg.id is missing/undefined, setAttribute("data-lvt-toast-item", msg.id) silently sets "undefined" — worth a null check or a console warning during development.

[Copilot]

Pull request overview

Adds a new client-side “toast directive” flow to LiveTemplate: server-rendered trigger elements provide data-pending JSON, and the client builds/controls an out-of-band toast stack (including click-outside dismissal) without relying on server DOM persistence.

Changes:

• Added handleToastDirectives() to parse data-pending from [data-toast-trigger] elements and append toast DOM into a global stack.
• Added setupToastClickOutside() to dismiss all toasts when clicking outside the stack.
• Wired toast handling into LiveTemplateClient.connect() (listener setup) and updateDOM() (directive execution after morphdom).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
livetemplate-client.ts	Calls toast setup at connect time and runs toast directive handling after each DOM patch.
dom/directives.ts	Implements toast stack creation, toast rendering, auto-dismiss timers, and click-outside dismissal logic.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

JSON.parse(pending) is assumed to return ToastMessage[], but runtime input could be a non-array (e.g. a string with a .length), which would pass the !messages.length guard and then throw at messages.forEach. Consider guarding with Array.isArray(...) (and optionally validating each item is an object with required fields) before iterating to avoid client-side crashes from malformed data-pending values.

[Copilot]

ToastMessage.type is currently unused when building the toast DOM, so the rendered markup can’t reflect the toast variant (info/success/warning/error). If styling/behavior depends on the type, consider adding a data-* attribute or class based on msg.type (or removing the field if it’s not intended to be used).

Suggested change

type: "info" | "success" | "warning" | "error";

[Copilot]

New toast behavior is introduced here (parsing data-pending, creating a global toast stack, click handlers, and auto-dismiss timers), but there are no corresponding unit tests alongside the existing directive tests. Adding tests for: (1) valid JSON array renders toast elements, (2) malformed/non-array JSON is ignored safely, (3) dismissMS removes toasts via fake timers, and (4) dismissible button removes a toast would help prevent regressions.

[claude]

Review: feat: add client-side toast directive

Overall clean implementation. A few issues worth addressing:

Bug: No runtime validation of parsed JSON

JSON.parse(pending) is cast directly to ToastMessage[] without field validation. If dismissMS arrives as a string (or is missing), msg.dismissMS > 0 silently fails to auto-dismiss. Recommend a guard:

if (typeof msg.dismissMS !== 'number' || msg.dismissMS > 0) { ... }

Or validate the shape before use.

Code quality: (trigger as any) for the processed-key cache

Using (trigger as any)[PENDING_PROCESSED_KEY] bypasses type safety and pollutes DOM elements. A WeakMap is the idiomatic alternative:

const processedPending = new WeakMap<HTMLElement, string>();
// ...
if (processedPending.get(trigger) === pending) return;
processedPending.set(trigger, pending);

UX concern: click-outside dismisses ALL toasts simultaneously

setupToastClickOutside removes every [data-lvt-toast-item] on any outside click. If multiple toasts are stacked (e.g. one auto-dismissing, one permanent), a stray click wipes them all. Is this intentional? If not, consider only dismissing dismissible ones.

Minor: aria-live="polite" without aria-atomic

The stack has aria-live="polite" but no aria-atomic="false". That's the default so it works, but being explicit avoids screen reader behavior surprises when multiple toasts arrive in one batch.

[claude]

Review

Good overall. XSS vectors are safe (.textContent throughout), JSON parse is guarded, and the dedup logic via the expando key is sound.

Issues worth fixing:

1. Click-outside discards unread error toasts
setupToastClickOutside removes all toasts on any outside click, including type=error ones with no auto-dismiss. Users can lose important messages before reading them. Consider skipping non-dismissible toasts, or at least error-type ones.

2. msg.id is not validated before use as an attribute
If the server omits id, setAttribute sets an empty attribute and DOM dedup by ID breaks. Add an early guard in createToastElement to skip messages with a falsy id.

3. dismissMS: Infinity creates a permanently stuck toast
The check (typeof msg.dismissMS === 'number' && msg.dismissMS > 0) passes for Infinity, so the timeout never fires. If dismissible is also false the toast cannot be removed. Add isFinite(msg.dismissMS) to the guard.

Minor:

• No cap on toast count — a server burst could flood the stack. A simple max (e.g. 10) would help.
• Expando props via (trigger as any) work but a WeakMap would be cleaner TypeScript.

[claude]

Review: feat: add client-side toast directive

Good overall — the XSS surface is clean (textContent throughout, no innerHTML), the dedup guard via PENDING_PROCESSED_KEY is smart, and the aria-live/role="alert" attributes are correct.

Two bugs worth fixing:

1. Click-outside handler dismisses non-dismissible toasts
setupToastClickOutside unconditionally removes every [data-lvt-toast-item], ignoring the dismissible field. A toast marked dismissible: false should survive an outside click.

// current
stack.querySelectorAll("[data-lvt-toast-item]").forEach((el) => el.remove());

// suggested
stack.querySelectorAll("[data-lvt-toast-item]").forEach((el) => {
  if (el.querySelector("button[aria-label='Dismiss']")) el.remove();
});

(or store the flag as a data-dismissible attribute on the element at creation time)

2. Position is silently ignored after the first stack is created
getOrCreateToastStack checks for any existing [data-lvt-toast-stack] and reuses it regardless of its position. If two triggers on the same page specify different positions, whichever fires first wins and the second is silently wrong. Consider keying the stack by position (e.g. [data-lvt-toast-stack="bottom-left"]) so each position gets its own container — this also fixes the setupToastClickOutside query which only targets a single stack.