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

[TSVB] Show tooltip on external pointer events #77306

Merged
merged 4 commits into from
Oct 8, 2020

Conversation

sulemanof
Copy link
Contributor

@sulemanof sulemanof commented Sep 14, 2020

Summary

Show tooltip on external pointer events in TSVB chart.

Fixes #76954

The regression was caused by this change in elastic-charts.

But with the change, TSVB charts do not work the same way as in 7.8:

  • prior to 7.9 only the Vertical cursor parallel to x axis where shown in external chart
    image

  • now the whole tooltip is visible:
    tsvb_tooltip

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@sulemanof sulemanof changed the title [TSVB] Show external pointer events [TSVB] Show tooltip on external pointer events Sep 14, 2020
@sulemanof
Copy link
Contributor Author

sulemanof commented Sep 14, 2020

I would kindly ask @markov00 & @nickofthyme to assist here.
Is there any way to achieve the same behavior as in previous kibana versions?

@nickofthyme
Copy link
Contributor

Looking to see if this changed on our side.

@nickofthyme
Copy link
Contributor

@sulemanof Yeah looks like the visible: false doesn't show any tooltip rather that showing the highlighted geometry.

Let me put up a PR to fix that.

@nickofthyme
Copy link
Contributor

@elasticmachine merge upstream

Copy link
Contributor

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

@sulemanof sorry for the delay, this should be good to go with the changes from elastic/elastic-charts#817.

@@ -139,6 +140,7 @@ export const TimeSeries = ({
type: tooltipMode === 'show_focused' ? TooltipType.Follow : TooltipType.VerticalCursor,
headerFormatter: tooltipFormatter,
}}
externalPointerEvents={{ tooltip: { visible: true } }}
Copy link
Contributor

Choose a reason for hiding this comment

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

This option now only controls the tooltip visibility. The crosshair is always shown when using dispatchExternalPointerEvent.

Suggested change
externalPointerEvents={{ tooltip: { visible: true } }}
externalPointerEvents={{ tooltip: { visible: false } }}

@sulemanof sulemanof added bug Fixes for quality problems that affect the customer experience Feature:TSVB TSVB (Time Series Visual Builder) release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.10.0 v7.11.0 v8.0.0 labels Oct 7, 2020
Copy link
Contributor

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

LGTM, works as expected.

Screen Recording 2020-10-07 at 09 07 AM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id before after diff
visTypeTimeseries 1.8MB 1.8MB +48.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@sulemanof sulemanof marked this pull request as ready for review October 7, 2020 15:11
@sulemanof sulemanof requested a review from a team October 7, 2020 15:11
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Nice! Tested it locally, regression is gone! Thanx @sulemanof and @nickofthyme for taking care of this ❤️

@sulemanof sulemanof merged commit 7c1e809 into elastic:master Oct 8, 2020
@sulemanof sulemanof deleted the fix/sync_tsvb_tooltip branch October 8, 2020 07:58
sulemanof added a commit to sulemanof/kibana that referenced this pull request Oct 8, 2020
* Show external pointer events in TSVB

* Disable tooltip visibility

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
sulemanof added a commit to sulemanof/kibana that referenced this pull request Oct 8, 2020
* Show external pointer events in TSVB

* Disable tooltip visibility

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@timroes
Copy link
Contributor

timroes commented Oct 8, 2020

@sulemanof @nickofthyme do we have the appropriate elastic charts version on 7.9 to backport this to 7.9? If so I think this should be backported to make it into 7.9.3.

@sulemanof
Copy link
Contributor Author

@sulemanof @nickofthyme do we have the appropriate elastic charts version on 7.9 to backport this to 7.9? If so I think this should be backported to make it into 7.9.3.

We don't have such a version in 7.9 unfortunately..
@nickofthyme do you have any plan to update charts version in 7.9 ?

sulemanof added a commit that referenced this pull request Oct 8, 2020
* Show external pointer events in TSVB

* Disable tooltip visibility

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
sulemanof added a commit that referenced this pull request Oct 8, 2020
* Show external pointer events in TSVB

* Disable tooltip visibility

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 8, 2020
* master: (217 commits)
  Fix dashboard "snapshot share" is not sharing panel state in view mode (elastic#79837)
  fix can't edit a scripted field with special char (elastic#79842)
  [ML] clear selection action (elastic#79834)
  [TSVB] Show tooltip on external pointer events (elastic#77306)
  Fixes bug where the same index was being passed in (elastic#79949)
  Adds date time query and return fields for timestamps and overrides (elastic#79911)
  [Security Solution][Detections] Reverts rules table tag filter to use AND operator (elastic#79920)
  add the correct class to truncate the names (elastic#79921)
  [kbn/optimizer] report limits with ci metrics (elastic#78205)
  [release notes] extract "dev docs" comment too (elastic#79351)
  Revert "skips test failing promotion (elastic#79777)" (elastic#79904)
  share tslib across bundles (elastic#79915)
  remove entire suite as partial skips aren't doing the trick
  skip flaky suite (elastic#78689)
  Skip failing suite (elastic#79522)
  skip flaky suite (elastic#79910)
  [es/mappings] remove doc_values from text fields (elastic#79869)
  remove skipped snapshots
  skip flaky tests (elastic#79891)
  chore(NA): add missing branches into backportrc configuration file (elastic#79848)
  ...
@nickofthyme
Copy link
Contributor

@sulemanof created backport with cherry-pick'd fix in @elastic/charts@19.8.3 #80016

sulemanof added a commit that referenced this pull request Oct 9, 2020
* [TSVB] Show tooltip on external pointer events (#77306)

* Show external pointer events in TSVB

* Disable tooltip visibility

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* update elastic/charts to v19.8.3

Co-authored-by: Daniil Suleiman <31325372+sulemanof@users.noreply.github.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported bug Fixes for quality problems that affect the customer experience Feature:TSVB TSVB (Time Series Visual Builder) release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.9.3 v7.10.0 v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TSVB visualization doesn't sync hover line correctly
6 participants