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.
This commit is contained in:
The Daniel
2026-07-29 15:12:41 -04:00
parent ac426531f3
commit adc36017d1
3 changed files with 45 additions and 23 deletions
@@ -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(
@@ -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<String?> { null }
val LocalCurrentExternalScope = staticCompositionLocalOf<String?> { 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()
}
}
}
@@ -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)
}
}
}