From eeb49732657e8cd711c3bec252b4e413251252bb Mon Sep 17 00:00:00 2001 From: TJ Dahunsi Date: Wed, 5 Jul 2023 14:18:29 +0100 Subject: [PATCH] PR feedback Change-Id: I48492e3c121ff8b2ee6bbbac08aa1829f6a6467f --- .../component/scrollbar/AppScrollbars.kt | 17 ++++++----- .../scrollbar/LazyScrollbarUtilities.kt | 3 ++ .../component/scrollbar/Scrollbar.kt | 30 ++++++++++++------- .../feature/bookmarks/BookmarksScreen.kt | 4 +-- .../feature/foryou/ForYouScreen.kt | 4 +-- .../feature/interests/TabContent.kt | 4 +-- .../feature/search/SearchScreen.kt | 4 +-- .../nowinandroid/feature/topic/TopicScreen.kt | 4 +-- 8 files changed, 43 insertions(+), 27 deletions(-) diff --git a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/AppScrollbars.kt b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/AppScrollbars.kt index 373bcb7d9..ca282aa7a 100644 --- a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/AppScrollbars.kt +++ b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/AppScrollbars.kt @@ -50,10 +50,13 @@ import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollba import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.ThumbState.Inactive import kotlinx.coroutines.delay -private const val INACTIVE_TO_DORMANT_COOL_DOWN = 2_000L +/** + * The time period for showing the scrollbar thumb after interacting with it, before it fades away + */ +private const val SCROLLBAR_INACTIVE_TO_DORMANT_TIME_IN_MS = 2_000L /** - * A [Scrollbar] that allows for fast scrolling of content. + * A [Scrollbar] that allows for fast scrolling of content by dragging its thumb. * Its thumb disappears when the scrolling container is dormant. * @param modifier a [Modifier] for the [Scrollbar] * @param state the driving state for the [Scrollbar] @@ -61,7 +64,7 @@ private const val INACTIVE_TO_DORMANT_COOL_DOWN = 2_000L * @param onThumbDisplaced the fast scroll implementation */ @Composable -fun ScrollableState.FastScrollbar( +fun ScrollableState.DraggableScrollbar( modifier: Modifier = Modifier, state: ScrollbarState, orientation: Orientation, @@ -74,7 +77,7 @@ fun ScrollableState.FastScrollbar( interactionSource = interactionSource, state = state, thumb = { - FastScrollbarThumb( + DraggableScrollbarThumb( interactionSource = interactionSource, orientation = orientation, ) @@ -115,7 +118,7 @@ fun ScrollableState.DecorativeScrollbar( * A scrollbar thumb that is intended to also be a touch target for fast scrolling. */ @Composable -private fun ScrollableState.FastScrollbarThumb( +private fun ScrollableState.DraggableScrollbarThumb( interactionSource: InteractionSource, orientation: Orientation, ) { @@ -137,7 +140,7 @@ private fun ScrollableState.FastScrollbarThumb( } /** - * A decorative scrollbar thumb for communicating a user's position in a list solely. + * A decorative scrollbar thumb used solely for communicating a user's position in a list. */ @Composable private fun ScrollableState.DecorativeScrollbarThumb( @@ -192,7 +195,7 @@ private fun ScrollableState.scrollbarThumbColor( true -> state = Active false -> if (state == Active) { state = Inactive - delay(INACTIVE_TO_DORMANT_COOL_DOWN) + delay(SCROLLBAR_INACTIVE_TO_DORMANT_TIME_IN_MS) state = Dormant } } diff --git a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/LazyScrollbarUtilities.kt b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/LazyScrollbarUtilities.kt index 2c1df0c66..c7ef8fe91 100644 --- a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/LazyScrollbarUtilities.kt +++ b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/LazyScrollbarUtilities.kt @@ -101,6 +101,9 @@ internal inline fun LazyState.scrol * of the scroll. * @param itemIndex a lookup function for index of an item in the layout relative to * the total amount of items available. + * + * @return a [Float] in the range [firstItemPosition..nextItemPosition) where nextItemPosition + * is the index of the consecutive item along the major axis. * */ internal inline fun LazyState.interpolateFirstItemIndex( visibleItems: List, diff --git a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/Scrollbar.kt b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/Scrollbar.kt index 4e28d340e..0571ff6d1 100644 --- a/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/Scrollbar.kt +++ b/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/Scrollbar.kt @@ -1,5 +1,5 @@ /* - * Copyright 2021 The Android Open Source Project + * Copyright 2023 The Android Open Source Project * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,8 +57,17 @@ import kotlinx.coroutines.delay import kotlin.math.max import kotlin.math.min -private const val SCROLLBAR_PRESS_DELAY = 10L -private const val SCROLLBAR_PRESS_DELTA = 0.02f +/** + * The delay between scrolls when a user long presses on the scrollbar track to initiate a scroll + * instead of dragging the scrollbar thumb. + */ +private const val SCROLLBAR_PRESS_DELAY_MS = 10L + +/** + * The percentage displacement of the scrollbar when scrolled by long presses on the scrollbar + * track. + */ +private const val SCROLLBAR_PRESS_DELTA_PCT = 0.02f /** * Class definition for the core properties of a scroll bar @@ -91,10 +100,11 @@ private value class ScrollbarTrack( } /** - * Creates a scrollbar state with the listed properties + * Creates a [ScrollbarState] with the listed properties * @param thumbSizePercent the thumb size of the scrollbar as a percentage of the total track size * @param thumbDisplacementPercent the distance the thumb has traveled as a percentage of total - * track size + * track size. Refers to either the thumb width (for horizontal scrollbars) + * or height (for vertical scrollbars). */ fun ScrollbarState( thumbSizePercent: Float, @@ -162,7 +172,7 @@ internal fun Orientation.valueOf(intOffset: IntOffset) = when (this) { } /** - * A Composable for drawing a Scrollbar + * A Composable for drawing a scrollbar * @param orientation the scroll direction of the scrollbar * @param state the state describing the position of the scrollbar * @param minThumbSize the minimum size of the scrollbar thumb @@ -219,7 +229,7 @@ fun Scrollbar( } } - // Scrollbar track container + // scrollbar track container Box( modifier = modifier .run { @@ -275,7 +285,7 @@ fun Scrollbar( a = with(localDensity) { thumbDisplacementPx.toDp() }, b = 0.dp, ) - // Scrollbar thumb container + // scrollbar thumb container Box( modifier = Modifier .align(Alignment.TopStart) @@ -319,7 +329,7 @@ fun Scrollbar( dimension = orientation.valueOf(pressedOffset), ) val isPositive = currentThumbDisplacement < destinationThumbDisplacement - val delta = SCROLLBAR_PRESS_DELTA * if (isPositive) 1f else -1f + val delta = SCROLLBAR_PRESS_DELTA_PCT * if (isPositive) 1f else -1f while (currentThumbDisplacement != destinationThumbDisplacement) { currentThumbDisplacement = when { @@ -334,7 +344,7 @@ fun Scrollbar( } onThumbDisplaced(currentThumbDisplacement) interactionThumbTravelPercent = currentThumbDisplacement - delay(SCROLLBAR_PRESS_DELAY) + delay(SCROLLBAR_PRESS_DELAY_MS) } } diff --git a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt index da4d32469..d4363f12c 100644 --- a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt +++ b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt @@ -62,7 +62,7 @@ import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleEventObserver import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaLoadingWheel -import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.FastScrollbar +import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.DraggableScrollbar import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.rememberFastScroller import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.scrollbarState import com.google.samples.apps.nowinandroid.core.designsystem.theme.LocalTintTheme @@ -208,7 +208,7 @@ private fun BookmarksGrid( val scrollbarState = scrollableState.scrollbarState( itemsAvailable = itemsAvailable, ) - scrollableState.FastScrollbar( + scrollableState.DraggableScrollbar( modifier = Modifier .fillMaxHeight() .windowInsetsPadding(WindowInsets.systemBars) diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt index ffbda5de3..dbcfe7eeb 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt @@ -92,7 +92,7 @@ import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaButto import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaIconToggleButton import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaOverlayLoadingWheel import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.DecorativeScrollbar -import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.FastScrollbar +import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.DraggableScrollbar import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.rememberFastScroller import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.scrollbarState import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons @@ -230,7 +230,7 @@ internal fun ForYouScreen( ) } } - state.FastScrollbar( + state.DraggableScrollbar( modifier = Modifier .fillMaxHeight() .windowInsetsPadding(WindowInsets.systemBars) diff --git a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/TabContent.kt b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/TabContent.kt index acc84e895..7ae652344 100644 --- a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/TabContent.kt +++ b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/TabContent.kt @@ -35,7 +35,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.testTag import androidx.compose.ui.unit.dp -import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.FastScrollbar +import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.DraggableScrollbar import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.rememberFastScroller import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.scrollbarState import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic @@ -83,7 +83,7 @@ fun TopicsTabContent( val scrollbarState = scrollableState.scrollbarState( itemsAvailable = topics.size, ) - scrollableState.FastScrollbar( + scrollableState.DraggableScrollbar( modifier = Modifier .fillMaxHeight() .windowInsetsPadding(WindowInsets.systemBars) diff --git a/feature/search/src/main/java/com/google/samples/apps/nowinandroid/feature/search/SearchScreen.kt b/feature/search/src/main/java/com/google/samples/apps/nowinandroid/feature/search/SearchScreen.kt index 14b60e564..08356d938 100644 --- a/feature/search/src/main/java/com/google/samples/apps/nowinandroid/feature/search/SearchScreen.kt +++ b/feature/search/src/main/java/com/google/samples/apps/nowinandroid/feature/search/SearchScreen.kt @@ -80,7 +80,7 @@ import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle -import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.FastScrollbar +import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.DraggableScrollbar import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.rememberFastScroller import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.scrollbarState import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons @@ -381,7 +381,7 @@ private fun SearchResultBody( val scrollbarState = state.scrollbarState( itemsAvailable = itemsAvailable, ) - state.FastScrollbar( + state.DraggableScrollbar( modifier = Modifier .fillMaxHeight() .windowInsetsPadding(WindowInsets.systemBars) diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt index f81c5b51c..84975b4ea 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt @@ -54,7 +54,7 @@ import com.google.samples.apps.nowinandroid.core.designsystem.component.DynamicA import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaBackground import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaFilterChip import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaLoadingWheel -import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.FastScrollbar +import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.DraggableScrollbar import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.rememberFastScroller import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.scrollbarState import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons @@ -151,7 +151,7 @@ internal fun TopicScreen( val scrollbarState = state.scrollbarState( itemsAvailable = itemsAvailable, ) - state.FastScrollbar( + state.DraggableScrollbar( modifier = Modifier .fillMaxHeight() .windowInsetsPadding(WindowInsets.systemBars)