Skip to content

Commit 895a6e4

Browse files
committed
fix(trigger-chat-agent): component correctness and accessibility
1 parent 8455459 commit 895a6e4

5 files changed

Lines changed: 69 additions & 18 deletions

File tree

trigger-chat-agent/src/components/error-notice.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,10 @@ export function ErrorNotice({
5151
const { title, detail } = explain(error);
5252

5353
return (
54-
<div className="flex items-start gap-3 rounded-xl border border-error/40 bg-error/5 px-4 py-3">
54+
<div
55+
role="alert"
56+
className="flex items-start gap-3 rounded-xl border border-error/40 bg-error/5 px-4 py-3"
57+
>
5558
<AlertTriangle className="mt-0.5 size-4 shrink-0 text-error" />
5659
<div className="min-w-0 flex-1">
5760
<div className="text-sm font-medium text-error">{title}</div>

trigger-chat-agent/src/components/flow-graph.tsx

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,20 @@ const nodeTypes = { flow: FlowNodeCard };
416416
export function FlowGraph({ title, nodes, edges, sequence }: FlowGraphProps) {
417417
const reduceMotion = !!useReducedMotion();
418418

419+
// Stream re-renders hand us fresh array identities with identical content (the
420+
// message part is re-cloned per streamed token). Key the expensive layout and
421+
// the status timeline on the CONTENT, not the array reference, so dagre isn't
422+
// re-run and an in-flight animation doesn't reset to t=0 on every token.
423+
const nodesSig = nodes
424+
.map((n) => `${n.id}|${n.label}|${n.sublabel ?? ""}|${n.kind}|${n.status ?? ""}`)
425+
.join(";");
426+
const edgesSig = edges
427+
.map((e) => `${e.from}>${e.to}|${e.kind ?? ""}|${e.label ?? ""}`)
428+
.join(";");
429+
const sequenceSig = (sequence ?? [])
430+
.map((s) => `${s.nodeId}|${s.status}|${s.atMs}`)
431+
.join(";");
432+
419433
// Robustness guard: a model can emit an edge whose `from`/`to` doesn't match
420434
// any node id (typo, stale reference, partial catalog data). React Flow
421435
// doesn't validate this itself — an edge pointing at a missing node id can
@@ -425,11 +439,13 @@ export function FlowGraph({ title, nodes, edges, sequence }: FlowGraphProps) {
425439
const safeEdges = useMemo(() => {
426440
const ids = new Set(nodes.map((n) => n.id));
427441
return edges.filter((e) => ids.has(e.from) && ids.has(e.to));
428-
}, [nodes, edges]);
442+
// eslint-disable-next-line react-hooks/exhaustive-deps
443+
}, [nodesSig, edgesSig]);
429444

430445
const { positions, handles: edgeHandles } = useMemo(
431446
() => computeLayout(nodes, safeEdges),
432-
[nodes, safeEdges]
447+
// eslint-disable-next-line react-hooks/exhaustive-deps
448+
[nodesSig, safeEdges]
433449
);
434450

435451
const revealDelays = useMemo(() => {
@@ -439,7 +455,8 @@ export function FlowGraph({ title, nodes, edges, sequence }: FlowGraphProps) {
439455
const out: Record<string, number> = {};
440456
for (const [id, o] of order) out[id] = 0.1 + o * step;
441457
return out;
442-
}, [nodes, safeEdges]);
458+
// eslint-disable-next-line react-hooks/exhaustive-deps
459+
}, [nodesSig, safeEdges]);
443460

444461
const [statuses, setStatuses] = useState<Record<string, FlowNodeStatus>>(() =>
445462
initialStatuses(nodes, sequence, reduceMotion)
@@ -456,7 +473,8 @@ export function FlowGraph({ title, nodes, edges, sequence }: FlowGraphProps) {
456473
}, Math.max(0, s.atMs))
457474
);
458475
return () => timers.forEach((t) => window.clearTimeout(t));
459-
}, [nodes, sequence, reduceMotion]);
476+
// eslint-disable-next-line react-hooks/exhaustive-deps
477+
}, [nodesSig, sequenceSig, reduceMotion]);
460478

461479
// Edges fade in mid-cascade so they don't dangle off still-hidden nodes.
462480
const [edgesVisible, setEdgesVisible] = useState(reduceMotion);
@@ -467,7 +485,9 @@ export function FlowGraph({ title, nodes, edges, sequence }: FlowGraphProps) {
467485
return;
468486
}
469487
setEdgesVisible(false);
470-
const t = window.setTimeout(() => setEdgesVisible(true), revealSpan * 500 + 150);
488+
// revealDelays are in SECONDS; the last node starts at ~revealSpan*1000ms,
489+
// so wait until it's begun animating in before the edges appear.
490+
const t = window.setTimeout(() => setEdgesVisible(true), revealSpan * 1000 + 150);
471491
return () => window.clearTimeout(t);
472492
}, [reduceMotion, revealSpan]);
473493

@@ -491,7 +511,8 @@ export function FlowGraph({ title, nodes, edges, sequence }: FlowGraphProps) {
491511
selectable: false,
492512
connectable: false,
493513
})),
494-
[nodes, positions, statuses, revealDelays, reduceMotion]
514+
// eslint-disable-next-line react-hooks/exhaustive-deps
515+
[nodesSig, positions, statuses, revealDelays, reduceMotion]
495516
);
496517

