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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Tooltip] Fix broken arrow position in rtl #27868
Conversation
@@ -63,32 +63,38 @@ const TooltipPopper = styled(Popper, { | |||
...(ownerState.arrow && { | |||
[`&[data-popper-placement*="bottom"] .${tooltipClasses.arrow}`]: { | |||
top: 0, | |||
left: 0, |
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.
These are already defined in the style
attribute, no need to specify them.
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.
Looks great. I guess the change of behavior between v4 and v5 is linked to the flip option that we do no longer have available: https://github.com/mui-org/material-ui/blob/120c564109245a3b070b1cbaf2c9f3f8659fd9fa/packages/material-ui/src/Tooltip/Tooltip.js#L652
ah I didn鈥檛 noticed it, but yes, looks like it comes from it. We missed to catch it during the migration. |
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.
Looks great. I guess the change of behavior between v4 and v5 is linked to the flip option that we do no longer have available:
I guess we need to go through every component that had this or rather check every usage of flip: false
from master
against the behavior of next
.
Yes will do this today |
Fixes #27858 The fix was to prevent some styles from flipping in rtl. As
/* @noflip */
is working only with string templates, I've added theisRtl
prop and added the appropriate styles based on it.Isolated codesandbox examples
Codesandbox in RTL on master:
https://codesandbox.io/s/material-demo-forked-qh2ds?file=/demo.js
Codesandbox in RTL on next:
https://codesandbox.io/s/basictooltip-material-demo-forked-5gc67?file=/demo.js
Codesandbox using the packages in the PR:
https://codesandbox.io/s/basictooltip-material-demo-forked-44bhg?file=/demo.js
-----
Docs demos:
Next docs 馃槩 :
Current PR docs: