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

[Lens] Removing updateVisualizationState updater #113779

Merged
merged 28 commits into from
Nov 3, 2021
Merged

[Lens] Removing updateVisualizationState updater #113779

merged 28 commits into from
Nov 3, 2021

Conversation

mbondyra
Copy link
Contributor

@mbondyra mbondyra commented Oct 4, 2021 •
edited
Loading

Summary

  • refactors generic updateState function with updaters into three separate setLayerDefaultDimension, removeOrClearLayer and addLayer.
  • instead of few actions in chart_switch (removeLayers for datasource and then updateVisualizationState for each vis layer) , we run removeLayers.
  • replaces non-serializable updateLayer with insertLayer
  • Adds separate EditVisualizationAction reducer to replace updateVisualizationState

Test changes:

  • dividing mocks file into few for readability

@mbondyra mbondyra added Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Lens auto-backport Deprecated - use backport:version if exact versions are needed v7.16.0 labels Oct 4, 2021
@mbondyra mbondyra force-pushed the lens/visualization_updater_removed branch 3 times, most recently from d2a9a5b to cd0ab56 Compare October 7, 2021 08:07
@mbondyra mbondyra force-pushed the lens/visualization_updater_removed branch 2 times, most recently from 9d510d3 to 6e0f7b5 Compare October 13, 2021 13:53
@mbondyra mbondyra force-pushed the lens/visualization_updater_removed branch from 6e0f7b5 to 50ee07b Compare October 14, 2021 11:11
@mbondyra mbondyra marked this pull request as ready for review October 14, 2021 11:11
dej611
dej611 reviewed Oct 14, 2021
View reviewed changes
Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Can I merge upstream here to test some changes on the dimension_editor in thresholds? They were affected by some updateState` special trick and would like to test with the new implementation.

x-pack/plugins/lens/public/state_management/index.ts Outdated Show resolved Hide resolved
@dej611
Copy link
Contributor

dej611 commented Oct 14, 2021

@elasticmachine merge upstream

dej611
dej611 approved these changes Oct 18, 2021
View reviewed changes
Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Tested locally and could not break it 👍

Will approve now even if the PR will be merged later on.

@mbondyra
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
lens 834 833 -1

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.0MB 1.0MB +1.8KB

History

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

@mbondyra
Copy link
Contributor Author

@elasticmachine merge upstream

@mbondyra mbondyra mentioned this pull request Oct 21, 2021
Merged
@mbondyra
Copy link
Contributor Author

@elasticmachine merge upstream

@mbondyra
Copy link
Contributor Author

mbondyra commented Nov 2, 2021

@elasticmachine merge upstream

@mbondyra mbondyra added backport:skip This commit does not require backporting and removed auto-backport Deprecated - use backport:version if exact versions are needed labels Nov 2, 2021
@mbondyra
Copy link
Contributor Author

mbondyra commented Nov 3, 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
lens 674 673 -1

Async chunks

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

id before after diff
lens 953.9KB 955.7KB +1.8KB

History

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

@mbondyra mbondyra merged commit 2ad1f91 into elastic:main Nov 3, 2021
@mbondyra mbondyra deleted the lens/visualization_updater_removed branch November 3, 2021 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dej611 dej611 dej611 approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mbondyra @elasticmachine @dej611 @kibanamachine