Improve popover positioining

- fallback to centered position for Safari due to foreign object issues
- add space around popover to allow for hitting viewport boundaries

Author: adamwoodnz

projects/js-packages/charts/src/components/line-chart/line-chart-annotation-label-popover.tsx

@@ -1,6 +1,7 @@
 import { Gridicon } from '@automattic/jetpack-components';
 import clsx from 'clsx';
 import { useEffect, useId, useRef, useState } from 'react';
+import { isSafari } from '../shared/utils';
 import styles from './line-chart.module.scss';
 import type { FC } from 'react';
 
@@ -31,9 +32,13 @@ const LineChartAnnotationLabelWithPopover: FC< LineChartAnnotationLabelWithPopov
 		if ( ! button || ! popover ) return;
 
 		const positionPopover = () => {
-			const buttonRect = button.getBoundingClientRect();
-			popover.style.left = `${ buttonRect.right + 10 }px`;
-			popover.style.top = `${ buttonRect.top }px`;
+			// Popover positioning in Safari is complicated due to issues with SVG foreign objects, so let it be positioned in the centre of the viewport.
+			if ( ! isSafari ) {
+				const buttonRect = button.getBoundingClientRect();
+				popover.style.left = `${ buttonRect.right }px`;
+				popover.style.top = `${ buttonRect.top }px`;
+			}
+
 			setIsPositioned( true );
 		};
 
@@ -74,7 +79,8 @@ const LineChartAnnotationLabelWithPopover: FC< LineChartAnnotationLabelWithPopov
 				{ ...( { popover: 'auto' } as any ) }
 				className={ clsx(
 					styles[ 'line-chart__annotation-label-popover' ],
-					! isPositioned && styles[ 'line-chart__annotation-label-popover--hidden' ]
+					! isPositioned && styles[ 'line-chart__annotation-label-popover--hidden' ],
+					isSafari && styles[ 'line-chart__annotation-label-popover--safari' ]
 				) }
 			>
 				<div className={ styles[ 'line-chart__annotation-label-popover-header' ] }>

projects/js-packages/charts/src/components/line-chart/line-chart-annotation.tsx

@@ -10,6 +10,7 @@ import { DataContext } from '@visx/xychart';
 import { merge } from 'lodash';
 import { useContext, useRef, useEffect, useState, useMemo } from 'react';
 import { useChartTheme } from '../../providers/theme/theme-provider';
+import { isSafari } from '../shared/utils';
 import LineChartAnnotationLabelWithPopover, {
 	POPOVER_BUTTON_SIZE,
 } from './line-chart-annotation-label-popover';
@@ -21,8 +22,6 @@ import type { LineSubjectProps } from '@visx/annotation/lib/components/LineSubje
 import type { TextProps } from '@visx/text';
 import type { FC } from 'react';
 
-const isSafari = /^((?!chrome|android).)*safari/i.test( navigator.userAgent );
-
 export type AnnotationStyles = {
 	circleSubject?: Omit< CircleSubjectProps, 'x' | 'y' > & { fill?: string };
 	lineSubject?: Omit< LineSubjectProps, 'x' | 'y' >;

projects/js-packages/charts/src/components/line-chart/line-chart.module.scss

@@ -56,16 +56,20 @@
 		min-width: 125px;
 		background: #fff;
 		border: none;
-		position: fixed;
-		margin: 0;
-		visibility: visible;
 		border-radius: 4px;
 		font-size: 14px;
 		box-shadow: 0 1px 2px rgba(0, 0, 0, 0.1);
+		position: fixed;
+		margin: 0.5rem 1rem;
 
 		&--hidden {
 			visibility: hidden;
 		}
+
+		&--safari {
+			position: initial;
+			margin: auto;
+		}
 	}
 
 	&__annotation-label-popover-header {

projects/js-packages/charts/src/components/line-chart/stories/annotation.stories.tsx

@@ -173,8 +173,8 @@ const customTopAnnotationArgs: Partial< LineChartAnnotationProps > = {
 		</span>
 	),
 	renderLabelPopover: () => (
-		<div style={ { display: 'flex', flexDirection: 'column', gap: '10px' } }>
-			<h4
+		<div style={ { display: 'flex', flexDirection: 'column', gap: '0.5rem' } }>
+			<div
 				style={ {
 					margin: 0,
 					display: 'flex',
@@ -184,8 +184,8 @@ const customTopAnnotationArgs: Partial< LineChartAnnotationProps > = {
 				} }
 			>
 				<DeployedIcon />
-				<span>Deploy finished</span>
-			</h4>
+				<strong>Deploy finished</strong>
+			</div>
 			<p style={ { margin: 0 } }>Thu. Apr 24, 2025. 09:57:23 UTC</p>
 		</div>
 	),
@@ -224,8 +224,8 @@ const customBottomAnnotationArgs: Partial< LineChartAnnotationProps > = {
 	title: 'Alert',
 	renderLabel: () => <AlertIcon />,
 	renderLabelPopover: () => (
-		<div style={ { display: 'flex', flexDirection: 'column', gap: '10px' } }>
-			<h4
+		<div style={ { display: 'flex', flexDirection: 'column', gap: '0.5rem' } }>
+			<div
 				style={ {
 					margin: 0,
 					display: 'flex',
@@ -235,8 +235,8 @@ const customBottomAnnotationArgs: Partial< LineChartAnnotationProps > = {
 				} }
 			>
 				<AlertIcon />
-				Origin HTTP 5xx Response Codes Rate Anomaly [Beta]
-			</h4>
+				<strong>Origin HTTP 5xx Response Codes Rate Anomaly [Beta]</strong>
+			</div>
 			<p style={ { margin: 0 } }>
 				Unusually high number of HTTP 5xx response codes detected on Origin
 			</p>

projects/js-packages/charts/src/components/shared/utils.ts

@@ -23,3 +23,5 @@ export const getLongestTickWidth = < T extends AnyD3Scale >(
 
 	return getStringWidth( longestTick, labelStyle );
 };
+
+export const isSafari = /^((?!chrome|android).)*safari/i.test( navigator.userAgent );