[ML] Migrates chart tooltip to react. #48122

walterra · 2019-10-14T15:58:30Z

Summary

Migrates the chart tooltip of Anomaly Explorer and Single Metric Viewer to use React.

The tooltip styling, code and features have been updated to be more in line with Elastic Charts tooltips. This prepares us should we want to use Elastic Charts' tooltip as is at some point.
jQuery is no longer used in mlChartTooltipService.
The tooltip content is no longer raw HTML, a mostly Elastic Charts compatible data structure is now passed around by mlChartTooltipService.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
~~Documentation was added for features that require explanation or tutorials~~
~~Unit or functional tests were updated or added to match the most common scenarios~~
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2019-10-14T15:58:33Z

Pinging @elastic/ml-ui (:ml)

elasticmachine · 2019-10-14T17:14:08Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 47417b8

x-pack/legacy/plugins/ml/public/components/chart_tooltip/chart_tooltip.tsx

darnautov · 2019-10-15T08:31:46Z

x-pack/legacy/plugins/ml/public/components/chart_tooltip/_chart_tooltip.scss

+  }
+
+  &--hidden {
+    opacity: 0;


you can try visibility property

darnautov · 2019-10-15T09:06:25Z

x-pack/legacy/plugins/ml/public/components/chart_tooltip/chart_tooltip_service.d.ts

+
+interface TooltipData {
+  name: string;
+  value: any;


string | ReactElement?

Fixed to reuse the types provided by Elastic Charts in a270966 (However, Elastic Charts treat this as any internally).

darnautov · 2019-10-15T09:08:30Z

x-pack/legacy/plugins/ml/public/components/chart_tooltip/chart_tooltip_service.js

-    clearTimeout(this.fadeTimeout);
-  }
+  // side bar width
+  const euiNavDrawer = document.getElementsByClassName('euiNavDrawer');


is it possible to use ref here instead of imperatively access the DOM?

That element is in a completely different area in Kibana we don't control. I'll leave this as-is replacement for the previous jQuery-based version in here for now.

x-pack/legacy/plugins/ml/public/explorer/explorer_charts/explorer_chart_distribution.js

elasticmachine · 2019-10-15T09:31:29Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: cfa2ab6

peteharverson · 2019-10-15T09:36:25Z

.../legacy/plugins/ml/public/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js

-      }, {
-        br: '<br />',
-        scheduledEventsValue: marker.scheduledEvents.map(mlEscape).join('<br/>')
+      tooltipData.push({


The layout for scheduled events need some tweaking I think:

Added a fix in 002ccfb to better handle multiple events within a tooltip. There's not much I can do in the case above, but the updated max-width: 512px might help.

peteharverson

Tested latest edits and all LGTM

jgowdyelastic

LGTM

elasticmachine · 2019-10-15T16:52:12Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 380a95c

- The tooltip styling, code and features have been updated to be more in line with Elastic Charts tooltips. This prepares us should we want to use Elastic Charts' tooltip as is at some point. - jQuery is no longer used in mlChartTooltipService. - The tooltip content is no longer raw HTML, a mostly Elastic Charts compatible data structure is now passed around by mlChartTooltipService.

elasticmachine · 2019-10-15T22:06:19Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 380a95c

[ml] Migrates chart tooltip to react.

47417b8

walterra added Feature:New Platform :ml Feature:Anomaly Detection ML anomaly detection v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.5.0 labels Oct 14, 2019

walterra requested review from darnautov, jgowdyelastic and peteharverson October 14, 2019 15:58

walterra requested review from a team as code owners October 14, 2019 15:58

walterra self-assigned this Oct 14, 2019

walterra added 2 commits October 15, 2019 09:38

Merge branch 'master' into ml-chart-tooltip-react

771bc7f

[ML] Fix dependencies, types.

cfa2ab6

peteharverson reviewed Oct 15, 2019

View reviewed changes

x-pack/legacy/plugins/ml/public/components/chart_tooltip/chart_tooltip.tsx Show resolved Hide resolved

peteharverson reviewed Oct 15, 2019

View reviewed changes

x-pack/legacy/plugins/ml/public/components/chart_tooltip/chart_tooltip.tsx Show resolved Hide resolved

darnautov reviewed Oct 15, 2019

View reviewed changes

peteharverson reviewed Oct 15, 2019

View reviewed changes

x-pack/legacy/plugins/ml/public/explorer/explorer_charts/explorer_chart_distribution.js Show resolved Hide resolved

peteharverson reviewed Oct 15, 2019

View reviewed changes

walterra added 5 commits October 15, 2019 16:35

[ML] Fix scheduled events.

002ccfb

[ML] Fix tooltip positioning.

52d673c

[ML] Fix tooltip max-width and word-wrap.

3972a67

[ML] Tweak types.

a270966

[ML] Fix entity lable.

380a95c

peteharverson approved these changes Oct 15, 2019

View reviewed changes

jgowdyelastic approved these changes Oct 15, 2019

View reviewed changes

walterra merged commit 0d0b38a into elastic:master Oct 15, 2019

walterra deleted the ml-chart-tooltip-react branch October 15, 2019 18:04

walterra mentioned this pull request Oct 15, 2019

[7.x] [ML] Migrates chart tooltip to react. (#48122) #48264

Merged

[ML] Migrates chart tooltip to react. #48122

[ML] Migrates chart tooltip to react. #48122

Uh oh!

Conversation

walterra commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Oct 14, 2019

Uh oh!

elasticmachine commented Oct 14, 2019

💔 Build Failed

Uh oh!

Uh oh!

Uh oh!

darnautov Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

darnautov Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

walterra Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

darnautov Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

walterra Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticmachine commented Oct 15, 2019

💚 Build Succeeded

Uh oh!

peteharverson Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

walterra Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

peteharverson left a comment

Choose a reason for hiding this comment

Uh oh!

jgowdyelastic left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Oct 15, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Oct 15, 2019

💚 Build Succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

walterra commented Oct 14, 2019 •

edited

Loading