Skip to content

Commit 97c834c

Browse files
committed
Use composition for annotations with chart context
1 parent 7176c2d commit 97c834c

File tree

5 files changed

+177
-185
lines changed

5 files changed

+177
-185
lines changed

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

Lines changed: 23 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,32 @@
11
import { DataContext } from '@visx/xychart';
22
import { useEffect, useState, useCallback } from 'react';
3-
import LineChartAnnotation from './line-chart-annotation';
3+
import { useChartContext } from '../../providers/chart-context';
44
import styles from './line-chart.module.scss';
5-
import type { LineChartRef } from './line-chart';
6-
import type { LineChartAnnotationProps } from './line-chart-annotation';
75
import type { AxisScale } from '@visx/axis';
8-
import type { FC, RefObject } from 'react';
6+
import type { FC, ReactNode } from 'react';
97

108
interface LineChartAnnotationsProps {
11-
chartRef: RefObject< LineChartRef >;
12-
annotations: LineChartAnnotationProps[];
13-
chartWidth: number;
14-
chartHeight: number;
9+
chartId: string;
10+
children?: ReactNode;
1511
}
1612

1713
interface ScaleData {
1814
xScale: AxisScale< Date >;
1915
yScale: AxisScale< number >;
2016
}
2117

22-
const LineChartAnnotationsOverlay: FC< LineChartAnnotationsProps > = ( {
23-
chartRef,
24-
annotations,
25-
chartWidth,
26-
chartHeight,
27-
} ) => {
18+
const LineChartAnnotationsOverlay: FC< LineChartAnnotationsProps > = ( { chartId, children } ) => {
19+
const { getChartData } = useChartContext();
20+
21+
let chartData = null;
22+
if ( chartId ) {
23+
chartData = getChartData( chartId );
24+
}
25+
26+
const chartRef = chartData?.chartRef;
27+
const chartWidth = chartData?.chartWidth || 0;
28+
const chartHeight = chartData?.chartHeight || 0;
29+
2830
const [ scales, setScales ] = useState< ScaleData | null >( null );
2931
const [ scalesStable, setScalesStable ] = useState< boolean >( false );
3032

@@ -42,7 +44,7 @@ const LineChartAnnotationsOverlay: FC< LineChartAnnotationsProps > = ( {
4244

4345
// Get scales from chart ref and return them with signature for comparison
4446
const getScalesData = useCallback( () => {
45-
if ( chartRef.current ) {
47+
if ( chartRef?.current ) {
4648
const scaleData = chartRef.current.getScales();
4749

4850
if ( scaleData ) {
@@ -107,6 +109,11 @@ const LineChartAnnotationsOverlay: FC< LineChartAnnotationsProps > = ( {
107109
};
108110
}, [ getScalesData, chartWidth, chartHeight ] );
109111

112+
// Early return if no chart data available
113+
if ( ! chartRef || ! children ) {
114+
return null;
115+
}
116+
110117
if ( ! scales || ! scalesStable ) {
111118
return null;
112119
}
@@ -116,16 +123,9 @@ const LineChartAnnotationsOverlay: FC< LineChartAnnotationsProps > = ( {
116123
const dataContextValue = {
117124
xScale: scales.xScale,
118125
yScale: scales.yScale,
119-
// Add minimal required properties for DataContext
120-
theme: {},
121126
margin: { top: 0, right: 0, bottom: 0, left: 0 },
122127
width: chartWidth,
123128
height: chartHeight,
124-
// Additional visx DataContext properties that may be needed
125-
colorScale: undefined,
126-
dataRegistry: new Map(),
127-
registerData: () => {},
128-
unregisterData: () => {},
129129
} as unknown as Parameters< typeof DataContext.Provider >[ 0 ][ 'value' ];
130130

131131
return (
@@ -136,13 +136,7 @@ const LineChartAnnotationsOverlay: FC< LineChartAnnotationsProps > = ( {
136136
className={ styles[ 'line-chart__annotations-overlay' ] }
137137
data-testid="line-chart-annotations-overlay"
138138
>
139-
{ annotations.map( ( annotation, index ) => (
140-
<LineChartAnnotation
141-
key={ `overlay-annotation-${ index }` }
142-
testId={ `overlay-annotation-${ index }` }
143-
{ ...annotation }
144-
/>
145-
) ) }
139+
{ children }
146140
</svg>
147141
</DataContext.Provider>
148142
);

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

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ import {
2121
useState,
2222
useRef,
2323
useCallback,
24+
Children,
25+
cloneElement,
26+
isValidElement,
2427
} from 'react';
2528
import { ChartProvider, useChartId, useChartRegistration } from '../../providers/chart-context';
2629
import { useXYChartTheme, useChartTheme } from '../../providers/theme/theme-provider';
@@ -30,9 +33,7 @@ import { useChartDataTransform } from '../shared/use-chart-data-transform';
3033
import { useChartMargin } from '../shared/use-chart-margin';
3134
import { useElementHeight } from '../shared/use-element-height';
3235
import { withResponsive } from '../shared/with-responsive';
33-
import LineChartAnnotationsOverlay from './line-chart-annotations-overlay';
3436
import styles from './line-chart.module.scss';
35-
import type { LineChartAnnotationProps } from './line-chart-annotation';
3637
import type { BaseChartProps, DataPoint, DataPointDate, SeriesData } from '../../types';
3738
import type { TickFormatter } from '@visx/axis';
3839
import type { GlyphProps } from '@visx/xychart';
@@ -143,7 +144,7 @@ interface LineChartProps extends BaseChartProps< SeriesData[] > {
143144
showVertical?: boolean;
144145
showHorizontal?: boolean;
145146
};
146-
annotations?: LineChartAnnotationProps[];
147+
children?: ReactNode;
147148
}
148149

149150
type TooltipDatum = {
@@ -294,11 +295,11 @@ const LineChartInternal = forwardRef< LineChartRef, LineChartProps >(
294295
renderTooltip = renderDefaultTooltip,
295296
withStartGlyphs = false,
296297
options = {},
297-
annotations,
298298
onPointerDown = undefined,
299299
onPointerUp = undefined,
300300
onPointerMove = undefined,
301301
onPointerOut = undefined,
302+
children,
302303
},
303304
ref
304305
) => {
@@ -404,13 +405,25 @@ const LineChartInternal = forwardRef< LineChartRef, LineChartProps >(
404405
);
405406

406407
// Register chart with context only if data is valid
407-
useChartRegistration( chartId, legendItems, providerTheme, 'line', isDataValid, {
408-
withGradientFill,
409-
smoothing,
410-
curveType,
411-
withStartGlyphs,
412-
withLegendGlyph,
413-
} );
408+
useChartRegistration(
409+
chartId,
410+
legendItems,
411+
providerTheme,
412+
'line',
413+
isDataValid,
414+
{
415+
withGradientFill,
416+
smoothing,
417+
curveType,
418+
withStartGlyphs,
419+
withLegendGlyph,
420+
},
421+
{
422+
chartRef: internalChartRef,
423+
chartWidth: width,
424+
chartHeight: height - ( showLegend ? legendHeight : 0 ),
425+
}
426+
);
414427

415428
const accessors = {
416429
xAccessor: ( d: DataPointDate ) => d?.date,
@@ -632,16 +645,6 @@ const LineChartInternal = forwardRef< LineChartRef, LineChartProps >(
632645
</XYChart>
633646
</div>
634647

635-
{ /* Render annotations as external overlay to avoid interaction blocking */ }
636-
{ annotations?.length && (
637-
<LineChartAnnotationsOverlay
638-
chartRef={ internalChartRef }
639-
annotations={ annotations }
640-
chartWidth={ width }
641-
chartHeight={ height - ( showLegend ? legendHeight : 0 ) }
642-
/>
643-
) }
644-
645648
{ showLegend && (
646649
<Legend
647650
items={ legendItems }
@@ -653,6 +656,15 @@ const LineChartInternal = forwardRef< LineChartRef, LineChartProps >(
653656
ref={ legendRef }
654657
/>
655658
) }
659+
660+
{ /* Render children here so they have access to the chart ID */ }
661+
{ Children.map( children, child => {
662+
if ( isValidElement( child ) ) {
663+
return cloneElement( child, { chartId } as { chartId: string } );
664+
}
665+
666+
return child;
667+
} ) }
656668
</div>
657669
);
658670
}

0 commit comments

Comments
 (0)