Skip to content

Save layer setting in the region map UI#12956

Merged
thomasneirynck merged 2 commits intoelastic:masterfrom
thomasneirynck:fix/save_layer_setting
Jul 27, 2017
Merged

Save layer setting in the region map UI#12956
thomasneirynck merged 2 commits intoelastic:masterfrom
thomasneirynck:fix/save_layer_setting

Conversation

@thomasneirynck
Copy link
Copy Markdown
Contributor

@thomasneirynck thomasneirynck commented Jul 18, 2017 •
edited
Loading

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

closes #12172.

@thomasneirynck thomasneirynck requested a review from ppisljar July 18, 2017 21:21
@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
ppisljar
ppisljar requested changes Jul 19, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@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.

src/core_plugins/region_map/public/region_map_vis_params.js Outdated
Copy link
Copy Markdown
Contributor

@ppisljar ppisljar Jul 19, 2017

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.

@thomasneirynck thomasneirynck force-pushed the fix/save_layer_setting branch from 0098b05 to fdd3883 Compare July 19, 2017 20:08
@thomasneirynck thomasneirynck requested a review from ppisljar July 19, 2017 20:09
@ppisljar
Copy link
Copy Markdown
Contributor

ppisljar commented Jul 20, 2017

jenkins, test this

ppisljar
ppisljar approved these changes Jul 20, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Contributor Author

thomasneirynck commented Jul 20, 2017

PR probably requires rebase, breaking on single_type issue.

@thomasneirynck thomasneirynck force-pushed the fix/save_layer_setting branch from fdd3883 to 58388d7 Compare July 20, 2017 15:14
@thomasneirynck
Copy link
Copy Markdown
Contributor Author

thomasneirynck commented Jul 20, 2017

jenkins, test this

2 similar comments
@thomasneirynck
Copy link
Copy Markdown
Contributor Author

thomasneirynck commented Jul 20, 2017

jenkins, test this

@thomasneirynck
Copy link
Copy Markdown
Contributor Author

thomasneirynck commented Jul 21, 2017

jenkins, test this

@thomasneirynck thomasneirynck force-pushed the fix/save_layer_setting branch from 58388d7 to 5171248 Compare July 21, 2017 14:43
@epixa epixa added v5.5.2 and removed v5.5.1 labels Jul 21, 2017
stacey-gammon
stacey-gammon approved these changes Jul 27, 2017
View reviewed changes
Copy link
Copy Markdown

@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
Save layer setting in the region map UI (elastic#12956)
233281c
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Jul 27, 2017
@thomasneirynck
Save layer setting in the region map UI (elastic#12956)
61bae16 
Includes manual edits to resolve merge conflicts.
@thomasneirynck
Copy link
Copy Markdown
Contributor Author

thomasneirynck commented Jul 27, 2017 •
edited
Loading

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
@thomasneirynck
Save layer setting in the region map UI (elastic#12956)
221cd0d 
Includes manual edits to resolve merge conflicts.
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Jul 27, 2017
@thomasneirynck
Save layer setting in the region map UI (elastic#12956)
1f534e2
thomasneirynck added a commit that referenced this pull request Jul 28, 2017
@thomasneirynck
Save layer setting in the region map UI (#12956) (#13168)
181435a
thomasneirynck added a commit that referenced this pull request Jul 28, 2017
@thomasneirynck
Save layer setting in the region map UI (#12956) (#13165)
78ed823
thomasneirynck added a commit that referenced this pull request Jul 28, 2017
@thomasneirynck
Save layer setting in the region map UI (#12956) (#13167)
dc6b9e2 
Includes manual edits to resolve merge conflicts.
epixa pushed a commit that referenced this pull request Jul 28, 2017
@thomasneirynck @epixa
Save layer setting in the region map UI (#12956) (#13166)
0a21221 
Includes manual edits to resolve merge conflicts.
@epixa
Copy link
Copy Markdown
Contributor

epixa commented Jul 28, 2017

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

@thomasneirynck thomasneirynck mentioned this pull request Aug 4, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ppisljar ppisljar ppisljar approved these changes

+1 more reviewer

@stacey-gammon stacey-gammon stacey-gammon approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

4 participants

@thomasneirynck @ppisljar @epixa @stacey-gammon