Skip to content

Commit eeb4973

Browse files
tunjidtunjid
committed
PR feedback
Change-Id: I48492e3c121ff8b2ee6bbbac08aa1829f6a6467f
1 parent 2dba522 commit eeb4973

File tree

8 files changed

+43
-27
lines changed

8 files changed

+43
-27
lines changed

‎core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/AppScrollbars.kt‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,18 +50,21 @@ import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollba
5050
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.ThumbState.Inactive
5151
import kotlinx.coroutines.delay
5252

53-
private const val INACTIVE_TO_DORMANT_COOL_DOWN = 2_000L
53+
/**
54+
* The time period for showing the scrollbar thumb after interacting with it, before it fades away
55+
*/
56+
private const val SCROLLBAR_INACTIVE_TO_DORMANT_TIME_IN_MS = 2_000L
5457

5558
/**
56-
* A [Scrollbar] that allows for fast scrolling of content.
59+
* A [Scrollbar] that allows for fast scrolling of content by dragging its thumb.
5760
* Its thumb disappears when the scrolling container is dormant.
5861
* @param modifier a [Modifier] for the [Scrollbar]
5962
* @param state the driving state for the [Scrollbar]
6063
* @param orientation the orientation of the scrollbar
6164
* @param onThumbDisplaced the fast scroll implementation
6265
*/
6366
@Composable
64-
fun ScrollableState.FastScrollbar(
67+
fun ScrollableState.DraggableScrollbar(
6568
modifier: Modifier = Modifier,
6669
state: ScrollbarState,
6770
orientation: Orientation,
@@ -74,7 +77,7 @@ fun ScrollableState.FastScrollbar(
7477
interactionSource = interactionSource,
7578
state = state,
7679
thumb = {
77-
FastScrollbarThumb(
80+
DraggableScrollbarThumb(
7881
interactionSource = interactionSource,
7982
orientation = orientation,
8083
)
@@ -115,7 +118,7 @@ fun ScrollableState.DecorativeScrollbar(
115118
* A scrollbar thumb that is intended to also be a touch target for fast scrolling.
116119
*/
117120
@Composable
118-
private fun ScrollableState.FastScrollbarThumb(
121+
private fun ScrollableState.DraggableScrollbarThumb(
119122
interactionSource: InteractionSource,
120123
orientation: Orientation,
121124
) {
@@ -137,7 +140,7 @@ private fun ScrollableState.FastScrollbarThumb(
137140
}
138141

139142
/**
140-
* A decorative scrollbar thumb for communicating a user's position in a list solely.
143+
* A decorative scrollbar thumb used solely for communicating a user's position in a list.
141144
*/
142145
@Composable
143146
private fun ScrollableState.DecorativeScrollbarThumb(
@@ -192,7 +195,7 @@ private fun ScrollableState.scrollbarThumbColor(
192195
true -> state = Active
193196
false -> if (state == Active) {
194197
state = Inactive
195-
delay(INACTIVE_TO_DORMANT_COOL_DOWN)
198+
delay(SCROLLBAR_INACTIVE_TO_DORMANT_TIME_IN_MS)
196199
state = Dormant
197200
}
198201
}

‎core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/LazyScrollbarUtilities.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ internal inline fun <LazyState : ScrollableState, LazyStateItem> LazyState.scrol
101101
* of the scroll.
102102
* @param itemIndex a lookup function for index of an item in the layout relative to
103103
* the total amount of items available.
104+
*
105+
* @return a [Float] in the range [firstItemPosition..nextItemPosition) where nextItemPosition
106+
* is the index of the consecutive item along the major axis.
104107
* */
105108
internal inline fun <LazyState : ScrollableState, LazyStateItem> LazyState.interpolateFirstItemIndex(
106109
visibleItems: List<LazyStateItem>,

‎core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/scrollbar/Scrollbar.kt‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2021 The Android Open Source Project
2+
* Copyright 2023 The Android Open Source Project
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -57,8 +57,17 @@ import kotlinx.coroutines.delay
5757
import kotlin.math.max
5858
import kotlin.math.min
5959

60-
private const val SCROLLBAR_PRESS_DELAY = 10L
61-
private const val SCROLLBAR_PRESS_DELTA = 0.02f
60+
/**
61+
* The delay between scrolls when a user long presses on the scrollbar track to initiate a scroll
62+
* instead of dragging the scrollbar thumb.
63+
*/
64+
private const val SCROLLBAR_PRESS_DELAY_MS = 10L
65+
66+
/**
67+
* The percentage displacement of the scrollbar when scrolled by long presses on the scrollbar
68+
* track.
69+
*/
70+
private const val SCROLLBAR_PRESS_DELTA_PCT = 0.02f
6271

6372
/**
6473
* Class definition for the core properties of a scroll bar
@@ -91,10 +100,11 @@ private value class ScrollbarTrack(
91100
}
92101

93102
/**
94-
* Creates a scrollbar state with the listed properties
103+
* Creates a [ScrollbarState] with the listed properties
95104
* @param thumbSizePercent the thumb size of the scrollbar as a percentage of the total track size
96105
* @param thumbDisplacementPercent the distance the thumb has traveled as a percentage of total
97-
* track size
106+
* track size. Refers to either the thumb width (for horizontal scrollbars)
107+
* or height (for vertical scrollbars).
98108
*/
99109
fun ScrollbarState(
100110
thumbSizePercent: Float,
@@ -162,7 +172,7 @@ internal fun Orientation.valueOf(intOffset: IntOffset) = when (this) {
162172
}
163173

164174
/**
165-
* A Composable for drawing a Scrollbar
175+
* A Composable for drawing a scrollbar
166176
* @param orientation the scroll direction of the scrollbar
167177
* @param state the state describing the position of the scrollbar
168178
* @param minThumbSize the minimum size of the scrollbar thumb
@@ -219,7 +229,7 @@ fun Scrollbar(
219229
}
220230
}
221231

222-
// Scrollbar track container
232+
// scrollbar track container
223233
Box(
224234
modifier = modifier
225235
.run {
@@ -275,7 +285,7 @@ fun Scrollbar(
275285
a = with(localDensity) { thumbDisplacementPx.toDp() },
276286
b = 0.dp,
277287
)
278-
// Scrollbar thumb container
288+
// scrollbar thumb container
279289
Box(
280290
modifier = Modifier
281291
.align(Alignment.TopStart)
@@ -319,7 +329,7 @@ fun Scrollbar(
319329
dimension = orientation.valueOf(pressedOffset),
320330
)
321331
val isPositive = currentThumbDisplacement < destinationThumbDisplacement
322-
val delta = SCROLLBAR_PRESS_DELTA * if (isPositive) 1f else -1f
332+
val delta = SCROLLBAR_PRESS_DELTA_PCT * if (isPositive) 1f else -1f
323333

324334
while (currentThumbDisplacement != destinationThumbDisplacement) {
325335
currentThumbDisplacement = when {
@@ -334,7 +344,7 @@ fun Scrollbar(
334344
}
335345
onThumbDisplaced(currentThumbDisplacement)
336346
interactionThumbTravelPercent = currentThumbDisplacement
337-
delay(SCROLLBAR_PRESS_DELAY)
347+
delay(SCROLLBAR_PRESS_DELAY_MS)
338348
}
339349
}
340350

‎feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ import androidx.lifecycle.Lifecycle
6262
import androidx.lifecycle.LifecycleEventObserver
6363
import androidx.lifecycle.compose.collectAsStateWithLifecycle
6464
import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaLoadingWheel
65-
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.FastScrollbar
65+
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.DraggableScrollbar
6666
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.rememberFastScroller
6767
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.scrollbarState
6868
import com.google.samples.apps.nowinandroid.core.designsystem.theme.LocalTintTheme
@@ -208,7 +208,7 @@ private fun BookmarksGrid(
208208
val scrollbarState = scrollableState.scrollbarState(
209209
itemsAvailable = itemsAvailable,
210210
)
211-
scrollableState.FastScrollbar(
211+
scrollableState.DraggableScrollbar(
212212
modifier = Modifier
213213
.fillMaxHeight()
214214
.windowInsetsPadding(WindowInsets.systemBars)

‎feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaButto
9292
import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaIconToggleButton
9393
import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaOverlayLoadingWheel
9494
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.DecorativeScrollbar
95-
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.FastScrollbar
95+
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.DraggableScrollbar
9696
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.rememberFastScroller
9797
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.scrollbarState
9898
import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons
@@ -230,7 +230,7 @@ internal fun ForYouScreen(
230230
)
231231
}
232232
}
233-
state.FastScrollbar(
233+
state.DraggableScrollbar(
234234
modifier = Modifier
235235
.fillMaxHeight()
236236
.windowInsetsPadding(WindowInsets.systemBars)

‎feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/TabContent.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import androidx.compose.ui.Alignment
3535
import androidx.compose.ui.Modifier
3636
import androidx.compose.ui.platform.testTag
3737
import androidx.compose.ui.unit.dp
38-
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.FastScrollbar
38+
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.DraggableScrollbar
3939
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.rememberFastScroller
4040
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.scrollbarState
4141
import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic
@@ -83,7 +83,7 @@ fun TopicsTabContent(
8383
val scrollbarState = scrollableState.scrollbarState(
8484
itemsAvailable = topics.size,
8585
)
86-
scrollableState.FastScrollbar(
86+
scrollableState.DraggableScrollbar(
8787
modifier = Modifier
8888
.fillMaxHeight()
8989
.windowInsetsPadding(WindowInsets.systemBars)

‎feature/search/src/main/java/com/google/samples/apps/nowinandroid/feature/search/SearchScreen.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ import androidx.compose.ui.tooling.preview.PreviewParameter
8080
import androidx.compose.ui.unit.dp
8181
import androidx.hilt.navigation.compose.hiltViewModel
8282
import androidx.lifecycle.compose.collectAsStateWithLifecycle
83-
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.FastScrollbar
83+
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.DraggableScrollbar
8484
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.rememberFastScroller
8585
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.scrollbarState
8686
import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons
@@ -381,7 +381,7 @@ private fun SearchResultBody(
381381
val scrollbarState = state.scrollbarState(
382382
itemsAvailable = itemsAvailable,
383383
)
384-
state.FastScrollbar(
384+
state.DraggableScrollbar(
385385
modifier = Modifier
386386
.fillMaxHeight()
387387
.windowInsetsPadding(WindowInsets.systemBars)

‎feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ import com.google.samples.apps.nowinandroid.core.designsystem.component.DynamicA
5454
import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaBackground
5555
import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaFilterChip
5656
import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaLoadingWheel
57-
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.FastScrollbar
57+
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.DraggableScrollbar
5858
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.rememberFastScroller
5959
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.scrollbarState
6060
import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons
@@ -151,7 +151,7 @@ internal fun TopicScreen(
151151
val scrollbarState = state.scrollbarState(
152152
itemsAvailable = itemsAvailable,
153153
)
154-
state.FastScrollbar(
154+
state.DraggableScrollbar(
155155
modifier = Modifier
156156
.fillMaxHeight()
157157
.windowInsetsPadding(WindowInsets.systemBars)

0 commit comments

Comments
 (0)