Skip to content

[Popover, Tooltip] Fix PopoverTrigger and TooltipTrigger prop types#1209

Merged
mnajdova merged 3 commits into
mui:masterfrom
okmr-d:fix/popover-trigger-props-type
Dec 23, 2024
Merged

[Popover, Tooltip] Fix PopoverTrigger and TooltipTrigger prop types#1209
mnajdova merged 3 commits into
mui:masterfrom
okmr-d:fix/popover-trigger-props-type

Conversation

@okmr-d

@okmr-d okmr-d commented Dec 20, 2024

Copy link
Copy Markdown
Contributor

Fix wrong prop types.
スクリーンショット 2024-12-21 5 09 47

@mui-bot

mui-bot commented Dec 20, 2024

Copy link
Copy Markdown

Netlify deploy preview

https://deploy-preview-1209--base-ui.netlify.app/

Generated by 🚫 dangerJS against a3e9ab1

}

export interface Props extends BaseUIComponentProps<any, State> {}
export interface Props extends BaseUIComponentProps<'button', State> {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tooltips can go on any type of element, not just buttons. It should at least have the props: GenericHTMLProps type instead of props: any though

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't know how to fix it cleanly, I have reverted TooltipTrigger fix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for pointing this out. I've made an extra PR here: #1213

@atomiks atomiks mentioned this pull request Dec 23, 2024
1 task
@mnajdova

Copy link
Copy Markdown
Member

It's a great first pull request on Base UI @okmr-d! Thank you for working on it 👌

@mnajdova mnajdova merged commit a9466a3 into mui:master Dec 23, 2024
@zannager zannager added component: tooltip Changes related to the tooltip component. component: popover Changes related to the popover component. labels Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: popover Changes related to the popover component. component: tooltip Changes related to the tooltip component.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants