Fix disappearing numbers with some keyboards #878
Conversation
jmartinesp
force-pushed
the
fix/jme/android-writing-numbers
branch
from
daff96f to
1bbf8dc
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #878 +/- ##
============================================
- Coverage 87.43% 87.28% -0.16%
- Complexity 406 409 +3
============================================
Files 165 133 -32
Lines 18956 17360 -1596
Branches 1045 405 -640
============================================
- Hits 16575 15152 -1423
+ Misses 2087 1937 -150
+ Partials 294 271 -23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
...d/library/src/main/java/io/element/android/wysiwyg/inputhandlers/InterceptInputConnection.kt
Show resolved
Hide resolved
| val start = compositionStart.coerceIn(0, editable.length) | ||
| val end = compositionEnd.coerceIn(0, editable.length) | ||
| val newComposition = editable.substring(start, end) | ||
| if (newComposition.isEmpty() || !newComposition.isDigitsOnly()) { |
There was a problem hiding this comment.
It seems a bit odd that replaceAll will not honour the given compositionStart and compositionEnd. Maybe the caller could be responsible for passing the right arguments? Or we can move the responsibility of setting the new composition out of replaceAll?
There was a problem hiding this comment.
I changed this on 88c81f9
This was actually hiding an issue with how the composition is set in `deleteSurroundingText`.
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -280,7 +281,7 @@ internal class InterceptInputConnection( | |||
| processInput(EditorInputAction.BackPress) | |||
| } | |||
| if (result != null) { | |||
| replaceAll(result.text, 0, editable.length) | |||
| replaceAll(result.text, result.selection.first, result.selection.last) | |||
There was a problem hiding this comment.
I guess this fixes a separate bug altogether?
Just to double check, is this new logic safe i.e. are the indices in result.selection guaranteed to be valid within result.text?
There was a problem hiding this comment.
Oh, maybe they're not. I forgot these were composer indexes that need to be translated to editor ones. Damn, I'll create a new PR with a fix for that.
* Ensure editor indexes are used for selection and composition: - Removes the responsibility of setting the composition from `replaceAll`. - Also removes `beginBatchEdit` and `endBatchEdit` so it's the calling functions who should manage that. - Calculate new indexes for selection and composition using `fun editorIndex(composerIndex, editable)`, do it just once per operation. * Apply a better solution for #878 * Add better tests * Simplify code * Input the while text as key presses in `testHardwareKeyboardInputWithDigits`, since it's way less flaky this way
Don't set composition if the replaced text is all digits. This seems to be the default behaviour in all keyboards I've tested (Gboard, AnySoftKeyboard, Simple Keyboard...), and prevents the composition (the last digit) from being replaced with a whitespace.
Fixes element-hq/element-x-android#1415.