-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Label style properties #52957
Conversation
Pinging @elastic/kibana-gis (Team:Geo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love this feature! I found one minor issue.
x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_text_property.js
Outdated
Show resolved
Hide resolved
Pinging @elastic/kibana-docs (Team:Docs) |
Beautiful job @nreese, having not only content but also text color and size driven by data is super cool!! 😍 Not totally sure, but it's quite typical for users to also want to change the font. Since the most common scenario is to use Maps with EMS basemaps this would not be a high priority to me at this moment since most of the visualization is already "defined" in cartographic terms, and a good default font already used in the basemap should be enough. But with custom basemaps also available, other fonts would be better suited, so we may need also that setting to be offered. |
I created #53127 to track that feature request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
holy smokes. killer feature.
Some initial comments. Will look more tomorrow.
I ran into two little bugs with styling with icons.
- saving removes icon
Steps:
- style a point layer with an icon
- add a label
- save the map
- refresh the browser
-> label is showing but icon is not showing
- labels are drawn under icons
Steps:
- style a point layer with an icon
- add a label
-> not that label is drawn under the icon.
(fwiw, there are bugs in mapbox wrt ordering of layers. e.g. heatmap layers sometimes behave oddly when reordering. I'm not sure if this is a similar issue with mb. It might also be us adding the symbol-layer on top of the text-layer).
.../legacy/plugins/maps/public/layers/styles/vector/components/color/dynamic_color_selection.js
Show resolved
Hide resolved
x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style_defaults.js
Outdated
Show resolved
Hide resolved
wrt @jsanz comment and custom fonts. Agreed on creating separate feature request. It's somewhat of a bigger issue than just offering that here. By design, font-libraries not configurable in Maps. Right now, the available fonts are a "blackboxed" hardcoded configuration specifically for EMS basemaps ((https://github.com/elastic/kibana/blob/8e9a8a84dccfa7965ce8a22362885e6cdef8b51f/src/legacy/server/config/schema.js)), and these cannot be modified (due to limitations in mapbox-gl API with registering font-libraries at runtime). I think we should punt on custom fonts until Maps supports some sort of configurable font-library and/or preset lists of guaranteed-to-exist fonts, and untie this from EMS. |
@thomasneirynck I have resolved the bugs you spotted. The icons where not displaying after saving because mapbox appears to not draw the symbol layer under another symbol layer at the same location. I resolved this by just using a single symbol layer when points are symbolized as icons. |
@nreese I'm getting an error when trying to enable labels No errors on the network tab, sharing console screenshot: Happening in both documents and aggregation layers, tested on Chromium and Firefox. Let me know if you need further details. |
@jsanz Thanks for finding this. Its all fixed now |
Working nicely on both Chromium and Firefox, apart from the decimals topic mentioned earlier I haven't find any other relevant issue 🎉 Maybe we can put this in a separate feature request but it would be great to allow labels to optionally have a halo (color and width). This is a very common setting to improve readability, specially when points have a color ramp, but also in many other situations. Sharing same data on Kibana and QGIS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly minor comments below.
x-pack/legacy/plugins/maps/public/layers/sources/es_search_source/es_search_source.js
Show resolved
Hide resolved
x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style_defaults.js
Outdated
Show resolved
Hide resolved
x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💣 💯 Will really empower users
lgtm on green.
agreed with @jsanz that addition of halos would help with readability, but that can always be done later.
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! love this feature.
tested in Chrome.
* text styling * label style editor UI * wire up styles to mb * allow string values * remove console.log * default getFields to provide ordinal fields for vector source * fix vector_style jest test * add label styles to docs * fix prettier errors * use index-pattern field formatter to format label * rename LABEL to LABEL_TEXT * review feedback * fix problem with icons not displaying with labels * fix functional tests * fix canno read name of null error * update jest expect * fix eslint errors * do not display label text in legend * always show all label styling properties in editor * review feedback
* text styling * label style editor UI * wire up styles to mb * allow string values * remove console.log * default getFields to provide ordinal fields for vector source * fix vector_style jest test * add label styles to docs * fix prettier errors * use index-pattern field formatter to format label * rename LABEL to LABEL_TEXT * review feedback * fix problem with icons not displaying with labels * fix functional tests * fix canno read name of null error * update jest expect * fix eslint errors * do not display label text in legend * always show all label styling properties in editor * review feedback
fixes #52607
Adds label, labelSize, and labelColor style properties to allow users to display a label for point features.