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
[material-ui][SpeedDial] Fix SpeedDial tooltip placement #41997
base: next
Are you sure you want to change the base?
Conversation
Netlify deploy previewhttps://deploy-preview-41997--material-ui.netlify.app/ packages/material-ui/material-ui.production.min.js: parsed: -0.05% 😍, gzip: -0.05% 😍 Bundle size reportDetails of bundle changes (Toolpad) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is already being addressed in #41414. Additionally, the modification you made constitutes a breaking change. The tooltip should remain white, as it was before.
@ZeeshanTamboli Hello! Thanks for the comment.
I use tooltipPlacement prop in tooltip component. |
@ZeeshanTamboli Thanks! |
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.
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. |
@ZeeshanTamboli It works now. Could you please check again?
I got it. Thank you for letting me know. |
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. |
@ZeeshanTamboli 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? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jjisabi There are a lot of file changes related to pigment-css
. Could you remove them?
5241dca
to
5609871
Compare
@ZeeshanTamboli sorry, my bad. I removed them all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jjisabi Thanks. Can you address the failing CI? Also, Argos CI flagged some unexpected changes, which needs attention.
with '#' will be ignored, and an empty message aborts the cmmit. rebase SpeedDialAction.js
rebase2
0880ac7
to
a5a484d
Compare
onClose={handleTooltipClose} | ||
onOpen={handleTooltipOpen} | ||
open={open && tooltipOpenProp} | ||
classes={TooltipClasses} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
classes={TooltipClasses} | |
classes={tooltipClasses} |
Fixes #41067
Closes #41414 (old inactive PR)
Before:
After:
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.