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

icon-text-fit should work with text-variable-anchor #8583

Closed
tristen opened this issue Aug 1, 2019 · 8 comments
Closed

icon-text-fit should work with text-variable-anchor #8583

tristen opened this issue Aug 1, 2019 · 8 comments
Assignees
Labels

Comments

@tristen
Copy link
Member

tristen commented Aug 1, 2019

Expected Behavior

When using these properties on a layer:

{
  'text-variable-anchor': ['left', 'top', 'right', 'bottom'],
  'text-justify': 'auto',
  'icon-text-fit': 'width'
}

I would expect both the icon that's been fitted under the label and the label to be variably placed.

Actual Behavior

Here's a label using a icon-text-fit background

Without text-variable-anchor With text-variable-anchor
Screen Shot 2019-08-01 at 9 08 34 AM Screen Shot 2019-08-01 at 9 17 17 AM

mapbox-gl-js version: v1.1.0
browser: Chrome Version 75.0.3770.142 (Official Build) (64-bit)

Link to Demonstration

https://jsfiddle.net/tristen/830prsyz/

@chloekraw chloekraw modified the milestone: release-queso Aug 2, 2019
@asheemmamoowala asheemmamoowala added this to the release-r milestone Aug 5, 2019
@asheemmamoowala
Copy link
Contributor

cc @alexshalamov

@alexshalamov
Copy link
Contributor

@asheemmamoowala yes, it is a bug, we need to offset an icon together with a text. Also, would be nice to document that in a spec as well.

@chloekraw
Copy link
Contributor

@alexshalamov, would be nice to document in the spec now that this property doesn't work with icon-text-fit, or to document that it does after we close this issue?

@alexshalamov
Copy link
Contributor

@chloekraw please disregard my comment, after checking the spec, I think it should be clear how icon-text-fit and variable anchors work together, e.g., icon would be shifted together with the text.

@alexshalamov
Copy link
Contributor

alexshalamov commented Aug 9, 2019

@ansis could you please take a look at gl-native fix in mapbox/mapbox-gl-native@19a49a5

Also, I had to fix it for combination of text-writing-mode with icon-text-fit.

@chloekraw chloekraw modified the milestones: release-r, release-queso Aug 12, 2019
@chloekraw
Copy link
Contributor

chloekraw commented Aug 12, 2019

@alexshalamov thanks for the investigation and proposed fix. per chat, I'm tracking that your next steps here are to split those commits out into a separate PR on GL-Native and to open a PR with render tests on GL-JS.

@alexshalamov
Copy link
Contributor

@chloekraw GL-Native PR #15367 render tests #8625

@kkaefer
Copy link
Contributor

kkaefer commented Sep 23, 2019

Fixed in v1.4.0-beta.1

@kkaefer kkaefer closed this as completed Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants