Configurable tile map #2605
Configurable tile map #2605
Conversation
jimmyjones2
force-pushed
the
configurable-tile-map
branch
2 times, most recently
from
9931e25 to
b1f112c
Compare
| @@ -43,6 +43,21 @@ define(function (require) { | |||
| value: 6, | |||
| description: 'The maximum geoHash size allowed in a tile map', | |||
| }, | |||
| 'visualization:tileMap:wms': { | |||
| value: false, | |||
| description: 'Set for WMS mapping provider' | |||
There was a problem hiding this comment.
Can you provide a more complete description of this? Maybe a link to a reference?
There was a problem hiding this comment.
@rashidkpc I tried, but HTML tags are shown as is rather than parsed. Any tips on fixing this, my angular knowledge is slim:(
jimmyjones2
force-pushed
the
configurable-tile-map
branch
from
f18b206 to
2ca321d
Compare
jimmyjones2
force-pushed
the
configurable-tile-map
branch
from
2ca321d to
04a77e2
Compare
|
@rashidkpc does this look better? Would you prefer the URL in the code as well and a null default? |
jimmyjones2
force-pushed
the
configurable-tile-map
branch
from
26a1bb1 to
7040887
Compare
| subdomains: '1234' | ||
| }); | ||
| var tileLayer; | ||
| var tileOptions = config.get('visualization:tileMap:options', '{' + |
There was a problem hiding this comment.
I'd rather see the 2nd argument of this be an object vs a concatenated string
jimmyjones2
force-pushed
the
configurable-tile-map
branch
from
cdc196d to
6542367
Compare
…indow and (hopefully) explain WMS option a bit better
jimmyjones2
force-pushed
the
configurable-tile-map
branch
from
567b0a8 to
5add4a8
Compare
|
@rashidkpc would you mind having another look? |
|
@jthomassie I believe is working on tests for the tile map, can you get with him and come up with some tests for the logic here? |
|
@rashidkpc sorry missed that, hopefully the last commit addresses it |
|
Thanks @jimmyjones2 we're going to hold on this pending tests for the tile map. Once those are ready you can write some tests for this and we can get this in. |
|
@jimmyjones2 I'll be working on tile map issues/enhancements over the next couple of weeks. I will get some tests in place for you to build upon. |
|
I've been thinking about this one and it might makes more sense to make this configurable on the per-visualization basis. Maybe we could make the value in this pull the defaults. On the topic of the options object, I'd rather break this out into individual options that are, again, configurable per-visualization. In general, it would be smart for us to start doing more vis-by-vis, index-by-index, and less global |
|
@rashidkpc I agree that per vis options would be nice, but I think this user config version would be used more widely, and it is an easier first step to employ. |
|
@jimmyjones2 tests for tile map are now in master, along with several other changes. |
|
@jthomassie Will get this sorted in the next week or so |
Conflicts: src/kibana/components/vislib/visualizations/tile_map.js
…MS sources to test with
|
@jthomassie Any suggestions how to write the tests - I'm no JavaScript pro! None of the existing tests get the map object, but even if I could do that, can't see a way to retrieve the current map URL to check if the config has applied? |
|
@jimmyjones2 no problem, I will get back to you soon with a suggestion for how to test this. |
|
@rashidkpc I know you guys are super busy with 4.1 but could you give me a few pointers for the tests? I can't see how to get and query the leaflet object for what URL is set? Or is there a better way to test? |
|
@jimmyjones2 Unrelated to the request for testings the Javascript, there are a lot of freely available WMS servers. Perhaps more importantly, a lot of the private map resources will be WMS servers rather than TMS servers. Anyway, here's a list of a ton of WMS servers to test with: http://www.skylab-mobilesystems.com/en/wms_serverlist.html |
|
Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'. |
|
Once #4701 goes in feel free to look at how the implementation was done and add TMS support. You probably want a new pull for that though. |
|
Closing in favor of #4701 |
Closes #1902
Expose configuration of Leaflet to allow custom mapping providers. Example alternative config: