HorizontalPager and PagerState don't expose pageCount [266965072]

Fixed

Bug

Status Update

No update yet.

Description

yu...@schimke.ee

created issue #1

Jan 29, 2023 04:13PM

Jetpack Compose version: 1.4.0-alpha05
Jetpack Compose component used: compose foundation

It's not possible to migrate off Accompanist Horizontal Pager to Compose Foundation, because the page count is not exposed as state.

https://github.com/google/horologist/blob/0ba46cee202c84154b4e20db4850d29757c7e776/compose-layout/src/main/java/com/google/android/horologist/compose/pager/PagerScreen.kt#L99

This is needed to implement androidx.wear.compose.material.PageIndicatorState from Wear Compose

Comments

ma...@google.com <ma...@google.com> Jan 30, 2023 09:41AM

Reassigned to le...@google.com.

le...@google.com <le...@google.com> #2Jan 30, 2023 11:15AM

Project: platform/frameworks/support
Branch: androidx-main
Author: Louis Pullen-Freilich <lpf@google.com>
Link: https://android-review.googlesource.com/3328184

Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling

Expand for full commit details

Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling 
 
These APIs allow overscroll to have events dispatched to it by one component, and rendered in a separate component. 
 
Fixes: b/266550551 
Fixes: b/204650733 
Fixes: b/255554340 
Fixes: b/229537244 
Test: OverscrollTest 
Relnote: "Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling APIs - these APIs create a wrapped instance of the provided overscroll effect that doesn't draw / handle events respectively, which allows for rendering  overscroll in a separate component from the component that is dispatching events. For example, disabling drawing the overscroll inside a lazy list, and then drawing the overscroll separately on top / elsewhere." 
Change-Id: Idbb3d91546b49c1987a041f959bce4b2b09a9f61

Files:

M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/OverscrollDemo.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/OverscrollSample.kt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/OverscrollTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Overscroll.kt

Hash: f64e25b7a473c757d080521e7dd97b3f6670f60d
Date: Fri Nov 01 18:43:56 2024

yu...@schimke.ee <yu...@schimke.ee> #3Jan 30, 2023 12:14PM

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

androidx.compose.foundation:foundation:1.8.0-alpha06
androidx.compose.foundation:foundation-android:1.8.0-alpha06
androidx.compose.foundation:foundation-jvmstubs:1.8.0-alpha06
androidx.compose.foundation:foundation-linuxx64stubs:1.8.0-alpha06

le...@google.com <le...@google.com> #4Jan 30, 2023 02:08PM

Sorry, what I mean by fixed is that, unlike LazyList, the number of pager is known upfront, as a parameter from the definition of the composable.

You can of course use a state backed parameter in this case. I understand that the pageCount inside the PagerState is available through LazyListState, but the end goal is that Pager will not be based off LazyLists so we couldn't open all APIs available in LazyListState for Pager. We're reconsidering this and I'll update the ticket with the result, thanks for the feedback btw :)

yu...@schimke.ee <yu...@schimke.ee> #5Jan 30, 2023 06:49PM

I'm using rememberUpdatedState for the count now, so I'm certainly not blocked.

https://github.com/google/horologist/blob/4a9bcff9bd165ba9223758a1efd3c6f7b7f53027/compose-layout/src/main/java/com/google/android/horologist/compose/pager/PagerScreen.kt#L83-L87

val countState = rememberUpdatedState(newValue = count)
val pagerScreenState = remember { PageScreenIndicatorState(state, countState) }
HorizontalPageIndicator(
modifier = Modifier.padding(6.dp),
pageIndicatorState = pagerScreenState
)

ap...@google.com <ap...@google.com> #6Apr 13, 2023 01:23PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit a9fe9fa615abc65c607ff14f7e638a4afbb8e8cb
Author: Levi Albuquerque <levima@google.com>
Date: Mon Mar 20 19:52:53 2023

Update pageCount use and application in Pager

pageCount should be used inside PagerState. This allows for better flexibility in the public API (it can be accessed from state), it also allows for usage in composition.

Relnote: Remove pageCount from Horizontal/VerticalPager. This should be provided at the state creation. Updated PagerState and rememberPagerState to accept the pageCount.

Test: Previous tests should pass.
Fixes: 266965072
Change-Id: Ifa3cb3f0444ed1493efbeb3e6268f50684dec44a

M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/pager/PagerCarrouselDemos.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/pager/PagerDemos.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/pager/PagerStateInteractionsDemos.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/PagerSamples.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/BasePagerTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/EmptyPagerTests.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PageLayoutPositionOnScrollingTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PageSizeTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerAccessibilityTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerContentPaddingTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerContentTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerCrossAxisTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerLayoutInfoTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerNestedScrollContentTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerOffscreenPageLimitPlacingTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerPinnableContainerTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerPrefetcherTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerScrollingTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerStateTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerSwipeEdgeTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/LazyLayoutPager.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/Pager.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/PagerMeasure.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/PagerMeasurePolicy.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/PagerState.kt
M compose/integration-tests/macrobenchmark-target/src/main/java/androidx/compose/integration/macrobenchmark/target/PagerActivity.kt
M compose/integration-tests/macrobenchmark-target/src/main/java/androidx/compose/integration/macrobenchmark/target/PagerAsCarouselActivity.kt
M paging/paging-compose/samples/src/main/java/androidx/paging/compose/samples/PagingFoundationSample.kt

