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

[dashboard][labs] Defer loading panels below the fold #99880

Merged
merged 4 commits into from
May 18, 2021

Conversation

clintandrewhall
Copy link
Contributor

@clintandrewhall clintandrewhall commented May 12, 2021

Fixes #76686
Addresses #14750
Addresses #3903

Summary

This PR does two things:

1/ introduces Labs to Dashboard, and
2/ adds a new Lab project for deferring the rendering of any panel below "the fold", or the area beneath the bottom edge of the viewport

The goal is to evaluate the change against a number of dashboard examples. I'd also like to see this demonstrated by usability study to anyone with high-throughput dashboards to see if the perceived performance is improved.

cc: @kmartastic @AlonaNadler @ryankeairns

Screen.Recording.2021-05-11.at.11.40.22.PM.mov

Implementation

This PR uses the Intersection Observer API to defer loading of any panel until it intersects with the viewport. After being rendered, the observer is ignored.

Future Work

  • Evaluate this change with stakeholders.
  • Determine if seeing panels load as you scroll makes Kibana Dashboards "still feel slow".
  • Once @ThomThomson is able to commit the state management changes, I'd love to hook each panel up to a "render is complete" event and evaluate if we can bulk load the lower portion of the dashboard without slowing down the UI.

@clintandrewhall clintandrewhall requested review from a team as code owners May 12, 2021 05:04
@clintandrewhall clintandrewhall added auto-backport Deprecated - use backport:version if exact versions are needed Feature:Dashboard Dashboard related features impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. loe:medium Medium Level of Effort release_note:enhancement review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.14.0 v8.0.0 labels May 12, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

Telemetry changes LGTM

@ryankeairns
Copy link
Contributor

This is really great. I'm eager to test out both the Labs UI itself and the 'projects'. Kent and I were discussing a baseline evaluation of Dashboards (in prep of the aesthetic improvements), so perhaps we can lump these things together given our limited bandwidth for research, at the moment.

@AlonaNadler
Copy link

Please think about what sort of validation you need in order to turn this ON by default

@afharo
Copy link
Member

afharo commented May 13, 2021

@clintandrewhall, how would this feature work with Reporting? Could it happen that some panels are not loaded in the reports?

@clintandrewhall
Copy link
Contributor Author

clintandrewhall commented May 13, 2021

@afharo I ran tests against Labs/No Labs/master... all of the PDFs were identical, in that they were broken the same way:

  • Optimized prints have an odd layout.
  • One of the maps didn't render.

So this Observer API change had no effect on reporting.

Examples

@clintandrewhall
Copy link
Contributor Author

@elasticmachine merge upstream

@ycombinator
Copy link
Contributor

Always nice to see when super old issues finally get addressed, in this case: #3903. It usually means a lot of foundational work needed to be done first, hence the long and worthwhile wait. ❤️

@clintandrewhall
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dashboard 190 191 +1
presentationUtil 98 102 +4
total +5

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 126 128 +2
presentationUtil 96 107 +11
total +13

Async chunks

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

id before after diff
dashboard 138.7KB 139.8KB +1.1KB

Page load bundle

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

id before after diff
dashboard 313.1KB 316.0KB +2.8KB
presentationUtil 41.6KB 43.9KB +2.3KB
total +5.2KB
Unknown metric groups

API count

id before after diff
dashboard 138 140 +2
presentationUtil 98 109 +11
total +13

References to deprecated APIs

id before after diff
canvas 29 25 -4
crossClusterReplication 8 6 -2
fleet 22 20 -2
globalSearch 4 2 -2
indexManagement 12 7 -5
infra 261 149 -112
lens 67 45 -22
licensing 18 15 -3
maps 286 208 -78
ml 121 115 -6
monitoring 109 56 -53
stackAlerts 101 95 -6
total -295

History

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

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.

Tested this locally on chrome and everything works as advertised. Loading tall dashboards (especially the flights dashboard) feels much faster.

I also looked through the code, and everything is looking really clean and well structured. I especially like the dashboard_grid_item implementation.

I agree that a good next step would be to load the panels below the fold if they come into view OR the panels above the fold have all completely finished loading. I also think we could attempt to turn this on by default soon to see which functional tests it would fail.

This is really exciting stuff! LGTM!

@clintandrewhall clintandrewhall merged commit 79e7694 into elastic:master May 18, 2021
@clintandrewhall clintandrewhall deleted the dashboard/defer branch May 18, 2021 21:44
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request May 18, 2021
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request May 18, 2021
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Clint Andrew Hall <clint.hall@elastic.co>
yctercero pushed a commit to yctercero/kibana that referenced this pull request May 25, 2021
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
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Dashboard Dashboard related features impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. loe:medium Medium Level of Effort release_note:enhancement review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dashboard] Lazy load off-screen visualizations
8 participants