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

[Visualize][Lens] Sync panels tooltips on dashboard level #130449

Merged
merged 8 commits into from
May 3, 2022

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Apr 18, 2022

Summary

Closes #113719

This PR adds an extra option on the dashboard settings page. It is false by default. When it is enabled, all the panels that support it are synchronizing and the tooltip is displayed when the user hovers over a chart.

The feature is supported on:

  • Lens XY
  • Viz editor XY (only for charts that have the Show detailed tooltip switch off)
  • Timelion (new implementation)
  • TSVB timeseries

sync

Checklist

Delete any items that are not applicable to this PR.

@stratoula stratoula changed the title [Vizualize][Lens] Sync tooltips on dashboard level [Visualize][Lens] Sync tooltips on dashboard level Apr 18, 2022
@stratoula stratoula added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Vis Editor Visualization editor issues Feature:Lens v8.3.0 release_note:enhancement backport:skip This commit does not require backporting labels Apr 20, 2022
@stratoula stratoula marked this pull request as ready for review April 20, 2022 07:25
@stratoula stratoula requested review from a team as code owners April 20, 2022 07:25
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

@stratoula stratoula changed the title [Visualize][Lens] Sync tooltips on dashboard level [Visualize][Lens] Sync panels tooltips on dashboard level Apr 20, 2022
@flash1293
Copy link
Contributor

@ppisljar I think it's fine for this PR but maybe we should think about another approach to handle this kind of options. We are looking into the direction of pulling more configuration options to the dashboard level (instead of having them defined per panel) and the current approach of how these options are passed through the layers doesn't scale well:

  • dashboard state
  • embeddable input
  • expression context
  • expression renderer

@botelastic botelastic bot added Feature:Embedding Embedding content via iFrame Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) labels Apr 21, 2022
@flash1293
Copy link
Contributor

Doesn't work for me for agg based and timelion - am I doing something wrong?
Screenshot 2022-04-27 at 10 23 31

@stratoula
Copy link
Contributor Author

@flash1293 as I mention on the description for agg based, it works only for charts that have the Show detailed tooltip switch off.
About timelion I dont know, it works for me 🤔 What spec are you using?

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

ACK on agg based, that works as expected.

For timelion - I found the problem. It's happening because timelion does not respect the configured time zone and in my case it was producing different non-matching buckets. I will open a separate issue for that.

Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

I very much agree with @flash1293 that we need an easier way to store options like this at the dashboard level, but it'll likely be on us to get this all straightened out.

In the meantime, this is exactly the way adding a new option to the dashboard should be done. LGTM!

Code only review.

Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

app services code LGTM

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
dashboard 140 141 +1
expressions 1713 1718 +5
total +6

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
canvas 1.0MB 1.0MB +29.0B
dashboard 301.4KB 302.2KB +756.0B
expressionXY 43.3KB 43.4KB +91.0B
lens 1.1MB 1.1MB +58.0B
visTypeTimelion 120.9KB 121.0KB +43.0B
visTypeTimeseries 465.2KB 465.3KB +104.0B
visTypeXy 60.2KB 60.3KB +138.0B
visualizations 165.6KB 165.8KB +205.0B
total +1.4KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dashboard 65.8KB 65.9KB +32.0B
embeddable 65.8KB 65.9KB +30.0B
expressions 98.7KB 98.9KB +221.0B
expressionXY 23.9KB 24.0KB +39.0B
visTypeTimelion 10.0KB 10.1KB +114.0B
visTypeTimeseries 16.4KB 16.5KB +118.0B
visTypeXy 43.0KB 43.2KB +146.0B
total +700.0B
Unknown metric groups

API count

id before after diff
dashboard 142 143 +1
expressions 2158 2166 +8
total +9

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Embedding Embedding content via iFrame Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) Feature:Lens Feature:Vis Editor Visualization editor issues release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Lens] Synchronize cursor Popup for X-axis across all Lens visualizations in a dashboard
6 participants