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

[Dashboard] Fixes Copy to dashboard includes filters. #136275

Merged
merged 17 commits into from
Jul 19, 2022
Merged

[Dashboard] Fixes Copy to dashboard includes filters. #136275

merged 17 commits into from
Jul 19, 2022

Conversation

Kuznietsov
Copy link
Contributor

@Kuznietsov Kuznietsov commented Jul 13, 2022 •
edited
Loading

Summary

Fixes: #122729.

Screen.Recording.2022-07-13.at.16.44.13.mov

@Kuznietsov Kuznietsov added Feature:Dashboard Dashboard related features release_note:fix Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:small Small Level of Effort impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. backport:skip This commit does not require backporting NeededFor:Dashboard v8.4.0 labels Jul 13, 2022
@Kuznietsov Kuznietsov self-assigned this Jul 13, 2022
ThomThomson
ThomThomson approved these changes Jul 13, 2022
View reviewed changes
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 fix locally, and everything LGTM! Looks like the usage of filterManager.getFilters() was meant to support pinned filters, but they still work great after this change.

src/plugins/dashboard/public/application/actions/copy_to_dashboard_modal.tsx Outdated Show resolved Hide resolved
@Kuznietsov Kuznietsov marked this pull request as ready for review July 13, 2022 16:33
@Kuznietsov Kuznietsov requested a review from a team as a code owner July 13, 2022 16:33
@elasticmachine
Copy link
Contributor

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

@Kuznietsov Kuznietsov enabled auto-merge (squash) July 13, 2022 16:51
@Kuznietsov Kuznietsov disabled auto-merge July 13, 2022 18:28
@Kuznietsov Kuznietsov requested a review from a team as a code owner July 14, 2022 14:49
@Kuznietsov Kuznietsov requested review from ThomThomson and removed request for a team July 14, 2022 18:47
@Kuznietsov
Copy link
Contributor Author

@elasticmachine merge upstream

@Kuznietsov Kuznietsov added the WIP Work in progress label Jul 15, 2022
@Kuznietsov
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

expected head sha didn’t match current head ref.

ThomThomson
ThomThomson approved these changes Jul 18, 2022
View reviewed changes
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 and everything looks good, it's also passing CI which is good because there is a lot of coverage in terms of dashboards with pinned and non pinned filters. Left one nit

src/plugins/dashboard/public/application/lib/convert_dashboard_state.ts Outdated Show resolved Hide resolved
@kibana-ci
Copy link
Collaborator

💚 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
dashboard 380.2KB 380.4KB +189.0B

History

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

cc @Kunzetsov

@Kuznietsov Kuznietsov removed the WIP Work in progress label Jul 19, 2022
@Kuznietsov Kuznietsov merged commit 568e631 into elastic:main Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexwizp alexwizp alexwizp approved these changes

@ThomThomson ThomThomson ThomThomson approved these changes

Assignees

@Kuznietsov Kuznietsov

Labels
backport:skip This commit does not require backporting Feature:Dashboard Dashboard related features impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort NeededFor:Dashboard release_note:fix Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.4.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Dashboard] Copy to dashboard includes filters
6 participants
@Kuznietsov @elasticmachine @kibanamachine @kibana-ci @ThomThomson @alexwizp