Skip to content

Commit 5c7e8c7

Browse files
samejrclaude
andcommitted
fix(webapp): move the column row focus ring to its hit area
- Drop the drag handle's hover fill; it's a handle, not an action. - Tighten the gap between a smart column's name and its bolt (8px to 4px) without changing the checkbox gap, via a nested group. - Clicking a row rang the checkbox because its ring keys off :focus, which a mouse press matches, not :focus-visible. The checkbox no longer rings at all. - Instead the label rings, since that's the actual hit area, and only on keyboard focus (has-[:focus-visible], 1px inset in the link colour). - The form column scrolls, and overflow-y-auto clips at the content box, which cut the inputs' focus ring on both sides. It now has 4px of padding with a matching negative margin, so the ring has room and the content stays on the grid (column titles still align, visible gap unchanged). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7d21b92 commit 5c7e8c7

2 files changed

Lines changed: 22 additions & 10 deletions

File tree

apps/webapp/app/components/runs/v3/AddSmartColumnDialog.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,8 @@ export function AddSmartColumnDialog({
188188
</Paragraph>
189189

190190
<div className="grid min-h-0 grid-cols-1 items-stretch gap-2.5 md:grid-cols-3">
191-
<div className="flex flex-col gap-4 overflow-y-auto scrollbar-thin scrollbar-track-transparent scrollbar-thumb-surface-control">
191+
{/* p-1/-m-1: overflow-y-auto clips at the content box, which cut the inputs' focus ring. */}
192+
<div className="-m-1 flex flex-col gap-4 overflow-y-auto p-1 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-surface-control">
192193
<InputGroup fullWidth>
193194
<div className={TITLE_ROW_CLASS}>
194195
<Label>Source</Label>

apps/webapp/app/components/runs/v3/RunsDisplayOptions.tsx

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,9 @@ export function RunsDisplayOptions({
287287
);
288288
}
289289

290+
/** The label owns the focus ring (see ColumnRow), so the checkbox itself never rings. */
291+
const CHECKBOX_NO_RING = "focus:ring-0 group-focus:ring-0 focus-visible:ring-0";
292+
290293
/**
291294
* The row's hover-revealed actions. Square, and hidden until the row is hovered or the
292295
* control itself takes keyboard focus (a checkbox click must not reveal them).
@@ -347,23 +350,28 @@ function ColumnRow({
347350
>
348351
{isOver && <div className="absolute inset-x-0 top-0 h-0.5 bg-indigo-500" />}
349352
{/* Native label so the whole name area toggles the column, matching CheckboxWithLabel. */}
353+
{/* The label is the hit area, so it carries the focus ring rather than the checkbox
354+
inside it, and only for keyboard focus -- a click must not ring anything. */}
350355
<label
351356
className={cn(
352-
"flex h-full min-w-0 flex-1 items-center gap-x-2 pl-2",
357+
"flex h-full min-w-0 flex-1 items-center gap-x-2 rounded-sm pl-2",
358+
"has-[:focus-visible]:outline has-[:focus-visible]:outline-1 has-[:focus-visible]:-outline-offset-1 has-[:focus-visible]:outline-text-link",
353359
locked ? "cursor-default" : "cursor-pointer"
354360
)}
355361
>
356362
{locked ? (
357-
<Checkbox checked disabled />
363+
<Checkbox checked disabled className={CHECKBOX_NO_RING} />
358364
) : (
359-
<Checkbox checked={checked} onChange={onToggle} />
365+
<Checkbox checked={checked} onChange={onToggle} className={CHECKBOX_NO_RING} />
360366
)}
361-
<span
362-
className={cn("truncate text-2sm", checked ? "text-text-bright" : "text-text-dimmed")}
363-
>
364-
{col.def.label}
367+
<span className="flex min-w-0 items-center gap-x-1">
368+
<span
369+
className={cn("truncate text-2sm", checked ? "text-text-bright" : "text-text-dimmed")}
370+
>
371+
{col.def.label}
372+
</span>
373+
{isSmart && <SmartColumnIcon className="size-3.5 flex-none text-text-dimmed" />}
365374
</span>
366-
{isSmart && <SmartColumnIcon className="size-3.5 flex-none text-text-dimmed" />}
367375
</label>
368376
<div className="flex flex-none items-center gap-0.5 pr-1">
369377
{onRemove && (
@@ -402,7 +410,10 @@ function ColumnRow({
402410
variant="minimal/small"
403411
aria-label={`Reorder ${col.def.label} (use arrow up and down)`}
404412
LeadingIcon={<GripVerticalIcon className="size-4" />}
405-
className={cn(ROW_ACTION_CLASS, "cursor-grab active:cursor-grabbing")}
413+
className={cn(
414+
ROW_ACTION_CLASS,
415+
"cursor-grab group-hover/button:bg-transparent active:cursor-grabbing"
416+
)}
406417
/>
407418
</span>
408419
</div>

0 commit comments

Comments
 (0)