Bug: PageSize.Fixed should be marked @Stable [300134276]

Fixed

Bug

Status Update

No update yet.

Description

ch...@gmail.com

created issue #1

Sep 12, 2023 09:21PM

Component used: androidx.paging:paging-compose
Version used: 3.2.1
Devices/Android versions reproduced on: N/A

PageSize.Fixed needs to be marked @Stable (and probably a data class). Currently it will cause recomposition of the Pager if recomposition starts above the Pager and you use PageSize in a way that it gets compared for stability.

The relevant line is here.

Example snippet (you'll need recomposeHighlighter):

@Composable
fun Example() {
    Column {
        var count by remember { mutableIntStateOf(0) }
        Button(onClick = { count++ }) {
            Text("Click me $count")
        }

        // The box wrappers force recomposition to start above the pager
        Box(modifier = Modifier.padding(bottom = count.dp)) {
            // This one's highlighter will not draw, indicating Pager was not recomposed
            Pager(pageSize = PageSize.Fill)
        }

        Box(modifier = Modifier.padding(bottom = count.dp)) {
            // This one's highlighter will increase, indicating Pager was not skipped
            Pager(pageSize = PageSize.Fixed(120.dp))
        }
    }
}

@Composable
fun Pager(pageSize: PageSize) {
    HorizontalPager(
        state = rememberPagerState { 2 },
        pageSize = pageSize,
        modifier = Modifier.recomposeHighlighter()
    ) {
        Text("$it", Modifier.padding(32.dp))
    }
}

In my specific use case, it made sense to remember the PageSize.Fixed, because I was using a dynamic calculation based on the screen width, so that's how I found and worked around this bug.

Comments

cl...@google.com <cl...@google.com> Sep 28, 2023 11:05PM

Reassigned to co...@google.com.

ma...@google.com <ma...@google.com> Oct 2, 2023 02:52PM

Reassigned to le...@google.com.

le...@google.com <le...@google.com> Nov 6, 2023 02:49PM

Accepted by le...@google.com.

ap...@google.com <ap...@google.com> #2Nov 6, 2023 05:23PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit a19b9ecab7e78804a61ba8481cc14cdb25d4d675
Author: Ralston Da Silva <ralu@google.com>
Date: Thu Dec 03 16:09:45 2020

Renaming focusObserver

Renaming Modifier.focusObserver() to Modifier.onFocusEvent() and Modifier.onFocusChanged

Bug: 168511863
Bug: 168511484
Test: ./gradlew compose:ui:ui:connectedCheck -P android.testInstrumentationRunnerArguments.class=androidx.compose.ui.focus.FocusChangedCountTest
Test: ./gradlew compose:ui:ui:connectedCheck -P android.testInstrumentationRunnerArguments.class=androidx.compose.ui.focus.FocusEventCountTest
Relnote: Modifier.focusObserver is deprecated. Use Modifier.onFocusChanged or Modifier.onFocusEvent instead
Change-Id: I30f17d06d60fa9b8c510ee0468464258894a467b

M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/text/ComposeInputFieldFocusTransition.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/TextFieldCursorTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/TextFieldFocusTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/TextFieldTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/text/CoreTextFieldInputServiceIntegrationTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Focusable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/CoreTextField.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/textfield/OutlinedTextFieldTest.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/textfield/TextFieldTest.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/TextFieldImpl.kt
M compose/ui/ui/api/current.txt
M compose/ui/ui/api/public_plus_experimental_current.txt
M compose/ui/ui/api/restricted_current.txt
M compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/autofill/ExplicitAutofillTypesDemo.kt
M compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/focus/FocusableDemo.kt
M compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/focus/ReuseFocusRequester.kt
M compose/ui/ui/integration-tests/ui-demos/src/main/java/androidx/compose/ui/demos/keyinput/KeyInputDemo.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/CaptureFocusTest.kt
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/FocusChangedCountTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/FocusChangedTest.kt
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/FocusEventCountTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/FocusManagerAmbientTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/FocusModifierAttachDetachTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/FocusRequesterTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/FreeFocusTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/OwnerFocusTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/ReusedFocusRequesterCaptureFocusTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/ReusedFocusRequesterFreeFocusTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/ReusedFocusRequesterTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/focus/SetRootFocusTest.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/FocusModifier.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/FocusObserverModifier.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusChangedModifier.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/focus/FocusEventModifier.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/DelegatingLayoutNodeWrapper.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/InnerPlaceable.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNode.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNodeWrapper.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/ModifiedFocusEventNode.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/ModifiedFocusNode.kt
M compose/ui/ui/src/test/kotlin/androidx/compose/ui/focus/FocusChangedModifierTest.kt
M compose/ui/ui/src/test/kotlin/androidx/compose/ui/focus/FocusEventModifierTest.kt

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

Issue 300134276

Description

Issue summary

Comments

cl...@google.com <cl...@google.com> Sep 28, 2023 11:05PM

ma...@google.com <ma...@google.com> Oct 2, 2023 02:52PM

le...@google.com <le...@google.com> Nov 6, 2023 02:49PM

ap...@google.com <ap...@google.com> #2Nov 6, 2023 05:23PM

Add comment

Issue metadata