Skip to content

feat(dashboard): simplify actors list header and compact rows - #5696

Open
NicholasKissel wants to merge 1 commit into
mainfrom
frontend/compact-actors-list
Open

feat(dashboard): simplify actors list header and compact rows#5696
NicholasKissel wants to merge 1 commit into
mainfrom
frontend/compact-actors-list

Conversation

@NicholasKissel

Copy link
Copy Markdown
Member
  • Replace the bordered Display, search, and create buttons in the actor list header with plain icons.
  • Add a single tooltip shared by the header icons that follows the cursor and glides between icons, preferring the right side of the cursor and falling back to the left or below when it would overflow.
  • Show the search shortcut as a kbd pill in the tooltip and register it with useHotkey.
  • Lighten the created time in actor rows to match the rest of the row text.
  • Reduce header bar height from 45px to 36px across the actor list and detail tabs, and shrink rows and the column header.
  • Remove the orange accent bar on the selected actor row.

@railway-app

railway-app Bot commented Sep 10, 2026

Copy link
Copy Markdown

🚅 Deployed to the actors-pr-5696 environment in rivet-frontend

Service Status Web Updated
kitchen-sink 😴 Sleeping (View Logs) Web Sep 12, 2026 at 3:46 am UTC
frontend-inspector 😴 Sleeping (View Logs) Web Sep 11, 2026 at 2:07 pm UTC
frontend-cloud 😴 Sleeping (View Logs) Web Sep 11, 2026 at 5:27 am UTC
ladle ✅ Success (View Logs) Web Sep 10, 2026 at 8:17 pm UTC
mcp-hub ✅ Success (View Logs) Web Sep 10, 2026 at 8:15 pm UTC
website ❌ Build Failed (View Logs) Web Sep 10, 2026 at 8:14 pm UTC

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 1 low-severity finding

Reviewed commit dc9dc88.

window.addEventListener("keydown", handler);
return () => window.removeEventListener("keydown", handler);
}, []);
useHotkey("Mod+K", () => setOpen(true));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Low · Search shortcut interrupts text editing

useHotkey defaults ignoreInputs to false for Mod shortcuts. As a result, pressing ⌘K/Ctrl+K while a filter field or the actor lookup field has focus opens this dialog and prevents the platform editing shortcut (for example, delete-to-end-of-line on macOS).

Pass { ignoreInputs: true } so the shortcut remains global outside editable controls.

@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Review

Nice cleanup overall — the placement math for the cursor tooltip is well isolated and the test coverage on cursor-tooltip-placement.ts is solid (boundary cases for right/left/bottom fallback and clamping are all exercised).

Bug: Cmd/Ctrl+K no longer ignores editable targets

frontend/src/components/actors/actors-list.tsx:169

useHotkey("Mod+K", () => setOpen(true));

The hand-rolled listener this replaces explicitly bailed when the event target was content-editable:

if (target?.isContentEditable) return;

The new useHotkey call drops that guard and doesn't pass ignoreInputs: true. Per frontend/CLAUDE.md and the existing convention in feedback-button.tsx (useHotkey("F", ..., { ignoreInputs: true })), ignoreInputs is exactly what restores this. As written, pressing Cmd/Ctrl+K while typing in the actor state editor (actor-editable-state.tsx uses CodeMirror, which is content-editable) or any other input on the page will now pop open the instance search dialog and steal focus, interrupting the user's typing. This looks like an unintentional regression from the refactor rather than a deliberate behavior change — worth adding { ignoreInputs: true } back.

Minor: tooltip content isn't associated with its trigger for assistive tech

frontend/src/components/ui/cursor-tooltip.tsx

The new CursorTooltipTrigger replaces Radix's Tooltip.Trigger/Tooltip.Content pair, which wires aria-describedby automatically. This custom implementation shows/hides the tooltip visually on focus/blur but never associates the portal content with the trigger via aria-describedby/aria-labelledby. Impact is limited here since every call site already sets an equivalent aria-label on the underlying button (Search, Display options, Expand details column, etc.), so the essential label still reaches screen readers — but the keyboard-shortcut hint (⌘K) rendered only inside the tooltip is not announced. Not blocking, but worth a follow-up if this component is reused for triggers that don't already have a redundant aria-label.

Nit: no story for the new interactive primitive

frontend/CLAUDE.md asks for a Ladle story on new UI components when it "would teach something a reader can't see from the source." CursorTooltipGroup/CursorTooltipTrigger introduce a genuinely new interaction (spring-animated, cursor-following tooltip that glides between triggers and falls back to left/below near viewport edges) that's hard to verify from source alone and doesn't require mocking routes/auth/data providers. A small story exercising the three placement fallbacks would make this easy to sanity-check in isolation and via visual regression later.

Things that look correct

  • Row/header height math is consistent throughout (h-9 bar → top-9 sticky offset for the list header, h-8 rows → estimateSize: () => 32 in the virtualizer).
  • Removing the SmallText wrapper for the row timestamp and setting text-xs font-normal directly fixes a real mismatch: SmallText's base text-sm font-medium wasn't previously overridden, so the timestamp rendered larger/bolder than the rest of the text-xs row — this change is a genuine fix, not just cosmetic churn.
  • CreateActorButton's new renderTooltip escape hatch is only consumed by the one new call site in actors-list.tsx; behavior for existing (non-icon-only) callers is unchanged (tooltip === null short-circuits to the old return content path).
  • Dropped the window-level keydown listener in favor of useHotkey, per this repo's convention — good direction, just needs the ignoreInputs option restored.

I wasn't able to run the frontend dev server in this environment to visually verify the compact header/rows across OSS and cloud flavors as frontend/CLAUDE.md recommends for visual changes — worth a manual pass before merging if that hasn't been done already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant