Skip to content

Comments

[Discover] Inline state modifying function to react#89543

Merged
kertal merged 15 commits intoelastic:masterfrom
kertal:kertal-pr-2021-01-28-deangularize-state-functions
Feb 8, 2021
Merged

[Discover] Inline state modifying function to react#89543
kertal merged 15 commits intoelastic:masterfrom
kertal:kertal-pr-2021-01-28-deangularize-state-functions

Conversation

@kertal
Copy link
Member

@kertal kertal commented Jan 28, 2021

Summary

This PR moves several function from discover angular controller to react components. Improving memoizing and therefore improving performance.

Part of #86166

Checklist

@kertal kertal added the Feature:Discover Discover Application label Jan 28, 2021
@kertal kertal self-assigned this Jan 28, 2021
@kertal kertal mentioned this pull request Feb 3, 2021
46 tasks
@kertal kertal marked this pull request as ready for review February 4, 2021 11:01
@kertal kertal requested review from a team and majagrubic February 4, 2021 11:01
@kertal kertal added release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v8.0.0 Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// labels Feb 4, 2021
@elasticmachine
Copy link
Contributor

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

@kertal
Copy link
Member Author

kertal commented Feb 8, 2021

@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
discover 337 339 +2

Async chunks

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

id before after diff
discover 418.1KB 420.2KB +2.1KB

History

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

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 this in Chrome on Mac OSX for basic Discover functionality:

  1. adding/removing columns
  2. switching index patterns
  3. saving a search
  4. adding a search to dashboard
  5. zooming in on histogram
  6. adding/removing filters

All works as expected. I didn't dig too deep into code changes, as it's a lot of moving code around.

return Promise.resolve();
};

$scope.setSortOrder = function setSortOrder(sort) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

hits="hits"
index-pattern="indexPattern"
minimum-visible-rows="minimumVisibleRows"
on-add-column="addColumn"
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

@kertal kertal merged commit 3d23733 into elastic:master Feb 8, 2021
kertal added a commit to kertal/kibana that referenced this pull request Feb 8, 2021
@kertal kertal deleted the kertal-pr-2021-01-28-deangularize-state-functions branch February 9, 2021 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.12.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants