From 7e59a1653250325dc3dfb2443610a3717d81e72c Mon Sep 17 00:00:00 2001 From: Jonas Kalderstam Date: Sun, 18 Aug 2024 06:57:14 +0200 Subject: [PATCH] Added scrollbar to reader --- .../ui/compose/feedarticle/ReaderView.kt | 456 +++++++++--------- settings.gradle.kts | 5 +- 2 files changed, 241 insertions(+), 220 deletions(-) diff --git a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/ReaderView.kt b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/ReaderView.kt index e5c1ec2294..b656eae81d 100644 --- a/app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/ReaderView.kt +++ b/app/src/main/java/com/nononsenseapps/feeder/ui/compose/feedarticle/ReaderView.kt @@ -11,8 +11,10 @@ import androidx.compose.foundation.layout.BoxWithConstraints import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.aspectRatio import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListScope @@ -70,6 +72,8 @@ import com.nononsenseapps.feeder.ui.compose.theme.LocalDimens import com.nononsenseapps.feeder.ui.compose.utils.ProvideScaledText import com.nononsenseapps.feeder.ui.compose.utils.ScreenType import com.nononsenseapps.feeder.ui.compose.utils.focusableInNonTouchMode +import my.nanihadesuka.compose.LazyColumnScrollbar +import my.nanihadesuka.compose.ScrollbarSettings import java.time.format.DateTimeFormatter import java.time.format.FormatStyle import java.util.Locale @@ -103,257 +107,271 @@ fun ReaderView( } SelectionContainer { - LazyColumn( + LazyColumnScrollbar( state = articleListState, - contentPadding = - PaddingValues( - bottom = 92.dp, - start = - when (screenType) { - ScreenType.DUAL -> 0.dp // List items have enough padding - ScreenType.SINGLE -> dimens.margin - }, - end = dimens.margin, + settings = + ScrollbarSettings.Default.copy( + thumbUnselectedColor = MaterialTheme.colorScheme.outlineVariant, ), - horizontalAlignment = Alignment.CenterHorizontally, - verticalArrangement = Arrangement.spacedBy(16.dp), - modifier = - modifier - .fillMaxWidth() - .focusGroup() - .safeSemantics { - testTag = "readerColumn" - }, ) { - item { - val goToFeedLabel = stringResource(R.string.go_to_feed, feedTitle) - Column( - verticalArrangement = Arrangement.spacedBy(8.dp), - modifier = - Modifier - .width(dimens.maxReaderWidth) - .semantics(mergeDescendants = true) { - try { - customActions = - listOf( - // TODO enclosure? - CustomAccessibilityAction(goToFeedLabel) { - onFeedTitleClick() - true - }, - ) - } catch (e: Exception) { - // Observed nullpointer exception when setting customActions - // No clue why it could be null - Log.e("FeederReaderScreen", "Exception in semantics", e) - } + LazyColumn( + state = articleListState, + contentPadding = + PaddingValues( + // Interferes with scrollbar +// bottom = 92.dp, + start = + when (screenType) { + ScreenType.DUAL -> 0.dp // List items have enough padding + ScreenType.SINGLE -> dimens.margin }, - ) { - if (articleTitle.isNotBlank()) { - WithBidiDeterminedLayoutDirection(paragraph = articleTitle) { - val interactionSource = remember { MutableInteractionSource() } - Text( - text = articleTitle, - style = MaterialTheme.typography.headlineLarge, - modifier = - Modifier - .indication(interactionSource, LocalIndication.current) - .focusableInNonTouchMode(interactionSource = interactionSource) - .width(dimens.maxReaderWidth), - ) - } - } - ProvideScaledText( - style = - MaterialTheme.typography.titleMedium.merge( - LinkTextStyle(), - ), + end = dimens.margin, + ), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.spacedBy(16.dp), + modifier = + modifier + .fillMaxWidth() + .focusGroup() + .safeSemantics { + testTag = "readerColumn" + }, + ) { + item { + val goToFeedLabel = stringResource(R.string.go_to_feed, feedTitle) + Column( + verticalArrangement = Arrangement.spacedBy(8.dp), + modifier = + Modifier + .width(dimens.maxReaderWidth) + .semantics(mergeDescendants = true) { + try { + customActions = + listOf( + // TODO enclosure? + CustomAccessibilityAction(goToFeedLabel) { + onFeedTitleClick() + true + }, + ) + } catch (e: Exception) { + // Observed nullpointer exception when setting customActions + // No clue why it could be null + Log.e("FeederReaderScreen", "Exception in semantics", e) + } + }, ) { - WithBidiDeterminedLayoutDirection(paragraph = feedTitle) { - Text( - text = feedTitle, - modifier = - Modifier - .width(dimens.maxReaderWidth) - .clearAndSetSemantics { - contentDescription = feedTitle - } - .clickable { - onFeedTitleClick() - }, - ) + if (articleTitle.isNotBlank()) { + WithBidiDeterminedLayoutDirection(paragraph = articleTitle) { + val interactionSource = remember { MutableInteractionSource() } + Text( + text = articleTitle, + style = MaterialTheme.typography.headlineLarge, + modifier = + Modifier + .indication(interactionSource, LocalIndication.current) + .focusableInNonTouchMode(interactionSource = interactionSource) + .width(dimens.maxReaderWidth), + ) + } } - } - if (authorDate != null) { - ProvideScaledText(style = MaterialTheme.typography.titleMedium) { - CompositionLocalProvider(LocalContentAlpha provides ContentAlpha.medium) { - WithBidiDeterminedLayoutDirection(paragraph = authorDate) { - val interactionSource = remember { MutableInteractionSource() } - Text( - text = authorDate, - modifier = - Modifier - .width(dimens.maxReaderWidth) - .indication(interactionSource, LocalIndication.current) - .focusableInNonTouchMode(interactionSource = interactionSource), - ) - } + ProvideScaledText( + style = + MaterialTheme.typography.titleMedium.merge( + LinkTextStyle(), + ), + ) { + WithBidiDeterminedLayoutDirection(paragraph = feedTitle) { + Text( + text = feedTitle, + modifier = + Modifier + .width(dimens.maxReaderWidth) + .clearAndSetSemantics { + contentDescription = feedTitle + } + .clickable { + onFeedTitleClick() + }, + ) } } - } - if (readTimeSecs > 0) { - ProvideScaledText(style = MaterialTheme.typography.titleMedium) { - CompositionLocalProvider(LocalContentAlpha provides ContentAlpha.medium) { - Row( - horizontalArrangement = Arrangement.spacedBy(4.dp), - verticalAlignment = Alignment.CenterVertically, - modifier = Modifier.width(dimens.maxReaderWidth), - ) { - Icon( - imageVector = Icons.Outlined.Timelapse, - contentDescription = null, - ) - val seconds = "%02d".format(readTimeSecs % 60) - val readTimeText = - pluralStringResource( - id = R.plurals.n_minutes, - count = readTimeSecs / 60, - ) - .format( - "${readTimeSecs / 60}:$seconds", - ) - WithBidiDeterminedLayoutDirection(paragraph = readTimeText) { - val interactionSource = - remember { MutableInteractionSource() } + if (authorDate != null) { + ProvideScaledText(style = MaterialTheme.typography.titleMedium) { + CompositionLocalProvider(LocalContentAlpha provides ContentAlpha.medium) { + WithBidiDeterminedLayoutDirection(paragraph = authorDate) { + val interactionSource = remember { MutableInteractionSource() } Text( - text = readTimeText, + text = authorDate, modifier = Modifier - .weight(1f) - .indication( - interactionSource, - LocalIndication.current, - ) + .width(dimens.maxReaderWidth) + .indication(interactionSource, LocalIndication.current) .focusableInNonTouchMode(interactionSource = interactionSource), ) } } } } - } - } - } - - if (enclosure.present) { - item { - // Image will be shown in block below - if (!enclosure.isImage) { - val openLabel = - if (enclosure.name.isBlank()) { - stringResource(R.string.open_enclosed_media) - } else { - stringResource(R.string.open_enclosed_media_file, enclosure.name) - } - ProvideScaledText( - style = - MaterialTheme.typography.bodyLarge.merge( - LinkTextStyle(), - ), - ) { - Text( - text = openLabel, - modifier = - Modifier - .width(dimens.maxReaderWidth) - .clickable { - onEnclosureClick() - } - .clearAndSetSemantics { - try { - customActions = - listOf( - CustomAccessibilityAction(openLabel) { - onEnclosureClick() - true - }, - ) - } catch (e: Exception) { - // Observed nullpointer exception when setting customActions - // No clue why it could be null - Log.e( - LOG_TAG, - "Exception in semantics", - e, + if (readTimeSecs > 0) { + ProvideScaledText(style = MaterialTheme.typography.titleMedium) { + CompositionLocalProvider(LocalContentAlpha provides ContentAlpha.medium) { + Row( + horizontalArrangement = Arrangement.spacedBy(4.dp), + verticalAlignment = Alignment.CenterVertically, + modifier = Modifier.width(dimens.maxReaderWidth), + ) { + Icon( + imageVector = Icons.Outlined.Timelapse, + contentDescription = null, + ) + val seconds = "%02d".format(readTimeSecs % 60) + val readTimeText = + pluralStringResource( + id = R.plurals.n_minutes, + count = readTimeSecs / 60, + ) + .format( + "${readTimeSecs / 60}:$seconds", ) - } - }, - ) + WithBidiDeterminedLayoutDirection(paragraph = readTimeText) { + val interactionSource = + remember { MutableInteractionSource() } + Text( + text = readTimeText, + modifier = + Modifier + .weight(1f) + .indication( + interactionSource, + LocalIndication.current, + ) + .focusableInNonTouchMode(interactionSource = interactionSource), + ) + } + } + } + } } } } - } - - // Don't show image for full text articles since it's typically inside the full article - if (isFeedText && image?.fromBody == false) { - val imageWidth: Int = - if (image.width?.let { it < 640 } == true) { - // Zero or too small, do not show - -1 - } else if (image.width != null) { - image.width ?: Int.MAX_VALUE - } else { - // Enclosures do not have a known width. This will be constrained below. - Int.MAX_VALUE - } - if (imageWidth > 0) { + if (enclosure.present) { item { - BoxWithConstraints( - contentAlignment = Alignment.Center, - modifier = - Modifier - .clip(RectangleShape) - .width(dimens.maxReaderWidth), - ) { - WithTooltipIfNotBlank(tooltip = stringResource(id = R.string.article_image)) { - val maxImageWidth by rememberMaxImageWidth() - val pixelDensity = LocalDensity.current.density - val contentScale = - remember(pixelDensity) { - RestrainedCropScaling(pixelDensity) - } - AsyncImage( - model = - ImageRequest.Builder(LocalContext.current) - .data(image.url) - .scale(Scale.FIT) - .size(imageWidth.coerceAtMost(maxImageWidth)) - .precision(Precision.INEXACT) - .build(), - contentDescription = enclosure.name, - placeholder = - rememberTintedVectorPainter( - Icons.Outlined.Terrain, - ), - error = rememberTintedVectorPainter(Icons.Outlined.ErrorOutline), - contentScale = contentScale, - alignment = Alignment.Center, + // Image will be shown in block below + if (!enclosure.isImage) { + val openLabel = + if (enclosure.name.isBlank()) { + stringResource(R.string.open_enclosed_media) + } else { + stringResource(R.string.open_enclosed_media_file, enclosure.name) + } + ProvideScaledText( + style = + MaterialTheme.typography.bodyLarge.merge( + LinkTextStyle(), + ), + ) { + Text( + text = openLabel, modifier = Modifier - .fillMaxWidth() - .run { - dimens.imageAspectRatioInReader?.let { ratio -> - aspectRatio(ratio) - } ?: this + .width(dimens.maxReaderWidth) + .clickable { + onEnclosureClick() + } + .clearAndSetSemantics { + try { + customActions = + listOf( + CustomAccessibilityAction(openLabel) { + onEnclosureClick() + true + }, + ) + } catch (e: Exception) { + // Observed nullpointer exception when setting customActions + // No clue why it could be null + Log.e( + LOG_TAG, + "Exception in semantics", + e, + ) + } }, ) } } } } - } - articleBody() + // Don't show image for full text articles since it's typically inside the full article + if (isFeedText && image?.fromBody == false) { + val imageWidth: Int = + if (image.width?.let { it < 640 } == true) { + // Zero or too small, do not show + -1 + } else if (image.width != null) { + image.width ?: Int.MAX_VALUE + } else { + // Enclosures do not have a known width. This will be constrained below. + Int.MAX_VALUE + } + + if (imageWidth > 0) { + item { + BoxWithConstraints( + contentAlignment = Alignment.Center, + modifier = + Modifier + .clip(RectangleShape) + .width(dimens.maxReaderWidth), + ) { + WithTooltipIfNotBlank(tooltip = stringResource(id = R.string.article_image)) { + val maxImageWidth by rememberMaxImageWidth() + val pixelDensity = LocalDensity.current.density + val contentScale = + remember(pixelDensity) { + RestrainedCropScaling(pixelDensity) + } + AsyncImage( + model = + ImageRequest.Builder(LocalContext.current) + .data(image.url) + .scale(Scale.FIT) + .size(imageWidth.coerceAtMost(maxImageWidth)) + .precision(Precision.INEXACT) + .build(), + contentDescription = enclosure.name, + placeholder = + rememberTintedVectorPainter( + Icons.Outlined.Terrain, + ), + error = rememberTintedVectorPainter(Icons.Outlined.ErrorOutline), + contentScale = contentScale, + alignment = Alignment.Center, + modifier = + Modifier + .fillMaxWidth() + .run { + dimens.imageAspectRatioInReader?.let { ratio -> + aspectRatio(ratio) + } ?: this + }, + ) + } + } + } + } + } + + articleBody() + + // Using spacer instead of content padding because padding interferes with scrollbar + item { + Spacer(modifier = Modifier.height(92.dp)) + } + } } } } diff --git a/settings.gradle.kts b/settings.gradle.kts index ff7ae12bc9..aad7fb3efa 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -64,6 +64,7 @@ dependencyResolutionManagement { version("accompanist", "0.30.1") version("coil", "2.4.0") version("androidWindow", "1.0.0") + version("lazycolumnscrollbar", "2.2.0") // Formerly customtabs version("androidxBrowser", "1.5.0") // Tests @@ -285,6 +286,7 @@ dependencyResolutionManagement { library("moshi-kotlin", "com.squareup.moshi", "moshi-kotlin").versionRef("moshi") library("moshi-adapters", "com.squareup.moshi", "moshi-adapters").versionRef("moshi") library("qrgen", "com.github.kenglxn.qrgen", "android").versionRef("qrgen") + library("lazycolumnscrollbar", "com.github.nanihadesuka", "LazyColumnScrollbar").versionRef("lazycolumnscrollbar") // Feel free to upgrade once we move to later sdk // Only necessary to fix a bad transitive dependency by Google @@ -356,7 +358,7 @@ dependencyResolutionManagement { "moshi-kotlin", "moshi-adapters", "qrgen", - "gofeed-android" + "gofeed-android", ), ) @@ -406,6 +408,7 @@ dependencyResolutionManagement { "compose-material3-windowsizeclass", "lifecycle-runtime-compose", "coil-compose", + "lazycolumnscrollbar", ), )