Skip to content
This repository has been archived by the owner on Sep 27, 2024. It is now read-only.

Commit

Permalink
[Android] Apply a better solution for #878 (#881)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@jmartinesp
jmartinesp authored Nov 20, 2023
1 parent ecb2bb0 commit 320f5c1
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 36 deletions.
52 changes: 42 additions & 10 deletions ...droid/library/src/androidTest/java/io/element/android/wysiwyg/EditorEditTextInputTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ import io.mockk.verify
import org.hamcrest.CoreMatchers
import org.hamcrest.CoreMatchers.containsString
import org.hamcrest.Description
import org.hamcrest.MatcherAssert
import org.hamcrest.Matchers
import org.junit.*
import org.junit.runner.RunWith
import uniffi.wysiwyg_composer.ActionState
Expand Down Expand Up @@ -513,16 +515,6 @@ class EditorEditTextInputTests {
confirmVerified(textWatcher)
}

@Test
fun testWritingOnlyDigits() {
onView(withId(R.id.rich_text_edit_text))
.perform(ImeActions.setComposingText("1"))
.perform(ImeActions.setComposingText("2"))
.perform(ImeActions.setComposingText("3"))
.perform(ImeActions.commitText(" "))
.check(matches(withText("123$NBSP")))
}

@Test
fun testPasteImage() {
val imageUri = Uri.parse("content://fakeImage")
Expand Down Expand Up @@ -648,6 +640,46 @@ class EditorEditTextInputTests {
})
}
}

@Test
fun testImeInputWithDigits() {
// This test emulates Gboard input
onView(withId(R.id.rich_text_edit_text))
.perform(EditorActions.setText(""))
.perform(ImeActions.commitText("1"))
.perform(ImeActions.commitText("2"))
.perform(ImeActions.commitText("3"))
.perform(ImeActions.commitText(" "))
.perform(ImeActions.setComposingText("hey"))
.perform(ImeActions.commitText("hey"))
.perform(ImeActions.commitText(" "))
.perform(ImeActions.setComposingText("1"))
.perform(ImeActions.commitText("1"))
.perform(ImeActions.commitText("2"))
.perform(ImeActions.commitText("3"))
.perform(ImeActions.commitText(" "))
.check(matches(withText("123 hey 123$NBSP")))
}

@Test
fun testHardwareKeyboardInputWithDigits() {
// This test emulates the input on AnySoftKeyboard, Simple Keyboard or SwiftKey
onView(withId(R.id.rich_text_edit_text))
.perform(EditorActions.setText(""))
.perform(pressKey(KeyEvent.KEYCODE_1))
.perform(pressKey(KeyEvent.KEYCODE_2))
.perform(pressKey(KeyEvent.KEYCODE_3))
.perform(pressKey(KeyEvent.KEYCODE_SPACE))
.perform(pressKey(KeyEvent.KEYCODE_H))
.perform(pressKey(KeyEvent.KEYCODE_E))
.perform(pressKey(KeyEvent.KEYCODE_Y))
.perform(pressKey(KeyEvent.KEYCODE_SPACE))
.perform(pressKey(KeyEvent.KEYCODE_1))
.perform(pressKey(KeyEvent.KEYCODE_2))
.perform(pressKey(KeyEvent.KEYCODE_3))
.perform(pressKey(KeyEvent.KEYCODE_SPACE))
.check(matches(withText("123 hey 123$NBSP")))
}
}

