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 styling and tooltip support to mapbox mvt vector tile sources #64488

Merged
merged 120 commits into from
Jul 2, 2020

Conversation

thomasneirynck
Copy link
Contributor

@thomasneirynck thomasneirynck commented Apr 27, 2020

Closes #46409

Adds tooltip-support and dynamic styling for .mvt vector tiles sources.

image

Opting to do his together since:

  • without tooltips it is really hard to write dynamic style rules (since there is no other way to inspect the data in the tile)
  • this PR is the main blocker to add full support for ES-sources (clusters/grids and document layers)

This PR adds UX-forms to:

  • add fields which are present in an MVT XYZ source
    • (note that down-the-line this could be determined automatically when Maps supports the TileJson specification)
  • add fields which should be added to the tooltips for an MVT XYZ source

Since all feature-data is contained in the tile and cannot be determined by inspecting the GeoJson or doing a backend AJAX call to a service, this PR introduces some light refactors in the existing code:

  • feature-properties/geometry cannot be determined, so the raw properties from Mapbox-gl component are passed along
  • feature-ids are not guaranteed to exist in a vector-tile source (For geojson sources, Maps will use _assignFeatureIds to forcibly assign one if it is missing). This id is now made optional in the corresponding layer and source APIs for tooltip-identification
  • the domain of a field's values (e.g. categorical set, or ordinal range) cannot be determined client-side for raw vector tile sources (XYZ). This requires disabling all automated ramping/color maps/icon palettes. This also requires disabled dynamic styling for sizes, since field-values cannot be entered manually.

Reviewer notes

The best way to test is to look at a TileJson (if your vector tile service has one).

Some examples from https://tiles.maps.elastic.co/data/v3.json

  • poi layer
    • name (String), name_en (String), class (String), agg_stop (Number)
  • transportation layer
    • class (String), brunnel (String), oneway (Number), layer (Number), level (Number), indoor (Number)

Note that the EMS tile-service is incredibly sparse in its populated fields. So not every property will be usable at any given scale.

