Fix dead dropdown menu actions after base-ui migration (Delete chat, Pin, Sign out)#186
Merged
Merged
Conversation
…Pin, Sign out) The base-ui migration (d82421b) moved dropdown menus onto base-ui's Menu.Item, which fires onClick and has no onSelect prop. Three menu items were left using Radix's onSelect, so base-ui silently ignored them and the handlers never ran — most visibly, "Delete chat" did nothing. Switch the affected items to onClick: - nav-threads.tsx: Delete chat and Pin chat - nav-user.tsx: Sign out The handlers ignore the event argument, so the swap is type-safe, and base-ui's closeOnClick defaults to true so the menu still closes. Verified: eslint (--max-warnings=0), prettier, typecheck, and 157 unit tests pass. Live browser test: clicking Delete chat now sends DELETE /api/threads (204), removes the thread from the sidebar, and deletes the row from Postgres. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Restores the dropdown menu actions that silently stopped working after the base-ui migration: Delete chat, Pin chat, and Sign out.
Why
Commit d82421b ("Consolidate UI primitives onto base-ui (drop radix-ui)") moved dropdown menus onto base-ui's
Menu.Item. base-ui menu items fireonClickand have noonSelectprop —DropdownMenuItemspreads props straight through, so any leftoveronSelect={...}is silently ignored and the handler never runs.Three menu items were missed in the migration and kept Radix's
onSelect:src/components/nav-threads.tsx— Delete chat (the reported bug) and Pin chatsrc/components/nav-user.tsx— Sign outSo clicking "Delete chat" did nothing.
Change
Swap
onSelect→onClickon the three affected items. The handlers ignore the event argument, so the swap is type-safe, and base-ui'scloseOnClickdefaults totrueso the menu still closes after the action.Verification
eslint --max-warnings=0,prettier --check,pnpm typecheck— all passpnpm test— 157/157 passDELETE /api/threads → 204, removes the thread from the sidebar, and deletes the row from Postgres. Other threads are untouched.Reviewer notes
onSelecton base-ui menu items in future migrations — it fails silently with no type error.🤖 Generated with Claude Code