-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
do not hide icons if text is an empty string #6164
Conversation
Empty text strings do not have any collision boxes so the collision index check was never done and the text was not counted as "placed". This fixes that by making "placed" the default when the feature has text. fix #6160
@ansis Can you please clarify when should one use 'text-optional' and 'icon-optional' layout properties? Shouldn't this require setting text-optional to true in the corresponding style? |
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.
Yeah this looks right to me.
@mb12 I was confused by the same thing regarding text/icon-optional
. The difference is that text-optional
means "show this icon even if its corresponding text has been hidden because of a collision". It doesn't have anything to do with whether the corresponding text is defined/empty. So in this case even though the text is empty, text-optional
still shouldn't be necessary (because the text isn't colliding with anything either).
Would this also fix the inverse case, "do not hide text if icon is an empty string" ? |
@dagjomar Mostly I think yes... from the point of view of the code, there's nothing special about an mapbox-gl-js/src/symbol/symbol_layout.js Lines 124 to 147 in 25f9afa
Along with: mapbox-gl-js/src/symbol/placement.js Line 162 in 25f9afa
So what that means is something like: "If the feature has an icon, but that icon's image is not in the image map, we won't create a shapedIcon and we won't add a symbol instance for just that icon. But if the same feature also has text, we'll bring the icon along for the ride, and |
hi. i have an apposite problem. when i add a text field the symbols disappear. i searched a lot and i used a lot of suggestions. but none of them didn't work for me. please help me. that's my code: ` |
fix #6160
Empty text strings do not have any collision boxes so the collision index check was never done and the text was not counted as "placed". This fixes that by making "placed" the default when the feature has text.
Alternative fix
Another possibility would be to create zero-sized collision boxes for empty strings. If we did this, should these zero-size boxes collide with other boxes if they are contained within them?
Launch Checklist