mui / material-ui

Material UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
91.73k stars 31.51k forks source link

[material-ui][SpeedDial] Fix SpeedDial tooltip placement #41997

Open jjisabi opened 1 week ago

jjisabi commented 1 week ago

Fixes #41067

Closes #41414 (old inactive PR)

Before: 스크린샷 2024-04-11 034214

After: 스크린샷 2024-04-11 034329

I used the tooltip component instead of SpeedDialActionStaticTooltip. Then fixed the tooltip placement by changing the fab component style (the transform property changes tooltip placement).

Deploy preview: https://deploy-preview-41997--material-ui.netlify.app/material-ui/react-speed-dial/#persistent-action-tooltips.

mui-bot commented 1 week ago

Netlify deploy preview

https://deploy-preview-41997--material-ui.netlify.app/

packages/material-ui/material-ui.production.min.js: parsed: -0.17% :heart_eyes:, gzip: -0.08% :heart_eyes: SpeedDialAction: parsed: -0.78% :heart_eyes:, gzip: -0.38% :heart_eyes: @material-ui/core: parsed: -0.17% :heart_eyes:, gzip: -0.09% :heart_eyes: @material-ui/lab: parsed: -0.34% :heart_eyes:, gzip: -0.13% :heart_eyes:

Bundle size report

Details of bundle changes (Toolpad) Details of bundle changes

Generated by :no_entry_sign: dangerJS against 0880ac709b80447f186849736eadc6952e6630b6

jjisabi commented 1 week ago

@ZeeshanTamboli Hello! Thanks for the comment. Is there any updates in #41414? or the issue has been solved with #41414? I can see it's still open.

Additionally, the modification you made constitutes a breaking change. The tooltip should remain white, as it was before.

I use tooltipPlacement prop in tooltip component. So this is the default color of the tooltip component, but i think i can make it white.

ZeeshanTamboli commented 1 week ago

Is there any updates in https://github.com/mui/material-ui/pull/41414? or the issue has been solved with https://github.com/mui/material-ui/pull/41414? I can see it's still open.

No new updates there. Feel free to address it in this PR.

jjisabi commented 1 week ago

@ZeeshanTamboli Thanks! 스크린샷 2024-04-23 143700 I make the tooltip white. Btw, in speedDial, all other tooltips are black. Is there a reason why only this should be white?

ZeeshanTamboli commented 1 week ago

@ZeeshanTamboli Thanks! 스크린샷 2024-04-23 143700 I make the tooltip white.

Are you sure? I can't see it as white in the docs preview: Link to docs preview when hovering over it. It should match the white tooltip in production: Link to production.

Btw, in speedDial, all other tooltips are black. Is there a reason why only this should be white?

Not entirely sure, but for consistent user experience, persistent tooltips should remain white to avoid any unexpected breaking changes for existing apps using them with the tooltipOpen prop.

jjisabi commented 1 week ago

@ZeeshanTamboli It works now. Could you please check again?

Not entirely sure, but for consistent user experience, persistent tooltips should remain white to avoid any unexpected breaking changes for existing apps using them with the tooltipOpen prop.

I got it. Thank you for letting me know.

ZeeshanTamboli commented 1 week ago

@ZeeshanTamboli It works now. Could you please check again?

Now it's white, but the size is smaller. You can see in the documentation preview I provided in the PR description. It should match the previous appearance exactly. Why not reuse the previous styles? This PR should only address the tooltip's positioning. Additionally, it should maintain the previous animation when opened.

jjisabi commented 1 week ago

@ZeeshanTamboli All styles, except for the animation, match the previous version. Could you please check?

When scale animation(previous animation) maintain, the tooltip finds the anchored element and is placed before the anchored element is placed. So the tooltip can't be placed in the correct position. Because of this issue, I remove the animation.

I tried to find a way to maintain it, but couldn't find it. Do you have any other suggestions or ways for this?

jjisabi commented 4 days ago

@ZeeshanTamboli sorry, my bad. I removed them all.