Skip to content

Commit 73c8a4d

Browse files
cardernecarderne
authored
fix(webapp): use native controls for inline actions (#4698)
## Summary Replace mouse-only dashboard actions with native buttons. Copy, remove, and stop-generation controls now expose keyboard focus and accessible names. Hover-revealed actions remain mounted so keyboard users can discover them, and a decorative clipboard icon no longer captures clicks. Base: [#4697](#4697)
1 parent 3d156df commit 73c8a4d

8 files changed

Lines changed: 180 additions & 134 deletions

File tree

‎apps/webapp/app/components/AskAI.tsx‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,8 +543,12 @@ function ChatInterface({ initialQuery }: { initialQuery?: string }) {
543543
/>
544544
{isGeneratingAnswer ? (
545545
<SimpleTooltip
546+
asChild
547+
tabbable
546548
button={
547-
<span
549+
<button
550+
type="button"
551+
aria-label="Stop generating"
548552
onClick={() => stopGeneration()}
549553
className="group relative z-10 flex size-10 min-w-10 cursor-pointer items-center justify-center"
550554
>
@@ -553,7 +557,7 @@ function ChatInterface({ initialQuery }: { initialQuery?: string }) {
553557
className="absolute inset-0 animate-spin"
554558
hoverEffect
555559
/>
556-
</span>
560+
</button>
557561
}
558562
content="Stop generating"
559563
/>

‎apps/webapp/app/components/code/TSQLResultsTable.tsx‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,37 @@ function CopyableCell({
843843
const [isHovered, setIsHovered] = useState(false);
844844
const { copy, copied } = useCopy(value);
845845

846+
// The button (with its aria-label) always sits in the same position in the tree, wrapped by
847+
// the same SimpleTooltip, so it is never unmounted/remounted on hover (which would drop
848+
// keyboard focus). The tooltip is left uncontrolled so Radix opens it only when the pointer or
849+
// keyboard focus is actually on the button, not whenever the pointer is anywhere in this
850+
// virtualized grid's cell. `focus-visible:` (not `focus:`) ensures keyboard focus reveals the
851+
// button without leaving it visible after a mouse click moves outside the cell.
852+
const copyButton = (
853+
<button
854+
type="button"
855+
aria-label={copied ? "Copied" : "Copy"}
856+
onClick={(e) => {
857+
e.stopPropagation();
858+
e.preventDefault();
859+
copy();
860+
}}
861+
className={cn(
862+
"absolute right-1 top-1/2 z-10 flex size-6 -translate-y-1/2 items-center justify-center rounded border border-border-bright bg-background-hover transition-opacity focus-visible:pointer-events-auto focus-visible:opacity-100",
863+
isHovered ? "opacity-100" : "pointer-events-none opacity-0",
864+
copied
865+
? "text-green-500"
866+
: "text-text-dimmed hover:border-border-bright hover:bg-background-raised hover:text-text-bright"
867+
)}
868+
>
869+
{copied ? (
870+
<ClipboardCheckIcon className="size-3.5" />
871+
) : (
872+
<ClipboardIcon className="size-3.5" />
873+
)}
874+
</button>
875+
);
876+
846877
return (
847878
<div
848879
className={cn(
@@ -856,37 +887,13 @@ function CopyableCell({
856887
onMouseLeave={() => setIsHovered(false)}
857888
>
858889
<span className="flex items-center truncate">{children}</span>
859-
{isHovered && (
860-
<span
861-
onClick={(e) => {
862-
e.stopPropagation();
863-
e.preventDefault();
864-
copy();
865-
}}
866-
className="absolute right-1 top-1/2 z-10 flex -translate-y-1/2 cursor-pointer"
867-
>
868-
<SimpleTooltip
869-
button={
870-
<span
871-
className={cn(
872-
"flex size-6 items-center justify-center rounded border border-border-bright bg-background-hover",
873-
copied
874-
? "text-green-500"
875-
: "text-text-dimmed hover:border-border-bright hover:bg-background-raised hover:text-text-bright"
876-
)}
877-
>
878-
{copied ? (
879-
<ClipboardCheckIcon className="size-3.5" />
880-
) : (
881-
<ClipboardIcon className="size-3.5" />
882-
)}
883-
</span>
884-
}
885-
content={copied ? "Copied!" : "Copy"}
886-
disableHoverableContent
887-
/>
888-
</span>
889-
)}
890+
<SimpleTooltip
891+
asChild
892+
tabbable
893+
button={copyButton}
894+
content={copied ? "Copied!" : "Copy"}
895+
disableHoverableContent
896+
/>
890897
</div>
891898
);
892899
}

‎apps/webapp/app/components/dashboard-agent/tooltip-accessible-name.test.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ const NO_AS_CHILD_BASELINE = new Set([
6060
"app/components/GitMetadata.tsx::LinkButton",
6161
"app/components/code/TSQLResultsTable.tsx::TextLink",
6262
"app/components/integrations/VercelLink.tsx::LinkButton",
63-
"app/components/primitives/CopyButton.tsx::Button",
6463
"app/components/runs/v3/RunTag.tsx::Link",
6564
"app/components/runs/v3/TaskRunsTable.tsx::DialogTrigger",
6665
"app/routes/account.tokens/route.tsx::DialogTrigger",
@@ -90,6 +89,16 @@ function attrOf(node: JsxNode, name: string) {
9089
return open.attributes.properties.find((p) => ts.isJsxAttribute(p) && p.name.getText() === name);
9190
}
9291

92+
function hasStaticTrueAttribute(node: JsxNode, name: string): boolean {
93+
const attribute = attrOf(node, name);
94+
if (!attribute || !ts.isJsxAttribute(attribute)) return false;
95+
if (!attribute.initializer) return true;
96+
return (
97+
ts.isJsxExpression(attribute.initializer) &&
98+
attribute.initializer.expression?.kind === ts.SyntaxKind.TrueKeyword
99+
);
100+
}
101+
93102
/** Text anywhere under the element, ignoring an expression that can render nothing. */
94103
function hasText(node: TsNode): boolean {
95104
if (!ts.isJsxElement(node)) return false;
@@ -179,7 +188,7 @@ function scanFile(file: string, relative: string): Violation[] {
179188
const initializer =
180189
buttonAttr && ts.isJsxAttribute(buttonAttr) ? buttonAttr.initializer : undefined;
181190
if (initializer && ts.isJsxExpression(initializer)) {
182-
const asChild = !!attrOf(node, "asChild");
191+
const asChild = hasStaticTrueAttribute(node, "asChild");
183192
for (const trigger of resolve(initializer.expression).flatMap(triggersIn)) {
184193
const named =
185194
!!attrOf(trigger, "aria-label") ||

‎apps/webapp/app/components/primitives/ClipboardField.tsx‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useEffect, useRef, useState } from "react";
1+
import { useEffect, useId, useState } from "react";
22
import { cn } from "~/utils/cn";
33
import { CopyButton } from "./CopyButton";
44

@@ -116,7 +116,7 @@ export function ClipboardField({
116116
fullWidth = true,
117117
}: ClipboardFieldProps) {
118118
const [isSecure, setIsSecure] = useState(secure !== undefined && secure);
119-
const inputIcon = useRef<HTMLInputElement>(null);
119+
const inputId = useId();
120120
const { container, input, buttonVariant, button, size } = variants[variant];
121121

122122
useEffect(() => {
@@ -128,16 +128,13 @@ export function ClipboardField({
128128
return (
129129
<span className={cn(container, fullWidth ? "w-full" : "max-w-fit", className)}>
130130
{icon && (
131-
<span
132-
onClick={() => inputIcon.current && inputIcon.current.focus()}
133-
className="flex items-center pl-1"
134-
>
131+
<label htmlFor={inputId} className="flex items-center pl-1">
135132
{icon}
136-
</span>
133+
</label>
137134
)}
138135
<input
136+
id={inputId}
139137
type="text"
140-
ref={inputIcon}
141138
value={isSecure ? maskedValue : value}
142139
readOnly={true}
143140
className={cn(

‎apps/webapp/app/components/primitives/CopyButton.tsx‎

Lines changed: 56 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -44,58 +44,69 @@ export function CopyButton({
4444

4545
const { icon: iconSize, button: buttonSize } = sizes[size];
4646

47-
const button =
48-
variant === "icon" ? (
49-
<span
50-
onClick={copy}
51-
className={cn(
52-
buttonSize,
53-
"flex shrink-0 items-center justify-center rounded border border-border-bright bg-background-hover",
54-
copied
55-
? "text-green-500"
56-
: "text-text-dimmed hover:border-border-bright hover:bg-background-raised hover:text-text-bright",
57-
buttonClassName
58-
)}
59-
>
60-
{copied ? (
61-
<ClipboardCheckIcon className={iconSize} />
62-
) : (
63-
<ClipboardIcon className={iconSize} />
64-
)}
47+
if (variant === "button") {
48+
return (
49+
<span className={className}>
50+
<Button
51+
variant={`${buttonVariant}/${size === "extra-small" ? "small" : size}`}
52+
onClick={copy}
53+
className={cn("shrink-0", buttonClassName)}
54+
tooltip={showTooltip ? (copied ? "Copied!" : "Copy") : undefined}
55+
aria-label={children ? undefined : copied ? "Copied" : "Copy"}
56+
LeadingIcon={
57+
copied ? (
58+
<ClipboardCheckIcon
59+
className={cn(
60+
iconSize,
61+
buttonVariant === "primary" ? "text-background-dimmed" : "text-green-500"
62+
)}
63+
/>
64+
) : (
65+
<ClipboardIcon
66+
className={cn(
67+
iconSize,
68+
buttonVariant === "primary" ? "text-background-dimmed" : "text-text-dimmed"
69+
)}
70+
/>
71+
)
72+
}
73+
>
74+
{children}
75+
</Button>
6576
</span>
66-
) : (
67-
<Button
68-
variant={`${buttonVariant}/${size === "extra-small" ? "small" : size}`}
69-
onClick={copy}
70-
className={cn("shrink-0", buttonClassName)}
71-
LeadingIcon={
72-
copied ? (
73-
<ClipboardCheckIcon
74-
className={cn(
75-
iconSize,
76-
buttonVariant === "primary" ? "text-background-dimmed" : "text-green-500"
77-
)}
78-
/>
79-
) : (
80-
<ClipboardIcon
81-
className={cn(
82-
iconSize,
83-
buttonVariant === "primary" ? "text-background-dimmed" : "text-text-dimmed"
84-
)}
85-
/>
86-
)
87-
}
88-
>
89-
{children}
90-
</Button>
9177
);
78+
}
9279

93-
if (!showTooltip) return <span className={className}>{button}</span>;
80+
const iconButton = (
81+
<button
82+
type="button"
83+
aria-label={copied ? "Copied" : "Copy"}
84+
onClick={copy}
85+
className={cn(
86+
buttonSize,
87+
"flex shrink-0 items-center justify-center rounded border border-border-bright bg-background-hover",
88+
copied
89+
? "text-green-500"
90+
: "text-text-dimmed hover:border-border-bright hover:bg-background-raised hover:text-text-bright",
91+
buttonClassName
92+
)}
93+
>
94+
{copied ? (
95+
<ClipboardCheckIcon className={iconSize} />
96+
) : (
97+
<ClipboardIcon className={iconSize} />
98+
)}
99+
</button>
100+
);
101+
102+
if (!showTooltip) return <span className={className}>{iconButton}</span>;
94103

95104
return (
96105
<span className={className}>
97106
<SimpleTooltip
98-
button={button}
107+
asChild
108+
tabbable
109+
button={iconButton}
99110
content={copied ? "Copied!" : "Copy"}
100111
className="font-sans"
101112
disableHoverableContent

‎apps/webapp/app/components/primitives/CopyableText.tsx‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,11 @@ export function CopyableText({
3939

4040
if (resolvedVariant === "icon-right") {
4141
const iconButton = (
42-
<span
42+
<button
43+
type="button"
44+
aria-label={copied ? "Copied" : "Copy"}
45+
onClick={copy}
46+
onMouseDown={(e) => e.stopPropagation()}
4347
className={cn(
4448
"ml-1 flex size-6 items-center justify-center rounded border border-border-bright bg-background-hover",
4549
asChild && "p-1",
@@ -53,7 +57,7 @@ export function CopyableText({
5357
) : (
5458
<ClipboardIcon className="size-3.5" />
5559
)}
56-
</span>
60+
</button>
5761
);
5862

5963
return (
@@ -72,24 +76,23 @@ export function CopyableText({
7276
{value}
7377
</span>
7478
<span
75-
onClick={copy}
76-
onMouseDown={(e) => e.stopPropagation()}
7779
className={cn(
78-
"absolute top-0 z-10 size-6 font-sans",
80+
"absolute top-0 z-10 flex size-6 font-sans transition-opacity has-focus-visible:pointer-events-auto has-focus-visible:opacity-100",
7981
// Truncated values reserve a right gutter, so the button sits inside it
8082
truncate ? "right-0" : "-right-6",
81-
isHovered ? "flex" : "hidden"
83+
isHovered ? "opacity-100" : "pointer-events-none opacity-0"
8284
)}
8385
>
8486
{hideTooltip ? (
8587
iconButton
8688
) : (
8789
<SimpleTooltip
90+
asChild
91+
tabbable
8892
button={iconButton}
8993
content={copied ? "Copied!" : "Copy"}
9094
className="font-sans"
9195
disableHoverableContent
92-
asChild={asChild}
9396
/>
9497
)}
9598
</span>

0 commit comments

Comments
 (0)