class TextViewMatcher(
Expand Down
56 changes: 30 additions & 26 deletions ...ibrary/src/main/java/io/element/android/wysiwyg/inputhandlers/InterceptInputConnection.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,15 @@ internal class InterceptInputConnection(
val result = processTextEntry(text, start, end)

return if (result != null) {
beginBatchEdit()
val newStart = start.coerceIn(0, result.text.length)
val newEnd = (newStart + text.length).coerceIn(newStart, result.text.length)

replaceAll(result.text, compositionStart = newStart, compositionEnd = newEnd)
setSelectionOnEditable(editable, result.selection.last, result.selection.last)
replaceAll(result.text)
val editorSelectionIndex = editorIndex(result.selection.last, editable)
setSelection(editorSelectionIndex, editorSelectionIndex)
setComposingRegion(newStart, newEnd)
endBatchEdit()
true
} else {
super.setComposingText(text, newCursorPosition)
Expand All @@ -125,8 +129,12 @@ internal class InterceptInputConnection(
val result = processTextEntry(text, start, end)

return if (result != null) {
replaceAll(result.text, compositionStart = end, compositionEnd = end)
setSelectionOnEditable(editable, result.selection.last, result.selection.last)
beginBatchEdit()
replaceAll(result.text)
val editorSelectionIndex = editorIndex(result.selection.last, editable)
setSelection(editorSelectionIndex, editorSelectionIndex)
setComposingRegion(editorSelectionIndex, editorSelectionIndex)
endBatchEdit()
true
} else {
super.commitText(text, newCursorPosition)
Expand Down Expand Up @@ -235,7 +243,13 @@ internal class InterceptInputConnection(
// append to the current composition with the hardware keyboard.
finishComposingText()

return setComposingText(newChars, 1)
return if (newChars.isDigitsOnly()) {
// Digits should be sent using `commitText`, as that's the default behaviour in the IME
commitText(newChars, 1)
} else {
// Any other printable character can be sent using `setComposingText`
setComposingText(newChars, 1)
}
}

override fun deleteSurroundingText(beforeLength: Int, afterLength: Int): Boolean {
Expand All @@ -260,9 +274,10 @@ internal class InterceptInputConnection(
processInput(action)
}
if (result != null) {
replaceAll(result.text, result.selection.first, result.selection.last)
setSelectionOnEditable(editable, result.selection.first, result.selection.last)
setComposingRegion(result.selection.first, result.selection.last)
replaceAll(result.text)
val editorSelectionIndex = editorIndex(result.selection.first, editable)
setSelection(editorSelectionIndex, editorSelectionIndex)
setComposingRegion(editorSelectionIndex, editorSelectionIndex)
}
// TODO: handle result == null
handled = true
Expand All @@ -281,9 +296,10 @@ internal class InterceptInputConnection(
processInput(EditorInputAction.BackPress)
}
if (result != null) {
replaceAll(result.text, result.selection.first, result.selection.last)
setSelectionOnEditable(editable, result.selection.first, result.selection.last)
setComposingRegion(result.selection.first, result.selection.last)
replaceAll(result.text)
val editorSelectionIndex = editorIndex(result.selection.first, editable)
setSelection(editorSelectionIndex, editorSelectionIndex)
setComposingRegion(editorSelectionIndex, editorSelectionIndex)
}
// TODO: handle result == null
handled = true
Expand Down Expand Up @@ -315,25 +331,13 @@ internal class InterceptInputConnection(
return start to end
}

private fun replaceAll(
charSequence: CharSequence,
compositionStart: Int,
compositionEnd: Int,
) {
beginBatchEdit()
private fun replaceAll(charSequence: CharSequence) {
editable.removeFormattingSpans()
editable.replace(0, editable.length, charSequence)
val newComposition = editable.substring(compositionStart, compositionEnd)
if (newComposition.isEmpty() || !newComposition.isDigitsOnly()) {
setComposingRegion(compositionStart, compositionEnd)
}
endBatchEdit()
}

private fun setSelectionOnEditable(editable: Editable, start: Int, end: Int = start) {
val newStart = min(EditorIndexMapper.editorIndexFromComposer(start, editable), editable.length)
val newEnd = min(EditorIndexMapper.editorIndexFromComposer(end, editable), editable.length)
Selection.setSelection(editable, newStart, newEnd)
private fun editorIndex(composerIndex: Int, editable: Editable): Int {
return min(EditorIndexMapper.editorIndexFromComposer(composerIndex, editable), editable.length)
}

private fun <T> withProcessor(block: EditorViewModel.() -> T): T {
Expand Down

0 comments on commit 320f5c1

Please sign in to comment.