Skip to content

Commit

Permalink
fix: annotation markers render when parent size is computed
Browse files Browse the repository at this point in the history
  • Loading branch information
markov00 committed Jun 3, 2020
1 parent 1fd26fb commit 2b0c472
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 28 deletions.
2 changes: 1 addition & 1 deletion .playground/playground.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License. */

import React from 'react';
import { Example } from '../stories/treemap/6_custom_style';
import { Example } from '../stories/annotations/lines/3_x_time';

export class Playground extends React.Component {
render() {
Expand Down
55 changes: 28 additions & 27 deletions src/chart_types/xy_chart/renderer/dom/annotations/annotations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,32 @@ interface AnnotationsOwnProps {

type AnnotationsProps = AnnotationsDispatchProps & AnnotationsStateProps & AnnotationsOwnProps;

function renderAnnotationLineMarkers(
chartDimensions: Dimensions,
annotationLines: AnnotationLineProps[],
id: AnnotationId,
) {
return annotationLines.reduce<JSX.Element[]>((markers, { marker }: AnnotationLineProps, index: number) => {
if (!marker) {
return markers;
}

const { icon, color, position } = marker;
const style = {
color,
top: chartDimensions.top + position.top,
left: chartDimensions.left + position.left,
};

markers.push(
<div className="echAnnotation" style={{ ...style }} key={`annotation-${id}-${index}`}>
{icon}
</div>,
);

return markers;
}, []);
}
const AnnotationsComponent = ({
tooltipState,
isChartEmpty,
Expand All @@ -65,31 +91,6 @@ const AnnotationsComponent = ({
chartId,
onPointerMove,
}: AnnotationsProps) => {
const renderAnnotationLineMarkers = useCallback(

This comment has been minimized.

Copy link
@nickofthyme

nickofthyme Jun 3, 2020

Collaborator

Was the useCallback the main culprit here?

This comment has been minimized.

Copy link
@markov00

markov00 Jun 10, 2020

Author Member

Removing the callback fixed the wrong marker positioning, I think the main issue was the [] dependency used, that runs this only at mount and unmount, but we probably need to run this whenever we have an updated marker position.

(annotationLines: AnnotationLineProps[], id: AnnotationId) =>
annotationLines.reduce<JSX.Element[]>((markers, { marker }: AnnotationLineProps, index: number) => {
if (!marker) {
return markers;
}

const { icon, color, position } = marker;
const style = {
color,
top: chartDimensions.top + position.top,
left: chartDimensions.left + position.left,
};

markers.push(
<div className="echAnnotation" style={{ ...style }} key={`annotation-${id}-${index}`}>
{icon}
</div>,
);

return markers;
}, []),
[], // eslint-disable-line react-hooks/exhaustive-deps
);

const renderAnnotationMarkers = useCallback((): JSX.Element[] => {
const markers: JSX.Element[] = [];

Expand All @@ -101,13 +102,13 @@ const AnnotationsComponent = ({

if (isLineAnnotation(annotationSpec)) {
const annotationLines = dimensions as AnnotationLineProps[];
const lineMarkers = renderAnnotationLineMarkers(annotationLines, id);
const lineMarkers = renderAnnotationLineMarkers(chartDimensions, annotationLines, id);
markers.push(...lineMarkers);
}
});

return markers;
}, [annotationDimensions, annotationSpecs, renderAnnotationLineMarkers]);
}, [chartDimensions, annotationDimensions, annotationSpecs]);

const onScroll = useCallback(() => {
onPointerMove({ x: -1, y: -1 }, new Date().getTime());
Expand Down

0 comments on commit 2b0c472

Please sign in to comment.