[Lens] Synchronize cursor position for X-axis across all Lens visualizations in a dashboard #106845

alexwizp · 2021-07-27T12:58:42Z

Closes: #77530

Summary

Implementation notes

The activeCursor service has been moved to the charts plugin and exposed through a contract.
TSVB, Lens, vis_type_xy and Timelion use activeCursor service from chart plugin
to reduce code duplication, use_active_cursor hook was created

Screens

Screen.Recording.2021-07-28.at.12.05.44.PM.mov

Screen.Recording.2021-07-28.at.12.09.11.PM.mov

Screen.Recording.2021-07-28.at.12.11.47.PM.mov

alexwizp · 2021-07-29T09:31:13Z

@elasticmachine merge upstream

…zations in a dashboard Closes: elastic#77530

alexwizp · 2021-07-31T09:28:19Z

@elasticmachine merge upstream

alexwizp · 2021-08-02T12:48:22Z

@elasticmachine merge upstream

elasticmachine · 2021-08-03T11:18:40Z

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

alexwizp · 2021-08-03T11:26:16Z

@elasticmachine merge upstream

dej611

Tested on Firefox 👍
Lens Code review seems fine to me.

dej611 · 2021-08-03T14:44:21Z

src/plugins/charts/public/services/active_cursor/active_cursor_utils.ts

