[Maps] Enable ability to provide custom renderer for tooltip #46150

nreese · 2019-09-19T15:28:27Z

This PR enhances factory.createFromState method with a renderTooltipContent parameter. When set, renderTooltipContent is used to render the tooltip content.

/**
 * Render custom tooltip content
 *
 * @param {function} addFilters
 * @param {function} closeTooltip
 * @param {Array} features - Vector features at tooltip location.
 * @param {boolean} isLocked
 * @param {function} loadFeatureProperties - Loads feature properties. Call with { layerId, featureId }. Returns Promise.
 *
 * @return {Component} A React Component.
 */
const renderTooltipContent = ({ addFilters, closeTooltip, features, isLocked, loadFeatureProperties}) => {
  return <div>Custom tooltip content</div>;
}

const mapEmbeddable = await factory.createFromState(state, input, parent, renderTooltipContent);

PR also adds unit tests for TooltipControl component

cc @spong

elasticmachine · 2019-09-19T15:28:29Z

Pinging @elastic/kibana-gis

spong · 2019-09-19T15:57:33Z

This may've been leftover from the previous implementation, but looks like the margins/centering of the loading spinner are a little off.

Hover / Click

elasticmachine · 2019-09-19T17:12:46Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-19T19:56:15Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-19T20:42:00Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…p_renderer

spong · 2019-09-23T16:46:30Z

Would it be possible to get access to findLayerById as well? Without this, to display all available layers we would have to call loadFeatureProperties() for each unique layerId to get _indexPattern.title off of the feature props.

kibana/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js

Line 284 in abc239d

findLayerById={this._findLayerById}

nreese · 2019-09-23T16:52:09Z

Would it be possible to get access to findLayerById as well? Without this, to display all available layers we would have to call loadFeatureProperties() for each unique layerId to get _indexPattern.title off of the feature props.

Would you just want a method to get layer name? Not sure we want to expose the entire layer API outside of maps application?

elasticmachine · 2019-09-23T17:57:21Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-24T22:06:36Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-25T01:05:54Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

kindsun · 2019-09-27T14:48:03Z

...ck/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js

+      getLayerName: this._getLayerName,
+    };
+
+    if (this.props.renderTooltipContent) {


Small thing, but I wonder if we should consider setting default props to false on renderTooltipContent

I do not understand what you mean. Could you elaborate?

Actually null would be more appropriate here. I think it just feels strange to rely on a prop that's undefined embedded several components down without having a default value. My first thought was, "we're relying on this value, what's the default if not overridden?" But yeah, undefined evaluates to false so it doesn't really make a difference.

kindsun

lgtm. pulled down the code and mostly just reviewed programatically since it's designed to be leveraged by other apps. It'd be nice to see some functional tests but I think that onus falls on the leveraging apps (like SIEM) if they think it'd be beneficial.

code review

…p_renderer

elasticmachine · 2019-09-27T18:29:54Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…#46150) * [Maps] Enable ability to provide custom renderer for tooltip * add unit test for on click handler * unit tests for mouseout behavior * add unit tests for on map move * clean up test * pass getLayerName to custom tooltip renderer * fix jest test

…#46843) * [Maps] Enable ability to provide custom renderer for tooltip * add unit test for on click handler * unit tests for mouseout behavior * add unit tests for on map move * clean up test * pass getLayerName to custom tooltip renderer * fix jest test

[Maps] Enable ability to provide custom renderer for tooltip

abc239d

nreese added WIP Work in progress [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.5.0 labels Sep 19, 2019

nreese added 4 commits September 19, 2019 12:02

add unit test for on click handler

87a571d

unit tests for mouseout behavior

d97996f

add unit tests for on map move

5cad844

clean up test

d5e88d4

spong mentioned this pull request Sep 20, 2019

[SIEM] Create custom tooltip for embedded map #46301

Closed

Merge branch 'master' of github.com:elastic/kibana into custom_toolti…

a77adb0

…p_renderer

nreese added 2 commits September 24, 2019 13:52

merge with master

c554df4

pass getLayerName to custom tooltip renderer

bcebb60

nreese requested a review from kindsun September 24, 2019 20:06

fix jest test

b8854c2

nreese removed the WIP Work in progress label Sep 26, 2019

kindsun reviewed Sep 27, 2019

View reviewed changes

kindsun approved these changes Sep 27, 2019

View reviewed changes

Merge branch 'master' of github.com:elastic/kibana into custom_toolti…

7eeec9f

…p_renderer

nreese merged commit c3f4f5d into elastic:master Sep 27, 2019

nreese mentioned this pull request Sep 27, 2019

[7.x] [Maps] Enable ability to provide custom renderer for tooltip (#46150) #46843

Merged

spong mentioned this pull request Sep 30, 2019

[SIEM] [Maps] Maps features for 7.5 #45281

Closed

spong mentioned this pull request Oct 24, 2019

[Maps] [Embeddables] Provide ability to retrieve number of features for a given layer #49236

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Maps] Enable ability to provide custom renderer for tooltip #46150

[Maps] Enable ability to provide custom renderer for tooltip #46150

nreese commented Sep 19, 2019 •

edited

Loading

elasticmachine commented Sep 19, 2019

spong commented Sep 19, 2019

elasticmachine commented Sep 19, 2019

elasticmachine commented Sep 19, 2019

elasticmachine commented Sep 19, 2019

spong commented Sep 23, 2019

nreese commented Sep 23, 2019

elasticmachine commented Sep 23, 2019

elasticmachine commented Sep 24, 2019

elasticmachine commented Sep 25, 2019

kindsun Sep 27, 2019

nreese Sep 27, 2019

kindsun Sep 27, 2019

kindsun left a comment

elasticmachine commented Sep 27, 2019

[Maps] Enable ability to provide custom renderer for tooltip #46150

[Maps] Enable ability to provide custom renderer for tooltip #46150

Conversation

nreese commented Sep 19, 2019 • edited Loading

elasticmachine commented Sep 19, 2019

spong commented Sep 19, 2019

Hover / Click

elasticmachine commented Sep 19, 2019

💔 Build Failed

elasticmachine commented Sep 19, 2019

💚 Build Succeeded

elasticmachine commented Sep 19, 2019

💚 Build Succeeded

spong commented Sep 23, 2019

nreese commented Sep 23, 2019

elasticmachine commented Sep 23, 2019

💚 Build Succeeded

elasticmachine commented Sep 24, 2019

💔 Build Failed

elasticmachine commented Sep 25, 2019

💚 Build Succeeded

kindsun Sep 27, 2019

Choose a reason for hiding this comment

nreese Sep 27, 2019

Choose a reason for hiding this comment

kindsun Sep 27, 2019

Choose a reason for hiding this comment

kindsun left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 27, 2019

💚 Build Succeeded

nreese commented Sep 19, 2019 •

edited

Loading