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

Introduce two component text radial offset #8609

Closed
wants to merge 2 commits into from

Conversation

pozdnyakov
Copy link
Contributor

@pozdnyakov pozdnyakov commented Aug 8, 2019

This PR introduces two-component text radial offset for variable text placement. This feature is useful when the text is placed around a symbol icon with different dimensions. The new behavior is described below:

  • if text-radial-offset is initialized with a number (to keep the existing styles working) or with an array of one element, we'll use the circle offset (same way we do now).

  • if text-radial-offset is initialized with an array of two elements, these two numbers will stand for the radius on the x and y axes of the ellipse offset.

Note: a new legacy-type field is added to the specification JSON file in order to allow usage of legacy types - in this particular case we still allow using plain number type with the text-radial-offset, so that the existing styles remain working.

Tagging @mapbox/studio @mapbox/maps-design @mapbox/gl-native

Fixes #8598

@asheemmamoowala
Copy link
Contributor

asheemmamoowala commented Aug 9, 2019

cc @mapbox/studio @mapbox/map-design-team @mapbox/gl-native

@ansis
Copy link
Contributor

ansis commented Aug 13, 2019

Is placing around an ellipse what we're looking for? It looks like placing around a rectangle might work better for #8598

// Layers with variable anchors use the `text-radial-offset` property and the [x, y] offset vector
// is calculated at placement time instead of layout time
if (radialOffset) {
if (radialOffset[0]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be false when radialOffset[0] == 0, regardless of radialOffset[1]'s value. This check needs to test both x and y.

@pozdnyakov
Copy link
Contributor Author

Closing in favor of #8642

@pozdnyakov pozdnyakov closed this Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

text-radial-offset does not account for icon size or bounding boxes
5 participants