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

[Maps] Add UI source for custom vector tile style #47058

Closed
wants to merge 1 commit into from

Conversation

nickpeihl
Copy link
Member

Summary

WIP. Please do not merge.

Partially fixes #46409.

This adds a new layer source supporting custom vector tiles from a user-specified URL to a style document matching the Mapbox Style Specification.

This naively passes the URL directly to mapbox-gl-js for fetching, validation, and rendering.

TEST URLS

TODO

  • Retrieve attribution from the style
  • Allow custom style URLs to be defined in kibana.yml
  • Tests

QUESTIONS

  1. Mapbox styles that use the mapbox:// URI protocol do not work without an [accessToken](https://docs.mapbox.com/mapbox-gl-js/api/#accesstoken) defined on the Map. Should we offer a text field for the user to attach their access token to the map?
  2. How smart should this be? Do we need to parse the style.json file and look for mapbox:// URI protocols before offering this option? Or simply have an optional Mapbox Access Token field.
  3. Do other vector tile providers have similar access token requirements that we may need to handle? (e.g. Nextzen, Carto, Esri, Here, ...)

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@bcamper
Copy link

bcamper commented Oct 1, 2019

  1. Mapbox styles that use the mapbox:// URI protocol do not work without an accessToken defined on the Map. Should we offer a text field for the user to attach their access token to the map?
  2. How smart should this be? Do we need to parse the style.json file and look for mapbox:// URI protocols before offering this option? Or simply have an optional Mapbox Access Token field.

I think a simple Mapbox Access Token field to start is fine. It's likely going to be the most common use case, so I don't think it warrants us parsing the style just to look for mapbox:// protocols. If we are going to be parsing it anyway for another reason, then I think it would be a nice touch though :)

  1. Do other vector tile providers have similar access token requirements that we may need to handle? (e.g. Nextzen, Carto, Esri, Here, ...)

I believe that all of these other providers will accept an API key/access token as a URL parameter, which can be baked into the tile source in the style JSON file (and naming/formatting will vary per provider anyway). I think that's an OK starting point, if we are always showing the MB access token field, we can just make sure to label it to indicate that it's unnecessary for non-MB sources.

@thomasneirynck thomasneirynck self-requested a review October 8, 2019 17:31
Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

This is a great PR because it addresses a real lack in Maps; the ability to consume external vector tiles and preserve smooth panning and zooming for that data.

The technical implementation too is really great. This PR introduces a new source-type (vector_tile_source), that can be visualized with the vector_tile_layer type introduced in 7.4 for EMS. Overall, it's a low footprint addition to Maps that makes a big impact.

I am not sure though if this is the right way to introduce support for vector tiles in Maps as a first pass.

  • This really adds support for bringing in custom styled mapbox maps, by linking to a mapbox stylesheet. Support for rendering these stylesheets to a high standard is really only provided by mapbox-g (especially wrt label and icon placement and orientation, also some of the more exotic styling parameters). This creates a significant dependency on Mapbox, and will hurt the ability of Maps to upgrade to a different toolkit in the future (e.g. should we want 3D support, multi-projection support, etc…)
  • It is unclear how this source-type unblocks a user. As presented here, it really only enables smooth panning/zooming for background-like data, but in the current state of Maps it does not offer any sort of functionality that I would expect a user to associate with vectors like dynamic filtering, highlighting, tooltip identification, filtering, …. Smooth zooming certainly has better appeal, but I am doubtful users are not using Maps only because today they would have to use a raster-TMS to achieve the same functionality (albeit sans smooth zooming)
  • In Maps, the font-library in Maps is statically defined. If Maps allows custom a vector stylesheet (where the glyph property maps to a font-library), it would also need to link to custom font libraries defined in the stylesheet. The multi-layered approach in Maps (since it does not have the concept of a single base map) somewhat precludes this.

While EMS-layers use the same approach behind the covers, this dependency on the mapbox styling language is a blackbox from a user’s perspective. As Elastic owns both sides of the stack here (EMS + Kibana), we are a lot more flexible in how we would provide upgrade paths, dial in new features, ... wrt basemapping.

Imho, the pathway to bring in vector tile support in Maps would be a little more low level, and focus on the consumption of a vector tile service directly (a service that publishes .mvt tiles (https://docs.mapbox.com/vector-tiles/specification/)

  • Unlike the stylesheets, .mvt services are a community standard that is widely supported in the industry by multiple server platforms (mapboxes own backend, but also competitors like ArcGIS, Geoserver, OpenmapTilesServer, …)
  • It does not create a lockin in mapbox-gl because most client-side rendering toolkits support rendering .mvt tiles.
  • It preserves more flexibility in how Maps can use these vector tiles. E.g. choropleth mapping, static/dynamic styling, … could all be ad-hoc defined in the Maps app, rather than in an offline map-creation exercise
  • It maps closer to how we could consume a possible Elasticsearch specific vector tiles service (e.g. vector tiles from an index)

cc @bcamper @nreese @alexfrancoeur @aaronjcaldwell

@nickpeihl
Copy link
Member Author

I think it's unlikely we will merge this. A better solution would be styling via the maps UI. Closing.

@nickpeihl nickpeihl closed this May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maps] Support custom vector tiles defined by user
6 participants