feat(annotations): add onClickHandler for annotations

[rshen91]

Summary

The onAnnotationClick prop is now available in the Settings component to listen to click events on LineAnnotation (marker only) and RectAnnnotation.

Details

Example code snippet from the new storybook example in annotations > rects > click-handle-rect-annotation:

 export const Example = () => {
  const onAnnotationClick = action('onAnnotationClick');

  return (
    <Chart>
      <Settings baseTheme={useBaseTheme()} onAnnotationClick={onAnnotationClick} />
      <RectAnnotation
        id="rect1"
        dataValues={[
          {
            coordinates: {
              x0: 0,
              x1: 1,
              y0: 0,
              y1: 4,
            },
            details: 'details about this annotation',
          },
        ]}
        style={{ fill: 'red' }}
      />
      <RectAnnotation
        id="rect2"
        dataValues={[
          {
            coordinates: {
              x0: 1,
              x1: 2,
              y0: 1,
              y1: 5,
            },
            details: 'details about this other annotation',
          },
        ]}
        style={{ fill: 'blue' }}
      />
      <LineAnnotation
        id="line_annotation"
        domainType={AnnotationDomainType.XDomain}
        dataValues={[{ dataValue: 2, details: `detail-${0}` }]}
        marker={<Icon type="alert" />}
        markerPosition={Position.Top}
      />
...

Issues

Closes #1211 request from the ML team

Checklist

• Proper documentation or storybook story was added for features that require explanation or tutorials
• Unit tests were updated or added to match the most common scenarios

[monfera]

What's the approach for the mouse capture area padding? For example, the actual line of a line annotation is very thin, and Fitts law needs significant meat for being able to reliably select. Then again, there may be some tightly spaced lines, in which case the generous padding is constrained so that the midline between two nearby parallel lines determines if this or that line gets selected (otherwise the capture zone of one line can occlude the other line). So, a one-dimensional Voronoi version of this 2D example—or the Voronoi picker @nickofthyme added to charts—at least conceptually, and the 1D Voronoi logic can be extracted into a common directory for later reuse.

Capture zone padding gets more complicated with mixed or overlapping annotations: both horizontal and vertical lines; lines over rectangles; overlapping rectangles; annotation over the text of some other annotation; annotation over bars, data point markers or other interactive elements.

So, just curious about what we have as current goals for this, maybe on the original issue, or on a separate issue that goes into this kind of detail (or discussed as part of the PR)

[rshen91]

What's the approach for the mouse capture area padding? For example, the actual line of a line annotation is very thin, and Fitts law needs significant meat for being able to reliably select. Then again, there may be some tightly spaced lines, in which case the generous padding is constrained so that the midline between two nearby parallel lines determines if this or that line gets selected (otherwise the capture zone of one line can occlude the other line). So, a one-dimensional Voronoi version of this 2D example—or the Voronoi picker @nickofthyme added to charts—at least conceptually, and the 1D Voronoi logic can be extracted into a common directory for later reuse.

Capture zone padding gets more complicated with mixed or overlapping annotations: both horizontal and vertical lines; lines over rectangles; overlapping rectangles; annotation over the text of some other annotation; annotation over bars, data point markers or other interactive elements.

So, just curious about what we have as current goals for this, maybe on the original issue, or on a separate issue that goes into this kind of detail (or discussed as part of the PR)

Hey! I might be misinterpreting, but this only should make the line annotation marker clickable if passed an onAnnotationClick listener not the whole line.

[monfera]

this only should make the line annotation marker clickable if passed an onAnnotationClick listener not the whole line

Thanks Rachel! It wasn't clear to me, as the issue (to which I need to compare the PR) mentions annotations, and specifically the line and rectangle annotations. Could the scope be added to the gh issue, ie. it's just the annotation markerand not the entire rectangle or line, so I only review things in the PR that are agreed to change.

I guess that for the annotation marker, it'll stay as it is, basically no padding. @markov00 do we want to add padding to the markers, and are there signs of needs for capture events on the lines/rectangles?

[nickofthyme]

Looking good! I made a few suggestions along with some questions. Ping me if something doesn't make sense.

[rshen91]

In aa8c050 I updated the failing vrts - these were exclusively screenshots that had line annotations and had slight pixel changes

[nickofthyme]

Playing with the pr locally, there is a pointer that is not clickable

[rshen91]

Thank you, good catch. I was hoping there was some easy fix with the get_cursor_pointer selector but it wasn't working. This fix in commit 828ab13 works - let me know what you think. Thanks

[nickofthyme]

The cursor pointer logic does not match the actual logic. The onAnnotationClick event should NOT fire over element geometry. Also check how this acts with line charts when clicking points above the rect annotations.

Screen.Recording.2021-08-30.at.02.13.54.PM.mp4

[rshen91]

The cursor pointer logic does not match the actual logic. The onAnnotationClick event should NOT fire over element geometry. Also check how this acts with line charts when clicking points above the rect annotations.

Screen.Recording.2021-08-30.at.02.13.54.PM.mp4

Please see 4c07301 for the fix thank you for catching this!

[nickofthyme]

Looks great @rshen91! Great work.

[nickofthyme]

🎉 This PR is included in version 35.0.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