Skip to content

Commit

Permalink
[@mantine/core] Fix incorrect arrow position for *-end and *-start po…
Browse files Browse the repository at this point in the history
…sitions in Tooltip, Popover, HoverCard and Menu components (#3065)

* [@mantine/core] always try centering arrow on target for popover

* [@mantine/core] always try centering arrow on target for tooltip

Co-authored-by: Jérémie van der Sande <jeremie.van-der-sande@ubisoft.com>
  • Loading branch information
jvdsande and Jérémie van der Sande committed Nov 28, 2022
1 parent 470dca2 commit 3fd2e64
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 78 deletions.
21 changes: 1 addition & 20 deletions src/mantine-core/src/Floating/FloatingArrow/FloatingArrow.tsx
@@ -1,36 +1,19 @@
import React, { forwardRef } from 'react';
import { useMantineTheme } from '@mantine/styles';
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;
arrowRadius: number;
arrowX: number;
arrowY: number;
visible: boolean;
}

export const FloatingArrow = forwardRef<HTMLDivElement, FloatingArrowProps>(
(
{
withBorder,
position,
arrowSize,
arrowOffset,
arrowRadius,
visible,
arrowX,
arrowY,
...others
},
ref
) => {
const theme = useMantineTheme();

({ withBorder, position, arrowSize, arrowRadius, visible, arrowX, arrowY, ...others }, ref) => {
if (!visible) {
return null;
}
Expand All @@ -43,9 +26,7 @@ export const FloatingArrow = forwardRef<HTMLDivElement, FloatingArrowProps>(
withBorder,
position,
arrowSize,
arrowOffset,
arrowRadius,
dir: theme.dir,
arrowX,
arrowY,
})}
Expand Down
@@ -1,47 +1,6 @@
import { CSSObject } from '@mantine/styles';
import type { FloatingPosition, FloatingSide, FloatingPlacement } from '../types';

function horizontalSide(
placement: FloatingPlacement | 'center',
arrowY: number,
arrowOffset: number
) {
if (placement === 'center') {
return { top: arrowY };
}

if (placement === 'end') {
return { bottom: arrowOffset };
}

if (placement === 'start') {
return { top: arrowOffset };
}

return {};
}

function verticalSide(
placement: FloatingPlacement | 'center',
arrowX: number,
arrowOffset: number,
dir: 'rtl' | 'ltr'
) {
if (placement === 'center') {
return { [dir === 'ltr' ? 'left' : 'right']: arrowX };
}

if (placement === 'end') {
return { [dir === 'ltr' ? 'right' : 'left']: arrowOffset };
}

if (placement === 'start') {
return { [dir === 'ltr' ? 'left' : 'right']: arrowOffset };
}

return {};
}

const radiusByFloatingSide: Record<
FloatingSide,
keyof Pick<
Expand All @@ -62,22 +21,18 @@ export function getArrowPositionStyles({
position,
withBorder,
arrowSize,
arrowOffset,
arrowRadius,
arrowX,
arrowY,
dir,
}: {
position: FloatingPosition;
withBorder: boolean;
arrowSize: number;
arrowOffset: number;
arrowRadius: number;
arrowX: number;
arrowY: number;
dir: 'rtl' | 'ltr';
}) {
const [side, placement = 'center'] = position.split('-') as [FloatingSide, FloatingPlacement];
const [side] = position.split('-') as [FloatingSide, FloatingPlacement];
const baseStyles = {
width: arrowSize,
height: arrowSize,
Expand All @@ -91,7 +46,7 @@ export function getArrowPositionStyles({
if (side === 'left') {
return {
...baseStyles,
...horizontalSide(placement, arrowY, arrowOffset),
top: arrowY,
right: arrowPosition,
borderLeft: 0,
borderBottom: 0,
Expand All @@ -101,7 +56,7 @@ export function getArrowPositionStyles({
if (side === 'right') {
return {
...baseStyles,
...horizontalSide(placement, arrowY, arrowOffset),
top: arrowY,
left: arrowPosition,
borderRight: 0,
borderTop: 0,
Expand All @@ -111,20 +66,20 @@ export function getArrowPositionStyles({
if (side === 'top') {
return {
...baseStyles,
...verticalSide(placement, arrowX, arrowOffset, dir),
left: arrowX,
bottom: arrowPosition,
borderTop: 0,
[dir === 'ltr' ? 'borderLeft' : 'borderRight']: 0,
borderLeft: 0,
};
}

if (side === 'bottom') {
return {
...baseStyles,
...verticalSide(placement, arrowX, arrowOffset, dir),
left: arrowX,
top: arrowPosition,
borderBottom: 0,
[dir === 'ltr' ? 'borderRight' : 'borderLeft']: 0,
borderRight: 0,
};
}

Expand Down
1 change: 0 additions & 1 deletion src/mantine-core/src/Popover/Popover.context.ts
Expand Up @@ -21,7 +21,6 @@ interface PopoverContext {
width?: PopoverWidth;
withArrow: boolean;
arrowSize: number;
arrowOffset: number;
arrowRadius: number;
trapFocus: boolean;
placement: FloatingPosition;
Expand Down
2 changes: 1 addition & 1 deletion src/mantine-core/src/Popover/Popover.tsx
Expand Up @@ -201,6 +201,7 @@ export function Popover(props: PopoverProps) {
position: getFloatingPosition(theme.dir, position),
offset: offset + (withArrow ? arrowSize / 2 : 0),
arrowRef,
arrowOffset,
onPositionChange,
positionDependencies,
opened,
Expand Down Expand Up @@ -257,7 +258,6 @@ export function Popover(props: PopoverProps) {
width,
withArrow,
arrowSize,
arrowOffset,
arrowRadius,
placement: popover.floating.placement,
trapFocus,
Expand Down
Expand Up @@ -63,6 +63,7 @@ export function PopoverDropdown({
<Box
{...accessibleProps}
tabIndex={-1}
key={ctx.placement}
ref={ctx.floating}
style={{
...style,
Expand Down Expand Up @@ -92,7 +93,6 @@ export function PopoverDropdown({
position={ctx.placement}
arrowSize={ctx.arrowSize}
arrowRadius={ctx.arrowRadius}
arrowOffset={ctx.arrowOffset}
className={classes.arrow}
/>
</Box>
Expand Down
3 changes: 2 additions & 1 deletion src/mantine-core/src/Popover/use-popover.ts
Expand Up @@ -26,6 +26,7 @@ interface UsePopoverOptions {
width: PopoverWidth;
middlewares: PopoverMiddlewares;
arrowRef: React.RefObject<HTMLDivElement>;
arrowOffset: number;
}

function getPopoverMiddlewares(options: UsePopoverOptions) {
Expand All @@ -43,7 +44,7 @@ function getPopoverMiddlewares(options: UsePopoverOptions) {
middlewares.push(inline());
}

middlewares.push(arrow({ element: options.arrowRef }));
middlewares.push(arrow({ element: options.arrowRef, padding: options.arrowOffset }));

return middlewares;
}
Expand Down
2 changes: 1 addition & 1 deletion src/mantine-core/src/Tooltip/Tooltip.tsx
Expand Up @@ -126,6 +126,7 @@ const _Tooltip = forwardRef<HTMLElement, TooltipProps>((props, ref) => {
opened,
events,
arrowRef,
arrowOffset,
offset: offset + (withArrow ? arrowSize / 2 : 0),
positionDependencies: [...positionDependencies, children],
inline,
Expand Down Expand Up @@ -170,7 +171,6 @@ const _Tooltip = forwardRef<HTMLElement, TooltipProps>((props, ref) => {
withBorder={false}
position={tooltip.placement}
arrowSize={arrowSize}
arrowOffset={arrowOffset}
arrowRadius={arrowRadius}
className={classes.arrow}
/>
Expand Down
3 changes: 2 additions & 1 deletion src/mantine-core/src/Tooltip/use-tooltip.ts
Expand Up @@ -26,6 +26,7 @@ interface UseTooltip {
opened?: boolean;
offset: number;
arrowRef?: React.RefObject<HTMLDivElement>;
arrowOffset: number;
events: { hover: boolean; focus: boolean; touch: boolean };
positionDependencies: any[];
inline: boolean;
Expand Down Expand Up @@ -69,7 +70,7 @@ export function useTooltip(settings: UseTooltip) {
offset(settings.offset),
shift({ padding: 8 }),
flip(),
arrow({ element: settings.arrowRef }),
arrow({ element: settings.arrowRef, padding: settings.arrowOffset }),
...(settings.inline ? [inline()] : []),
],
});
Expand Down

0 comments on commit 3fd2e64

Please sign in to comment.