From 49e18a3e7a698fcc7b5ec2b2034824bde05f3a42 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Fri, 23 Feb 2024 15:00:24 +0000 Subject: [PATCH] Chore: replace `react-popper` with `floating-ui` in `Popover` (#82922) * replace react-popper with floating-ui in Popover * update HoverCard * fix unit tests * mock useTransitionStyles to ensure consistent unit test results --- .betterer.results | 7 - .../components/ColorPicker/ColorPicker.tsx | 17 -- .../src/components/Table/FilterPopup.tsx | 1 - .../src/components/Table/Table.test.tsx | 8 + .../src/components/Tooltip/Popover.tsx | 170 +++++++++--------- .../alerting/unified/components/HoverCard.tsx | 76 +++----- 6 files changed, 117 insertions(+), 162 deletions(-) diff --git a/.betterer.results b/.betterer.results index f04979e6beb..d305a5130af 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1662,13 +1662,6 @@ exports[`better eslint`] = { "public/app/features/alerting/unified/components/GrafanaAlertmanagerDeliveryWarning.tsx:5381": [ [0, 0, 0, "Styles should be written using objects.", "0"] ], - "public/app/features/alerting/unified/components/HoverCard.tsx:5381": [ - [0, 0, 0, "Styles should be written using objects.", "0"], - [0, 0, 0, "Styles should be written using objects.", "1"], - [0, 0, 0, "Styles should be written using objects.", "2"], - [0, 0, 0, "Styles should be written using objects.", "3"], - [0, 0, 0, "Styles should be written using objects.", "4"] - ], "public/app/features/alerting/unified/components/Label.tsx:5381": [ [0, 0, 0, "Styles should be written using objects.", "0"], [0, 0, 0, "Styles should be written using objects.", "1"], diff --git a/packages/grafana-ui/src/components/ColorPicker/ColorPicker.tsx b/packages/grafana-ui/src/components/ColorPicker/ColorPicker.tsx index 65912a39d6a..57fc7cfe6ce 100644 --- a/packages/grafana-ui/src/components/ColorPicker/ColorPicker.tsx +++ b/packages/grafana-ui/src/components/ColorPicker/ColorPicker.tsx @@ -92,23 +92,6 @@ const getStyles = stylesFactory((theme: GrafanaTheme2) => { color: theme.colors.text.primary, maxWidth: '400px', fontSize: theme.typography.size.sm, - // !important because these styles are also provided to popper via .popper classes from Tooltip component - // hope to get rid of those soon - padding: '15px !important', - '& [data-placement^="top"]': { - paddingLeft: '0 !important', - paddingRight: '0 !important', - }, - '& [data-placement^="bottom"]': { - paddingLeft: '0 !important', - paddingRight: '0 !important', - }, - '& [data-placement^="left"]': { - paddingTop: '0 !important', - }, - '& [data-placement^="right"]': { - paddingTop: '0 !important', - }, }), }; }); diff --git a/packages/grafana-ui/src/components/Table/FilterPopup.tsx b/packages/grafana-ui/src/components/Table/FilterPopup.tsx index efa1e848e02..422dfe7a4f5 100644 --- a/packages/grafana-ui/src/components/Table/FilterPopup.tsx +++ b/packages/grafana-ui/src/components/Table/FilterPopup.tsx @@ -108,7 +108,6 @@ const getStyles = (theme: GrafanaTheme2) => ({ backgroundColor: theme.colors.background.primary, border: `1px solid ${theme.colors.border.weak}`, padding: theme.spacing(2), - margin: theme.spacing(1, 0), boxShadow: theme.shadows.z3, borderRadius: theme.shape.radius.default, }), diff --git a/packages/grafana-ui/src/components/Table/Table.test.tsx b/packages/grafana-ui/src/components/Table/Table.test.tsx index 5d855d25a4c..e6ed28d2a8b 100644 --- a/packages/grafana-ui/src/components/Table/Table.test.tsx +++ b/packages/grafana-ui/src/components/Table/Table.test.tsx @@ -7,6 +7,14 @@ import { applyFieldOverrides, createTheme, DataFrame, FieldType, toDataFrame } f import { Table } from './Table'; import { Props } from './types'; +// mock transition styles to ensure consistent behaviour in unit tests +jest.mock('@floating-ui/react', () => ({ + ...jest.requireActual('@floating-ui/react'), + useTransitionStyles: () => ({ + styles: {}, + }), +})); + function getDefaultDataFrame(): DataFrame { const dataFrame = toDataFrame({ name: 'A', diff --git a/packages/grafana-ui/src/components/Tooltip/Popover.tsx b/packages/grafana-ui/src/components/Tooltip/Popover.tsx index cb760b3cf5e..81527f3cde6 100644 --- a/packages/grafana-ui/src/components/Tooltip/Popover.tsx +++ b/packages/grafana-ui/src/components/Tooltip/Popover.tsx @@ -1,96 +1,102 @@ -import { Placement, VirtualElement } from '@popperjs/core'; -import React, { PureComponent } from 'react'; -import { Manager, Popper as ReactPopper, PopperArrowProps } from 'react-popper'; -import Transition from 'react-transition-group/Transition'; +import { + FloatingArrow, + arrow, + autoUpdate, + flip, + offset, + shift, + useFloating, + useTransitionStyles, +} from '@floating-ui/react'; +import React, { useLayoutEffect, useRef } from 'react'; +import { useTheme2 } from '../../themes'; +import { getPlacement } from '../../utils/tooltipUtils'; import { Portal } from '../Portal/Portal'; -import { PopoverContent } from './types'; - -const defaultTransitionStyles = { - transitionProperty: 'opacity', - transitionDuration: '200ms', - transitionTimingFunction: 'linear', - opacity: 0, -}; - -const transitionStyles: { [key: string]: object } = { - exited: { opacity: 0 }, - entering: { opacity: 0 }, - entered: { opacity: 1, transitionDelay: '0s' }, - exiting: { opacity: 0, transitionDelay: '500ms' }, -}; - -export type RenderPopperArrowFn = (props: { arrowProps: PopperArrowProps; placement: string }) => JSX.Element; +import { PopoverContent, TooltipPlacement } from './types'; interface Props extends Omit, 'content'> { show: boolean; - placement?: Placement; + placement?: TooltipPlacement; content: PopoverContent; - referenceElement: HTMLElement | VirtualElement; + referenceElement: HTMLElement; wrapperClassName?: string; - renderArrow?: RenderPopperArrowFn; + renderArrow?: boolean; } -class Popover extends PureComponent { - render() { - const { content, show, placement, className, wrapperClassName, renderArrow, referenceElement, ...rest } = - this.props; +export function Popover({ + content, + show, + placement, + className, + wrapperClassName, + referenceElement, + renderArrow, + ...rest +}: Props) { + const theme = useTheme2(); + const arrowRef = useRef(null); - return ( - - - {(transitionState) => { - return ( - - - {({ ref, style, placement, arrowProps, update }) => { - return ( -
-
- {typeof content === 'string' && content} - {React.isValidElement(content) && React.cloneElement(content)} - {typeof content === 'function' && - content({ - updatePopperPosition: update, - })} - {renderArrow && - renderArrow({ - arrowProps, - placement, - })} -
-
- ); - }} - - - ); - }} - - + // the order of middleware is important! + // `arrow` should almost always be at the end + // see https://floating-ui.com/docs/arrow#order + const middleware = [ + offset(8), + flip({ + fallbackAxisSideDirection: 'end', + // see https://floating-ui.com/docs/flip#combining-with-shift + crossAxis: false, + boundary: document.body, + }), + shift(), + ]; + + if (renderArrow) { + middleware.push( + arrow({ + element: arrowRef, + }) ); } -} -export { Popover }; + const { context, refs, floatingStyles } = useFloating({ + open: show, + placement: getPlacement(placement), + middleware, + whileElementsMounted: autoUpdate, + strategy: 'fixed', + }); + + useLayoutEffect(() => { + refs.setReference(referenceElement); + }, [referenceElement, refs]); + + const { styles: placementStyles } = useTransitionStyles(context, { + initial: () => ({ + opacity: 0, + }), + duration: theme.transitions.duration.enteringScreen, + }); + + return show ? ( + +
+
+ {renderArrow && } + {typeof content === 'string' && content} + {React.isValidElement(content) && React.cloneElement(content)} + {typeof content === 'function' && content({})} +
+
+ + ) : undefined; +} diff --git a/public/app/features/alerting/unified/components/HoverCard.tsx b/public/app/features/alerting/unified/components/HoverCard.tsx index 5d592b1113d..62edfd1c790 100644 --- a/public/app/features/alerting/unified/components/HoverCard.tsx +++ b/public/app/features/alerting/unified/components/HoverCard.tsx @@ -53,17 +53,13 @@ export const HoverCard = ({
- : () => <> - } + renderArrow={arrow} /> )} @@ -82,55 +78,25 @@ export const HoverCard = ({ }; const getStyles = (theme: GrafanaTheme2) => ({ - popover: (offset: number) => css` - border-radius: ${theme.shape.radius.default}; - box-shadow: ${theme.shadows.z3}; - background: ${theme.colors.background.primary}; - border: 1px solid ${theme.colors.border.medium}; - - margin-bottom: ${theme.spacing(offset)}; - `, + popover: css({ + borderRadius: theme.shape.radius.default, + boxShadow: theme.shadows.z3, + background: theme.colors.background.primary, + border: `1px solid ${theme.colors.border.medium}`, + }), card: { - body: css` - padding: ${theme.spacing(1)}; - `, - header: css` - padding: ${theme.spacing(1)}; - background: ${theme.colors.background.secondary}; - border-bottom: solid 1px ${theme.colors.border.medium}; - `, - footer: css` - padding: ${theme.spacing(0.5)} ${theme.spacing(1)}; - background: ${theme.colors.background.secondary}; - border-top: solid 1px ${theme.colors.border.medium}; - `, - }, - // TODO currently only works with bottom placement - arrow: (placement: string) => { - const ARROW_SIZE = '9px'; - - return css` - width: 0; - height: 0; - - border-left: ${ARROW_SIZE} solid transparent; - border-right: ${ARROW_SIZE} solid transparent; - /* using hex colors here because the border colors use alpha transparency */ - border-top: ${ARROW_SIZE} solid ${theme.isLight ? '#d2d3d4' : '#2d3037'}; - - &:after { - content: ''; - position: absolute; - - border: ${ARROW_SIZE} solid ${theme.colors.background.primary}; - border-bottom: 0; - border-left-color: transparent; - border-right-color: transparent; - - margin-top: 1px; - bottom: 1px; - left: -${ARROW_SIZE}; - } - `; + body: css({ + padding: theme.spacing(1), + }), + header: css({ + padding: theme.spacing(1), + background: theme.colors.background.secondary, + borderBottom: `solid 1px ${theme.colors.border.medium}`, + }), + footer: css({ + padding: theme.spacing(0.5, 1), + background: theme.colors.background.secondary, + borderTop: `solid 1px ${theme.colors.border.medium}`, + }), }, });