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.

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

LazyListState.isScrollInProgress is always false on desktop #1423

Closed
JoeSteven opened this issue Nov 19, 2021 · 7 comments · Fixed by JetBrains/compose-multiplatform-core#1055
Labels
bug Something isn't working desktop scroll

Comments

@JoeSteven
Copy link

JoeSteven commented Nov 19, 2021

Compose-jb version: 1.0.0-beta5
kotlin version: 1.5.31
Hi, I want to save the first item position after scroll, here is my code:

            MaterialTheme {
                val listState = rememberLazyListState()
                LaunchedEffect(Unit) {
                    snapshotFlow { listState.isScrollInProgress }
                        .distinctUntilChanged()
                        .filter {
                            println("is scroll in Progress:$it")
                            !it
                        }.collect {
                            //save state here..
                        }
                }
                LazyColumn(state = listState) {
                    items(500) { index ->
                        Text("Test item $index")
                    }
                }
            }

it works perfect on Android, isScrollInProgress updated every time the list start to scroll and stop scroll

2021-11-19 14:26:37.012 16032-16032/? I/System.out: is scroll in Progress:true
2021-11-19 14:26:37.495 16032-16032/? I/System.out: is scroll in Progress:false
2021-11-19 14:26:37.860 16032-16032/? I/System.out: is scroll in Progress:true
2021-11-19 14:26:39.333 16032-16032/? I/System.out: is scroll in Progress:false
2021-11-19 14:26:58.640 16032-16032/? I/System.out: is scroll in Progress:true
2021-11-19 14:27:00.484 16032-16032/? I/System.out: is scroll in Progress:false

But on desktop (MacOS BigSur/ Windows10) I found that isScrollInProgress is always false even list is clearly in scroll。BTW, I've tested this code both on my project and Compose-jb/examples/imageViewer, the results is the same.

@akurasov akurasov self-assigned this Dec 8, 2021
@akurasov akurasov added bug Something isn't working desktop labels Dec 8, 2021
@zacharee
Copy link

zacharee commented Dec 12, 2021

I'm running into this issue too. It seems like on desktop DefaultScrollableState.scroll() isn't being called. Instead, dispatchRawDelta() is. Since that second method doesn't affect isScrollInProgress, it's always false.

I think it's because of the touchScrollable() vs mouseScroll() differences here.

@SebastianAigner
Copy link
Member

SebastianAigner commented Mar 10, 2022

Encountered this problem while trying to synchronize the scroll position of multiple parallel LazyColumns.
Inspired by these answers on StackOverflow:

https://stackoverflow.com/questions/71206165/shared-lazyliststate-across-multiple-lazyrow-result-in-strange-behavior?noredirect=1&lq=1,
https://stackoverflow.com/questions/69565303/scroll-multiple-lists-at-different-speeds-in-jetpack-compose/69613111#69613111

Temporarily worked around this by using firstVisibleItemIndex and firstVisibleItemScrollOffset as triggers to perform the scrollToItem:

val idx = lazyListStateA.firstVisibleItemIndex
val off = lazyListStateA.firstVisibleItemScrollOffset

LaunchedEffect(idx, off) {
    lazyListStateB.scrollToItem(idx, off)
}

@MatkovIvan
Copy link
Member

It should be mitigated after implementing smooth (animated) scrolling, but if you need to check if there was a scroll, you should subscribe to firstVisibleItemIndex and firstVisibleItemScrollOffset (as @SebastianAigner mentioned above) instead:

snapshotFlow { listState.firstVisibleItemIndex to listState.firstVisibleItemScrollOffset }
    .distinctUntilChanged()

Semantics of isScrollInProgress returns true only if we have a scroll across multiple frames, i.e. we have something continuous, not instant. Unlike dragging where we can detect start, move and end of gesture (in different frames), it's not a case with single event from mouse wheel. As I mentioned above it can be mitigated by introduction animated transaction, but still even after that, there can be a situation where transaction doesn't have an animation, and scroll position will be changed from one place to another in one single frame.

@MatkovIvan
Copy link
Member

It worth to mention that smooth (animated) scrolling is merged, so currently there is a TODO in code about this flag. Please note that it's still not exactly about mouse wheel, since it's just single-frame event (as explained above).

@MatkovIvan MatkovIvan removed their assignment Mar 28, 2023
@YektaDev
Copy link
Contributor

ScrollState has a similar field. It's also always false on desktop.

@aljohnston112
Copy link

aljohnston112 commented Aug 3, 2023

It should be mitigated after implementing smooth (animated) scrolling, but if you need to check if there was a scroll, you should subscribe to firstVisibleItemIndex and firstVisibleItemScrollOffset (as @SebastianAigner mentioned above) instead:

snapshotFlow { listState.firstVisibleItemIndex to listState.firstVisibleItemScrollOffset }
    .distinctUntilChanged()

Semantics of isScrollInProgress returns true only if we have a scroll across multiple frames, i.e. we have something continuous, not instant. Unlike dragging where we can detect start, move and end of gesture (in different frames), it's not a case with single event from mouse wheel. As I mentioned above it can be mitigated by introduction animated transaction, but still even after that, there can be a situation where transaction doesn't have an animation, and scroll position will be changed from one place to another in one single frame.

I tried this

    LaunchedEffect(lazyListState) {
        snapshotFlow {
            lazyListState.firstVisibleItemIndex to
                    lazyListState.firstVisibleItemScrollOffset
        }.distinctUntilChanged().collect { (i, offset) ->
            println("Changed")
            lazyListState.scrollToItem(i, offset)
        }
    }

and it only triggers twice and then scrolling stops working.

@MatkovIvan
Copy link
Member

@aljohnston112 It's about #3366. Fixed in JetBrains/compose-multiplatform-core#696 It was because conflicting simultaneous animations (triggered by mouse wheel and by your scrollToItem)

MatkovIvan added a commit to JetBrains/compose-multiplatform-core that referenced this issue Feb 14, 2024
## Proposed Changes

- Use `dispatchScroll` with a new `NestedScrollSource.Wheel` instead of
direct call of `scrollBy`/`dispatchRawDelta`
- Set scrolling priority to `MutatePriority.UserInput` during mouse
wheel scroll animation
- Move all scroll delta dispatching into a single coroutine
- Remove threshold (logic where small delta was applied without
animation), `shouldApplyImmediately` flag based on
`isPreciseWheelScroll` handles it instead.
- Wait `ProgressTimeout` after each mouse wheel event before resetting
`isScrollInProgress` flag
- Enable this logic by default (old "raw" dispatching failed our tests
on iOS)

## Testing

Test: Check "NestedScroll" page in mpp demo + unit tests

## Issues Fixed

Fixes JetBrains/compose-multiplatform#653
Fixes JetBrains/compose-multiplatform#1423
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working desktop scroll
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants