IssueTracker

Thanks! This looks like a bug, thank you for reporting!

Project: platform/frameworks/support
Branch: androidx-main
Author: Jossi Wolf <jossiwolf@google.com>
Link:      https://android-review.googlesource.com/3278651

Update AnchoredDraggable target calculation logic

Update AnchoredDraggable target calculation logic

We were previously relying on currentValue and the next closest anchor in the drag direction, but the simpler and more reliable way is to look at the left and right anchors as the window.

Relnote: Fixed a bug where positional thresholds passed to AnchoredDraggableDefaults.flingBehavior were not considered correctly in some scenarios.
Test: anchoredDraggable_fling_offsetPastHalfwayBetweenAnchors_beforePosThreshold_doesntAdvance
Bug: 367660226
Bug: 366003852
Change-Id: Ifdf0dfcf3d7ff8288affee56e7092bbed473d6ab

The following release(s) address this bug.It is possible this bug has only been partially addressed:

• androidx.compose.foundation:foundation:1.8.0-alpha04
• androidx.compose.foundation:foundation-android:1.8.0-alpha04
• androidx.compose.foundation:foundation-jvmstubs:1.8.0-alpha04

Using the new version 1.8.0-alpha04 does not seem to resolve the issue for me. I tested with the same setting as the reported issue:

            // it doesn't matter what you put here a the multiplier, the positionalThreshold is always 50%
            positionalThreshold = { d -> d * 0.9f },
            velocityThreshold = { Float.POSITIVE_INFINITY },

Same. I've run this again on 1.8.0-alpha04. When using

positionalThreshold = { d -> d * 0.1f }

Dragging up to 10% or between 50% and 90% will cause it to snap back to the start anchor. Dragging between 10% and 50% or past 90% will cause it to snap to the end anchor.

I would expect It to only return if dragged less than or equal to 10% and snap to the end if dragged greater than 10%.

When using

positionalThreshold = { d -> d * 0.9f }

It acts as if the threshold has been set to 50%.

From what I can tell, nothing has changed about the behavior.

Thanks, we will investigate and update this issue.

Project: platform/frameworks/support
Branch: androidx-main
Author: Jossi Wolf <jossiwolf@google.com>
Link:      https://android-review.googlesource.com/3343747

Update AnchoredDraggable target calculation logic

Update AnchoredDraggable target calculation logic 
 
There was a missed case when the velocity was negative. 
 
Test: New tests with more parameter permutations 
Fixes: 367660226 
Bug: 376931805 
Relnote: "Follow-up fix for an issue in AnchoredDraggable's target calculation where it could settle at the wrong anchor for specific swipes." 
Change-Id: I23b8773d3a694aa3c8de6a8f34b9d7fb54cfe560

What is the fix version?

We will comment on the issue when it has been included in a release. Until then, you can try the snapshot versions from androidx.dev.

The following release(s) address this bug.It is possible this bug has only been partially addressed:

• androidx.compose.foundation:foundation:1.8.0-alpha07
• androidx.compose.foundation:foundation-android:1.8.0-alpha07
• androidx.compose.foundation:foundation-jvmstubs:1.8.0-alpha07
• androidx.compose.foundation:foundation-linuxx64stubs:1.8.0-alpha07

#15, please report a new issue with a full reproduction case and the versions you are using. Thank you!

Running compose foundation 1.8.0-alpha07

I'm using AnchoredDraggable with nested scrolling. When I drag and release the lazy layout sometimes the draggable settles correctly and sometimes it doesn't even when i drag and release past the threshold (positionalThreshold = { distance -> distance / 2f }.

    internal fun nestedScrollConnection(
        dragState: AnchoredDraggableState<Anchors>,
        decayAnimationSpec: DecayAnimationSpec<Float>,
        orientation: Orientation = Orientation.Vertical,
    ): NestedScrollConnection = object : NestedScrollConnection {

        override fun onPreScroll(
            available: Offset,
            source: NestedScrollSource,
        ): Offset {
            val availableDelta = available.toFloat()

            return if (
                availableDelta < 0f
            ) dragState.dispatchRawDelta(availableDelta).toOffset()
            else Offset.Zero
        }

        override fun onPostScroll(
            consumed: Offset,
            available: Offset,
            source: NestedScrollSource,
        ): Offset {
            val availableDelta = available.toFloat()

            return if (
                availableDelta >= 0f
            ) dragState.dispatchRawDelta(availableDelta).toOffset()
            else Offset.Zero
        }

        override suspend fun onPreFling(
            available: Velocity,
        ): Velocity {
            val availableDelta = available.toFloat()

            val consumedOffset = if (availableDelta < 0f) dragState.animateToWithDecay(
                velocity = availableDelta,
                targetValue = Anchors.Collapsed,
                decayAnimationSpec = decayAnimationSpec,
            ) else 0f

            return Velocity(
                x = 0f,
                y = consumedOffset,
            )
        }

        override suspend fun onPostFling(
            consumed: Velocity,
            available: Velocity,
        ): Velocity {
            val availableDelta = available.toFloat()

            if (
                availableDelta > 0f
            ) dragState.animateToWithDecay(
                velocity = availableDelta,
                targetValue = Anchors.Expanded,
                decayAnimationSpec = decayAnimationSpec,
            )

            dragState.settle(animationSpec = spring())

            return Velocity(x = 0f, y = availableDelta)
        }

        private fun Float.toOffset(): Offset = Offset(
            y = if (orientation == Orientation.Vertical) this else 0f,
            x = if (orientation == Orientation.Horizontal) this else 0f,
        )

        @JvmName("offsetToFloat")
        private fun Offset.toFloat() = if (orientation == Orientation.Horizontal) x else y

        @JvmName("velocityToFloat")
        private fun Velocity.toFloat() = if (orientation == Orientation.Horizontal) x else y
    }

Hi, this seems to be a different issue. Please provide a full reproducer in a new issue so we can investigate. Thank you!

Looking at the release notes (https://developer.android.com/jetpack/androidx/releases/compose-foundation) is likely the most straightforward way. Since this issue has a few +1 votes, please keep the comments to the original issue :)

Unfortunately, this still does not work. I'm on 1.8.0-alpha07 and when I use 0.9f from the original example it still snaps at 50%.

#21, are you using exactly the same code shared in #1?

Yes, I'm the OP.

sdkmanager "build-tools;build-tools-version"

Apologies #23, but I am not able to reproduce the issue with the sample from #1, running on Compose 1.8.0-alpha08. Can you please provide a full reproducer project as a zip? Thanks!

Our use case is a list of items of various types that are sorted arbitrarily by domain-level business logic. There multiple (3+) types of interactions with list items which trigger an asynchronous update of the list that the composable receives. The composable doesn't know how the index will change on each operation. The list may be reordered, items may be removed, added, put to the end or to the top of the list. As such, it is not possible to call requestScrollToItem within a single measure pass. And yes, the requirements are such that although the list is changing, the position of the user (scroll state) should always stay the same.

If I may talk about some basic use case is having a list sorted by date of modification or date of last played.

The user is in the middle of the list, trigger an action on an item that can be delayed, like playing or changing something via a dialog or whatever, the database change the data is refreshed the item have changed position, the list scrolls to that item.

In a few cases it's not wanted, typically the user is doing operations at the end of a list to update things on the oldest stuff and every time he's jumped at start, or he want to play oldest stuff in order and the list keeps jumping ....

I'm not the OP but my use case would be a to-do list where marking the first item done should not scroll the list to the end where the checked item gets moved to upon completion. The current solution is way too cumbersome for such a basic task.

Great, thanks for that detail in #comment28!

Re #comment28, #comment29, and #comment30:

It sounds like your use-cases best match the test-case requestScrollToItem_withFirstVisibleIndex_firstVisibleItemMoved_staysScrolledAtSameIndex.

I will give a code-snippet below to show the template for the solution to these use-cases. I will then provide some detail below the snippet on how this solution works.

val state = rememberLazyListState()

// The content is inside of its own fun, so it gets recomposed when [list] changes and
// thus also triggers SideEffect.
@Composable
fun Content(list: List<T>) {
  LazyColumn(state = state) {
   items(list, key = { ... }) { ... }
  }

  SideEffect {
   state.requestScrollToItem(index = state.firstVisibleItemIndex)
  }
}

Content(list = list)

The solution above uses SideEffect which "guarantees that the effect executes after every successful recomposition".

Adding list: List<T> as a parameter to a @Composable fun, will cause the fun to be recomposed when the list changes.

Since the recomposition will always happen after the data changes, but before the next measure pass, SideEffect will be called after each data change, before the next measure pass. This will essentially fully opt-out of maintaining the scroll position based on key, and rather will maintain the currently scrolled index and offset.

Does that fully answer your questions/concerns?

Some of us lives in the future and use immutablelists.

And how does this handle state restoration? Specially if there's a delay on the query and so initial redraw have no data, state is skipped but you force a scroll, then it's broken.

There's so many side effects and things to handle this manually that it's still more an hack than something stable that won't bite in the future.

I did manage to solve this earlier with one of the test cases but the point is still that it is a very complex API for such basic use cases. There should be a flag that completely disables the auto-scrolling behavior (see comment #21).

Solution from #31 is not an acceptable one to me. You suggest to trigger a scroll operation on every recomposition, which in a complex screen will happen much more frequently than when just the list is mutated. This solution is subpar in terms of performance and honestly looks like a hack. Doesn't the compose official documentation discourage the usage of SideEffect function? Then why is this suggested as a default solution for 116+ people who upvoted this topic?

This solution is subpar in terms of performance

In most cases recomposition of the composable containing LazyColumn will anyway trigger remeasure for lazy column. Then calling requestScrollToItem is not making it worse. Also remeasuring lazy column is not as expensive as you think. We basically do this operation on every frame when the list is scrolling and this operation is heavily optimized.

And how does this handle state restoration?

We are not aware of any issues related to that. requestScrollToItem called with the same index/offset shouldn't change the behavior here. Please file a separate bug if you think there is an issue.

We are not saying that we will never introduce additional api to completely switch to maintaining the scroll position by index. For now we introduced requestScrollToItem api which is very flexible and keeping the same index is just one of many possible use cases for it. We are also slightly surprised it is considered to be so common to want to stay on the same index. This logic is not working great with reorderings. Every time there are new items added before the first visible one it is causing a visible to the user content jump.

To sum up, if you want to always stay on the same index, those are three options we can offer at the moment:

1. You can use SideEffect and request scroll to the same position everytime list composable is recomposed:

    @Composable
    fun MyList(list: List<Int>) {
        val state = rememberLazyListState()
        LazyColumn(state) {
            items(list, key = { it }) {
                ...
            }
        }

        SideEffect {
            state.requestScrollToItem(
                index = state.firstVisibleItemIndex,
                scrollOffset = state.firstVisibleItemScrollOffset
            )
        }
    }

2. If you don't use immutable lists and the list might be updated without recomposition then you can try this pattern.

Here we wrap it into withoutReadObservation because without that reading state.firstVisibleItemIndex or state.firstVisibleItemScrollOffset will subscribe us to each scroll position updates causing us to do unnecessary work.

    val list by remember { mutableStateListOf<String>() }
    val state = rememberLazyListState()
    LazyColumn(state) {
        Snapshot.withoutReadObservation {
            state.requestScrollToItem(
                index = state.firstVisibleItemIndex,
                scrollOffset = state.firstVisibleItemScrollOffset
            )
        }
        items(list, key = { it }) {
            ...
        }
    }

3. If you fully control all the list modifications you can just call requestScrollToItem manually every time you change the list and want to stay on the same index. For example from the button click lambdas.

We understand that it might still look too complex for some of you and you would just expect to have something like rememberLazyListState(maintainScrollPositionByKey = false) instead.

But we are not yet sure it is a scaleable enough API as there are many different context specific use cases, in some of them it is reasonable to stay on the same key, in others to stay on the same index, but in rare cases you might want to just end up on a completely different position after the data set change.

We will still monitor the feedback and are open to consider introducing more APIs to make common use cases simpler.

For the state restoration the lazy have hacks to not restore with paging for example when there's no items to avoid loosing the state, it wait for at least one item present.

With the side effect and no data yet restored it will try to scroll to an item that is not yet there so will fail, so will corrupt the state or crash or will have unpredictable effect.

This is something that needs to be taken in account.

Then there's the other API that was refused but can't find back the previous issue, it's proper control of that state restoration delay, since if you have an header and a paging source, the data is still restore too early and we need to do

    val emptyState = remember(lazyPagingItems.itemCount) {
        LazyGridState(
            firstVisibleItemIndex = lazyGridState.firstVisibleItemIndex,
            firstVisibleItemScrollOffset = lazyGridState.firstVisibleItemScrollOffset,
        )
    }

LazyVerticalGrid(
    state = if (lazyPagingItems.itemCount == 0) emptyState else lazyGridState,
)

And now we need to mix both of those and hope there's no other things that will bite us.

Another question, is what happens when a fling is running and the content is changed, if currently this is properly handled and the fling keeps going, now it will behave differently.

Lazy stuff is complex enough with many quirks to handle already, adding something ultra invasive as this kind of forced scroll will be problematic at some moment and will be insanely hard to debug.

For the state restoration the lazy have hacks to not restore with paging for example when there's no items to avoid loosing the state, it wait for at least one item present. With the side effect and no data yet restored it will try to scroll to an item that is not yet there so will fail, so will corrupt the state or crash or will have unpredictable effect.

As I said, I expect the new requestScrollToItem() to not change this behavior. The scroll position will not be "corrupted" while we don't have any items, in the same way as it works right now. If it is not what you experience please file a separate bug.

You need a compose foundation version of 1.7.0-alpha07+.

I tried the solution in #35 but if there's a scroll in progress when list changes, scrolling abrubtly stops.

Re #comment40:

If you'd like, you can first check the LazyListState for isScrollInProgress.

Similar to this test case.

That's what I ended up doing as a workaround. But now when list changes, LazyList tries to maintain scroll position if scroll is in progress. So it ends up scrolling to an unexpected position. It's very rare that this creates a problem, but I would still prefer a proper fix.

Are there any examples that incorporate Paging3?