+  const isDateHistogram =
+    Boolean(dataTables.length) &&
+    dataTables.every((dataTable) =>
+      dataTable.columns.find((c) => Boolean(c.meta.sourceParams?.appliedTimeRange))


nit: maybe .some is more suited than find here?
No big deal, in terms of perf I think they are equivalent, it's just about semantic

x-pack/plugins/lens/public/pie_visualization/render_function.tsx

stratoula · 2021-08-04T09:45:47Z

It works well for all charts, sometimes it is laggy, not sure how it will perform with big dashboards and a lot of data.
@ghudgins can you also check it and just to confirm, we decided to go without a setting to disable it?

mbondyra · 2021-08-04T10:03:13Z

@elasticmachine merge upstream

mbondyra

tested on a dashboard with 10 synchronizing visualizations on Safari. Works really well 🆗 Code LGTM!

VladLasitsa

Code LGTM!

alexwizp · 2021-08-04T13:41:23Z

It works well for all charts, sometimes it is laggy, not sure how it will perform with big dashboards and a lot of data.
@ghudgins can you also check it and just to confirm, we decided to go without a setting to disable it?

@stratoula I've added a small debounce to be sure that that PR will not be a reason of any performance issues.

stratoula

LGTM, thanx Alex :D

ghudgins · 2021-08-04T14:16:03Z

i've tested a mix of different vis types on 30 panels and profiled. it feels like it's doing what it should

nickofthyme

This is fantastic! 🎉

Ideally this would be simpler if we on the charts side had a registry of all the charts on a page. This in not in the near future so I'm glad you were able to achieve this as is.

I just have just one comment, see below 👇

nickofthyme · 2021-08-04T15:52:00Z

src/plugins/vis_type_xy/public/components/xy_settings.tsx

      xDomain={adjustedXDomain}
      rotation={rotation}
-      theme={[themeOverrides, theme]}
+      theme={[theme, themeOverrides]}


This is flipped, the lower the index the higher the priority. So in this case themeOverrides is used only when the value does not exist on the theme.

Are you running into issue with this? There may be an issue where if a value is undefined on the first object this will not be overridden by the second.

Interestingly, after the modifying, it began to work as expected. The same approach is used elsewhere, so if you're right the problem is everywhere

Yeah. Also if the theme is a full theme, (i.e. defines all available property) that means the overrides are pointless and will never be used.

kibanamachine · 2021-08-04T18:50:28Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`charts`	68	72	+4
`visTypeTimelion`	66	65	-1
`visTypeTimeseries`	538	537	-1
total			+2

Async chunks

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

id	before	after	diff
`lens`	1.6MB	1.6MB	+402.0B
`visTypeTimelion`	89.3KB	89.1KB	-198.0B
`visTypeTimeseries`	1004.1KB	1003.9KB	-154.0B
`visTypeXy`	113.6KB	113.9KB	+314.0B
total			+364.0B

Page load bundle

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

id	before	after	diff
`charts`	88.7KB	92.2KB	+3.5KB
`visTypeTimelion`	24.6KB	25.1KB	+446.0B
`visTypeTimeseries`	24.6KB	24.5KB	-20.0B
`visTypeXy`	62.1KB	62.5KB	+359.0B
total			+4.2KB

Unknown metric groups

API count

id	before	after	diff
`charts`	195	199	+4

API count missing comments

id	before	after	diff
`charts`	164	168	+4

Non-exported public API item count

id	before	after	diff
`charts`	1	3	+2

History

💚 Build #142999 succeeded 039ca4e
💔 Build #142952 failed 73d1b25
💚 Build #142892 succeeded 0dc7c71
💚 Build #142781 succeeded 3661503
💚 Build #142588 succeeded ee76dea

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

cc @alexwizp

…zations in a dashboard (elastic#106845) * [Lens] Synchronize cursor position for X-axis across all Lens visualizations in a dashboard Closes: elastic#77530 * add mocks for active_cursor service * fix jest tests * fix jest tests * apply PR comments * fix cursor style * update heatmap, jest * add tests * fix wrong import * replace cursor for timelion * update tsvb_dashboard baseline * fix CI * update baseline * Update active_cursor_utils.ts * add debounce * remove cursor from heatmap and pie * add tests for debounce * return theme order back Co-authored-by: Kibana Machine <[email protected]>

…zations in a dashboard (#106845) (#107691) * [Lens] Synchronize cursor position for X-axis across all Lens visualizations in a dashboard Closes: #77530 * add mocks for active_cursor service * fix jest tests * fix jest tests * apply PR comments * fix cursor style * update heatmap, jest * add tests * fix wrong import * replace cursor for timelion * update tsvb_dashboard baseline * fix CI * update baseline * Update active_cursor_utils.ts * add debounce * remove cursor from heatmap and pie * add tests for debounce * return theme order back Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

…zations in a dashboard (elastic#106845) * [Lens] Synchronize cursor position for X-axis across all Lens visualizations in a dashboard Closes: elastic#77530 * add mocks for active_cursor service * fix jest tests * fix jest tests * apply PR comments * fix cursor style * update heatmap, jest * add tests * fix wrong import * replace cursor for timelion * update tsvb_dashboard baseline * fix CI * update baseline * Update active_cursor_utils.ts * add debounce * remove cursor from heatmap and pie * add tests for debounce * return theme order back Co-authored-by: Kibana Machine <[email protected]>

alexwizp requested review from stratoula and VladLasitsa July 28, 2021 10:14

alexwizp self-assigned this Jul 28, 2021

alexwizp added 5 commits July 29, 2021 13:03

[Lens] Synchronize cursor position for X-axis across all Lens visuali…

3d45332

…zations in a dashboard Closes: elastic#77530

add mocks for active_cursor service

a541e6c

fix jest tests

2c1c4ad

fix jest tests

07b3688

apply PR comments

1e51546

alexwizp force-pushed the cursor branch from 02356b1 to 1e51546 Compare July 30, 2021 14:37

alexwizp added 2 commits July 30, 2021 18:11

fix cursor style

3ec0abf

update heatmap, jest

cbc53b5

alexwizp added 2 commits August 2, 2021 14:01

add tests

ba8f4df

Merge remote-tracking branch 'upstream/master' into cursor

800971b

alexwizp force-pushed the cursor branch from dd10fb1 to 800971b Compare August 2, 2021 11:07

alexwizp added 2 commits August 2, 2021 14:20

fix wrong import

429adfe

replace cursor for timelion

55cc57f

alexwizp requested a review from markov00 August 2, 2021 12:12

kibanamachine and others added 3 commits August 2, 2021 08:48

Merge branch 'master' into cursor

e87d15e

update tsvb_dashboard baseline

ab1dae5

Merge remote-tracking branch 'upstream/master' into cursor

3aec4bb

alexwizp requested a review from a team August 3, 2021 11:18

Merge branch 'master' into cursor

ee76dea

alexwizp added the Feature:Timelion Timelion app and visualization label Aug 3, 2021

stratoula requested review from mbondyra and dej611 August 3, 2021 15:50

dej611 approved these changes Aug 3, 2021

View reviewed changes

Update active_cursor_utils.ts

3661503

alexwizp commented Aug 4, 2021

View reviewed changes

x-pack/plugins/lens/public/pie_visualization/render_function.tsx Outdated Show resolved Hide resolved

Merge branch 'master' into cursor

0dc7c71

mbondyra approved these changes Aug 4, 2021

View reviewed changes

VladLasitsa approved these changes Aug 4, 2021

View reviewed changes

alexwizp added 2 commits August 4, 2021 16:33

add debounce

e8c1aa8

remove cursor from heatmap and pie

73d1b25

stratoula approved these changes Aug 4, 2021

View reviewed changes

add tests for debounce

039ca4e

nickofthyme approved these changes Aug 4, 2021

View reviewed changes

return theme order back

ac3eb6a

alexwizp merged commit f62a0a1 into elastic:master Aug 4, 2021

alexwizp mentioned this pull request Aug 4, 2021

[7.x] [Lens] Synchronize cursor position for X-axis across all Lens visualizations in a dashboard (#106845) #107691

Merged

ghudgins mentioned this pull request Sep 14, 2021

[ML] Embeddable can be "brushed" in a dashboard #112106

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] Synchronize cursor position for X-axis across all Lens visualizations in a dashboard #106845

[Lens] Synchronize cursor position for X-axis across all Lens visualizations in a dashboard #106845

alexwizp commented Jul 27, 2021 •

edited

Loading

alexwizp commented Jul 29, 2021

alexwizp commented Jul 31, 2021

alexwizp commented Aug 2, 2021

elasticmachine commented Aug 3, 2021

alexwizp commented Aug 3, 2021

dej611 left a comment

dej611 Aug 3, 2021

stratoula commented Aug 4, 2021

mbondyra commented Aug 4, 2021

mbondyra left a comment

VladLasitsa left a comment

alexwizp commented Aug 4, 2021 •

edited

Loading

stratoula left a comment

ghudgins commented Aug 4, 2021

nickofthyme left a comment

nickofthyme Aug 4, 2021

alexwizp Aug 4, 2021

nickofthyme Aug 4, 2021

kibanamachine commented Aug 4, 2021

API count

API count missing comments

Non-exported public API item count

[Lens] Synchronize cursor position for X-axis across all Lens visualizations in a dashboard #106845

[Lens] Synchronize cursor position for X-axis across all Lens visualizations in a dashboard #106845

Conversation

alexwizp commented Jul 27, 2021 • edited Loading

Summary

Implementation notes

Screens

alexwizp commented Jul 29, 2021

alexwizp commented Jul 31, 2021

alexwizp commented Aug 2, 2021

elasticmachine commented Aug 3, 2021

alexwizp commented Aug 3, 2021

dej611 left a comment

Choose a reason for hiding this comment

dej611 Aug 3, 2021

Choose a reason for hiding this comment

stratoula commented Aug 4, 2021

mbondyra commented Aug 4, 2021

mbondyra left a comment

Choose a reason for hiding this comment

VladLasitsa left a comment

Choose a reason for hiding this comment

alexwizp commented Aug 4, 2021 • edited Loading

stratoula left a comment

Choose a reason for hiding this comment

ghudgins commented Aug 4, 2021

nickofthyme left a comment

Choose a reason for hiding this comment

nickofthyme Aug 4, 2021

Choose a reason for hiding this comment

alexwizp Aug 4, 2021

Choose a reason for hiding this comment

nickofthyme Aug 4, 2021

Choose a reason for hiding this comment

kibanamachine commented Aug 4, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

API count

API count missing comments

Non-exported public API item count

History

alexwizp commented Jul 27, 2021 •

edited

Loading

alexwizp commented Aug 4, 2021 •

edited

Loading