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

Save layer setting in the region map UI #12956

Merged
merged 2 commits into from
Jul 27, 2017

Conversation

thomasneirynck
Copy link
Contributor

@thomasneirynck thomasneirynck commented Jul 18, 2017

Release Note: The layer selection is now preserved in the UI dropdown when saving a Region Map.


closes #12172.

@thomasneirynck thomasneirynck added review v5.5.1 v5.6.0 v6.0.0 Feature:Visualizations Generic visualization features (in case no more specific feature label is available) labels Jul 18, 2017
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

works well, have one minor comment regarding implementation, we should stay consistent with how we define other settings.

@@ -15,6 +15,14 @@ uiModules.get('kibana/region_map')

$scope.collections = $scope.vis.type.editorConfig.collections;

$scope.initializeLayerSetting = function () {
$scope.vis.params.selectedLayer = $scope.collections.vectorLayers[0];
Copy link
Member

Choose a reason for hiding this comment

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

this should be defined in the vis definition (visConfig.defaults) as we do with all the other settings, and then you don't need the ng-init part.

@ppisljar
Copy link
Member

jenkins, test this

Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

LGTM, ci is failling

@thomasneirynck
Copy link
Contributor Author

PR probably requires rebase, breaking on single_type issue.

@thomasneirynck
Copy link
Contributor Author

jenkins, test this

2 similar comments
@thomasneirynck
Copy link
Contributor Author

jenkins, test this

@thomasneirynck
Copy link
Contributor Author

jenkins, test this

Copy link
Contributor

@stacey-gammon stacey-gammon left a comment

Choose a reason for hiding this comment

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

lgtm!

@thomasneirynck thomasneirynck merged commit 9b949a5 into elastic:master Jul 27, 2017
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Jul 27, 2017
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Jul 27, 2017
Includes manual edits to resolve merge conflicts.
@thomasneirynck
Copy link
Contributor Author

thomasneirynck commented Jul 27, 2017

Backport:
5.5: #13167
5.6: 0a21221
5.x: #13166
6.0: #13165
6.x: #13168

thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Jul 27, 2017
Includes manual edits to resolve merge conflicts.
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Jul 27, 2017
thomasneirynck added a commit that referenced this pull request Jul 28, 2017
Includes manual edits to resolve merge conflicts.
thomasneirynck added a commit that referenced this pull request Jul 28, 2017
Includes manual edits to resolve merge conflicts.
epixa pushed a commit that referenced this pull request Jul 28, 2017
Includes manual edits to resolve merge conflicts.
@epixa
Copy link
Contributor

epixa commented Jul 28, 2017

This should go into 5.6 rather than 5.x. I cherry-picked it over 0a21221

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 v5.5.2 v5.6.0 v6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Region Map] Saved map does not persist vectors originally defined
4 participants