Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Pager] Migrate to Modifier.scrollable #236

Merged
merged 7 commits into from
Mar 17, 2021
Merged

[Pager] Migrate to Modifier.scrollable #236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b99abf7
Try out using Modifier.scrollable
chrisbanes Mar 11, 2021
628efd3
Add reverseLayout parameter
chrisbanes Mar 16, 2021
6bea8ef
Improve semantics implementation
chrisbanes Mar 16, 2021
aa149c4
Update to Compose SNAPSHOT 7212271
chrisbanes Mar 16, 2021
95cc0e9
Improve scroll snapping
chrisbanes Mar 16, 2021
97d01f8
Expose animation specs via HorizontalPager
chrisbanes Mar 16, 2021
d42994e
Tweak Pager tests for new snap thresholds
chrisbanes Mar 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion buildSrc/src/main/java/com/google/accompanist/buildsrc/dependencies.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ object Libs {
const val appcompat = "androidx.appcompat:appcompat:1.3.0-beta01"

object Compose {
const val snapshot = "7205803"
const val snapshot = "7212271"
const val version = "1.0.0-SNAPSHOT"

@JvmStatic
Expand Down
7 changes: 5 additions & 2 deletions pager/api/pager.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ public abstract interface annotation class com/google/accompanist/pager/Experime
}

public final class com/google/accompanist/pager/Pager {
public static final fun HorizontalPager (Lcom/google/accompanist/pager/PagerState;Landroidx/compose/ui/Modifier;ILkotlin/jvm/functions/Function4;Landroidx/compose/runtime/Composer;II)V
public static final fun HorizontalPager (Lcom/google/accompanist/pager/PagerState;Landroidx/compose/ui/Modifier;ZILandroidx/compose/animation/core/DecayAnimationSpec;Landroidx/compose/animation/core/AnimationSpec;Lkotlin/jvm/functions/Function4;Landroidx/compose/runtime/Composer;II)V
}

public abstract interface class com/google/accompanist/pager/PagerScope : androidx/compose/foundation/layout/BoxScope {
Expand All @@ -15,15 +15,18 @@ public final class com/google/accompanist/pager/PagerScope$DefaultImpls {
public static fun matchParentSize (Lcom/google/accompanist/pager/PagerScope;Landroidx/compose/ui/Modifier;)Landroidx/compose/ui/Modifier;
}

public final class com/google/accompanist/pager/PagerState {
public final class com/google/accompanist/pager/PagerState : androidx/compose/foundation/gestures/ScrollableState {
public static final field Companion Lcom/google/accompanist/pager/PagerState$Companion;
public fun <init> (IIF)V
public synthetic fun <init> (IIFILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun animateScrollToPage (IFFLkotlin/coroutines/Continuation;)Ljava/lang/Object;
public static synthetic fun animateScrollToPage$default (Lcom/google/accompanist/pager/PagerState;IFFLkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object;
public fun dispatchRawDelta (F)F
public final fun getCurrentPage ()I
public final fun getCurrentPageOffset ()F
public final fun getPageCount ()I
public fun isScrollInProgress ()Z
public fun scroll (Landroidx/compose/foundation/MutatePriority;Lkotlin/jvm/functions/Function2;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
public final fun scrollToPage (IFLkotlin/coroutines/Continuation;)Ljava/lang/Object;
public static synthetic fun scrollToPage$default (Lcom/google/accompanist/pager/PagerState;IFLkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object;
public final fun setPageCount (I)V
Expand Down
4 changes: 2 additions & 2 deletions pager/src/androidTest/java/com/google/accompanist/pager/PagerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ import org.junit.runner.RunWith
import org.junit.runners.Parameterized

private const val MediumSwipeDistance = 0.8f
private const val ShortSwipeDistance = 0.6f
private const val ShortSwipeDistance = 0.45f

private const val FastVelocity = 4000f
private const val SlowVelocity = 400f
private const val SlowVelocity = 600f

@OptIn(ExperimentalPagerApi::class) // Pager is currently experimental
@LargeTest
Expand Down
96 changes: 59 additions & 37 deletions pager/src/main/java/com/google/accompanist/pager/Pager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,15 @@ package com.google.accompanist.pager

import android.util.Log
import androidx.annotation.IntRange
import androidx.compose.animation.splineBasedDecay
import androidx.compose.animation.core.AnimationSpec
import androidx.compose.animation.core.DecayAnimationSpec
import androidx.compose.animation.core.spring
import androidx.compose.animation.defaultDecayAnimationSpec
import androidx.compose.foundation.gestures.FlingBehavior
import androidx.compose.foundation.gestures.Orientation
import androidx.compose.foundation.gestures.draggable
import androidx.compose.foundation.gestures.ScrollScope
import androidx.compose.foundation.gestures.scrollBy
import androidx.compose.foundation.gestures.scrollable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.BoxScope
import androidx.compose.runtime.Composable
Expand All @@ -33,10 +39,10 @@ import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clipToBounds
import androidx.compose.ui.layout.Layout
import androidx.compose.ui.layout.Measurable
import androidx.compose.ui.layout.ParentDataModifier
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.platform.LocalLayoutDirection
import androidx.compose.ui.semantics.ScrollAxisRange
import androidx.compose.ui.semantics.horizontalScrollAxisRange
Expand All @@ -49,19 +55,19 @@ import androidx.compose.ui.unit.LayoutDirection
import kotlinx.coroutines.launch
import kotlin.math.roundToInt

/**
* The scroll threshold for moving to the next page. The value is used in both directions
* (so both negative and positive).
*/
internal const val ScrollThreshold = 0.35f

/**
* Library-wide switch to turn on debug logging.
*/
internal const val DebugLog = false

private const val LogTag = "Pager"

/**
* This attempts to mimic ViewPager's custom scroll interpolator. It's not a perfect match
* (and we may not want it to be), but this seem to match in terms of scroll duration and 'feel'
*/
private const val SnapSpringStiffness = 2750f

@RequiresOptIn(message = "Accompanist Pager is experimental. The API may be changed in the future.")
@Retention(AnnotationRetention.BINARY)
@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION)
Expand All @@ -87,8 +93,13 @@ private val Measurable.page: Int
*
* @param state the state object to be used to control or observe the list's state.
* @param modifier the modifier to apply to this layout.
* @param reverseLayout reverse the direction of scrolling and layout, when `true` items will be
* composed from the end to the start and [PagerState.currentPage] == 0 will mean
* the first item is located at the end.
* @param offscreenLimit the number of pages that should be retained on either side of the
* current page. This value is required to be `1` or greater.
* @param decayAnimationSpec The decay animation spec to use for decayed flings.
* @param snapAnimationSpec The animation spec to use when snapping.
* @param content a block which describes the content. Inside this block you can reference
* [PagerScope.currentPage] and other properties in [PagerScope].
*/
Expand All @@ -97,51 +108,62 @@ private val Measurable.page: Int
fun HorizontalPager(
state: PagerState,
modifier: Modifier = Modifier,
reverseLayout: Boolean = false,
@IntRange(from = 1) offscreenLimit: Int = 1,
content: @Composable PagerScope.(page: Int) -> Unit
decayAnimationSpec: DecayAnimationSpec<Float> = defaultDecayAnimationSpec(),
snapAnimationSpec: AnimationSpec<Float> = spring(stiffness = SnapSpringStiffness),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's is an idea: since your internal PagerState.fling uses only public API available (if not, it's still a good idea :) ) what you can do here is to replace two animation specs with one FlingBehaviour, e.g.

object PagerDefaults {
     @Composable
     fun defaultPagerFlingConfig(state: PagerState, decayAnimationSpec: DecayAnimationSpec<Float>, snapAnimationSpec: AnimationSpec<Float>): FlingConfig { ... }
}

Or similar, then you can just open the whole flingbehaviour for overriding (make FlingBehaviour a param in HorizontalPager), so people can:

  1. Use default behavior by providing nothing
  2. Use default with tweaked params
  3. Use their own sophisticated or slightly different fling behaviour by providing their own instance.
  4. The API for customization will be consistent with LazyColumn and friends.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I like it, I'll follow up after #254

content: @Composable PagerScope.(page: Int) -> Unit,
) {
require(offscreenLimit >= 1) { "offscreenLimit is required to be >= 1" }

val reverseScroll = LocalLayoutDirection.current == LayoutDirection.Rtl

val density = LocalDensity.current
val decay = remember(density) { splineBasedDecay<Float>(density) }
val isRtl = LocalLayoutDirection.current == LayoutDirection.Rtl
val reverseDirection = if (isRtl) !reverseLayout else reverseLayout
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do if (isRtl) reverseLayout else !reverseLayout to "reverse the reverse", do you still need the shenanigans with minuses in the rest of the logic?


val coroutineScope = rememberCoroutineScope()

val semanticsAxisRange = remember(state, reverseDirection) {
ScrollAxisRange(
value = { state.currentPage + state.currentPageOffset },
maxValue = { state.lastPageIndex.toFloat() },
)
}
val semantics = Modifier.semantics {
if (state.pageCount > 0) {
horizontalScrollAxisRange = ScrollAxisRange(
value = { (state.currentPage + state.currentPageOffset) * state.pageSize },
maxValue = { state.lastPageIndex.toFloat() * state.pageSize },
reverseScrolling = reverseScroll
)
// Hook up scroll actions to our state
scrollBy { x: Float, _ ->
coroutineScope.launch {
state.draggableState.drag { dragBy(x) }
}
true
horizontalScrollAxisRange = semanticsAxisRange
// Hook up scroll actions to our state
scrollBy { x: Float, _ ->
coroutineScope.launch {
state.scrollBy(if (reverseDirection) x else -x)
}
// Treat this as a selectable group
selectableGroup()
true
}
// Treat this as a selectable group
selectableGroup()
}

val draggable = Modifier.draggable(
state = state.draggableState,
startDragImmediately = true,
onDragStopped = { velocity ->
launch { state.performFling(velocity, decay) }
},
val flingBehavior = remember(state, decayAnimationSpec, snapAnimationSpec) {
object : FlingBehavior {
override suspend fun ScrollScope.performFling(initialVelocity: Float): Float {
return state.fling(
initialVelocity = -initialVelocity,
decayAnimationSpec = decayAnimationSpec,
snapAnimationSpec = snapAnimationSpec,
scrollBy = { deltaPixels -> -scrollBy(-deltaPixels) },
)
}
}
}

val scrollable = Modifier.scrollable(
orientation = Orientation.Horizontal,
reverseDirection = reverseScroll,
flingBehavior = flingBehavior,
reverseDirection = reverseDirection,
state = state,
)

Layout(
modifier = modifier
.then(semantics)
.then(draggable),
.then(scrollable)
.clipToBounds(),
content = {
val firstPage = (state.currentPage - offscreenLimit).coerceAtLeast(0)
val lastPage = (state.currentPage + offscreenLimit).coerceAtMost(state.lastPageIndex)
Expand Down
Loading