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] Fast Navigation Between Dashboards #157437

Merged
merged 16 commits into from
May 25, 2023

Conversation

ThomThomson
Copy link
Contributor

@ThomThomson ThomThomson commented May 11, 2023

Summary

This PR makes all navigation from one Dashboard to another feel snappier. Instead of destroying the Dashboard container entirely and rebuilding it from scratch, this PR loads the new Dashboard saved object in the background, then reinitializes subscriptions and replaces the last Dashboard state with the new state.

Navigation

This results in the following changes:

  • On save, when the new state matches the old state exactly but the ID has changed, the Dashboard container is not destroyed and rebuilt so the screen no longer flashes white. This can be seen when:
    • Saving a new Dashboard. When the URL changes from create to /view/{id}
    • Saving a dashboard as a copy or cloning. When the URL changes from /view/{oldid} to /view/{newid}
  • On navigation between one dashboard and another the panels and Controls of the last dashboard stay visible until the new Dashboard is ready, then they are replaced in one render with the contents of the new Dashboard. This is visible when:
    • Navigating between Dashboards on drilldown.
    • Navigating between Dashboards with any navigation system - i.e. markdown and the future Navigation Embeddable.

Navigation Embeddable

This will allow our new Navigation Embeddable to move between Dashboards in a more native tab-like way, because the Navigation Embeddable itself will not disappear as the navigation happens.

A complication with this is on navigation click, the page won't immediately go white, so we may need a loading state to provide feedback to the user when they click on a nav link while the new Dashboard loads in the background. Otherwise, the delay while loading the next Dashboard could make the Navigation Embeddable seem unresponsive. This loading state could show specifically inside the Navigation Embeddable to make it impossible for a user to immediately try to click another link.

Flaky Test runner

I ran a broad Flaky test runner to ensure that this PR didn't introduce any extra flakiness. Note that it ran into 2 failures out of 400 test runs, and that these two failures seem unrelated. I added a commit: 0276c77 that should resolve them by making the presence of the toast optional when adding panels from the library - as requiring toasts to be present can be quite flaky.

Checklist

Delete any items that are not applicable to this PR.

@ThomThomson ThomThomson changed the title Navigate between dashboards without destroying dashboard container [Dashboard] Fast Navigation Between Dashboards May 16, 2023
@ThomThomson ThomThomson added Feature:Dashboard Dashboard related features Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:medium Medium Level of Effort impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. release_note:enhancement labels May 16, 2023
@ThomThomson
Copy link
Contributor Author

@elasticmachine merge upstream

@ThomThomson
Copy link
Contributor Author

@elasticmachine merge upstream

@ThomThomson ThomThomson marked this pull request as ready for review May 18, 2023 13:34
@ThomThomson ThomThomson requested review from a team as code owners May 18, 2023 13:34
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@angorayc angorayc left a comment

Choose a reason for hiding this comment

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

Thanks for the enhancement!

@cqliu1 cqliu1 self-requested a review May 22, 2023 19:22
Comment on lines -211 to -214
untilDashboardReady().then(async (container) => {
container.setScrollToPanelId(incomingEmbeddable.embeddableId);
container.setHighlightPanelId(incomingEmbeddable.embeddableId);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was the scrolling behavior removed here for incoming embeddables? Maybe got lost in a merge conflict?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah true, good catch! While fixing this, I also noticed that the code didn't scroll to a newly added panel, only one that was edited, so I added some extra logic to make it scroll to newly added panels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 6120d3f

@ThomThomson
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@cqliu1 cqliu1 left a comment

Choose a reason for hiding this comment

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

LGTM! I pulled it down and navigated between the sample dashboards in various different states, and things like view mode, filters, and unsaved changes are preserved between reloads.

The only thing I noticed was some of the panels flying around with the CSS transforms on while navigating between dashboards looked a little odd, but it's not a blocker.

May-23-2023.16-13-21.mp4

Besides that, the navigation between dashboard is so seamless with this change. Huge performance win and great prep for the navigation embeddable. Great work!

@ThomThomson
Copy link
Contributor Author

@cqliu1 thanks for the review! Can you test out 5e2b47e and let me know if this solves the problem of panels animating while Navigating?

In a followup, we'll also need to figure out how to indicate that the next Dashboard is loading up in the background.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dashboard 498 372 -126

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
controls 297 299 +2

Async chunks

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

id before after diff
controls 188.1KB 188.4KB +314.0B
dashboard 436.4KB 355.5KB -80.9KB
securitySolution 9.2MB 9.2MB +9.0B
total -80.6KB

Page load bundle

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

id before after diff
dashboard 39.8KB 39.7KB -41.0B
Unknown metric groups

API count

id before after diff
controls 304 306 +2

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 399 403 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 479 483 +4
total +6

History

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

@ThomThomson ThomThomson merged commit 5342563 into elastic:main May 25, 2023
@kibanamachine kibanamachine added v8.9.0 backport:skip This commit does not require backporting labels May 25, 2023
@ThomThomson
Copy link
Contributor Author

Backporting this into 8.8 to support this fix also being backported.

@ThomThomson
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

ThomThomson added a commit to ThomThomson/kibana that referenced this pull request Jun 15, 2023
## Summary
Makes all navigation from one Dashboard to another feel snappier.

(cherry picked from commit 5342563)
ThomThomson added a commit that referenced this pull request Jun 16, 2023
# Backport

This will backport the following commits from `main` to `8.8`:
- [[Dashboard] Fast Navigation Between Dashboards
(#157437)](#157437)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Devon
Thomson","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-25T18:40:48Z","message":"[Dashboard]
Fast Navigation Between Dashboards (#157437)\n\n## Summary\r\nMakes all
navigation from one Dashboard to another feel
snappier.","sha":"5342563a22a54223180a727a542a7b6c99caea7f","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","Feature:Dashboard","Team:Presentation","loe:days","impact:medium","backport:skip","v8.8.0","v8.9.0"],"number":157437,"url":"https://github.com/elastic/kibana/pull/157437","mergeCommit":{"message":"[Dashboard]
Fast Navigation Between Dashboards (#157437)\n\n## Summary\r\nMakes all
navigation from one Dashboard to another feel
snappier.","sha":"5342563a22a54223180a727a542a7b6c99caea7f"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/157437","number":157437,"mergeCommit":{"message":"[Dashboard]
Fast Navigation Between Dashboards (#157437)\n\n## Summary\r\nMakes all
navigation from one Dashboard to another feel
snappier.","sha":"5342563a22a54223180a727a542a7b6c99caea7f"}}]}]
BACKPORT-->

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Patryk Kopyciński <[email protected]>
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:Dashboard Dashboard related features impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:medium Medium Level of Effort release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.8.0 v8.8.2 v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants