Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Fix issues for use-cases when icon-text-fit is combined with text-variable-anchor #15367

Merged
merged 3 commits into from
Aug 20, 2019
Merged

[core] Fix issues for use-cases when icon-text-fit is combined with text-variable-anchor #15367

merged 3 commits into from
Aug 20, 2019

Conversation

alexshalamov
Copy link
Contributor

There are multiple issues related to icon-text-fit when it is used in combination with text-variable-anchor and text-writing-mode

  • Collision feature box for an icon has wrong size
  • Collision feature should be shifted together with text box
  • Collision debug box for an icon should be shifted together with text
  • Vertically oriented text box might be of different size, thus vertical icon might be needed

Fixes: #15346
Adresses: mapbox/mapbox-gl-js#8583

@alexshalamov alexshalamov requested a review from ansis August 13, 2019 11:06
This was referenced Aug 13, 2019
Merged
Closed
@alexshalamov alexshalamov force-pushed the alexshalamov_fix_icon-text-fit branch from 205fdf3 to c9cc7d2 Compare August 13, 2019 13:20
@chloekraw chloekraw added this to the release-queso milestone Aug 13, 2019
@alexshalamov alexshalamov force-pushed the alexshalamov_fix_icon-text-fit branch from c9cc7d2 to 336532c Compare August 14, 2019 21:03
@alexshalamov alexshalamov self-assigned this Aug 14, 2019
@alexshalamov alexshalamov force-pushed the alexshalamov_fix_icon-text-fit branch from 336532c to 5c78282 Compare August 15, 2019 15:35
@alexshalamov alexshalamov marked this pull request as ready for review August 15, 2019 15:37
@alexshalamov alexshalamov added the needs changelog Indicates PR needs a changelog entry prior to merging. label Aug 15, 2019
ansis
ansis approved these changes Aug 19, 2019
View reviewed changes
Copy link
Contributor

@ansis ansis left a comment

Choose a reason for hiding this comment

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

Everything looks great!

@chloekraw
Copy link
Contributor

chloekraw commented Aug 20, 2019 •
edited
Loading

Possible changelog entry:

Fixed rendering and collision detection issues with using text-variable-anchor and icon-text-fit properties on the same layer.

(Noooo sorry I closed the pr by mistake.)

@chloekraw chloekraw closed this Aug 20, 2019
@chloekraw chloekraw reopened this Aug 20, 2019
@alexshalamov alexshalamov force-pushed the alexshalamov_fix_icon-text-fit branch from 5c78282 to b105b5b Compare August 20, 2019 08:17
@alexshalamov alexshalamov requested a review from 1ec5 as a code owner August 20, 2019 08:17
@alexshalamov alexshalamov requested a review from a team August 20, 2019 08:17
@alexshalamov alexshalamov removed the needs changelog Indicates PR needs a changelog entry prior to merging. label Aug 20, 2019
@alexshalamov alexshalamov force-pushed the alexshalamov_fix_icon-text-fit branch from b105b5b to 0cf61b7 Compare August 20, 2019 08:41
@alexshalamov alexshalamov merged commit 1625aa0 into master Aug 20, 2019
@alexshalamov alexshalamov deleted the alexshalamov_fix_icon-text-fit branch August 20, 2019 09:31
@chloekraw chloekraw mentioned this pull request Aug 21, 2019
Open
This was referenced Sep 3, 2019
Closed
Closed
ansis added a commit to mapbox/mapbox-gl-js that referenced this pull request Sep 7, 2019
@ansis
wip port mapbox/mapbox-gl-native#15367
8aa109d
@ansis ansis mentioned this pull request Sep 7, 2019
Closed
@alexshalamov alexshalamov mentioned this pull request Sep 13, 2019
Merged
@ansis ansis mentioned this pull request Sep 25, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pozdnyakov pozdnyakov pozdnyakov approved these changes

@ansis ansis ansis approved these changes

@1ec5 1ec5 Awaiting requested review from 1ec5

Assignees

@alexshalamov alexshalamov

Labels
None yet
Projects
None yet
Milestone
release-queso
Development

Successfully merging this pull request may close these issues.

[core] Combination of text-variable-anchor and icon-text-fit is broken
4 participants
@alexshalamov @chloekraw @pozdnyakov @ansis