Skip to content

fix: annotation tooltip display when remounting specs#1167

Merged
nickofthyme merged 3 commits intoelastic:masterfrom
nickofthyme:fix-annotation-tooltip
May 27, 2021
Merged

fix: annotation tooltip display when remounting specs#1167
nickofthyme merged 3 commits intoelastic:masterfrom
nickofthyme:fix-annotation-tooltip

Conversation

@nickofthyme
Copy link
Copy Markdown
Collaborator

@nickofthyme nickofthyme commented May 26, 2021

Summary

fix #1063

Fixes issue where annotation tooltip would not render when hovering annotation marker when using useContext.

Details

The main issue here is that apm uses useContext on the onPointerUpdate listener callback. This causes the chart to rerender and re-upsert the chart specs. Doing this changes the object reference between hoveredDOMElement.datum and the datum in annotationDimensions map.

This was a check to ensure the correct tooltip is rendered but since the is of all specs must be unique, this check can be removed without concern.

This is a demo of the linked charts library with the changes included in this PR. Notice the annotation tooltip being shown in all hover cases.

Screen.Recording.2021-05-26.at.10.38.22.AM.mp4

Checklist

  • Unit tests were updated or added to match the most common scenarios

@nickofthyme nickofthyme added bug Something isn't working :annotation Annotation (line, rect, text) related issue labels May 26, 2021
@nickofthyme nickofthyme requested review from markov00 and rshen91 May 26, 2021 15:48
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.

Perfectly agree, thanks Nick for the investigation on this.
The id is used here comes from getAnnotationLinePropsId that already take in consideration the following properties:

[specId, verticalValue, horizontalValue, datum.header, datum.details, index]

and can be considered unique

@nickofthyme nickofthyme reopened this May 27, 2021
@nickofthyme nickofthyme enabled auto-merge (squash) May 27, 2021 16:44
@nickofthyme nickofthyme merged commit 8408600 into elastic:master May 27, 2021
@nickofthyme nickofthyme deleted the fix-annotation-tooltip branch May 28, 2021 02:06
nickofthyme pushed a commit that referenced this pull request Jun 4, 2021
# [30.0.0](v29.2.0...v30.0.0) (2021-06-04)

### Bug Fixes

* **domain:** custom domain should not filter data ([#1181](#1181)) ([76e8dca](76e8dca)), closes [#1129](#1129)
* **value_labels:** zero as a valid value for textBorder and borderWidth ([#1182](#1182)) ([a64f333](a64f333))
* annotation tooltip display when remounting specs ([#1167](#1167)) ([8408600](8408600))
* render nodeLabel formatted text into the nodes ([#1173](#1173)) ([b44bdff](b44bdff))

### Features

* **axis:** allow pixel domain padding for y axes  ([#1145](#1145)) ([7c1fa8e](7c1fa8e))
* apply value formatter to the default legend item label ([#1190](#1190)) ([71474a5](71474a5))
* **tooltip:** stickTo vertical middle of the cursor ([#1163](#1163)) ([380363b](380363b)), closes [#1108](#1108)
* **wordcloud:** click and over events on text ([#1180](#1180)) ([196fb6a](196fb6a)), closes [#1156](#1156)

### BREAKING CHANGES

* **value_labels:** the `textBorder` of `ValueFillDefinition` is now optional or a number only
@nickofthyme
Copy link
Copy Markdown
Collaborator Author

🎉 This PR is included in version 30.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nickofthyme nickofthyme added the released Issue released publicly label Jun 4, 2021
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
# [30.0.0](elastic/elastic-charts@v29.2.0...v30.0.0) (2021-06-04)

### Bug Fixes

* **domain:** custom domain should not filter data ([opensearch-project#1181](elastic/elastic-charts#1181)) ([92ba84c](elastic/elastic-charts@92ba84c)), closes [opensearch-project#1129](elastic/elastic-charts#1129)
* **value_labels:** zero as a valid value for textBorder and borderWidth ([#1182](elastic/elastic-charts#1182)) ([880fbf1](elastic/elastic-charts@880fbf1))
* annotation tooltip display when remounting specs ([opensearch-project#1167](elastic/elastic-charts#1167)) ([7163951](elastic/elastic-charts@7163951))
* render nodeLabel formatted text into the nodes ([opensearch-project#1173](elastic/elastic-charts#1173)) ([0de9688](elastic/elastic-charts@0de9688))

### Features

* **axis:** allow pixel domain padding for y axes  ([#1145](elastic/elastic-charts#1145)) ([6787728](elastic/elastic-charts@6787728))
* apply value formatter to the default legend item label ([opensearch-project#1190](elastic/elastic-charts#1190)) ([20108bb](elastic/elastic-charts@20108bb))
* **tooltip:** stickTo vertical middle of the cursor ([#1163](elastic/elastic-charts#1163)) ([b858fb3](elastic/elastic-charts@b858fb3)), closes [opensearch-project#1108](elastic/elastic-charts#1108)
* **wordcloud:** click and over events on text ([opensearch-project#1180](elastic/elastic-charts#1180)) ([adbf341](elastic/elastic-charts@adbf341)), closes [opensearch-project#1156](elastic/elastic-charts#1156)

### BREAKING CHANGES

* **value_labels:** the `textBorder` of `ValueFillDefinition` is now optional or a number only
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 released Issue released publicly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Annotation tooltip not being displayed.

2 participants