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

Change arrow positioning logic for ArrowPositionRules.ALIGN_ANCHOR #560

Merged
merged 2 commits into from
Dec 21, 2023

Conversation

nkhar
Copy link
Contributor

@nkhar nkhar commented Dec 20, 2023

minPosition and maxPosition are randomly chosen so they don't always give correct results. Here we tried to solve on edge case as noted in issue: #413

PR help needed

Hello I am Nika, I have been using this library for quite some time, I wanted to help in some way.
This is my first PR to public repo, I am new to Open source contribution, please feel free to provide any criticism that can improve this PR or my abilities, thank you.

🎯 Goal

Goal of this PR is to solve arrow positioning issues such as #413

🛠 Implementation details

For now only a certain edge case that was in issue#413 has been fixed. Logic seems to be quite complicated. We will have to calculate size of the balloon, size of the anchor, their positioning relative to each other and also size of the arrow.
As mentioned above minPosition and maxPosition are random numbers based on arrow size, some multiplying factor and padding.
private fun getMinArrowPosition(): Float { return (builder.arrowSize.toFloat() * builder.arrowAlignAnchorPaddingRatio) + builder.arrowAlignAnchorPadding }

Code in this PR return 0 for position if arrow position is below the x position of balloon. On the other hand if anchor is within the borders of ballon the code returns arrow position with balloon position substracted, since arrow is already positioned relative to balloon.
val tipArrowPosition = anchorX + (anchor.width) * builder.arrowPosition if ((tipArrowPosition - builder.arrowHalfSize) <= balloonX) { return 0.0f // + builder.cornerRadius } if ((tipArrowPosition - builder.arrowHalfSize) > balloonX) { if (anchor.width <= getMeasuredWidth() - builder.marginRight - builder.marginLeft) { return tipArrowPosition - builder.arrowHalfSize - balloonX } }

Further feedback is required

This code is not enough to fix all the edge cases, some further changes depend on the preferences of developer, such as how to calculate min and max positions.

nkhar and others added 2 commits December 20, 2023 22:25
…give correct results. Here we tried to solve on edge case as noted in issue: skydoves#413
Change arrow positioning logic for ArrowPositionRules.ALIGN_ANCHOR
@nkhar nkhar requested a review from skydoves as a code owner December 20, 2023 18:58
Copy link
Owner

@skydoves skydoves left a comment

Choose a reason for hiding this comment

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

This is awesome.

Certainly, this solution won't address every edge case, like scenarios involving a large corner radius that demand complex mathematical calculations. Nevertheless, this contribution is poised to be significantly beneficial for many users. I will take a look at other cases if I have a chance. Thank you so much for your contribution!

@skydoves skydoves merged commit 06cc1ff into skydoves:main Dec 21, 2023
3 checks passed
@nkhar
Copy link
Contributor Author

nkhar commented Dec 21, 2023

Thank you, I appreciate the kind words and I am truly grateful for your generous feedback.

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.

2 participants