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

[Android] Apply a better solution for #878 #881

Merged
merged 5 commits into from
Nov 20, 2023
Merged

[Android] Apply a better solution for #878 #881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
32e8227
Ensure editor indexes are used for selection and composition:
jmartinesp Nov 20, 2023
a8d79a8
Apply a better solution for #878
jmartinesp Nov 20, 2023
c9d8092
Add better tests
jmartinesp Nov 20, 2023
e34f743
Simplify code
jmartinesp Nov 20, 2023
6a2a012
Input the while text as key presses in `testHardwareKeyboardInputWith…
jmartinesp Nov 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 42 additions & 10 deletions ...droid/library/src/androidTest/java/io/element/android/wysiwyg/EditorEditTextInputTests.kt
View file
Open in desktop
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
Open in desktop
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()
jonnyandrew marked this conversation as resolved.
Show resolved Hide resolved
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()) {
jonnyandrew marked this conversation as resolved.
Show resolved Hide resolved
// 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)
Comment on lines +277 to +280
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think it's reasonable to assume that selection.first == selection.last but it feels a bit strange not to trust the Rust model. Anyway, it's not a big deal but if we did manage to refactor the result handling into a function (as mentioned in my other comment), that might fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mostly did it following what was already there and trying to avoid calculating indexes several times, I see no need to convert twice index x from the composer to get the same y editor index in both cases, doing extra work. Also, the composer doesn't have the concept of 'composition', so we some times may have to get a bit creative with selections.

In the future we could add this concept to the composer, since it seems to be used in both Android an iOS (on iOS I believe it's called 'marked text'), but right now that would be way too much work for the time we have to work on the library.

}
// 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
Loading