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

[Pager] HorizontalPager height changes are not consistent #1050

Closed
boswelja opened this issue Feb 25, 2022 · 18 comments
Closed

[Pager] HorizontalPager height changes are not consistent #1050

boswelja opened this issue Feb 25, 2022 · 18 comments
Assignees
Labels
stale Stale issues which are marked for closure

Comments

@boswelja
Copy link

Description

Currently when paging with HorizontalPager, if the pages have varying heights the transition isn't smooth. When swiping forward, Pager seems to adjust its height based on the next page off the screen, and when swiping backward the pager waits until swipe has started to adjust its height. This can lead to visual jank when swiping through pages of varying height.

Steps to reproduce

  1. Set up a HorizontalPager with pages of varying heights
  2. Swipe back and forth between pages
  3. Notice the inconsistent height changes causing jank

Expected behavior

HorizontalPager adjusts it's height once the new page has been snapped (or some other default, as long as it's consistent)

Additional context

My use case is a calendar month view. The number of rows in any given month can vary, which is when I notice this

@boswelja boswelja changed the title [Pager] HorizontalPager height changes are not smooth [Pager] HorizontalPager height changes are not consistent Feb 25, 2022
@vicpinm
Copy link

vicpinm commented Mar 9, 2022

Same problem here

@andretortolano
Copy link

This is a side problem to this, but I would love to be able to change the contentSize behavior maybe through a modifier.

the one thing I will be trying to implement on top of the Pager is the ability to know the size(width/height) of the next page I'm scrolling to, and gradually change the side as I'm moving to it.

right now the only thing I was able to do was to Modifier.animateContentSize() and I can see that it animates but it dependes on the side of those contents, I wish there was a way for me to get the process information on scroll or swipe gesture so I can animate the content size based on it.

@MobileSam
Copy link

@andretortolano were you able to achieve your goal? I'm looking into something similar and I was wondering if this lib is a good starting point.

@andretortolano
Copy link

@MobileSam I had to prioritize other things, but if this doesnt get updated here I will probably spend more time trying to build my own solution that lets me handle content size change better

@boswelja
Copy link
Author

I ended up switching to a straight LazyRow, but this workaround should still work for a HorizontalPager
https://github.com/boswelja/Ephemeris/blob/24eb24804ee7e4de0e34f54852f6f204851f5067/android/compose/src/main/kotlin/com/boswelja/ephemeris/compose/InfiniteHorizontalPager.kt#L74
It does assume there will only ever be at most one item in view at a time (but with some modification you could change that), and it's not very tidy but it works

@MobileSam
Copy link

@boswelja I was able to use your strategy of pagerHeight with this lib. It works great, thank you for the inspiration.

@andretortolano
Copy link

andretortolano commented May 5, 2022

Hey all, today I got some time and I got to work on this. inspired bu @boswelja but also connected to HorizontalPager I made this work:

@ExperimentalPagerApi
private class HorizontalPagerSize constructor(private val pagerState: PagerState) {

    val heightMap = mutableStateMapOf<Int, Int>()

    val pagerContainerSize
        get() = run {
            val currentPageSize = heightMap[pagerState.currentPage] ?: 0
            val nextPageSize = when {
                pagerState.currentPageOffset > 0 -> heightMap[pagerState.currentPage + 1] ?: 0
                pagerState.currentPageOffset < 0 -> heightMap[pagerState.currentPage - 1] ?: 0
                else -> currentPageSize
            }

            val offsetNormalized = if (pagerState.currentPageOffset < 0)
                pagerState.currentPageOffset * -1
            else
                pagerState.currentPageOffset

            (nextPageSize - currentPageSize) * offsetNormalized + currentPageSize
        }

    fun onHeightChanged(index: Int, height: Int) {
        if (height != 0 && heightMap[index] ?: 0 == 0) {
            heightMap[index] = height
        }
    }
}

val horizontalPagerSize = remember { HorizontalPagerSize(pagerState) }

    HorizontalPager(
        modifier = Modifier.size(with(LocalDensity.current) { horizontalPagerSize.pagerContainerSize.toDp() }),
        count = containerPages.pageCount,
        state = pagerState,
        verticalAlignment = Alignment.Top
    ) { index ->
        Box(
            // gets the size of each page and store it in a remembered state
            modifier = Modifier.onSizeChanged { horizontalPagerSize.onHeightChanged(index, it.height) }
        ) {
            Composition(index)
        }
    }

