Skip to content

Commit 2cf8757

Browse files
adamwoodnzkangzj
andauthored
Line chart - custom annotations (#44131)
* Enhance LineChartAnnotation component with custom label support and improved label positioning * Refactor LineChartAnnotation to accept custom label rendering via renderLabel prop. * Update label positioning logic to accommodate custom dx and dy values. * Modify annotation props to include additional label positioning options. * Adjust LineChart component to pass down annotation props more efficiently. * Add a new story for custom label rendering in annotation stories. * Add changelog * Fix regression on default annotation positioning * Fix react popover attr naming * Configure bottom custom annotation in story * Wrap annotations in svg * Refactor LineChart component to support forwarding refs and expose scale data * Introduced LineChartRef interface to provide methods for accessing chart scales and dimensions. * Refactored LineChart to use forwardRef, allowing parent components to interact with the chart's internal state. * Created LineChartInternal component to manage data context and provide scale data via ref. * Updated chart rendering logic to maintain existing functionality while enhancing flexibility. * Use an overlay for positioning annotations above the interaction layer * Remove debug annotations * Fix positioning issues * Refactor LineChartAnnotation and LineChartAnnotations components to remove external positioning logic and utilize DataContext for scale management. Simplified annotation rendering by directly passing props without calculating chart bounds. * Improve data context types * Wait for scales to be ready in tests * Simplify scales ready state * Fix svg getBBox not being available in jsdom * Improve syncing of overlay scale * Remove React from imports * Fix custom label positoning in Safari * Add @automattic/jetpack-components as a dependency in charts package * Add LineChartAnnotationLabelWithPopover component for enhanced label popover functionality in line charts. Update LineChartAnnotation to support popover rendering and simplify annotation prop passing in LineChartAnnotations. Update stories to demonstrate new popover feature. * Add missing position on line chart wrapper after rebase * Refactor LineChartAnnotation to utilize POPOVER_BUTTON_SIZE constant for consistent button dimensions and simplify Safari label positioning logic. * Move popoveer styles to scss * Minor code cleanup * Refactor LineChartAnnotation to remove unused dx and dy props, simplifying label positioning logic and improving code clarity. * Add DeployedIcon and AlertIcon components for improved annotation rendering in LineChart. Update renderLabel methods to utilize these new components, enhancing code clarity and maintainability. * Unify tooltip and popover styles * Add tests * Introduce scalesStable state to track when scales have settled, ensuring proper rendering only when scales are stable. * Improve popover positioining - fallback to centered position for Safari due to foreign object issues - add space around popover to allow for hitting viewport boundaries * Add popover api types * Add popover tests * Fix typecheck * Add annotation overlay tests * Rename LineChartAnnotations component to LineChartAnnotationsOverlay for clarity and update import references accordingly. * Add popover tests * Enhance LineChart annotation tests to verify overlay rendering and dimensions. Update tests to ensure proper behavior when annotations are provided, absent, or empty. * Expose unwrapped LineChart component for testing and add tests for ref interface methods. * Fix tooltip keyboard navigation by moving annotations out of focusable area * Use annotation title for button aria-label * Improve Safari detection and comments * Improve story styling * Use composition for annotations with chart context * Switch to compound component pattern * fix build errors * Update tests * 💄 Fix lint * Update changelog Include the breaking change to the annotations API --------- Co-authored-by: Jasper Kang <[email protected]>
1 parent 656519b commit 2cf8757

15 files changed

+1404
-448
lines changed

pnpm-lock.yaml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Significance: minor
2+
Type: added
3+
4+
Line chart: Add support for custom interactive annotations. Includes a breaking change to the annotations API, from an `annotations` prop to a compound component pattern.

projects/js-packages/charts/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
"./style.css": "./dist/mjs/style.css"
4949
},
5050
"dependencies": {
51+
"@automattic/jetpack-components": "workspace:*",
5152
"@automattic/number-formatters": "workspace:*",
5253
"@babel/runtime": "7.27.6",
5354
"@react-spring/web": "9.7.5",
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import { Gridicon } from '@automattic/jetpack-components';
2+
import clsx from 'clsx';
3+
import { useEffect, useId, useRef, useState } from 'react';
4+
import { isSafari } from '../shared/utils';
5+
import styles from './line-chart.module.scss';
6+
import type { ButtonWithPopover, PopoverElement, ToggleEvent } from '../../types';
7+
import type { FC } from 'react';
8+
9+
export const POPOVER_BUTTON_SIZE = 44;
10+
11+
interface LineChartAnnotationLabelWithPopoverProps {
12+
title: string;
13+
subtitle?: string;
14+
renderLabel: FC< { title: string; subtitle?: string } >;
15+
renderLabelPopover: FC< { title: string; subtitle?: string } >;
16+
}
17+
18+
const LineChartAnnotationLabelWithPopover: FC< LineChartAnnotationLabelWithPopoverProps > = ( {
19+
title,
20+
subtitle,
21+
renderLabel,
22+
renderLabelPopover,
23+
} ) => {
24+
const popoverId = useId();
25+
const buttonRef = useRef< HTMLButtonElement >( null );
26+
const popoverRef = useRef< HTMLDivElement >( null );
27+
const [ isPositioned, setIsPositioned ] = useState( false );
28+
const isBrowserSafari = isSafari();
29+
30+
useEffect( () => {
31+
const button = buttonRef.current;
32+
const popover = popoverRef.current;
33+
34+
if ( ! button || ! popover ) return;
35+
36+
const positionPopover = () => {
37+
// Popover positioning in Safari is complicated due to issues with SVG foreign objects (https://bugs.webkit.org/show_bug.cgi?id=23113), so let it be positioned in the centre of the viewport.
38+
if ( ! isBrowserSafari ) {
39+
const buttonRect = button.getBoundingClientRect();
40+
popover.style.left = `${ buttonRect.right }px`;
41+
popover.style.top = `${ buttonRect.top }px`;
42+
}
43+
44+
setIsPositioned( true );
45+
};
46+
47+
// Position when popover shows
48+
popover.addEventListener( 'toggle', ( e: ToggleEvent ) => {
49+
if ( e.newState === 'open' ) {
50+
positionPopover();
51+
}
52+
} );
53+
54+
// Initial positioning if already open
55+
try {
56+
if ( popover.matches( ':popover-open' ) ) {
57+
positionPopover();
58+
}
59+
} catch {
60+
// Ignore errors in test environments (e.g., JSDOM does not support :popover-open)
61+
}
62+
}, [ isBrowserSafari ] );
63+
64+
return (
65+
<div className={ styles[ 'line-chart__annotation-label' ] }>
66+
<button
67+
ref={ buttonRef }
68+
{ ...( { popovertarget: popoverId } as ButtonWithPopover ) }
69+
className={ styles[ 'line-chart__annotation-label-trigger-button' ] }
70+
style={ {
71+
width: `${ POPOVER_BUTTON_SIZE }px`,
72+
height: `${ POPOVER_BUTTON_SIZE }px`,
73+
transform: `translate(${ POPOVER_BUTTON_SIZE / 2 }px, 0)`,
74+
} }
75+
aria-label={ title || 'View details' }
76+
>
77+
{ renderLabel( { title, subtitle } ) }
78+
</button>
79+
<div
80+
ref={ popoverRef }
81+
id={ popoverId }
82+
{ ...( { popover: 'auto' } as PopoverElement ) }
83+
className={ clsx(
84+
styles[ 'line-chart__annotation-label-popover' ],
85+
isPositioned && styles[ 'line-chart__annotation-label-popover--visible' ],
86+
isBrowserSafari && styles[ 'line-chart__annotation-label-popover--safari' ]
87+
) }
88+
data-testid="line-chart-annotation-label-popover"
89+
>
90+
<div className={ styles[ 'line-chart__annotation-label-popover-header' ] }>
91+
<div className={ styles[ 'line-chart__annotation-label-popover-content' ] }>
92+
{ renderLabelPopover( { title, subtitle } ) }
93+
</div>
94+
<button
95+
{ ...( {
96+
popovertarget: popoverId,
97+
popovertargetaction: 'hide',
98+
} as ButtonWithPopover ) }
99+
className={ styles[ 'line-chart__annotation-label-popover-close-button' ] }
100+
aria-label="Close"
101+
>
102+
<Gridicon icon="cross" size={ 16 } />
103+
</button>
104+
</div>
105+
</div>
106+
</div>
107+
);
108+
};
109+
110+
export default LineChartAnnotationLabelWithPopover;

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

Lines changed: 121 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,19 @@
1-
import { Annotation, CircleSubject, Connector, Label, LineSubject } from '@visx/annotation';
1+
import {
2+
Annotation,
3+
CircleSubject,
4+
Connector,
5+
HtmlLabel,
6+
Label,
7+
LineSubject,
8+
} from '@visx/annotation';
29
import { DataContext } from '@visx/xychart';
310
import { merge } from 'lodash';
411
import { useContext, useRef, useEffect, useState, useMemo } from 'react';
512
import { useChartTheme } from '../../providers/theme/theme-provider';
13+
import { isSafari } from '../shared/utils';
14+
import LineChartAnnotationLabelWithPopover, {
15+
POPOVER_BUTTON_SIZE,
16+
} from './line-chart-annotation-label-popover';
617
import type { DataPointDate } from '../../types';
718
import type { CircleSubjectProps } from '@visx/annotation/lib/components/CircleSubject';
819
import type { ConnectorProps } from '@visx/annotation/lib/components/Connector';
@@ -15,7 +26,10 @@ export type AnnotationStyles = {
1526
circleSubject?: Omit< CircleSubjectProps, 'x' | 'y' > & { fill?: string };
1627
lineSubject?: Omit< LineSubjectProps, 'x' | 'y' >;
1728
connector?: Omit< ConnectorProps, 'x' | 'y' | 'dx' | 'dy' >;
18-
label?: Omit< LabelProps, 'title' | 'subtitle' >;
29+
label?: Omit< LabelProps, 'title' | 'subtitle' | 'x' | 'y' > & {
30+
x?: number | 'start' | 'end';
31+
y?: number | 'start' | 'end';
32+
};
1933
};
2034

2135
type SubjectType = 'circle' | 'line-vertical' | 'line-horizontal';
@@ -30,6 +44,8 @@ export type LineChartAnnotationProps = {
3044
subjectType?: SubjectType;
3145
styles?: AnnotationStyles;
3246
testId?: string;
47+
renderLabel?: FC< { title: string; subtitle?: string } >;
48+
renderLabelPopover?: FC< { title: string; subtitle?: string } >;
3349
};
3450

3551
export const getLabelPosition = ( {
@@ -79,6 +95,7 @@ export const getLabelPosition = ( {
7995

8096
if ( effectiveX + annotationMaxWidth > xMax ) {
8197
isFlippedHorizontally = true;
98+
8299
if ( subjectType === 'circle' ) {
83100
dx = -dx; // Just flip to the left side with same offset
84101
} else if ( subjectType === 'line-vertical' ) {
@@ -142,7 +159,7 @@ const getVerticalAnchor = (
142159
return y - height < yMax ? 'start' : 'end';
143160
}
144161

145-
return 'middle';
162+
return 'start';
146163
}
147164

148165
return undefined;
@@ -155,6 +172,8 @@ const LineChartAnnotation: FC< LineChartAnnotationProps > = ( {
155172
subjectType = 'circle',
156173
styles: datumStyles,
157174
testId,
175+
renderLabel,
176+
renderLabelPopover,
158177
} ) => {
159178
const providerTheme = useChartTheme();
160179
const { xScale, yScale } = useContext( DataContext ) || {};
@@ -166,7 +185,7 @@ const LineChartAnnotation: FC< LineChartAnnotationProps > = ( {
166185

167186
// Measure the label height once after initial render
168187
useEffect( () => {
169-
if ( labelRef.current ) {
188+
if ( labelRef.current?.getBBox ) {
170189
const bbox = labelRef.current.getBBox();
171190
setHeight( bbox.height );
172191
}
@@ -183,6 +202,22 @@ const LineChartAnnotation: FC< LineChartAnnotationProps > = ( {
183202
const [ yMin, yMax ] = yScale.range().map( Number );
184203
const [ xMin, xMax ] = xScale.range().map( Number );
185204

205+
// If a custom label is provided, use the provided position
206+
if ( renderLabel ) {
207+
return {
208+
x,
209+
dx: 0,
210+
y,
211+
dy: 0,
212+
yMin,
213+
yMax,
214+
xMin,
215+
xMax,
216+
isFlippedHorizontally: false,
217+
isFlippedVertically: false,
218+
};
219+
}
220+
186221
const position = getLabelPosition( {
187222
subjectType,
188223
x,
@@ -195,13 +230,61 @@ const LineChartAnnotation: FC< LineChartAnnotationProps > = ( {
195230
} );
196231

197232
return { x, y, yMin, yMax, xMin, xMax, ...position };
198-
}, [ datum, xScale, yScale, subjectType, styles?.label?.maxWidth, height ] );
233+
}, [ datum, xScale, yScale, subjectType, styles?.label?.maxWidth, height, renderLabel ] );
199234

200235
if ( ! positionData ) return null;
201236

202237
const { x, y, yMin, yMax, xMin, xMax, dx, dy, isFlippedHorizontally, isFlippedVertically } =
203238
positionData;
204239

240+
const getLabelY = () => {
241+
const labelY = styles?.label?.y;
242+
243+
if ( labelY === 'start' ) return yMax;
244+
if ( labelY === 'end' ) return yMin;
245+
246+
return labelY;
247+
};
248+
249+
const getLabelX = () => {
250+
const labelX = styles?.label?.x;
251+
252+
if ( labelX === 'start' ) return xMin;
253+
if ( labelX === 'end' ) return xMax;
254+
255+
return labelX;
256+
};
257+
258+
const labelPosition = {
259+
x: getLabelX(),
260+
y: getLabelY(),
261+
};
262+
263+
// Safari has a bug where children of an SVG foreignObject are not positioned correctly https://bugs.webkit.org/show_bug.cgi?id=23113
264+
// This is a workaround to position the label correctly
265+
const getSafariHTMLLabelPosition = () => {
266+
const labelWidth = POPOVER_BUTTON_SIZE;
267+
const labelHeight = POPOVER_BUTTON_SIZE;
268+
269+
return isSafari()
270+
? {
271+
transform: `translate(${
272+
x +
273+
( dx || 0 ) +
274+
( typeof labelPosition.x === 'number' ? labelPosition.x - x : 0 ) -
275+
labelWidth
276+
}px, ${
277+
y +
278+
( dy || 0 ) +
279+
( typeof labelPosition.y === 'number' ? labelPosition.y - y : 0 ) -
280+
labelHeight
281+
}px)`,
282+
width: labelWidth,
283+
height: labelHeight,
284+
}
285+
: undefined;
286+
};
287+
205288
return (
206289
<g data-testid={ testId }>
207290
<Annotation x={ x } y={ y } dx={ dx } dy={ dy }>
@@ -221,21 +304,39 @@ const LineChartAnnotation: FC< LineChartAnnotationProps > = ( {
221304
{ ...{ ...styles?.lineSubject, orientation: 'horizontal' } }
222305
/>
223306
) }
224-
<g ref={ labelRef }>
225-
<Label
226-
title={ title }
227-
subtitle={ subtitle }
228-
{ ...styles?.label }
229-
horizontalAnchor={ getHorizontalAnchor( subjectType, isFlippedHorizontally ) }
230-
verticalAnchor={ getVerticalAnchor(
231-
subjectType,
232-
isFlippedVertically,
233-
y,
234-
yMax,
235-
height ?? ANNOTATION_INIT_HEIGHT
236-
) }
237-
/>
238-
</g>
307+
{ renderLabel ? (
308+
<HtmlLabel { ...styles?.label } { ...labelPosition }>
309+
<div style={ getSafariHTMLLabelPosition() }>
310+
{ renderLabelPopover ? (
311+
<LineChartAnnotationLabelWithPopover
312+
title={ title }
313+
subtitle={ subtitle }
314+
renderLabel={ renderLabel }
315+
renderLabelPopover={ renderLabelPopover }
316+
/>
317+
) : (
318+
renderLabel( { title, subtitle } )
319+
) }
320+
</div>
321+
</HtmlLabel>
322+
) : (
323+
<g ref={ labelRef }>
324+
<Label
325+
title={ title }
326+
subtitle={ subtitle }
327+
{ ...styles?.label }
328+
{ ...labelPosition }
329+
horizontalAnchor={ getHorizontalAnchor( subjectType, isFlippedHorizontally ) }
330+
verticalAnchor={ getVerticalAnchor(
331+
subjectType,
332+
isFlippedVertically,
333+
y,
334+
yMax,
335+
height ?? ANNOTATION_INIT_HEIGHT
336+
) }
337+
/>
338+
</g>
339+
) }
239340
</Annotation>
240341
</g>
241342
);

0 commit comments

Comments
 (0)