fix(ui): forward button refs#892
Conversation
Allow the shared Button component to receive forwarded refs while preserving its polymorphic as prop API. Co-authored-by: Cursor <cursoragent@cursor.com>
👷 Deploy request for tanstack pending review.Visit the deploys page to approve it
|
📝 WalkthroughWalkthroughThe Button component is refactored to support React.forwardRef. An internal ButtonInner function is introduced to accept a ref parameter, the Button export wraps it with React.forwardRef, and the children prop is made optional in ButtonOwnProps. Type definitions are updated to include the ButtonRef type and a ref-compatible ButtonComponent signature. ChangesButton Ref Forwarding
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/ui/Button.tsx (1)
150-152: 💤 Low valueOptional: set an explicit
displayNameon the forwarded component.After wrapping with
React.forwardRef, React DevTools will show the component asForwardRef(ButtonInner)instead ofButton. Setting an explicitdisplayNameimproves the debugging experience without affecting runtime behavior.♻️ Proposed addition
export const Button = React.forwardRef( ButtonInner as React.ForwardRefRenderFunction<unknown, ButtonProps>, ) as unknown as ButtonComponent + +;(Button as unknown as { displayName?: string }).displayName = 'Button'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/Button.tsx` around lines 150 - 152, The exported forwarded component currently uses React.forwardRef with ButtonInner and thus will appear as ForwardRef(ButtonInner) in DevTools; set an explicit displayName on the resulting Button component (e.g., Button.displayName = 'Button') immediately after the forwardRef assignment so React DevTools shows the desired name; reference the exported symbol Button and the inner component ButtonInner when applying this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/ui/Button.tsx`:
- Around line 150-152: The exported forwarded component currently uses
React.forwardRef with ButtonInner and thus will appear as
ForwardRef(ButtonInner) in DevTools; set an explicit displayName on the
resulting Button component (e.g., Button.displayName = 'Button') immediately
after the forwardRef assignment so React DevTools shows the desired name;
reference the exported symbol Button and the inner component ButtonInner when
applying this change.
Summary
Fixes the
CopyPageDropdownmenu not showing its items by addingReact.forwardRefsupport to the sharedButtoncomponent.CopyPageDropdownuses the Button through dropdown trigger composition, which requires refs to be forwarded correctly for the dropdown primitives to anchor and render the menu items.Changes
React.forwardRefchildrenoptional to support icon-only/ref-driven button usageTest plan
pnpm testpnpm buildSummary by CodeRabbit
New Features
Changes