-
-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Fix BalloonMarker's text position calculation, consider insets #3035
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3035 +/- ##
=======================================
Coverage 19.77% 19.77%
=======================================
Files 113 113
Lines 13663 13663
=======================================
Hits 2702 2702
Misses 10961 10961 Continue to review full report at Codecov.
|
please give a context what's wrong. |
This seems to be a good change, but as @liuxuan30 said: What is the problem in the current library? |
@liuxuan30 @jjatie Has no effect on the current library, repair logic is not rigorous, ignored the horizontal center. |
again, we understand this should not impact the current library, we just need to know what's the change is about. You changed the x position and width to respect inset, we want to know the user case. Did you meet any issue? Or the x direction inset broke something on your side? I guess when it was implemented, @danielgindi might considered this already, and intended only y direction. |
@yangcaimu This looks like a good change, we just want to document it better so if there is a question or issue in the future, we can quickly understand the justification behind what's there. |
Thank you for your patience reply, I'll make it clearer. At present, the text is centered because the If you do not understand, I can provide demo. |
ok. So you change to |
@yangcaimu can you post compare images before/after changing the inset? |
there is also this solution |
Before: After: |
@liuxuan30 I like this change. While it's already currently possible to achieve the same look, I think this way is much more intuitive. If you're okay with it let's merge. |
sure thing. once we know how it looks after fix, it makes sense then. |
Consider insets, adjust the position of the label in the context of the center.