with this I was able to do what I was looking for in the beginning which is to change the overall content size while I'm scrolling.

@NitroG42
Copy link

Here's my take on it which is simpler but works in my case :

val pagerState = rememberPagerState()
    val selectedTabIndex = pagerState.currentPage
    TabRow(selectedTabIndex = selectedTabIndex,
        backgroundColor = MaterialTheme.colors.surface,
        indicator = { positions ->
            TabRowDefaults.Indicator(
                modifier = Modifier.pagerTabIndicatorOffset(pagerState, positions),
                color = MaterialTheme.colors.secondary
            )
        },
        tabs = {
            tabs.forEachIndexed { index, vehicleTab ->
//                Tab(selected = , onClick = { /*TODO*/ }) {
//
//                }
            }
        })


    HorizontalPager(
        state = pagerState,
        count = tabs.count(),
        modifier = Modifier
            .animateContentSize()
            .background(color = MaterialTheme.colors.background)
            .padding(contentPadding),
        verticalAlignment = Alignment.Top
    ) { page ->
        tabs.getOrNull(page)?.let {
            val scrolling by remember(pagerState) {
                derivedStateOf {
                    pagerState.currentPageOffset != 0f
                }
            }
            val heightModifier = if (pagerState.currentPage == page || scrolling) {
                Modifier.wrapContentHeight()
            } else {
                Modifier.requiredHeightIn(max = 320.dp)
            }
            Box(
                contentAlignment = Alignment.TopCenter,
                modifier = heightModifier.fillMaxWidth()
            ) {
                //card
            }
        }
    }

The main concept is to put every pages other than the one displayed on the screen to an arbitrary value (320.dp). As soon as the user start to scroll to another page, we display those pages in their full height (wrapcontent).
An amelioration would be to have something more nice than the arbitrary value (like the "maximum" height of the pager, or the remaining space in the screen).

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale Stale issues which are marked for closure label Jun 18, 2022
@boswelja
Copy link
Author

Still an issue

@github-actions github-actions bot removed the stale Stale issues which are marked for closure label Jun 19, 2022
@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale Stale issues which are marked for closure label Jul 19, 2022
@boswelja
Copy link
Author

Yep still an issue

@boswelja
Copy link
Author

Very cool thanks bot

@andkulikov
Copy link
Collaborator

Hello. Is there a chance you can try to reproduce this inconsistency issue it with just LazyRow with pages taking the parent size via Modifier.fillParentMaxWidth(), not Pager and file it as a bug in the Compose UI tracker https://issuetracker.google.com/issues?q=componentid:612128? Thanks

@joshing-dev
Copy link

Is there an issuetracker link to follow for this? I'm also having this issue and wondering how to stay up to date with it.

@andkulikov
Copy link
Collaborator

Hey! Pager from accompanist is not maintained anymore. It was replaced with a HorizontalPager we added directly into our main compose.foundation library. Please try to see if it works fine there, and if not file a bug in the compose bug tracker

@joshing-dev
Copy link

Thank you, I realized this was for accompanist and not the official foundation library after posting. I've been using the official one from foundation and this issue was the closest to my scenario when using it for a calendar view. I created a bug report here in case anyone else stumbles upon this: https://issuetracker.google.com/issues/330194851

It seems to unreliably adjust height depending on how many pages you give it.

@BadrAtt
Copy link

BadrAtt commented Jun 13, 2024

it's still there,
I did a workaround to fix this:

    HorizontalPager(
        modifier = modifier,
        state = pagerState,
        pageSize = pageSize,
        userScrollEnabled = false
    ) { page ->

        val currentPage = pagerState.currentPage
        if(currentPage  == page && pagerState.currentPageOffsetFraction == 0f){
            pages[page]()
        }
    }

I've firstly disabled the userScroll then, I display the Page only when it's fully snapped

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale issues which are marked for closure
Projects
None yet
Development

No branches or pull requests

8 participants