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.

Sign up for GitHub

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

Jump to bottom

[Discover][Main] Improve state related code #102028

Merged
merged 10 commits into from
Jun 22, 2021
Merged

[Discover][Main] Improve state related code #102028

merged 10 commits into from
Jun 22, 2021

Conversation

kertal
Copy link
Member

@kertal kertal commented Jun 14, 2021 •
edited
Loading

Summary

This PR further centralizes state management in Discover Main.

  • Declaration of Discover Layout component code taking care of index pattern and query change moved to used_discover_state hook, no more need for searchSessionManager here
  • When discover:searchOnPageLoad is set to Off this now also works when switching index patterns (it started searching before this PR)
  • Initializing state syncing logic was migrated to discover_state.ts
  • useDiscoverState hook now uses useSearchSession and useSavedSearchData hook
  • Other minor cleanups

Checklist

Delete any items that are not applicable to this PR.

@kertal kertal self-assigned this Jun 14, 2021
@kertal kertal added the Feature:Discover Discover Application label Jun 14, 2021
@kertal
Copy link
Member Author

kertal commented Jun 14, 2021

@elasticmachine merge upstream

@kertal kertal marked this pull request as ready for review June 15, 2021 08:28
@kertal kertal requested a review from a team June 15, 2021 08:28
@kertal kertal added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Jun 15, 2021
@elasticmachine
Copy link
Contributor

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

@kertal kertal added release_note:skip Skip the PR/issue when compiling release notes v7.14.0 v8.0.0 labels Jun 15, 2021
@kertal kertal requested review from majagrubic and timroes June 15, 2021 10:35
@kertal kertal added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 16, 2021
@kertal
Copy link
Member Author

kertal commented Jun 16, 2021

@elasticmachine merge upstream

@kertal kertal requested a review from dimaanj June 18, 2021 06:30
@kertal
Copy link
Member Author

kertal commented Jun 21, 2021

@elasticmachine merge upstream

majagrubic
majagrubic approved these changes Jun 21, 2021
View reviewed changes
Copy link
Contributor

@majagrubic majagrubic left a comment

Choose a reason for hiding this comment

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

Tested in Chrome on Mac OSX, looks good. Great improvement!

@kertal
Copy link
Member Author

kertal commented Jun 22, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
discover 430.4KB 430.3KB -105.0B

History

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

cc @kertal

@kertal kertal merged commit 21f6a1b into elastic:master Jun 22, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 22, 2021
@kertal @kibanamachine
[Discover][Main] Improve state related code (elastic#102028)
65938f0
@kibanamachine kibanamachine mentioned this pull request Jun 22, 2021
Merged
@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 Jun 22, 2021
@kibanamachine @kertal
[Discover][Main] Improve state related code (#102028) (#102959)
11dec59 
Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@majagrubic majagrubic majagrubic approved these changes

@timroes timroes Awaiting requested review from timroes

@dimaanj dimaanj Awaiting requested review from dimaanj

Assignees

@kertal kertal

Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.14.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kertal @elasticmachine @kibanamachine @majagrubic