497518
const rfEdges: Edge[] = useMemo(
@@ -527,7 +548,8 @@ export function FlowGraph({ title, nodes, edges, sequence }: FlowGraphProps) {
527548
const bottoms = nodes.map((n) => (positions[n.id]?.y ?? 0) + (positions[n.id]?.height ?? 48));
528549
const maxY = Math.max(0, ...bottoms);
529550
return Math.min(480, Math.max(180, maxY + 24));
530-
}, [nodes, positions]);
551+
// eslint-disable-next-line react-hooks/exhaustive-deps
552+
}, [nodesSig, positions]);
531553

532554
return (
533555
<motion.div

trigger-chat-agent/src/components/prompt-card.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"use client";
22

33
import { motion, useReducedMotion } from "motion/react";
4-
import { useState } from "react";
4+
import { useEffect, useRef, useState } from "react";
55
import { cn } from "@/lib/utils";
66
import { reducedVariants, revealBlur, staggerContainer } from "@/lib/motion";
77

@@ -27,12 +27,20 @@ export function PromptCard({
2727
const item = reduceMotion ? reducedVariants : revealBlur;
2828
const container = reduceMotion ? staggerContainer(0, 0) : staggerContainer(0.05);
2929
const [copied, setCopied] = useState(false);
30+
const resetTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
31+
32+
// Clear a pending reset on unmount so it can't fire setState afterwards.
33+
useEffect(() => () => {
34+
if (resetTimer.current) clearTimeout(resetTimer.current);
35+
}, []);
3036

3137
const onCopy = async () => {
3238
try {
3339
await navigator.clipboard.writeText(prompt);
3440
setCopied(true);
35-
window.setTimeout(() => setCopied(false), 1500);
41+
// Restart the window on each click instead of stacking timers.
42+
if (resetTimer.current) clearTimeout(resetTimer.current);
43+
resetTimer.current = setTimeout(() => setCopied(false), 1500);
3644
} catch {
3745
// Clipboard unavailable (insecure context or denied permission) —
3846
// leave the button idle rather than showing a false "Copied".

trigger-chat-agent/src/components/quiz.tsx

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,26 @@ export function Quiz({
3333
>
3434
<div className="mb-1 font-mono text-2xs uppercase tracking-widest text-dimmed/70">Quiz</div>
3535
<p className="mb-4 font-title text-lg font-medium text-bright">{question}</p>
36-
<div className="space-y-2">
36+
{/* aria-disabled (not the `disabled` attribute) keeps answered options in
37+
the tab order, so a keyboard user can still move across and read the
38+
revealed correct/incorrect states; a click guard blocks re-answering. */}
39+
<div className="space-y-2" role="group" aria-label={question}>
3740
{options.map((o, i) => {
3841
const isCorrect = Boolean(o.correct);
3942
const reveal = answered && (i === picked || isCorrect);
4043
return (
4144
<button
4245
key={i}
4346
type="button"
44-
disabled={answered}
45-
onClick={() => setPicked(i)}
47+
aria-disabled={answered}
48+
onClick={() => {
49+
if (!answered) setPicked(i);
50+
}}
4651
className={cn(
4752
"flex min-h-11 w-full items-center gap-3 rounded-xl border px-4 py-2.5 text-left text-sm leading-5 transition-colors duration-150",
48-
!reveal && "border-charcoal-700 bg-charcoal-800 text-bright enabled:hover:bg-charcoal-700",
53+
!reveal && "border-charcoal-700 bg-charcoal-800 text-bright",
54+
!reveal && !answered && "cursor-pointer hover:bg-charcoal-700",
55+
answered && "cursor-default",
4956
reveal && isCorrect && "border-apple-500/60 bg-apple-500/10 text-apple-200",
5057
reveal && !isCorrect && "border-error/60 bg-error/10 text-error"
5158
)}
@@ -57,9 +64,13 @@ export function Quiz({
5764
);
5865
})}
5966
</div>
60-
{answered && explanation && (
61-
<p className="mt-4 border-t border-grid-bright pt-3 text-sm leading-relaxed text-dimmed">{explanation}</p>
62-
)}
67+
{/* Live region present before the answer lands, so screen readers
68+
announce the explanation when it appears. */}
69+
<div aria-live="polite">
70+
{answered && explanation && (
71+
<p className="mt-4 border-t border-grid-bright pt-3 text-sm leading-relaxed text-dimmed">{explanation}</p>
72+
)}
73+
</div>
6374
</motion.div>
6475
);
6576
}

trigger-chat-agent/src/components/stat-card.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,18 @@ function AnimatedValue({ value, active, reduceMotion }: { value: string; active:
8282
const [, prefix, numText, suffix] = match;
8383
const target = Number(numText.replace(/,/g, ""));
8484
const decimals = numText.includes(".") ? numText.split(".")[1].length : 0;
85+
// Keep the authored digit grouping: "1,234" must animate to "1,234", not
86+
// "1234". toFixed drops separators, so re-group when the source had them.
87+
const grouped = numText.includes(",");
88+
const format = (v: number) =>
89+
grouped
90+
? v.toLocaleString("en-US", { minimumFractionDigits: decimals, maximumFractionDigits: decimals })
91+
: v.toFixed(decimals);
8592
const controls = animate(0, target, {
8693
duration: 1,
8794
delay: 0.25,
8895
ease: easings.outExpo,
89-
onUpdate: (v) => setDisplay(`${prefix}${v.toFixed(decimals)}${suffix}`),
96+
onUpdate: (v) => setDisplay(`${prefix}${format(v)}${suffix}`),
9097
});
9198
return () => controls.stop();
9299
// eslint-disable-next-line react-hooks/exhaustive-deps

0 commit comments

Comments
 (0)