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

Add multiple colorramps to coordinate maps #17403

Merged
merged 4 commits into from
Apr 25, 2018
Merged

Add multiple colorramps to coordinate maps #17403

merged 4 commits into from
Apr 25, 2018

Conversation

thomasneirynck
Copy link
Contributor

@thomasneirynck thomasneirynck commented Mar 26, 2018 •
edited
Loading

Closes #5517.

todo:

@thomasneirynck thomasneirynck added release_note:enhancement WIP Work in progress Feature:Visualizations Generic visualization features (in case no more specific feature label is available) v7.0.0 v6.3.0 labels Mar 26, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck thomasneirynck requested a review from timroes April 11, 2018 18:32
@thomasneirynck thomasneirynck removed the WIP Work in progress label Apr 11, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

timroes
timroes approved these changes Apr 17, 2018
View reviewed changes
Copy link
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Once tests are added, LGTM

src/core_plugins/region_map/public/choropleth_layer.js
@@ -6,6 +6,7 @@ import { KibanaMapLayer } from 'ui/vis/map/kibana_map_layer';
import { truncatedColorMaps } from 'ui/vislib/components/color/truncated_colormaps';
import * as topojson from 'topojson-client';
import { toastNotifications } from 'ui/notify';
import * as colorUtil from 'ui/vis/map/color_util';
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we - in a separate PR - maybe move the map folder out of the vis folder? I would prefer in the long run, if we could remove anything from that folder, that is not core vis model classes (like vis and vis types) and classes that are used in the overall rendering pipeline (like request and response handlers). It's not urgent at all, but maybe we could keep this in the back of our minds, once a feature freeze is not close :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, makes sense, but will do in separate PR indeed

ppisljar
ppisljar approved these changes Apr 19, 2018
View reviewed changes
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

@thomasneirynck
Copy link
Contributor Author

jenkins, test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck
initial checkin
4cb6660 
remove cruft

rebase fixes

ensure styler function is recreated

remove duplication
@thomasneirynck
Copy link
Contributor Author

rebased

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@thomasneirynck
Copy link
Contributor Author

Run CI once more.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck
Copy link
Contributor Author

jenkins, test this

@thomasneirynck
Copy link
Contributor Author

thomasneirynck commented Apr 20, 2018 •
edited
Loading

Selenium failure

✖ fail: "visualize app visualize lab mode disabling does not break loading saved searches"
18:12:21      │        tryForTime timeout: [POST http://localhost:9515/session/1b34abfea6ddf51755849f9f555f0af6/element / {"using":"partial link text","value":"visualize_lab_mode_test"}] no such element: Unable to locate element: {"method":"partial link text","selector":"visualize_lab_mode_test"}

seems completely unrelated to last commit, which was an addition to the unit tests. This PR also makes no changes to any of the lab-mode code.

Unit test suite completed OK https://kibana-ci.elastic.co/job/elastic+kibana+pull-request+multijob-intake/2115/.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck
Copy link
Contributor Author

jenkins, test this

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@thomasneirynck thomasneirynck merged commit dde52f0 into elastic:master Apr 25, 2018
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Apr 25, 2018
@thomasneirynck
Add multiple colorramps to coordinate maps (elastic#17403)
183f24f
@thomasneirynck thomasneirynck mentioned this pull request Apr 25, 2018
Merged
thomasneirynck added a commit that referenced this pull request Apr 26, 2018
@thomasneirynck
Add multiple colorramps to coordinate maps (#17403) (#18586)
67fed21
@thomasneirynck thomasneirynck mentioned this pull request Dec 4, 2018
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

@timroes timroes timroes approved these changes

@nreese nreese Awaiting requested review from nreese

Assignees
No one assigned
Labels
Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:enhancement v6.4.0 v7.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Custom colors and gradients for the Tile Maps
4 participants
@thomasneirynck @elasticmachine @timroes @ppisljar