From adc36017d133ee9b0bd4414c8ed8a4beb505e89c Mon Sep 17 00:00:00 2001 From: The Daniel Date: Wed, 29 Jul 2026 15:12:41 -0400 Subject: [PATCH] fix(nip73): honor data-saver gate and fix preview icon touch target Review feedback from PR #3810: - Gate both new preview call sites behind settings.showUrlPreview(), so a user on WIFI_ONLY over cellular or NEVER no longer triggers an outbound OpenGraph request and image download per external-scoped comment. Falls back to the plain link chip / URL header. - Wrap the open-in-browser icon in an IconButton so it gets a real touch target instead of a 14dp one nested inside the card's own clickable, where a near-miss silently navigated instead. - Only show that icon when onCardClick is set; without it the icon and the card tap did the same thing, so existing note-body link cards stay as-is. - staticCompositionLocalOf for LocalCurrentExternalScope (constant per screen, avoids read tracking per feed row). - Inset the loading/error URL header to match the card, and use the hoisted Size14Modifier. --- .../amethyst/ui/components/UrlPreviewCard.kt | 30 ++++++++++--------- .../note/nip22Comments/DisplayExternalId.kt | 28 ++++++++++++----- .../ui/screen/loggedIn/url/UrlScreen.kt | 10 ++++++- 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/UrlPreviewCard.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/UrlPreviewCard.kt index 99d47a2dff..3f26837655 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/UrlPreviewCard.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/UrlPreviewCard.kt @@ -21,12 +21,11 @@ package com.vitorpamplona.amethyst.ui.components import androidx.compose.foundation.ExperimentalFoundationApi -import androidx.compose.foundation.clickable import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.layout.size +import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -40,7 +39,6 @@ import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalClipboard import androidx.compose.ui.platform.LocalUriHandler import androidx.compose.ui.text.style.TextOverflow -import androidx.compose.ui.unit.dp import coil3.compose.AsyncImage import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.icons.symbols.Icon @@ -50,6 +48,7 @@ import com.vitorpamplona.amethyst.ui.components.util.setText import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.amethyst.ui.theme.DoubleVertSpacer import com.vitorpamplona.amethyst.ui.theme.MaxWidthWithHorzPadding +import com.vitorpamplona.amethyst.ui.theme.Size14Modifier import com.vitorpamplona.amethyst.ui.theme.innerPostModifier import com.vitorpamplona.amethyst.ui.theme.previewCardImageModifier import kotlinx.coroutines.launch @@ -134,17 +133,20 @@ fun UrlPreviewCard( overflow = TextOverflow.Ellipsis, ) - Spacer(modifier = Modifier.size(4.dp)) - - Icon( - symbol = MaterialSymbols.AutoMirrored.OpenInNew, - contentDescription = stringRes(R.string.url_preview_open_in_browser), - modifier = - Modifier - .size(14.dp) - .clickable { runCatching { uri.openUri(url) } }, - tint = Color.Gray, - ) + // Only meaningful when the card's own tap does something else (e.g. opening the + // comment thread); otherwise it would duplicate the card's open-in-browser tap. + if (onCardClick != null) { + IconButton( + onClick = { runCatching { uri.openUri(url) } }, + ) { + Icon( + symbol = MaterialSymbols.AutoMirrored.OpenInNew, + contentDescription = stringRes(R.string.url_preview_open_in_browser), + modifier = Size14Modifier, + tint = Color.Gray, + ) + } + } } Text( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/nip22Comments/DisplayExternalId.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/nip22Comments/DisplayExternalId.kt index 458defdcff..deca589468 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/nip22Comments/DisplayExternalId.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/nip22Comments/DisplayExternalId.kt @@ -28,7 +28,7 @@ import androidx.compose.material3.LocalTextStyle import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable -import androidx.compose.runtime.compositionLocalOf +import androidx.compose.runtime.staticCompositionLocalOf import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.text.LinkAnnotation @@ -62,7 +62,7 @@ import com.vitorpamplona.quartz.nip73ExternalIds.urls.UrlId * the URL thread screen) provides this so nested comments sharing that same scope don't * redundantly repeat the preview the screen itself already shows. */ -val LocalCurrentExternalScope = compositionLocalOf { null } +val LocalCurrentExternalScope = staticCompositionLocalOf { null } @Composable fun DisplayExternalId( @@ -96,12 +96,8 @@ fun DisplayUrlExternalId( nav: INav, ) { val url = externalId.url - when (val state = rememberUrlPreviewState(url, accountViewModel)) { - is UrlPreviewState.Loaded -> { - UrlPreviewCard(url, state.previewInfo, onCardClick = { nav.nav(Route.Url(url)) }) - } - - else -> { + val chip = + @Composable { DisplayExternalIdChip( symbol = MaterialSymbols.Link, contentDescription = stringRes(id = R.string.external_url_scope), @@ -109,6 +105,22 @@ fun DisplayUrlExternalId( linkInteractionListener = { nav.nav(Route.Url(url)) }, ) } + + // Respect the data-saver/privacy gate: fetching the preview reaches out to the + // third-party page, so when previews are off this stays a plain link chip. + if (!accountViewModel.settings.showUrlPreview()) { + chip() + return + } + + when (val state = rememberUrlPreviewState(url, accountViewModel)) { + is UrlPreviewState.Loaded -> { + UrlPreviewCard(url, state.previewInfo, onCardClick = { nav.nav(Route.Url(url)) }) + } + + else -> { + chip() + } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/url/UrlScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/url/UrlScreen.kt index ccb5b0333e..d323345e19 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/url/UrlScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/url/UrlScreen.kt @@ -44,6 +44,7 @@ import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.screen.loggedIn.url.dal.UrlFeedViewModel import com.vitorpamplona.amethyst.ui.screen.loggedIn.url.datasource.UrlFilterAssemblerSubscription import com.vitorpamplona.amethyst.ui.stringRes +import com.vitorpamplona.amethyst.ui.theme.MaxWidthWithHorzPadding import com.vitorpamplona.quartz.nip73ExternalIds.urls.UrlId @Composable @@ -131,13 +132,20 @@ fun UrlScreenPreview( url: String, accountViewModel: AccountViewModel, ) { + // Respect the data-saver/privacy gate: fetching the preview reaches out to the + // third-party page, so when previews are off this stays the plain URL header. + if (!accountViewModel.settings.showUrlPreview()) { + DisplayUrlHeader(url, MaxWidthWithHorzPadding) + return + } + when (val state = rememberUrlPreviewState(url, accountViewModel)) { is UrlPreviewState.Loaded -> { UrlPreviewCard(url, state.previewInfo) } else -> { - DisplayUrlHeader(url, Modifier) + DisplayUrlHeader(url, MaxWidthWithHorzPadding) } } }