fix(web): use --surface-hover token for menu and listbox elements#31273
Conversation
Replace --ghost-hover with --surface-hover on the skills filter dropdown trigger and listbox option rows. Per semantic token conventions, --ghost-hover is for ghost Button hover states only; menus and list rows should use --surface-hover. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| aria-haspopup="listbox" | ||
| aria-expanded={open} | ||
| className="inline-flex w-40 items-center justify-between gap-2 rounded-lg border bg-[var(--surface-active)] px-3 py-2 text-body-medium-lighter transition-colors hover:bg-[var(--ghost-hover)]" | ||
| className="inline-flex w-40 items-center justify-between gap-2 rounded-lg border bg-[var(--surface-active)] px-3 py-2 text-body-medium-lighter transition-colors hover:bg-[var(--surface-hover)]" |
There was a problem hiding this comment.
🚩 Inconsistent hover token usage within the skills domain after this change
This PR changes --ghost-hover → --surface-hover in FilterDropdown (line 463) and FilterGroup (line 544), but three sibling files in the same skills/ directory still use --ghost-hover:
category-sidebar.tsx:80—hover:bg-[var(--ghost-hover)]skill-row.tsx:46—hover:bg-[var(--ghost-hover)]skill-detail.tsx:184—hover:bg-[var(--ghost-hover)]
These tokens resolve to different values: --ghost-hover is solid (e.g. #F2F0EE in light mode via --surface-active), while --surface-hover is semi-transparent (rgba(185, 180, 172, 0.08)). The visual difference is noticeable. If this is an intentional migration toward --surface-hover, CONVENTIONS.md:905-906 says "When fixing a convention violation, audit the broader codebase for the same violation and fix all instances." If the intent is design-specific (only these two components should change), the inconsistency within the same feature area is worth confirming with the designer.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — audited the three sibling files:
skill-row.tsx:46—<div role="button">card row → changed to--surface-hover✓skill-detail.tsx:184—<button>file tree entry → changed to--surface-hover✓category-sidebar.tsx:80—<Button variant="ghost">→ this IS a ghost Button, so--ghost-hoveris semantically correct here. No change needed.
Pushed the fix.
There was a problem hiding this comment.
✦ APPROVE
Value: Closes the semantic token gap flagged in #31271 — menus and listbox rows now use the correct hover token.
What this does: Swaps --ghost-hover → --surface-hover on two elements in skills-tab.tsx: the filter dropdown trigger and the listbox option rows.
Both changes are correct per our design token conventions. --ghost-hover is for ghost Button states only; --surface-hover is the right token for menus, list rows, and popovers. Two-line diff, nothing else touched.
CI: ✅ Lint/Type/Build, Socket Security · Reviewed at dc81e37f
Vellum Constitution — Inviting: consistent, intentional token usage makes the UI feel coherent rather than assembled.
skill-row.tsx uses a div[role=button] card row and skill-detail.tsx uses a button for file tree entries — neither is a ghost Button, so --surface-hover is the correct token. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
afca175
Prompt / plan
Follow-up from #31271 review: several elements in the skills domain used
--ghost-hover, which is semantically intended for ghostButtonhover states only. Menus, list rows, card rows, and file tree entries should use--surface-hoverper the design token conventions.Four occurrences fixed across three files:
skills-tab.tsx:463— filter dropdown trigger (<button aria-haspopup="listbox">)skills-tab.tsx:544— listbox option rowsskill-row.tsx:46— skill card row (<div role="button">)skill-detail.tsx:184— file tree entry (<button>)One usage intentionally left unchanged:
category-sidebar.tsx:80—<Button variant="ghost">— this IS a ghost Button, so--ghost-hoveris semantically correct.Test plan
bunx tsc --noEmit— passesbun run lint— passesLink to Devin session: https://app.devin.ai/sessions/c4696ace57fe43649f2475d7661ac273
Requested by: @ashleeradka