Skip to content

Commit

Permalink
fix: properly take padding into consideration for selection events on…
Browse files Browse the repository at this point in the history
… Android (#774)

## 📜 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

<!-- High level overview of important changes -->
<!-- For example: fixed status bar manipulation; added new types
declarations; -->
<!-- If your changes don't affect one of platform/language below - then
remove this platform/language -->

### 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):

<img width="381" alt="image"
src="https://github.com/user-attachments/assets/61036446-da2d-4c45-b1ef-8e2ddd723bec"
/>

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
  • Loading branch information
kirillzyusko authored Jan 24, 2025
1 parent 241d157 commit f4eb088
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,15 @@ class KeyboardControllerSelectionWatcher(
0
}
val gravity = editText.gravity and Gravity.VERTICAL_GRAVITY_MASK
val paddingVertical = editText.paddingTop + editText.paddingBottom
val lineHeightHalfHearted = editText.lineHeight / 2
val verticalOffset =
when (gravity) {
Gravity.CENTER_VERTICAL -> (editTextHeight - textHeight) / 2 + editText.paddingTop
Gravity.BOTTOM -> editTextHeight - textHeight
Gravity.CENTER_VERTICAL ->
(editTextHeight - paddingVertical - textHeight) / 2 + editText.paddingTop + lineHeightHalfHearted
Gravity.BOTTOM -> editTextHeight - textHeight - editText.paddingBottom + lineHeightHalfHearted
// Default to Gravity.TOP or other cases
else -> editText.paddingTop * 2
else -> editText.paddingTop + lineHeightHalfHearted
}

cursorPositionStartX = layout.getPrimaryHorizontal(realStart)
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit f4eb088

Please sign in to comment.