Skip to content

fix: annotation details tooltip throwing with hooks#1949

Merged
nickofthyme merged 3 commits intoelastic:mainfrom
nickofthyme:anno-details-tooltip-w-hooks
Jan 27, 2023
Merged

fix: annotation details tooltip throwing with hooks#1949
nickofthyme merged 3 commits intoelastic:mainfrom
nickofthyme:anno-details-tooltip-w-hooks

Conversation

@nickofthyme
Copy link
Copy Markdown
Collaborator

@nickofthyme nickofthyme commented Jan 26, 2023

Summary

This fixes a bug that throws an error when react hooks are used inside of a customTooltipDetails component, under rare circumstances.

BREAKING CHANGE: The customTooltipDetails type is now passing details as props using a ComponentType.

- const OldPrompt = (details) => {
-   return <div>{details}</div>;
- };

+ const NewPrompt = (props) => {
+   return <div>{props.details}</div>;
+ };

 return (
   <Chart>
     <RectAnnotation
       id="annotation"
       dataValues={dataValues}
-      customTooltipDetails={OldPrompt}
+      customTooltipDetails={NewPrompt}
     />
   </Chart>
 );

Before

Throws error entering the LineAnnotation marker icon directly from a RectAnnotation, not from the outside.

Screen Recording 2023-01-26 at 09 45 57 AM

After

Screen Recording 2023-01-26 at 09 43 38 AM

Details

We were treating the customTooltipDetails option as a simple JSX.Element that somehow did not fully register the hooks inside when called as a function in JSX rather than a react component. The solution is to treat customTooltipDetails as a ComponentType instead.

Issues

Kibana issue elastic/kibana#149414

Checklist

  • The proper chart type label has been added (e.g. :xy, :partition)
  • The proper feature labels have been added (e.g. :interactions, :axis)
  • All related issues have been linked (i.e. closes #123, fixes #123)
  • New public API exports have been added to packages/charts/src/index.ts
  • Unit tests have been added or updated to match the most common scenarios

@nickofthyme nickofthyme added bug Something isn't working :annotation Annotation (line, rect, text) related issue :xy Bar/Line/Area chart related labels Jan 26, 2023
@nickofthyme nickofthyme requested a review from markov00 January 26, 2023 16:55
@nickofthyme nickofthyme enabled auto-merge (squash) January 26, 2023 17:50
Copy link
Copy Markdown
Collaborator

@markov00 markov00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nickofthyme nickofthyme merged commit 779b7f3 into elastic:main Jan 27, 2023
@nickofthyme nickofthyme deleted the anno-details-tooltip-w-hooks branch January 27, 2023 10:22
nickofthyme pushed a commit that referenced this pull request Jan 27, 2023
# [52.0.0](v51.3.0...v52.0.0) (2023-01-27)

### Bug Fixes

* annotation details tooltip throwing with hooks ([#1949](#1949)) ([779b7f3](779b7f3))
* **deps:** update dependency @elastic/eui to v72 ([#1914](#1914)) ([8814c80](8814c80))
* **deps:** update dependency @elastic/eui to v73 ([#1941](#1941)) ([4eeafa7](4eeafa7))

### BREAKING CHANGES

* The `customTooltipDetails` type is now passing `details` as `props` using a `ComponentType`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:annotation Annotation (line, rect, text) related issue bug Something isn't working :xy Bar/Line/Area chart related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants