Status Update
Comments
lp...@google.com <lp...@google.com> #2
Branch: androidx-main
commit 0d92f9936b46c7f380d4114c1334f1eca5e2818f
Author: Zach Klippenstein <klippenstein@google.com>
Date: Fri Mar 10 14:27:30 2023
Rewrite PlatformTextInput* to be more session-oriented.
This completely changes how text input clients interact with the
PlatformTextInput system. Instead of reading a composition local to get
the registry, and then asking the registry for the adapter for their
plugin, they now must implement a Modifier.Node that implements the
PlatformTextInputModifierNode interface. Such nodes have access to a
suspend function that will show the keyboard and ask the system to start
a new input session. The function takes platform-specific input that
specifies how to wire up to the platform APIs. The lifetime of the
coroutine is tied to the lifetime of the input session: the function
will suspend the calling coroutine until the session is closed, and
cancelling the coroutine early will close the session. When the last
session is closed the keyboard will automatically be hidden.
See go/platform-text-input for more info.
PlatformTextInput APIs were originally introduced in aosp/2406439.
Fixes:
Fixes:
Bug:
Test: AndroidTextInputSessionRequestTest
Test: AndroidPlatformTextInputSessionTest
Test: NullableInputConnectionWrapperTest
Test: SessionMutexTest
Test: PlatformTextInputViewIntegrationTest
Test: DesktopTextInputSessionTest
Relnote: "Completely redesigned `PlatformTextInput*` API."
Change-Id: I6c93a1111561b5cb55c6a34e2fc3738be3c8941d
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/text/PlatformTextInputAdapterDemo.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/text/TextDemos.kt
M compose/foundation/foundation/integration-tests/foundation-demos/src/main/java/androidx/compose/foundation/demos/text2/BasicTextField2Demos.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/text2/BasicTextField2ImmIntegrationTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/text2/BasicTextField2Test.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/text2/TextFieldKeyboardActionsTest.kt
D compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/text2/input/internal/AndroidTextInputAdapterTest.kt
A compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/text2/input/internal/AndroidTextInputSessionTest.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/BasicTextField2.kt
D compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/internal/AndroidTextInputAdapter.kt
D compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/internal/AndroidTextInputPlugin.kt
A compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/internal/AndroidTextInputSession.android.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/internal/TextFieldDecoratorModifier.kt
D compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/text2/input/internal/TextInputCommandExecutorTest.kt
M compose/ui/ui-text/api/current.txt
M compose/ui/ui-text/api/restricted_current.txt
D compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/input/PlatformTextInputAdapter.android.kt
A compose/ui/ui-text/src/androidMain/kotlin/androidx/compose/ui/text/input/PlatformTextInputMethodRequest.android.kt
D compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/input/PlatformTextInputAdapter.kt
A compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/input/PlatformTextInputMethodRequest.kt
M compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/input/TextInputService.kt
D compose/ui/ui-text/src/desktopMain/kotlin/androidx/compose/ui/text/input/PlatformTextInputAdapter.desktop.kt
A compose/ui/ui-text/src/desktopMain/kotlin/androidx/compose/ui/text/input/PlatformTextInputMethodRequest.desktop.kt
M compose/ui/ui/api/current.ignore
M compose/ui/ui/api/current.txt
M compose/ui/ui/api/restricted_current.ignore
M compose/ui/ui/api/restricted_current.txt
A compose/ui/ui/samples/src/main/java/androidx/compose/ui/samples/PlatformTextInputModifierNodeSample.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/AndroidComposeViewAccessibilityDelegateCompatTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/input/pointer/HitPathTrackerTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/input/pointer/PointerInputEventProcessorTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/layout/Helpers.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/platform/LocalSoftwareKeyboardControllerTest.kt
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/text/input/AndroidPlatformTextInputSessionTest.kt
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/text/input/NullableInputConnectionWrapperTest.kt
D compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/text/input/PlatformTextInputAdapterRegistryTest.kt
D compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/text/input/PlatformTextInputEditTextIntegrationTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/text/input/PlatformTextInputViewIntegrationTest.kt
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/text/input/TestInputMethodRequest.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt
A compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidPlatformTextInputSession.kt
A compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/PlatformTextInputSession.android.kt
D compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/text/input/AndroidTextInputServicePlugin.kt
A compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/text/input/NullableInputConnectionWrapper.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/text/input/TextInputServiceAndroid.android.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/AtomicReference.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/SessionMutex.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/Owner.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/platform/CompositionLocals.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/platform/PlatformTextInputModifierNode.kt
M compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposeLayer.desktop.kt
M compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/DesktopPlatformInput.desktop.kt
A compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/DesktopTextInputSession.kt
M compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/PlatformComponent.desktop.kt
A compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/PlatformTextInputSession.desktop.kt
A compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/platform/DesktopTextInputSessionTest.kt
A compose/ui/ui/src/jvmMain/kotlin/androidx/compose/ui/AtomicReference.jvm.kt
M compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/PlatformInput.skiko.kt
M compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/SkiaBasedOwner.skiko.kt
A compose/ui/ui/src/test/kotlin/androidx/compose/ui/SessionMutexTest.kt
M compose/ui/ui/src/test/kotlin/androidx/compose/ui/node/LayoutNodeTest.kt
M compose/ui/ui/src/test/kotlin/androidx/compose/ui/node/ModifierLocalConsumerEntityTest.kt
M compose/ui/ui/src/test/kotlin/androidx/compose/ui/text/input/TextInputServiceAndroidCommandDebouncingTest.kt
M development/build_log_simplifier/messages.ignore
ap...@google.com <ap...@google.com> #3
Branch: androidx-main
commit 646912a194282dae0f26239faf838f1b0abb7149
Author: Zach Klippenstein <klippenstein@google.com>
Date: Mon Jun 26 13:40:30 2023
Move BasicTextField2 to common.
Bug:
Test: existing test coverage, no behavior change just moving source sets
Relnote: n/a
Change-Id: I62b95e65d196e0f1e5a8c27845d7e7d0b3a473ba
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/internal/ComposeInputMethodManager.android.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/internal/StatelessInputConnection.android.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/text2/input/internal/TextInputSession.android.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/DeadKeyCombiner.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/BasicSecureTextField.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/BasicTextField2.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/AllCapsFilter.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/CodepointTransformation.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/MaxLengthFilter.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/TextEditFilter.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/TextEditResult.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/TextFieldBuffer.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/TextFieldBufferWithSelection.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/TextFieldCharSequence.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/TextFieldLineLimits.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/TextFieldState.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/TextObfuscationMode.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/ApplyEditCommand.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/ChangeTracker.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/EditCommand.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/EditProcessor.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/EditingBuffer.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/GapBuffer.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/MathUtils.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/TextFieldCoreModifier.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/TextFieldDecoratorModifier.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/TextFieldKeyEventHandler.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/TextFieldTextLayoutModifier.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/TextLayoutState.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/input/internal/TextPreparedSelection.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text2/selection/TextFieldSelectionState.kt
A compose/foundation/foundation/src/desktopMain/kotlin/androidx/compose/foundation/text2/input/internal/DesktopTextInputSession.desktop.kt
ap...@google.com <ap...@google.com> #4
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.compose.foundation:foundation:1.6.0-alpha02
androidx.compose.foundation:foundation-android:1.6.0-alpha02
androidx.compose.ui:ui:1.6.0-alpha02
androidx.compose.ui:ui-android:1.6.0-alpha02
androidx.compose.ui:ui-text:1.6.0-alpha02
androidx.compose.ui:ui-text-android:1.6.0-alpha02
lp...@google.com <lp...@google.com>
ma...@google.com <ma...@google.com>
ap...@google.com <ap...@google.com> #5
Branch: androidx-main
commit ea035f9a4c64467bc175122c8f9efafae978080b
Author: Mariano Martin <marianomartin@google.com>
Date: Wed Mar 02 11:04:52 2022
Added a Modifier to query ancestors scroll info
Test: Added tests
Fixes: 223405612, 203141462
Relnote: """Added an Modifier API to query ancestors scroll info.
Used in Clickable to correctly delay press interactions, when getures could become scroll events.
Fixed Clickables not correctly delaying ripples, when used inside an Scrollable ViewGroup.
Updated Drawers and Sheets to correctly delay presses in case gestures can become scroll events.
"""
Change-Id: I2ba9d6d55f853e5d2775fe9a9f15e7a41d41e359
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ClickableInScrollableViewGroupTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ScrollableTest.kt
M compose/foundation/foundation/src/androidMain/kotlin/androidx/compose/foundation/Clickable.android.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Clickable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/Draggable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/Scrollable.kt
M compose/foundation/foundation/src/desktopMain/kotlin/androidx/compose/foundation/Clickable.desktop.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/DrawerTest.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/ModalBottomSheetTest.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/Drawer.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/ModalBottomSheet.kt
M compose/material3/material3/src/androidAndroidTest/kotlin/androidx/compose/material3/DismissibleNavigationDrawerTest.kt
M compose/material3/material3/src/androidAndroidTest/kotlin/androidx/compose/material3/ModalNavigationDrawerTest.kt
M compose/material3/material3/src/commonMain/kotlin/androidx/compose/material3/NavigationDrawer.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
A compose/ui/ui/samples/src/main/java/androidx/compose/ui/samples/ScrollableContainerSample.kt
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/input/nestedscroll/ScrollContainerInfoTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/viewinterop/AndroidViewTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/viewinterop/ComposeViewTest.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/viewinterop/AndroidViewHolder.android.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/input/ScrollContainerInfo.kt
ju...@google.com <ju...@google.com> #6
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.compose.foundation:foundation:1.4.0-alpha03
androidx.compose.material:material:1.4.0-alpha03
androidx.compose.material3:material3:1.1.0-alpha03
androidx.compose.ui:ui:1.4.0-alpha03
jo...@muzz.com <jo...@muzz.com> #7
We have clickable items within a swipeable Composable and have ended up applying a scrollable modifier which never scrolls in order to apply this local modifier and have the clickable items delay the ripple in case the user is performing a swipe.
If not should swipeable or draggable use the same modifier local to say the composable is inside a scrollable container?
lp...@google.com <lp...@google.com> #8
Yes, we have plans to make it public in the future, but it is not prioritized currently.
We don't want to globally enable this for draggable / swipeable, because the ordering can get tricky and not every component would need this, but we would explicitly add it for known components where it is required, such as in a drawer / a sheet.
jd...@google.com <jd...@google.com> #9
Reopening this bug because we would need ModifierLocalScrollableContainer
to be public so we can set it in our implementation of go/sysui-transition-layout. We have a custom gesture handler to implement transitions between different SystemUI scenes, and currently the ripple is instantly started when swiping down on clickables which has a noticeable impact on jank.
Any change this can be prioritized? :-) It looks like everything is already implement and "only" exposing a public API is missing.
lp...@google.com <lp...@google.com> #10
That said there might be other use cases that care about scrollable containers separately, such as scrolling screenshots, so it's unclear what the overlap is and what the API(s) that would accomplish these use cases in a consistent manner are. I might be able to do a brief exploration though to see what we would need to do to unblock at least some of the current use cases.
For your custom gesture handler, what are the 'rules' for the transitions here - does it convert horizontal / vertical swipes past a touch slop into a transition?
jd...@google.com <jd...@google.com> #11
Thanks for the quick reply!
For your custom gesture handler, what are the 'rules' for the transitions here - does it convert horizontal / vertical swipes past a touch slop into a transition?
Yes that's exactly what it does, using awaitHorizontalTouchSlopOrCancellation
and awaitVerticalTouchSlopOrCancellation
:-) We had to fork Draggable
because we also needed to support multiple fingers down. See the code in
jd...@google.com <jd...@google.com> #12
@lpf any update on this by any chance? :-)
lp...@google.com <lp...@google.com> #13
jd...@google.com <jd...@google.com> #14
Any news? :-) Pinging this again because this is preventing us from correctling measuring performance on some of our benchmarks: when the ripple is triggered during a transition, it's causing a lot of frames to be marked as dropped. See
lp...@google.com <lp...@google.com> #15
There's work planned in this space in our general tracker, but not at a high priority compared to other ongoing work.
For the benchmark case specifically, you could still end up in cases where the ripple is triggered if the gesture is 'slow' so that the move doesn't start until after the minimum press time: what specifically is your goal here for the benchmarks?
jd...@google.com <jd...@google.com> #16
what specifically is your goal here for the benchmarks?
I want the benchmark to be stable and have the least number of dropped frames. But we also want to make sure that in most cases this path is not triggered in production, so I don't want to just disable ripples in the benchmark either. Once we can disable the ripple inside our custom draggable, I'll make sure that the benchmark gesture is fast enough to not trigger the ripple.
jj...@google.com <jj...@google.com> #17
FYI this is real jank, not just showing up in the benchmark.
lp...@google.com <lp...@google.com> #18
I'll make sure that the benchmark gesture is fast enough to not trigger the ripple.
Hard to say without looking into some method traces for this, but it is likely that most of the cost isn't in drawing the ripple itself (because this is all done on the render thread), but in creating the ripple node in preparation to draw it. So even if you were to delay the interaction, it might still be slower regardless.
Do you have a link to the component with the ripple?
jj...@google.com <jj...@google.com> #19
updates. Some history here:
On Fri, May 24, 2024 at 6:21 PM lpf <buganizer-system+lpf@google.com> wrote:
lp...@google.com <lp...@google.com> #20
Not sure I understand the discussion in that bug, but it sounds like a framework issue on some API levels? Is there something actionable from the Compose side to mitigate this issue?
jj...@google.com <jj...@google.com> #21
Friendly ping on this - note this is a clear regression from the View framework. View's are able to handle this properly.
jd...@google.com <jd...@google.com> #22
To make sure there is no misunderstanding of the bug, here is an example that shows that the ripple is not triggered when dragging a clickable View
but that is is triggered when dragging a clickable Composable :-) See the video below.
import android.widget.Button
import androidx.compose.foundation.border
import androidx.compose.foundation.gestures.draggable2D
import androidx.compose.foundation.gestures.rememberDraggable2DState
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.offset
import androidx.compose.foundation.layout.size
import androidx.compose.material3.Button
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.round
import androidx.compose.ui.viewinterop.AndroidView
@Composable
fun DraggableDemo(modifier: Modifier = Modifier) {
Column(modifier.fillMaxSize()) {
var offset by remember { mutableStateOf(Offset.Zero) }
Box(
Modifier.offset { offset.round() }
.draggable2D(rememberDraggable2DState { offset += it })
.size(200.dp)
.border(1.dp, Color.Red),
contentAlignment = Alignment.Center,
) {
Column {
Button(onClick = {}) { Text("Compose button") }
AndroidView(factory = { Button(it).apply { text = "View button" } })
}
}
}
}
lp...@google.com <lp...@google.com> #23
For this issue specifically, all AndroidViews inside Compose are treated defensively as if they might scroll, so ripples will always be delayed (even if there is nothing scrollable in the hierarchy). In the future when we have public API we would implement this to follow the same logic used within compose.
For the compose case, we only apply this logic to scrollable() and other scrolling containers, not to draggable. If the 'issue' here is that you want the compose case to not instantly show a ripple, as a workaround if you add an empty scrollable with zero size somewhere in the hierarchy above the rippling component, it might work.
Note that this work is tracked in our backlog, but not being actively worked on / won't be for a minimum of a few months.
jd...@google.com <jd...@google.com> #24
For the compose case, we only apply this logic to scrollable() and other scrolling containers, not to draggable
Any chance we can have an early experimental version of a public API to be able to do the same in our custom pointer inputs/draggables?
lp...@google.com <lp...@google.com> #25
Current update:
There are some similar issues in the same problem space that we aim to solve together, rather than trying to patch over these issues with different hard-to-maintain APIs. Basically there's a general issue here of gesture coordination, where high level gestures (e.g 'drag') need some level of coordination between each other. This is true for this case, cases like handling touch slop for multiple lazy lists, etc.
The current loose idea is building some higher level abstraction around gestures, built on top of pointerInput. E.g you could imagine some Modifier.gesture() API, that allows for some higher level logic, and communication between gestures in a hierarchy. As mentioned previously this is on our backlog, and this should solve this specific bug too when we get around to implementing it
Description
In b/168524931 we globally added a delay before emitting a
PressInteraction
in case we are in a scrollable container, and in case the 'down' event is actually part of a scroll. This is important so that we don't show a ripple if the user is actually beginning a scroll motion.However, currently because we don't have any 'in scrollable container' metadata, this is globally enabled, whether or not we actually need to delay. This has the unfortunate effect of delaying
PressInteraction
s unnecessarily, and hence causing ripples and other indication to be delayed and feel laggy.With the recently added
ModifierLocal
APIs, we can now provide and consume this metadata, so we should now implement support for this so we only delayPressInteraction
s if necessary.Compose work:
ModifierLocal
to represent scrollable container metadataModifier.scrollable
- this is used in other APIs such as lazy lists andModifier.verticalScroll
, so just setting it here should cover all cases.PressInteraction
, so that we only delay if we are truly inside a scrollable containerInterop work:
ComposeView
is in a scrollable container, as even if there are no scrollable containers in the Compose part of the hierarchy, if theComposeView
itself is in a scrollableViewGroup
, we should still delay pressesAndroidViewHolder
should also consume this metadata and use it to overrideViewGroup#shouldDelayChildPressedState()
, so anyView
s hosted inside of Compose can also delay their pressed state if they are inside scrollable Compose containers, or inside non-scrollable Compose containers inside a non-scrollableAndroidView
inside a scrollable Compose container...Because the interop work requires APIs between
ui
andfoundation
(presumably theModifierLocal
needs to be public API inui
, so we can use it fromfoundation
), for now we can just ignore theAndroidViewHolder
support, and just queryLocalView
instead of setting this metadata at theComposeView
layer to unblock the most common cases.