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
[@mantine/core] always try centering arrow on target for popover #3065
[@mantine/core] always try centering arrow on target for popover #3065
Conversation
d15e5bd
to
a80de1f
Compare
import { getArrowPositionStyles } from './get-arrow-position-styles'; | ||
import { FloatingPosition } from '../types'; | ||
|
||
interface FloatingArrowProps extends React.ComponentPropsWithoutRef<'div'> { | ||
withBorder: boolean; | ||
position: FloatingPosition; | ||
arrowSize: number; | ||
arrowOffset: number; |
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.
We don't need the arrow offset anymore in this component, as the offset is now used as a configuration for the arrow
middleware of floating UI directly.
}, | ||
ref | ||
) => { | ||
const theme = useMantineTheme(); |
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.
We don't need the theme at all anymore since the RTL direction is already handled by Floating UI
} | ||
|
||
return {}; | ||
} |
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.
Those two functions are not needed anymore, as position is correctly handled by Floating UI, including RTL. So we can directly set left
for vertical and top
for horizontal sides
top: arrowPosition, | ||
borderBottom: 0, | ||
[dir === 'ltr' ? 'borderRight' : 'borderLeft']: 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.
This check was actually messing things up for RTL, simply removing it fixes things
Looks good, thanks! |
This PR tries to fix some outstanding issues introduced with the switch to Floating UI:
-end
or `-start``-end
and-start
placement now only impact the dropdown, not the arrow. The arrow will always try to align itself on the center of the target.arrowOffset
now acts as a min distance from dropdown border, rather than a fixed position in-end
and-start
placement.