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

fixes angular vis type so it correctly propagates events up to visualize #15629

Merged
merged 1 commit into from
Dec 18, 2017

Conversation

ppisljar
Copy link
Member

fixes angular vis type so it correctly propagates events up to visualize

the update event was not correctly propagated up to visualize when calling updateState() from visualization if it was using angular visualization type.

the clone was there for historical reasons i guess, and should not be needed anymore.

@ppisljar ppisljar added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:fix review v6.2.0 v7.0.0 labels Dec 15, 2017
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

LGTM.
Note I tested this by using the same edit in a 6.1.1 install I have running with the swimlane visualization (https://github.com/prelert/kibana-swimlane-vis) which is an AngularJS viz type, which I am currently upgrading to work in 6.x Kibana. This fixes the issue I was seeing previously, where a change to $scope.vis.aggs was not correctly updating the aggs in the vis.

@ppisljar
Copy link
Member Author

jenkins, test this

@thomasneirynck thomasneirynck removed their request for review December 15, 2017 15:44
@timroes
Copy link
Contributor

timroes commented Dec 15, 2017

With the removal of this, we don't need the clone function on Vis anymore. Could we remove it, the tests for it, also as far as see, it was the last usage of the uiState parameter to the Vis constructor.

I think regarding the amount of dead code we're already having, we might want to consider throwing this out directly :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:fix review v6.2.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants