Skip to content

Commit aa81fda

Browse files
samejrclaude
andcommitted
feat(webapp): clearer disabled Reset, keyboard focus, new Columns icon
The disabled "Reset to default" differed from its enabled state by exactly one thing: opacity 0.75 on the inner span. Text and icon colours were identical and the background never changed (hover can't fire under pointer-events-none), so the only cue was a 25% fade on already-dim text. It now carries three cues: opacity drops to 50%, the label steps from text-bright down to text-dimmed so the label/icon hierarchy flattens, and the cursor reads not-allowed. A tooltip explains the condition ("Columns are already at their default") rather than leaving a dead control with no reason. Both levers are theme-safe: verified near-black-on-white vs mid-grey-at-50% in light, bright vs dimmed in dark. Opening the popover with the shortcut now lets Radix focus the first row so the list can be tabbed immediately; mouse opens still keep focus put, since moving it would reveal that row's hover-only reorder handle. The popover is controlled so the two paths can be told apart. Also updates ColumnsIcon to the wider 18x16 body with lines at x=9 and x=15. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0c37f67 commit aa81fda

2 files changed

Lines changed: 42 additions & 16 deletions

File tree

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
export function ColumnsIcon({ className }: { className?: string }) {
22
return (
33
<svg className={className} viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
4-
<rect x="4" y="5" width="16" height="14" rx="2" stroke="currentColor" strokeWidth="2" />
5-
<line x1="9.33334" y1="19" x2="9.33333" y2="5" stroke="currentColor" strokeWidth="2" />
6-
<line x1="14.6667" y1="19" x2="14.6667" y2="5" stroke="currentColor" strokeWidth="2" />
4+
<rect x="3" y="4" width="18" height="16" rx="2" stroke="currentColor" strokeWidth="2" />
5+
<line x1="9" y1="19" x2="9" y2="5" stroke="currentColor" strokeWidth="2" />
6+
<line x1="15" y1="19" x2="15" y2="5" stroke="currentColor" strokeWidth="2" />
77
</svg>
88
);
99
}

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

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,17 @@ export function RunsDisplayOptions({
6262
const [editing, setEditing] = useState<SmartEditTarget | null>(null);
6363
const [dragKey, setDragKey] = useState<string | null>(null);
6464
const [overKey, setOverKey] = useState<string | null>(null);
65-
const triggerRef = useRef<HTMLButtonElement>(null);
65+
const [open, setOpen] = useState(false);
66+
// Whether this open came from the shortcut, which decides if focus moves into the list.
67+
const openedByShortcut = useRef(false);
6668

6769
useShortcutKeys({
6870
shortcut: COLUMNS_SHORTCUT,
6971
action: (event) => {
7072
event.preventDefault();
7173
event.stopPropagation();
72-
triggerRef.current?.click();
74+
openedByShortcut.current = true;
75+
setOpen((previous) => !previous);
7376
},
7477
});
7578

@@ -156,7 +159,13 @@ export function RunsDisplayOptions({
156159

157160
return (
158161
<>
159-
<Popover>
162+
<Popover
163+
open={open}
164+
onOpenChange={(next) => {
165+
setOpen(next);
166+
if (!next) openedByShortcut.current = false;
167+
}}
168+
>
160169
<SimpleTooltip
161170
asChild
162171
side="bottom"
@@ -166,7 +175,7 @@ export function RunsDisplayOptions({
166175
// so the tooltip anchor can't be the Button itself (same as NotificationPanel).
167176
<div className="flex">
168177
<PopoverTrigger asChild>
169-
<Button ref={triggerRef} variant="secondary/small" LeadingIcon={ColumnsIcon}>
178+
<Button variant="secondary/small" LeadingIcon={ColumnsIcon}>
170179
Columns
171180
</Button>
172181
</PopoverTrigger>
@@ -182,9 +191,13 @@ export function RunsDisplayOptions({
182191
<PopoverContent
183192
align="end"
184193
className="w-64 p-0"
185-
// Radix otherwise focuses the first item on open, and the row's hover-revealed
186-
// reorder handle would show through :focus-within before the mouse ever gets there.
187-
onOpenAutoFocus={(event) => event.preventDefault()}
194+
// Opened by shortcut: let Radix focus the first row so the list can be tabbed
195+
// straight away. Opened by mouse: keep focus put, or the first row's
196+
// hover-revealed reorder handle would appear before the cursor ever got there.
197+
onOpenAutoFocus={(event) => {
198+
if (!openedByShortcut.current) event.preventDefault();
199+
openedByShortcut.current = false;
200+
}}
188201
>
189202
<div className="flex items-center justify-between px-3 py-2">
190203
<span className="text-xs font-medium text-text-dimmed">Columns</span>
@@ -243,12 +256,25 @@ export function RunsDisplayOptions({
243256
className="h-8"
244257
/>
245258
)}
246-
<PopoverMenuItem
247-
icon={ArrowUturnLeftIcon}
248-
title="Reset to default"
249-
onClick={reset}
250-
disabled={!layout.isCustomized}
251-
className="h-8"
259+
<SimpleTooltip
260+
asChild
261+
side="bottom"
262+
disableHoverableContent
263+
hidden={layout.isCustomized}
264+
button={
265+
// The disabled button has pointer-events-none, so it can host neither the
266+
// not-allowed cursor nor a tooltip; the wrapper carries both.
267+
<div className={cn("flex", !layout.isCustomized && "cursor-not-allowed")}>
268+
<PopoverMenuItem
269+
icon={ArrowUturnLeftIcon}
270+
title="Reset to default"
271+
onClick={reset}
272+
disabled={!layout.isCustomized}
273+
className="h-8 group-disabled/button:opacity-50 group-disabled/button:[&_span]:text-text-dimmed"
274+
/>
275+
</div>
276+
}
277+
content="Columns are already at their default"
252278
/>
253279
</div>
254280
</PopoverContent>

0 commit comments

Comments
 (0)