https://android-review.googlesource.com/2490595

ap...@google.com <ap...@google.com> #7Apr 17, 2023 02:00PM

Project: platform/frameworks/support
Branch: androidx-main

commit 64c80bd7c114fc4a1ec29441277b6efa644a79dd
Author: Levi Albuquerque <levima@google.com>
Date: Thu Apr 13 18:03:40 2023

Update pageCount use and application in Pager

pageCount should be used inside PagerState. This allows for better flexibility in the public API (it can be accessed from state), it also allows for usage in composition.

Relnote: Remove pageCount from Horizontal/VerticalPager. This should be provided at the state creation. Updated PagerState and rememberPagerState to accept the pageCount.

Test: Previous tests should pass.
Fixes: 266965072
Change-Id: Ieb52d10f08c4a45beaa4ce0a72734a44afb60ce7

M compose/foundation/foundation/api/public_plus_experimental_current.txt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/pager/PagerCarrouselDemos.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/pager/PagerDemos.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/pager/PagerStateInteractionsDemos.kt
M compose/foundation/foundation/samples/src/main/java/androidx/compose/foundation/samples/PagerSamples.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/BasePagerTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/EmptyPagerTests.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PageLayoutPositionOnScrollingTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PageSizeTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerAccessibilityTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerContentPaddingTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerContentTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerCrossAxisTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerLayoutInfoTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerNestedScrollContentTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerOffscreenPageLimitPlacingTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerPinnableContainerTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerPrefetcherTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerScrollingTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerStateTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerSwipeEdgeTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/LazyLayoutPager.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/Pager.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/PagerMeasure.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/PagerMeasurePolicy.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/PagerState.kt
M compose/integration-tests/macrobenchmark-target/src/main/java/androidx/compose/integration/macrobenchmark/target/PagerActivity.kt
M compose/integration-tests/macrobenchmark-target/src/main/java/androidx/compose/integration/macrobenchmark/target/PagerAsCarouselActivity.kt
M paging/paging-compose/samples/src/main/java/androidx/paging/compose/samples/PagingFoundationSample.kt

https://android-review.googlesource.com/2535728

ys...@google.com <ys...@google.com> #8Apr 25, 2023 06:08PM

Nice, works well. https://github.com/google/horologist/pull/1231/files

One footgun is partially updating the API usage, I removed the count from the HorizontalPager, but missed one location calling rememberPagerState, was confused. But working after that. I wonder if that invalid combination should fail with a more obvious error.

But looks good.

na...@google.com <na...@google.com> #9May 3, 2023 06:24PM

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

androidx.paging:paging-compose:1.0.0-alpha19

na...@google.com <na...@google.com> #10May 10, 2023 07:09PM

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

androidx.compose.foundation:foundation:1.5.0-alpha04

[Deleted User] <[Deleted User]> #11Jun 2, 2023 08:03PM

Started using this recently. But running into issues when using key on the HorizontalPager. If an update occurs on the data set the count seems to be getting mismatched? Get an "IndexOutOfBoundsException" when trying to access data set with index for the keys.

le...@google.com <le...@google.com> #12Jun 5, 2023 09:12AM

Hi, could you open another ticket with a reproducible sample? Thank you :D

[Deleted User] <[Deleted User]> #13Jun 5, 2023 09:27PM

Thanks for the reply, will do!

Issue 266965072

Description

Issue summary

Comments

ma...@google.com <ma...@google.com> Jan 30, 2023 09:41AM

le...@google.com <le...@google.com> #2Jan 30, 2023 11:15AM

yu...@schimke.ee <yu...@schimke.ee> #3Jan 30, 2023 12:14PM

le...@google.com <le...@google.com> #4Jan 30, 2023 02:08PM

yu...@schimke.ee <yu...@schimke.ee> #5Jan 30, 2023 06:49PM

ap...@google.com <ap...@google.com> #6Apr 13, 2023 01:23PM

ap...@google.com <ap...@google.com> #7Apr 17, 2023 02:00PM

ys...@google.com <ys...@google.com> #8Apr 25, 2023 06:08PM

na...@google.com <na...@google.com> #9May 3, 2023 06:24PM

na...@google.com <na...@google.com> #10May 10, 2023 07:09PM

[Deleted User] <[Deleted User]> #11Jun 2, 2023 08:03PM

le...@google.com <le...@google.com> #12Jun 5, 2023 09:12AM

[Deleted User] <[Deleted User]> #13Jun 5, 2023 09:27PM

Add comment

Issue metadata