Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow base button to be wrapped with tooltip #10376

Closed
wants to merge 2 commits into from

Conversation

sbeaury
Copy link

@sbeaury sbeaury commented Nov 23, 2024

Problem

Solves #10223

Solution

Adds React.forwardRefso that MUI's tooltip can wrap RA custom button component

How To Test

This will now work

<Tooltip title="This is a button">
  <Button>My Button</Button>
</Tooltip>

A ButtonWithTooltip.tsx component could be added

@fzaninotto
Copy link
Member

I don't think this solves the linked issue, as other react-admin buttons don't forward the ref.

Also, can you add a story demonstrating this use case?

@sbeaury
Copy link
Author

sbeaury commented Nov 27, 2024

I don't think this solves the linked issue, as other react-admin buttons don't forward the ref.

Also, can you add a story demonstrating this use case?

It does, the issue was specifically for custom buttons (so any custom button deriving from the custom Button like CreateButton, EditButton, etc will inherit from this added forwardRef). Added the example in a story.

@fzaninotto
Copy link
Member

CreateButton and other high-level buttons don't use forwardRef, so even if the lower-level Button does use it, your change doesn't allow a Tooltip wrapping a CreateButton, which is the issue pointed in #10223.

@sbeaury sbeaury closed this Dec 9, 2024
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.

2 participants