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

fix: properly take padding into consideration for selection events on Android #774

Merged
merged 2 commits into from
Jan 24, 2025

Conversation

kirillzyusko
Copy link
Owner

@kirillzyusko kirillzyusko commented Jan 23, 2025

📜 Description

Correctly handling padding for selection events, especially when it's different (comparing top/bottom).

💡 Motivation and Context

If you apply different paddingTop/paddingBottom, then you will notice, that selection coordinates are not reporting expected values. It happens because of the fact, that we need to take padding into consideration, when we calculate relative text position inside input.

For different gravity we need to use different formulas:

  • center: we need to exclude text layout and vertical paddings, then divide by 2 (we'll get a distance that put text in center relative to EditText), and then add paddingTop
  • for bottom: we need to exclude layout and bottom padding
  • for top we just need to exclude top padding

And after that we need to add lineHeight / 2 (because otherwise caret will be in the middle of the text,

To understand the problem try various combinations of paddingTop=60/paddingBottom=20/textAlignVertical="top|center|bottom" (reverse paddings as well).

You'll see, how it produces different values. With this PR in place we always will get the same relative position (blue box will always follow a carret).

📢 Changelog

Android

  • take padding into consideration when we calculate vertical offset (which is based on gravity);
  • add lineHeight / 2 to final vertical offset.

🤔 How Has This Been Tested?

Tested on Medium Phone API 35.

📸 Screenshots (if appropriate):

image

📝 Checklist

  • CI successfully passed
  • I added new mocks and corresponding unit-tests if library API was changed

@kirillzyusko kirillzyusko added 🐛 bug Something isn't working 🤖 android Android specific focused input 📝 Anything about focused input functionality labels Jan 23, 2025
@kirillzyusko kirillzyusko self-assigned this Jan 23, 2025
Copy link
Contributor

github-actions bot commented Jan 23, 2025

📊 Package size report

Current size Target Size Difference
168507 bytes 168453 bytes 54 bytes 📈

@kirillzyusko kirillzyusko marked this pull request as ready for review January 24, 2025 10:43
@kirillzyusko kirillzyusko merged commit f4eb088 into main Jan 24, 2025
21 checks passed
@kirillzyusko kirillzyusko deleted the fix/selection-proper-padding-consideration branch January 24, 2025 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 android Android specific 🐛 bug Something isn't working focused input 📝 Anything about focused input functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant