Pullrefresh zIndex will interfeer with other composables [271777421]

Fixed

Bug

Status Update

No update yet.

Description

gd...@blissapplications.com

created issue #1

Mar 6, 2023 01:01PM

Jetpack Compose version:

androidxCompose = "1.4.0-beta02"
androidxComposeMaterial3 = "1.1.0-alpha07"
accompanist = "0.29.1-alpha"

Jetpack Compose component used:

androidx.compose.material.pullrefresh
androidx.compose.material3.TabRow

Android Studio Build: Android Studio Electric Eel | 2022.1.1 Patch 2
Kotlin version: 1.8.10

Steps to Reproduce or Code Sample to Reproduce:
Create the following structure:

Scaffold
 -- Column
       -- TabRow
            -- Tabs
       -- HorizontalPager
            -- PullToRefresh

And you'll notice that the tabs will lose some of their touch area (the middle one). The quick fix is this:

TabRow(
    modifier = Modifier.zIndex(Float.MAX_VALUE),

Video in attachment and more information on this thread: https://kotlinlang.slack.com/archives/CJLTWPH7S/p1678101507857829

Screen Recording 2023-03-06 at 12.34.50.mov

590 KB

Download

Comments

lp...@google.com <lp...@google.com> Mar 6, 2023 05:56PM

Reassigned to lp...@google.com.

ri...@google.com <ri...@google.com> #2Mar 6, 2023 06:07PM

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

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

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

Message last modified on Mar 6, 2023 06:09PM

lp...@google.com <lp...@google.com> #3Mar 6, 2023 08:17PM

Looks like this is happening because PullRefreshIndicator uses a Surface, which internally uses Modifier.pointerInput {} to block siblings from seeing input. In this particular case it's a problem because we offset the indicator off screen, and the indicator is below the tabs, so it ends up being displayed on top of the tabs and intercepting input.

We should consider:

a) whether we can fully remove the indicator when it's not visible anyway b) not using surface, and hence not blocking input

lp...@google.com <lp...@google.com> #4Mar 16, 2023 04:30PM

I think the best solution here is to replace the Surface with just a box with modifiers, to avoid blocking input - and keep it always being emitted. That way we won't break cases like using Modifier.onGloballyPositioned, which might not even be emitted until the indicator is shown

ap...@google.com <ap...@google.com> #5Mar 17, 2023 05:10PM

Marked as fixed.

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

commit b62278a33435277fd29f4508146eaea171fac45b
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Thu Mar 16 19:00:54 2023

Fixes PullRefreshIndicator intercepting clicks / pointer events

Surface internally blocks input for cases like when you have a drawer / sheet displayed over other content. We don't want that behaviour here, so stop using Surface.

Fixes:

b/271777421

Test: PullRefreshIndicatorTest
Change-Id: I9a1c0251cc4eb933cd41297fb660b6529ec1a0c5

M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/pullrefresh/PullRefreshIndicatorTest.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/pullrefresh/PullRefreshIndicator.kt

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

na...@google.com <na...@google.com> #6Apr 5, 2023 06:27PM

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

androidx.compose.material:material:1.5.0-alpha02

ju...@telesoftas.com <ju...@telesoftas.com> #7Jul 18, 2023 10:01AM

Still occurs using material = 1.8.0.

gd...@blissapplications.com <gd...@blissapplications.com> #8Jul 18, 2023 10:17AM

Using:

androidxCompose = "1.6.0-alpha01"
androidxComposeMaterial3 = "1.2.0-alpha03"

androidx-compose-material = { group = "androidx.compose.material", name = "material", version.ref = "androidxCompose" }
androidx-compose-material3 = { group = "androidx.compose.material3", name = "material3", version.ref = "androidxComposeMaterial3" }

and it works fine

Message last modified on Jul 18, 2023 10:24AM