From a573bebcae2c9a90c0cc55bb7950185f9ef083ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Saleniuk?= <30429749+saleniuk@users.noreply.github.com> Date: Fri, 14 Jun 2024 14:46:04 +0200 Subject: [PATCH] fix: make learn more links clickable for automation v2 [WPB-5888] (#3064) --- .../ui/common/PreviewTextWithLinkSuffix.kt | 53 ++++++++ .../android/ui/common/TextWithLinkSuffix.kt | 124 +++++++++++++----- 2 files changed, 144 insertions(+), 33 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/ui/common/PreviewTextWithLinkSuffix.kt b/app/src/main/kotlin/com/wire/android/ui/common/PreviewTextWithLinkSuffix.kt index b293680821b..701545a8c2a 100644 --- a/app/src/main/kotlin/com/wire/android/ui/common/PreviewTextWithLinkSuffix.kt +++ b/app/src/main/kotlin/com/wire/android/ui/common/PreviewTextWithLinkSuffix.kt @@ -20,12 +20,15 @@ package com.wire.android.ui.common import androidx.compose.foundation.layout.width import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalDensity +import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.text.AnnotatedString import androidx.compose.ui.text.rememberTextMeasurer import androidx.compose.ui.text.style.TextDecoration import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.LayoutDirection import com.wire.android.ui.theme.WireTheme import com.wire.android.ui.theme.wireTypography import com.wire.android.util.ui.PreviewMultipleThemes @@ -52,6 +55,8 @@ private fun PreviewTextWithLinkSuffixBuilder( ) } +// ----- LTR ----- + @PreviewMultipleThemes @Composable fun PreviewTextWithLinkSuffixWithoutALink() = WireTheme { @@ -87,3 +92,51 @@ fun PreviewTextWithLinkSuffixMultilineNotFittingInLastLine() = WireTheme { linkText = "link" ) { lastTextLineWidthDp, linkWidthDp -> lastTextLineWidthDp + (linkWidthDp / 2) } } + +// ----- RTL ----- + +@PreviewMultipleThemes +@Composable +fun PreviewTextWithLinkSuffixWithoutALinkRtl() = WireTheme { + CompositionLocalProvider(LocalLayoutDirection provides LayoutDirection.Rtl) { + TextWithLinkSuffix(text = AnnotatedString("This is a text without a link")) + } +} + +@PreviewMultipleThemes +@Composable +fun PreviewTextWithLinkSuffixFittingInSameLineRtl() = WireTheme { + CompositionLocalProvider(LocalLayoutDirection provides LayoutDirection.Rtl) { + PreviewTextWithLinkSuffixBuilder { lastTextLineWidthDp, linkWidthDp -> lastTextLineWidthDp + linkWidthDp } + } +} + +@PreviewMultipleThemes +@Composable +fun PreviewTextWithLinkSuffixNotFittingInSameLineRtl() = WireTheme { + CompositionLocalProvider(LocalLayoutDirection provides LayoutDirection.Rtl) { + PreviewTextWithLinkSuffixBuilder { lastTextLineWidthDp, linkWidthDp -> lastTextLineWidthDp + (linkWidthDp / 2) } + } +} + +@PreviewMultipleThemes +@Composable +fun PreviewTextWithLinkSuffixMultilineFittingInLastLineRtl() = WireTheme { + CompositionLocalProvider(LocalLayoutDirection provides LayoutDirection.Rtl) { + PreviewTextWithLinkSuffixBuilder( + textLines = listOf("This is a text with a link", "This is a text with a"), + linkText = "link", + ) { lastTextLineWidthDp, linkWidthDp -> lastTextLineWidthDp + linkWidthDp } + } +} + +@PreviewMultipleThemes +@Composable +fun PreviewTextWithLinkSuffixMultilineNotFittingInLastLineRtl() = WireTheme { + CompositionLocalProvider(LocalLayoutDirection provides LayoutDirection.Rtl) { + PreviewTextWithLinkSuffixBuilder( + textLines = listOf("This is a text with a", "This is a text with a"), + linkText = "link" + ) { lastTextLineWidthDp, linkWidthDp -> lastTextLineWidthDp + (linkWidthDp / 2) } + } +} diff --git a/core/ui-common/src/main/kotlin/com/wire/android/ui/common/TextWithLinkSuffix.kt b/core/ui-common/src/main/kotlin/com/wire/android/ui/common/TextWithLinkSuffix.kt index 95f2485413a..114c64e2d97 100644 --- a/core/ui-common/src/main/kotlin/com/wire/android/ui/common/TextWithLinkSuffix.kt +++ b/core/ui-common/src/main/kotlin/com/wire/android/ui/common/TextWithLinkSuffix.kt @@ -18,15 +18,25 @@ package com.wire.android.ui.common import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.size import androidx.compose.foundation.text.InlineTextContent import androidx.compose.foundation.text.appendInlineContent import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier +import androidx.compose.ui.geometry.Offset import androidx.compose.ui.graphics.Color +import androidx.compose.ui.layout.Layout +import androidx.compose.ui.layout.layoutId +import androidx.compose.ui.layout.onGloballyPositioned +import androidx.compose.ui.layout.positionInParent import androidx.compose.ui.platform.LocalDensity -import androidx.compose.ui.platform.testTag import androidx.compose.ui.text.AnnotatedString import androidx.compose.ui.text.Placeholder import androidx.compose.ui.text.PlaceholderVerticalAlign @@ -35,23 +45,78 @@ import androidx.compose.ui.text.TextStyle import androidx.compose.ui.text.buildAnnotatedString import androidx.compose.ui.text.rememberTextMeasurer import androidx.compose.ui.text.style.TextDecoration +import androidx.compose.ui.unit.DpSize import com.wire.android.ui.theme.wireColorScheme import com.wire.android.ui.theme.wireTypography @Composable fun TextWithLinkSuffix( text: AnnotatedString, + modifier: Modifier = Modifier, linkText: String? = null, onLinkClick: () -> Unit = {}, - linkTag: String = "link", textStyle: TextStyle = MaterialTheme.wireTypography.body01, textColor: Color = MaterialTheme.wireColorScheme.onBackground, linkStyle: TextStyle = MaterialTheme.wireTypography.body02, linkColor: Color = MaterialTheme.wireColorScheme.primary, linkDecoration: TextDecoration = TextDecoration.Underline, - onTextLayout: (TextLayoutResult) -> Unit = {}, - modifier: Modifier = Modifier, + onTextLayout: (TextLayoutResult) -> Unit = {} ) { + var linkPosition by remember { mutableStateOf(Offset(0f, 0f)) } + val (inlineText, inlineContent) = buildInlineText( + text = text, + linkText = linkText, + linkStyle = linkStyle, + linkDecoration = linkDecoration, + onLinkPositionCalculated = { linkPosition = it } + ) + + // For some reason automation tests can't find inlined text content, so it needs to be added directly in layout. Inlined text content + // is still used to get the size of the link text and its position so that it works no matter what locale is used. Position of the link + // is then used in this layout and the proper text composable that can be found by automation tests is placed where it should be. + Layout( + modifier = modifier, + content = { + Text( + text = inlineText, + style = textStyle, + color = textColor, + inlineContent = inlineContent, + onTextLayout = onTextLayout, + modifier = Modifier.layoutId("text") + ) + + if (linkText != null) { + Text( + text = linkText, + style = linkStyle, + color = linkColor, + textDecoration = linkDecoration, + modifier = Modifier.layoutId("link") + .clickable(onClick = onLinkClick) + ) + } + }, + measurePolicy = { measurables, constraints -> + val measureConstraints = constraints.copy(minWidth = 0, minHeight = 0) + val textPlaceable = measurables.first { it.layoutId == "text" }.measure(measureConstraints) + val linkPlaceable = measurables.firstOrNull { it.layoutId == "link" }?.measure(measureConstraints) + layout(width = textPlaceable.width, height = textPlaceable.height) { + textPlaceable.placeRelative(0, 0) + linkPlaceable?.place(linkPosition.x.toInt(), linkPosition.y.toInt()) + } + } + ) +} + +@Composable +private fun buildInlineText( + text: AnnotatedString, + linkText: String?, + linkStyle: TextStyle, + linkDecoration: TextDecoration, + onLinkPositionCalculated: (Offset) -> Unit +): Pair> { val textMeasurer = rememberTextMeasurer() val linkId = "link" val inlineText = linkText?.let { @@ -68,42 +133,35 @@ fun TextWithLinkSuffix( text = linkText, style = linkStyle.copy(textDecoration = linkDecoration), ) - val textSize = textLayoutResult.size val density = LocalDensity.current val (linkWidthSp, linkHeightSp) = with(density) { - textSize.width.toSp() to textSize.height.toSp() + textLayoutResult.size.width.toSp() to textLayoutResult.size.height.toSp() + } + val linkSizeDp = with(density) { + DpSize(textLayoutResult.size.width.toDp(), textLayoutResult.size.height.toDp()) } - put( linkId, InlineTextContent( - placeholder = Placeholder( - width = linkWidthSp, - height = linkHeightSp, - placeholderVerticalAlign = PlaceholderVerticalAlign.Bottom - ), - children = { - Text( - text = linkText, - style = linkStyle, - color = linkColor, - textDecoration = linkDecoration, - modifier = Modifier - .testTag(linkTag) - .clickable(onClick = onLinkClick) - ) - } - ) + placeholder = Placeholder( + width = linkWidthSp, + height = linkHeightSp, + placeholderVerticalAlign = PlaceholderVerticalAlign.Bottom + ), + children = { + Box( + modifier = Modifier // It's only a placeholder as well, just to get the real size and position of the link. + .size(linkSizeDp) + .onGloballyPositioned { + it.parentLayoutCoordinates?.let { + onLinkPositionCalculated(it.positionInParent()) + } + } + ) + } + ) ) } } - - Text( - text = inlineText, - style = textStyle, - color = textColor, - inlineContent = inlineContent, - onTextLayout = onTextLayout, - modifier = modifier, - ) + return inlineText to inlineContent }