Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Maps] Add filter actions to tooltips #33635

Merged
merged 29 commits into from
Apr 8, 2019

Conversation

thomasneirynck
Copy link
Contributor

@thomasneirynck thomasneirynck commented Mar 21, 2019

Closes #32736

This PR contains:

Refactors

  • creates typing for individual tooltip-properties

Changes/additions

  • use grid layout for tooltips
  • show filter actions only in read-only mode (ie. when embedded in dashboard)
  • add phrase filtering for any string property on any ES-source
  • add phrase filtering for any property that is joined to other ES-sources (e.g. EMS layer)

Known issues:

  • joined data on the map does not update based on changing filters (regardless if the filter is sent from the map, or from another visualization)

Other todos:

  • add functional tests for tooltips use jest tests for contents
  • detect presence of filter-bar iso using isReadOnly embeddables are always filterable right now
  • design (it needs it!)
    • do not show close button when hovering
    • do not show filters when hovering
    • change to + icon for filter creation button
    • change mouse-pointer when tooltip is "lockable" on click

Discuss:

  • should the map take "ownership" of filters (similar to the input controls visualization) (?) NO. maybe in another phase
  • should tooltips remain on the map after filtering (?) if so, what would be the best mechanism to introduce this (e.g. track doc-ids across data-updates. (?) NO. tackle this as separate improvement. right now, tooltips are removed after filtering

image

@thomasneirynck thomasneirynck added WIP Work in progress [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.2.0 labels Mar 21, 2019
@thomasneirynck thomasneirynck requested a review from a team as a code owner March 21, 2019 03:40
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@thomasneirynck thomasneirynck removed the WIP Work in progress label Mar 29, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

for (let i = 0; i < this._joins.length; i++) {
const propsFromJoin = await this._joins[i].filterAndFormatPropertiesForTooltip(properties);
Object.assign(tooltipsFromSource, propsFromJoin);
allTooltips = [...allTooltips, ...propsFromJoin];
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of recreating the array on each iteration, how about something more like this?

  async getPropertiesForTooltip(properties) {
    const sourceTooltipProperties =  await this._source.filterAndFormatPropertiesToHtml(properties);
    this._addJoinsToSourceTooltips(sourceTooltipProperties);

    // tooltip properties added by joins. For example, aggregation metrics
    const joinTooltipProperties = [];
    for (let i = 0; i < this._joins.length; i++) {
      const propsFromJoin = await this._joins[i].filterAndFormatPropertiesForTooltip(properties);
      joinTooltipProperties.push(...propsFromJoin);
    }
    return [...sourceTooltipProperties, ...joinTooltipProperties];
  }

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

lgtm
code review, tested in chrome

@thomasneirynck thomasneirynck merged commit ae977f8 into elastic:master Apr 8, 2019
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Apr 8, 2019
chandlerprall pushed a commit to chandlerprall/kibana that referenced this pull request Apr 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maps] Improve tooltip menus
4 participants