Skip to content

[Tooltip] Migrate to emotion#24571

Merged
oliviertassinari merged 13 commits intomui:nextfrom
queengooborg:migrate/Tooltip
Feb 26, 2021
Merged

[Tooltip] Migrate to emotion#24571
oliviertassinari merged 13 commits intomui:nextfrom
queengooborg:migrate/Tooltip

Conversation

@queengooborg
Copy link
Contributor

This PR migrates the Tooltip component to the new emotion format as a part of #24405.

@oliviertassinari oliviertassinari added the scope: tooltip Changes related to the tooltip. label Jan 23, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Jan 23, 2021

@material-ui/core: parsed: +0.26% , gzip: +0.21%
@material-ui/lab: parsed: +0.29% , gzip: +0.24%

Details of bundle changes

Generated by 🚫 dangerJS against 949065e

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

See the comment regarding the overridesResolver. Let me know if you are willing to take a look on this or if you need more help

const overridesResolver = (props, styles) => {
const { styleProps } = props;

return deepmerge(styles.popper || {}, {
Copy link
Member

Choose a reason for hiding this comment

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

As this component does not have a Root I would recommend creating two separate components TooltipPopper - you already have this one, and one wrapper for the children that will have the classes related to the tooltip. With that we will create two overridesResolver functions, once for each. At this moment I don't have better idea for solving this component. Cc @oliviertassinari

PS: The overrides resolver just spreads these styles on the root element, which in this case does not exists, that's why we need two separate overrides resolvers for this component.

Copy link
Member

@oliviertassinari oliviertassinari Jan 26, 2021

Choose a reason for hiding this comment

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

and one wrapper for the children

I don't follow the use case. From what I understand, we don't need to apply styles to the children. The component has no root.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Feb 8, 2021
@oliviertassinari oliviertassinari self-assigned this Feb 23, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Feb 25, 2021
@oliviertassinari oliviertassinari dismissed mnajdova’s stale review February 25, 2021 17:11

fix build and regressions

@oliviertassinari
Copy link
Member

@vinyldarkscratch Thanks!

@queengooborg queengooborg deleted the migrate/Tooltip branch February 26, 2021 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: tooltip Changes related to the tooltip.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants