From 65b72e225f8d4ed41dbfa939df4720953c14d743 Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Tue, 28 Nov 2023 17:01:17 +0100 Subject: [PATCH] [Android] Fixes to make `EditorStyleText` work properly in Compose (#891) * Fix link click detection. Before this, the `TextView` would consume the touch event even if no link or mention was clicked. * Fix race condition that incorrectly rendered the mentions, as they weren't being detected as mentions at all. * Add `TextStyle.lineHeight` property to modify line height. * Bump JNA library version. --- .../wysiwyg/compose/EditorStyledText.kt | 2 +- .../wysiwyg/compose/RichTextEditorDefaults.kt | 2 + .../wysiwyg/compose/RichTextEditorStyle.kt | 1 + .../internal/RichTextEditorStyleExt.kt | 8 ++++ platforms/android/library/build.gradle | 2 +- .../android/wysiwyg/EditorStyledTextView.kt | 44 ++++++++++++------- .../android/wysiwyg/utils/ResourcesHelper.kt | 6 +++ .../android/wysiwyg/utils/RustCleanerTask.kt | 13 ++++++ 8 files changed, 61 insertions(+), 17 deletions(-) create mode 100644 platforms/android/library/src/main/java/io/element/android/wysiwyg/utils/RustCleanerTask.kt diff --git a/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/EditorStyledText.kt b/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/EditorStyledText.kt index 2141f65bd..313825f3b 100644 --- a/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/EditorStyledText.kt +++ b/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/EditorStyledText.kt @@ -53,9 +53,9 @@ fun EditorStyledText( EditorStyledTextView(context) }, update = { view -> + view.updateStyle(style.toStyleConfig(view.context), mentionDisplayHandler) view.applyStyleInCompose(style) view.typeface = typeface - view.updateStyle(style.toStyleConfig(view.context), mentionDisplayHandler) if (text is Spanned) { view.setText(text, TextView.BufferType.SPANNABLE) } else { diff --git a/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/RichTextEditorDefaults.kt b/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/RichTextEditorDefaults.kt index df18fb70a..9408aa0e0 100644 --- a/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/RichTextEditorDefaults.kt +++ b/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/RichTextEditorDefaults.kt @@ -144,6 +144,7 @@ object RichTextEditorDefaults { fun textStyle( color: Color = MaterialTheme.colorScheme.onSurface, fontSize: TextUnit = MaterialTheme.typography.bodyLarge.fontSize, + lineHeight: TextUnit = MaterialTheme.typography.bodyLarge.lineHeight, fontFamily: FontFamily? = MaterialTheme.typography.bodyLarge.fontFamily, fontWeight: FontWeight? = MaterialTheme.typography.bodyLarge.fontWeight, fontStyle: FontStyle? = MaterialTheme.typography.bodyLarge.fontStyle, @@ -151,6 +152,7 @@ object RichTextEditorDefaults { ) = TextStyle( color = color, fontSize = fontSize, + lineHeight = lineHeight, fontFamily = fontFamily, fontWeight = fontWeight, fontStyle = fontStyle, diff --git a/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/RichTextEditorStyle.kt b/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/RichTextEditorStyle.kt index 5cdb9b532..19ae8d470 100644 --- a/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/RichTextEditorStyle.kt +++ b/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/RichTextEditorStyle.kt @@ -51,6 +51,7 @@ data class PillStyle( data class TextStyle( val color: Color, val fontSize: TextUnit, + val lineHeight: TextUnit, val fontFamily: FontFamily?, val fontWeight: FontWeight?, val fontStyle: FontStyle?, diff --git a/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/internal/RichTextEditorStyleExt.kt b/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/internal/RichTextEditorStyleExt.kt index a6995da87..59a99be34 100644 --- a/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/internal/RichTextEditorStyleExt.kt +++ b/platforms/android/library-compose/src/main/java/io/element/android/wysiwyg/compose/internal/RichTextEditorStyleExt.kt @@ -89,6 +89,14 @@ internal fun TextStyle.rememberTypeface(): State { internal fun TextView.applyStyleInCompose(style: RichTextEditorStyle) { setTextColor(style.text.color.toArgb()) + val lineHeightInPx = TypedValue.applyDimension( + TypedValue.COMPLEX_UNIT_DIP, + style.text.lineHeight.value, + context.resources.displayMetrics + ) + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + lineHeight = lineHeightInPx.roundToInt() + } setTextSize(TypedValue.COMPLEX_UNIT_SP, style.text.fontSize.value) if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { val cursorDrawable = ContextCompat.getDrawable(context, R.drawable.cursor) diff --git a/platforms/android/library/build.gradle b/platforms/android/library/build.gradle index cabec9ca3..8415ec3bd 100644 --- a/platforms/android/library/build.gradle +++ b/platforms/android/library/build.gradle @@ -86,7 +86,7 @@ kotlin { dependencies { - implementation "net.java.dev.jna:jna:5.7.0@aar" + implementation "net.java.dev.jna:jna:5.13.0@aar" implementation libs.kotlin.coroutines.android implementation libs.kotlin.coroutines diff --git a/platforms/android/library/src/main/java/io/element/android/wysiwyg/EditorStyledTextView.kt b/platforms/android/library/src/main/java/io/element/android/wysiwyg/EditorStyledTextView.kt index 6830d552c..87452a41c 100644 --- a/platforms/android/library/src/main/java/io/element/android/wysiwyg/EditorStyledTextView.kt +++ b/platforms/android/library/src/main/java/io/element/android/wysiwyg/EditorStyledTextView.kt @@ -10,9 +10,11 @@ import androidx.appcompat.widget.AppCompatTextView import androidx.core.graphics.withTranslation import androidx.core.text.getSpans import androidx.core.view.GestureDetectorCompat +import com.sun.jna.internal.Cleaner import io.element.android.wysiwyg.display.MentionDisplayHandler import io.element.android.wysiwyg.internal.view.EditorEditTextAttributeReader import io.element.android.wysiwyg.utils.HtmlConverter +import io.element.android.wysiwyg.utils.RustCleanerTask import io.element.android.wysiwyg.view.StyleConfig import io.element.android.wysiwyg.view.inlinebg.SpanBackgroundHelper import io.element.android.wysiwyg.view.inlinebg.SpanBackgroundHelperFactory @@ -28,7 +30,18 @@ import uniffi.wysiwyg_composer.newMentionDetector */ open class EditorStyledTextView : AppCompatTextView { - private var mentionDetector: MentionDetector? = null + // Used to automatically clean up the native resources when this instance is GCed + private val cleaner = Cleaner.getCleaner() + + private val mentionDetector: MentionDetector? by lazy { + if (!isInEditMode) { + val detector = newMentionDetector() + cleaner.register(this, RustCleanerTask(detector)) + detector + } else { + null + } + } private lateinit var inlineCodeBgHelper: SpanBackgroundHelper private lateinit var codeBlockBgHelper: SpanBackgroundHelper @@ -43,13 +56,20 @@ open class EditorStyledTextView : AppCompatTextView { private val spannableFactory = ReuseSourceSpannableFactory() - private var mentionDisplayHandler: MentionDisplayHandler? = null + var mentionDisplayHandler: MentionDisplayHandler? = null private var htmlConverter: HtmlConverter? = null var onLinkClickedListener: ((String) -> Unit)? = null // This gesture detector will be used to detect clicks on spans private val gestureDetector = GestureDetectorCompat(context, object : GestureDetector.SimpleOnGestureListener() { + + override fun onDown(e: MotionEvent): Boolean { + // Find any spans in the coordinates + val spans = findSpansForTouchEvent(e) + return spans.any { it is LinkSpan || it is PillSpan || it is CustomMentionSpan } + } + override fun onSingleTapUp(e: MotionEvent): Boolean { // Find any spans in the coordinates val spans = findSpansForTouchEvent(e) @@ -59,17 +79,20 @@ open class EditorStyledTextView : AppCompatTextView { when (span) { is LinkSpan -> { onLinkClickedListener?.invoke(span.url) + return true } is PillSpan -> { span.url?.let { onLinkClickedListener?.invoke(it) } + return true } is CustomMentionSpan -> { span.url?.let { onLinkClickedListener?.invoke(it) } + return true } else -> Unit } } - return true + return false } }) @@ -138,18 +161,9 @@ open class EditorStyledTextView : AppCompatTextView { override fun onAttachedToWindow() { super.onAttachedToWindow() - mentionDetector = if (isInEditMode) null else newMentionDetector() - updateStyle(styleConfig, mentionDisplayHandler) } - override fun onDetachedFromWindow() { - super.onDetachedFromWindow() - - mentionDetector?.destroy() - mentionDetector = null - } - private fun createHtmlConverter(styleConfig: StyleConfig, mentionDisplayHandler: MentionDisplayHandler?): HtmlConverter { return HtmlConverter.Factory.create( context = context, @@ -165,11 +179,11 @@ open class EditorStyledTextView : AppCompatTextView { override fun onTouchEvent(event: MotionEvent?): Boolean { // We pass the event to the gesture detector - event?.let { gestureDetector.onTouchEvent(it) } + val handled = event?.let { gestureDetector.onTouchEvent(it) } // This will handle the default actions for any touch event in the TextView super.onTouchEvent(event) - // We need to return true to be able to detect any events - return true + // We return if we handled the event and want to intercept it or not + return handled ?: false } private fun findSpansForTouchEvent(event: MotionEvent): Array { diff --git a/platforms/android/library/src/main/java/io/element/android/wysiwyg/utils/ResourcesHelper.kt b/platforms/android/library/src/main/java/io/element/android/wysiwyg/utils/ResourcesHelper.kt index c48db5e2c..c46d66f39 100644 --- a/platforms/android/library/src/main/java/io/element/android/wysiwyg/utils/ResourcesHelper.kt +++ b/platforms/android/library/src/main/java/io/element/android/wysiwyg/utils/ResourcesHelper.kt @@ -6,6 +6,9 @@ import androidx.annotation.ColorRes import androidx.annotation.Dimension import androidx.core.content.res.ResourcesCompat +/** + * This class provides access to resources needed to convert HTML to spans. + */ internal interface ResourcesHelper { fun getDisplayMetrics(): DisplayMetrics @@ -14,6 +17,9 @@ internal interface ResourcesHelper { fun getColor(@ColorRes colorId: Int): Int } +/** + * This class provides access to Android resources needed to convert HTML to spans. + */ internal class AndroidResourcesHelper( private val application: Application, ) : ResourcesHelper { diff --git a/platforms/android/library/src/main/java/io/element/android/wysiwyg/utils/RustCleanerTask.kt b/platforms/android/library/src/main/java/io/element/android/wysiwyg/utils/RustCleanerTask.kt new file mode 100644 index 000000000..4142a4d86 --- /dev/null +++ b/platforms/android/library/src/main/java/io/element/android/wysiwyg/utils/RustCleanerTask.kt @@ -0,0 +1,13 @@ +package io.element.android.wysiwyg.utils + +import timber.log.Timber +import uniffi.wysiwyg_composer.Disposable + +internal class RustCleanerTask( + private val disposable: Disposable, +) : Runnable { + override fun run() { + Timber.d("Cleaning up disposable: $disposable") + disposable.destroy() + } +} \ No newline at end of file