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

fixing multiple metrics #62929

Merged
merged 2 commits into from
Apr 8, 2020
Merged

Conversation

ppisljar
Copy link
Member

@ppisljar ppisljar commented Apr 8, 2020

Summary

resolves #62771

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@ppisljar ppisljar added review v8.0.0 Team:AppArch release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Apr 8, 2020
@ppisljar ppisljar requested a review from a team April 8, 2020 11:05
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@ppisljar ppisljar force-pushed the fix/multiplemetrics branch from 64066a9 to 377627e Compare April 8, 2020 11:14
@flash1293
Copy link
Contributor

Tested and visualizations with more complex metrics and axes configurations render fine again.

Could you elaborate on how this fix works? It seems like this broke because something generic (in how state management works, it seems) got changed, but the fix only addresses a single local instance.

Is it because this is happening in a special tab?

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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

@ppisljar
Copy link
Member Author

ppisljar commented Apr 8, 2020

vis.setState was updated to allow changing type (where before if you changed type it would actually just change the .type property it would now really recreate that type (get the type def from registry, init default parameters etc). so when changing type default parameters got applied as nothing else got passed in. We are now passing whole state in and just updating the type.

Copy link
Contributor

@flash1293 flash1293 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 explanation @ppisljar tested and works as expected. A quick code search didn't show any other places where setState was called the wrong way.

@ppisljar ppisljar merged commit 90e6f2c into elastic:master Apr 8, 2020
ppisljar added a commit to ppisljar/kibana that referenced this pull request Apr 8, 2020
ppisljar added a commit to ppisljar/kibana that referenced this pull request Apr 8, 2020
ppisljar added a commit that referenced this pull request Apr 9, 2020
ppisljar added a commit that referenced this pull request Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes review v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visualizations with multiple series and axes don't work
4 participants