From 31c3b0c881a08303e933c584178d5d0ec27777fc Mon Sep 17 00:00:00 2001 From: Colin S <3526918+cbs228@users.noreply.github.com> Date: Sat, 25 Jan 2025 21:07:22 -0600 Subject: [PATCH 1/2] messages: open web links in full web browser In [1], all hyperlinks within `MessagesNode` switched to using activity.openUrlInChromeCustomTab() instead of with `openUrlInExternalApp()`. This causes web links in chat messages to open in an "in-app" Chrome Custom Tab [2] instead of the user's configured "full" web browser. This is a UX nuisance. Open web links in a full browser instead. Closes #3885 [3]. [1]: 5baefd479f0045110918b5abbcb7a21fbf28c01e (Identity change: handle click on "learn more") [2]: https://developer.chrome.com/docs/android/custom-tabs/guide-get-started#opening_a_custom_tab [3]: https://github.com/element-hq/element-x-android/issues/3885 --- .../element/android/features/messages/impl/MessagesNode.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesNode.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesNode.kt index 2ab3bbbc862..2f439e6a870 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesNode.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesNode.kt @@ -40,6 +40,7 @@ import io.element.android.features.messages.impl.timeline.di.LocalTimelineItemPr import io.element.android.features.messages.impl.timeline.di.TimelineItemPresenterFactories import io.element.android.features.messages.impl.timeline.model.TimelineItem import io.element.android.libraries.androidutils.browser.openUrlInChromeCustomTab +import io.element.android.libraries.androidutils.system.openUrlInExternalApp import io.element.android.libraries.androidutils.system.toast import io.element.android.libraries.architecture.NodeInputs import io.element.android.libraries.architecture.inputs @@ -148,7 +149,9 @@ class MessagesNode @AssistedInject constructor( is PermalinkData.RoomLink -> { handleRoomLinkClick(activity, permalink, eventSink) } - is PermalinkData.FallbackLink, + is PermalinkData.FallbackLink -> { + activity.openUrlInExternalApp(url) + } is PermalinkData.RoomEmailInviteLink -> { activity.openUrlInChromeCustomTab(null, darkTheme, url) } From ebdd9459e7b9ff18d3545de90f826d2fd7f73053 Mon Sep 17 00:00:00 2001 From: Colin S <3526918+cbs228@users.noreply.github.com> Date: Tue, 28 Jan 2025 19:45:47 -0600 Subject: [PATCH 2/2] messages: separate custom tab links from normal links Some links in the `MessagesView` are part of Element X itself, such as the help pages in `LearnMoreConfig` [1]. These links should open in an "in-app" Chrome Custom Tab, because they are basically part of the app. Web links from chat messages, on the other hand, should open in the user's preferred web browser as regular tabs. Separate "regular" links from "custom tab" links with a new parameter `onLinkClick(..., customTab)`. If true, the link opens in a custom tab. Links within `TimelineView` are always opened in a normal tab. [1]: appconfig/src/main/kotlin/io/element/android/appconfig/LearnMoreConfig.kt --- .../android/features/messages/impl/MessagesNode.kt | 9 +++++++-- .../android/features/messages/impl/MessagesView.kt | 10 +++++----- .../impl/crypto/identity/IdentityChangeStateView.kt | 6 +++--- .../identity/MessagesViewWithIdentityChangePreview.kt | 2 +- .../android/features/messages/impl/MessagesViewTest.kt | 3 ++- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesNode.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesNode.kt index 2f439e6a870..4f7cd1b2cb0 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesNode.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesNode.kt @@ -139,6 +139,7 @@ class MessagesNode @AssistedInject constructor( darkTheme: Boolean, url: String, eventSink: (TimelineEvents) -> Unit, + customTab: Boolean ) { when (val permalink = permalinkParser.parse(url)) { is PermalinkData.UserLink -> { @@ -150,7 +151,11 @@ class MessagesNode @AssistedInject constructor( handleRoomLinkClick(activity, permalink, eventSink) } is PermalinkData.FallbackLink -> { - activity.openUrlInExternalApp(url) + if (customTab) { + activity.openUrlInChromeCustomTab(null, darkTheme, url) + } else { + activity.openUrlInExternalApp(url) + } } is PermalinkData.RoomEmailInviteLink -> { activity.openUrlInChromeCustomTab(null, darkTheme, url) @@ -236,7 +241,7 @@ class MessagesNode @AssistedInject constructor( onRoomDetailsClick = this::onRoomDetailsClick, onEventContentClick = this::onEventClick, onUserDataClick = this::onUserDataClick, - onLinkClick = { url -> onLinkClick(activity, isDark, url, state.timelineState.eventSink) }, + onLinkClick = { url, customTab -> onLinkClick(activity, isDark, url, state.timelineState.eventSink, customTab) }, onSendLocationClick = this::onSendLocationClick, onCreatePollClick = this::onCreatePollClick, onJoinCallClick = this::onJoinCallClick, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt index 02ad1cf87f2..381d2dca41d 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt @@ -111,7 +111,7 @@ fun MessagesView( onRoomDetailsClick: () -> Unit, onEventContentClick: (event: TimelineItem.Event) -> Boolean, onUserDataClick: (UserId) -> Unit, - onLinkClick: (String) -> Unit, + onLinkClick: (String, Boolean) -> Unit, onSendLocationClick: () -> Unit, onCreatePollClick: () -> Unit, onJoinCallClick: () -> Unit, @@ -273,7 +273,7 @@ private fun MessagesViewContent( state: MessagesState, onContentClick: (TimelineItem.Event) -> Unit, onUserDataClick: (UserId) -> Unit, - onLinkClick: (String) -> Unit, + onLinkClick: (String, Boolean) -> Unit, onReactionClick: (key: String, TimelineItem.Event) -> Unit, onReactionLongClick: (key: String, TimelineItem.Event) -> Unit, onMoreReactionsClick: (TimelineItem.Event) -> Unit, @@ -347,7 +347,7 @@ private fun MessagesViewContent( state = state.timelineState, timelineProtectionState = state.timelineProtectionState, onUserDataClick = onUserDataClick, - onLinkClick = onLinkClick, + onLinkClick = { url -> onLinkClick(url, false) }, onContentClick = onContentClick, onMessageLongClick = onMessageLongClick, onSwipeToReply = onSwipeToReply, @@ -396,7 +396,7 @@ private fun MessagesViewContent( private fun MessagesViewComposerBottomSheetContents( subcomposing: Boolean, state: MessagesState, - onLinkClick: (String) -> Unit, + onLinkClick: (String, Boolean) -> Unit, ) { if (state.userEventPermissions.canSendMessage) { Column(modifier = Modifier.fillMaxWidth()) { @@ -537,7 +537,7 @@ internal fun MessagesViewPreview(@PreviewParameter(MessagesStateProvider::class) onRoomDetailsClick = {}, onEventContentClick = { false }, onUserDataClick = {}, - onLinkClick = {}, + onLinkClick = { _, _ -> }, onSendLocationClick = {}, onCreatePollClick = {}, onJoinCallClick = {}, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateView.kt index 5dbdeeb4052..a7b78ca84b0 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateView.kt @@ -26,7 +26,7 @@ import io.element.android.libraries.ui.strings.CommonStrings @Composable fun IdentityChangeStateView( state: IdentityChangeState, - onLinkClick: (String) -> Unit, + onLinkClick: (String, Boolean) -> Unit, modifier: Modifier = Modifier, ) { // Pick the first identity change to PinViolation @@ -73,7 +73,7 @@ fun IdentityChangeStateView( url = LinkAnnotation.Url( url = LearnMoreConfig.IDENTITY_CHANGE_URL, linkInteractionListener = { - onLinkClick(LearnMoreConfig.IDENTITY_CHANGE_URL) + onLinkClick(LearnMoreConfig.IDENTITY_CHANGE_URL, true) } ), start = learnMoreStartIndex, @@ -93,6 +93,6 @@ internal fun IdentityChangeStateViewPreview( ) = ElementPreview { IdentityChangeStateView( state = state, - onLinkClick = {}, + onLinkClick = { _, _ -> }, ) } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/MessagesViewWithIdentityChangePreview.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/MessagesViewWithIdentityChangePreview.kt index a72baa9e003..5b695e1a9e0 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/MessagesViewWithIdentityChangePreview.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/MessagesViewWithIdentityChangePreview.kt @@ -35,7 +35,7 @@ internal fun MessagesViewWithIdentityChangePreview( onRoomDetailsClick = {}, onEventContentClick = { false }, onUserDataClick = {}, - onLinkClick = {}, + onLinkClick = { _, _ -> }, onSendLocationClick = {}, onCreatePollClick = {}, onJoinCallClick = {}, diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesViewTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesViewTest.kt index 63491b0103e..3557d84bd5d 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesViewTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesViewTest.kt @@ -58,6 +58,7 @@ import io.element.android.tests.testutils.EnsureCalledOnceWithParam import io.element.android.tests.testutils.EnsureNeverCalled import io.element.android.tests.testutils.EnsureNeverCalledWithParam import io.element.android.tests.testutils.EnsureNeverCalledWithParamAndResult +import io.element.android.tests.testutils.EnsureNeverCalledWithTwoParams import io.element.android.tests.testutils.EventsRecorder import io.element.android.tests.testutils.clickOn import io.element.android.tests.testutils.ensureCalledOnce @@ -514,7 +515,7 @@ private fun AndroidComposeTestRule.setMessa onRoomDetailsClick: () -> Unit = EnsureNeverCalled(), onEventClick: (event: TimelineItem.Event) -> Boolean = EnsureNeverCalledWithParamAndResult(), onUserDataClick: (UserId) -> Unit = EnsureNeverCalledWithParam(), - onLinkClick: (String) -> Unit = EnsureNeverCalledWithParam(), + onLinkClick: (String, Boolean) -> Unit = EnsureNeverCalledWithTwoParams(), onSendLocationClick: () -> Unit = EnsureNeverCalled(), onCreatePollClick: () -> Unit = EnsureNeverCalled(), onJoinCallClick: () -> Unit = EnsureNeverCalled(),