To test

  1. Set xpack.maps.enableVectorTiles: true in kibana.dev.yml
  2. Add new Vector Tile Layer layer. with the following configurations (check https://tiles.maps.elastic.co/data/v3.json for more examples).
  • transportation layer:
    • Url: https://tiles.maps.elastic.co/data/v3/{z}/{x}/{y}.pbf?elastic_tile_service_tos=agree&my_app_name=kibana&my_app_version=8.0.0&license={PLUGIN_LICENSE_KEY}
    • Layer name: transportation
    • Zoom range: 4 to 14
    • fields:
      • name: class, type string
      • name: name, type string
  • poi layer:
    • Url: https://tiles.maps.elastic.co/data/v3/{z}/{x}/{y}.pbf?elastic_tile_service_tos=agree&my_app_name=kibana&my_app_version=8.0.0&license={PLUGIN_LICENSE_KEY}
    • Layer name: poi
    • Zoom range: 10 to 14
    • fields:
      • name: class, type string
  • water layer:
    • Url: https://tiles.maps.elastic.co/data/v3/{z}/{x}/{y}.pbf?elastic_tile_service_tos=agree&my_app_name=kibana&my_app_version=8.0.0&license={PLUGIN_LICENSE_KEY}
    • Layer name: water
    • Zoom range: 0 to 14

Screen Shot 2020-06-24 at 12 22 58 PM

Bugs todo

  • Adding/clearing fields to the source-settings requires a save for the settings to be available
  • The field editing is not behaving independently of the min/zoom/layername settings. (e.g. field-changes get cleared when other settings are modified)
  • should not show tooltips when not configured
  • supportsAutoDomain error in console

@thomasneirynck thomasneirynck added WIP Work in progress [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation enhancement New value added to drive a business result labels Apr 27, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@thomasneirynck thomasneirynck marked this pull request as ready for review June 9, 2020 20:24
@thomasneirynck thomasneirynck requested a review from a team as a code owner June 9, 2020 20:24
@thomasneirynck thomasneirynck added release_note:enhancement and removed enhancement New value added to drive a business result labels Jun 9, 2020
Copy link
Contributor

@miukimiu miukimiu left a comment

Choose a reason for hiding this comment

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

Hi @thomasneirynck,

I created a PR with a few design improvements thomasneirynck#17.

This way we don't need to use too much space:

  • Changed all forms to compressed
  • Some of the forms are now using a column layout
  • Moved the help text to tooltips.

source settings@2x

Let me know what you think.

Copy link
Contributor

@kindsun kindsun left a comment

Choose a reason for hiding this comment

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

Thank you so much for all the work on this and the final polish! This adds some great foundational pieces that pave the way for a whole new phase in how Maps will look and feel. 🌞

Functionally works well when tested locally with the understanding that it currently requires a good understanding of the shape of your data source. lgtm w/ green CI!

  • code review
  • tested locally in chrome

@nreese
Copy link
Contributor

nreese commented Jul 1, 2020

Can you update MVTSingleLayerVectorSource to implement getValueSuggestions? That way, typeahead will work when creating color maps and icon maps. The implementation can just be a simple filter against fields to see if name includes search value.

Screen Shot 2020-07-01 at 11 57 11 AM

Disregard, getValueSuggestions needs access to values which are not available.

@thomasneirynck thomasneirynck requested a review from a team as a code owner July 1, 2020 18:19
@thomasneirynck
Copy link
Contributor Author

Thx @miukimiu , looks great. Can you take 2nd look?

@thomasneirynck thomasneirynck changed the title [Maps] Add tooltips and dynamic styling to vector tile sources [Maps] Add styling and tooltip support to mapbox mvt vector tile sources Jul 1, 2020
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

Nice, this is really powerful for displaying 3rd party vector tiles.

LGTM
code review, tested in chrome

Copy link
Contributor

@miukimiu miukimiu left a comment

Choose a reason for hiding this comment

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

Tested locally and it looks good! 🎉

There's something that I'm not sure if it's possible. Is there a way to make the transportation bold without the quotes?

Screenshot 2020-07-01 at 19 43 51

Like this:
Screenshot 2020-07-01 at 20 16 21

Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

App Arch code change LGTM!

@thomasneirynck
Copy link
Contributor Author

Is there a way to make the transportation bold without the quotes?

@miukimiu yes, I just pushed this up. thx!

@thomasneirynck
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@thomasneirynck thomasneirynck merged commit 9c76f19 into elastic:master Jul 2, 2020
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Jul 2, 2020
…ces (elastic#64488)

* tmp commit

* rename

* more boilerpalte

* more boiler

* more boilerpalte

* typing

* fix import

* boilerplate

* more boiler

* enable custom palettes

* fix label text and orientation

* fix merge errors

* remove dupe import

* stash commit

* tmp commit

* debounce settings

* return null

* slight rearrangement

* tooltip guard

* minor tweaks

* feedback

* ts fixes

* ts fixes

* more ts fixes

* ts fixes

* jest test

* fix typo

* spacing

* fix typing

* add unit test

* add more tests

* add snapshot test

* add snapshot

* add field editor snapshot test

* fix snapshot

* add snapshot

* remove unused import

* test stub for mvt layer

fix optional param

more checks

* add snapshot test

more unit tests

more unit tests

ts fixes

* add data syncing unit test

* fix autorefactor

* fix merge and replace snapshots

* field editor changes

* field editor changes

* ts fixes

* update snapshots

* fix things

* fix names

* fix tooltip

* add more error handling

* improve copy

* styling changes

* style option box a little better

* ts fixes

* fix console error

* remove mbProperties from interface

* remove unused method

* remove cruft

* rename for consistency

* remove unused param

* feedback

* feedback

* ensure properties are always present

* handle possible null values

* feedback

* typo

* update SIEM

* feedback

* remove cruft

* remove unused translations

* feedback

* improve readability

* fix brittle test

* fix snapshot after master merge

* remove unused method

* feedback

* revert some feedback

* remove micro-optimization

* initialize in constructor

* simplify wording

* add snapshot

* naming

* add clarifying comment

* remove unused import

* sanitize tooltips

* remove cruft

* feedback

* fix typo

* remove export

* Design fixes

* clean up supportsAutoDomain

* remove patch.txt

* cleanup

* clean-up

* Merge in styling changes

* Tweak message format

* fix broken import

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: miukimiu <elizabet.oliveira@elastic.co>
Co-authored-by: Nathan Reese <reese.nathan@gmail.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 2, 2020
* master:
  Update known-plugins.asciidoc (elastic#69370)
  [ML] Anomaly Explorer swim lane pagination (elastic#70063)
  [Ingest Manager] Update asset paths to use _ instead of - (elastic#70320)
  Fix discover, tsvb and Lens chart theming issues (elastic#69695)
  Allow Saved Object type mappings to set a field's `doc_values` property (elastic#70433)
  [S&R] Support data streams (elastic#68078)
  [Maps] Add styling and tooltip support to mapbox mvt vector tile sources (elastic#64488)
  redirect to default app if hash can not be forwarded (elastic#70417)
  [APM] Don't fetch dynamic index pattern in setupRequest (elastic#70308)
  [Security_Solution][Endpoint] Leveraging msearch and ancestry array for resolver (elastic#70134)
  Update docs for api authentication usage (elastic#66819)
  chore(NA): disable alerts_detection_rules cypress suites (elastic#70577)
  add getVisibleTypes API to SO type registry (elastic#70559)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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