[tooltip] Simplify RTL styles using CSS logical properties#48351
Conversation
Bundle size
Deploy previewhttps://deploy-preview-48351--material-ui.netlify.app/ Check out the code infra dashboard for more information about this PR. |
…ing" This reverts commit 0a9b9d8.
There was a problem hiding this comment.
Pull request overview
This PR simplifies Tooltip RTL-specific styling by replacing left/right physical CSS properties with CSS logical properties, removing the need to thread isRtl through ownerState and collapsing multiple RTL/LTR variants into fewer shared rules (aiming to reduce complexity and bundle size).
Changes:
- Removed
useRtl()usage fromTooltipand eliminatedisRtlfromownerState. - Replaced direction-dependent
left/right+marginLeft/marginRightarrow/margin rules with logical properties (insetInlineStart/End,marginInlineStart/End). - Simplified style variant conditions (including a refactor of the popper
pointerEventsvariants).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: sai chand <60743144+sai6855@users.noreply.github.com>
mj12albert
left a comment
There was a problem hiding this comment.
Codex review:
Blocker: Tooltip RTL side offsets now depend on DOM dir, not MUI’s RTL context.
The new logical properties in
Tooltip.jsresolve from CSS/DOM direction, but this PR removesuseRtl()from Tooltip.Popperstill reads RTL fromRtlProviderand passes direction for placement logic, butBasePopperconsumes that prop and does not set DOM/CSS direction on the portaled root. In aThemeProviderwithdirection: 'rtl'but no globaldir="rtl", placement is RTL-aware while these arrow/margin rules stay LTR, regressing the previousownerState.isRtlbehavior.
Per https://mui.com/material-ui/customization/right-to-left/#1-set-the-html-direction, consumers must set dir="rtl" on (or on a local wrapper with a configured portal container). With this being said, CSS logical properties in Tooltip resolve correctly on the portaled popper. I verified this visually in all three configurations, LTR, , and ThemeProvider({ direction: 'rtl' }) only—and arrows correctly point at anchors in each case. |
|
Added suggested test coverage for the changes:
|
Collapse 15 style variants down to 5 in
Tooltip.jsby replacing physical CSS properties (marginLeft/marginRight,left/right) with logical properties (marginInlineStart/marginInlineEnd,insetInlineStart/insetInlineEnd), letting the browser handle RTL flipping natively.Goal of this PR is simplify css and reduce bundle size
Why this is better
1. RTL handling moves from JS to the browser. Previously, the component read
useRtl()on every render, storedisRtlonownerState, and every direction-sensitive style had to be authored twice (once for LTR, once for RTL). Logical properties flipautomatically based on
dir/direction, so one rule covers both.2. Fewer style variants → simpler styling pipeline.
TooltipPopper: 7 variants → 2 (the!openvariant was also redundant; merged into the!disableInteractivecondition)TooltipTooltip: 8 variants → 3 (the twotouchtop/bottom variants also merged)3. Less state to thread through the component.
isRtlis gone fromownerState, and theuseRtl()call + import are removed. Onefewer hook call per render, one fewer field to reason about.
4. Aligned with the rest of the codebase's direction. Other MUI components (Popper, Drawer internals) already use logical properties
where applicable. This brings Tooltip in line.
I visually don't see any difference in https://deploy-preview-48351--material-ui.netlify.app/material-ui/react-tooltip/ and in https://mui.com/material-ui/react-tooltip/ in rtl version