"Could not find destination in the navigation graph, ignoring the deep link from Intent" for deep link in nested navigation [154532067]

Fixed

Bug

Status Update

No update yet.

Description

ja...@gmail.com

created issue #1

Apr 21, 2020 07:20AM

Component used: Navigation

Version used: 2.2.2

Devices/Android versions reproduced on: Emulator Android R, OnePlus 7 with Android 10

The issue appeared in 2.2.2, 2.2.1 works just fine. Error log:

I/NavController: Could not find destination com.my.app:id/main_navigation in the navigation graph, ignoring the deep link from Intent { act=android.intent.action.MAIN cat=[android.intent.category.LAUNCHER] flg=0x1040c000 pkg=com.my.app cmp=com.my.app/com.my.app.MainActivity (has extras) }

This log appears TWICE, so both graphs failed while lookup. In version 2.2.1 this log appeared once, as expected, for the nested graph.

The issue here is that in main_navigation i have other navigation graph for intro flow (splash screen, onboarding etc). If in main_navigation i have only fragments - everything works fine

Scenario to reproduce problem on attached project

Run app
Click "go to main page"
Click "Create notification"
Now hide the application and click notification

Pending intent in notification should point to main page, but it can't. Why? Because method findInvalidDestinationDisplayNameInDeepLink is looking for deep link handler twice. Once in proper main_navigation. But the second time - it is looking for it in intro_navigation, which doesn't have destination we are looking for. The changed line that broke things is graph.findNode(destinationId, false);. And it seems reasonable to me - intro_navigation can't find proper node, but it's parent can. I can confirm that evaluating graph.findNode(destinationId, true); returns proper node which is expected behavior.

Please ask if i can provide you with more information. Full reproduction sample provided.

TestApplication.zip

148 KB

Download

Comments

il...@google.com <il...@google.com> #2Apr 21, 2020 04:25PM

just query LocalView instead of setting this metadata at the ComposeView layer to unblock the most common cases.

As we can't know when / if the ComposeView is moved to a different part of the View hierarchy, we don't have a good callback / place to update this value within ComposeView - theoretically since shouldDelayChildPressedState is a function, it could return different values at different points in time in any case. So we should instead continue to query the view hierarchy during a press, to see if at that point in time we should delay the press or not.

So instead we just need to provide whether there is a scrollable Compose node, and then consumers can combine querying that with whether there is a scrollable ViewGroup, to work out whether they should delay presses or not. AndroidView can solely query whether it is in a scrollable Compose node, as anything inside it will look up the view hierarchy anyway

[Deleted User] <[Deleted User]> #3Apr 21, 2020 09:39PM

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

commit b9c029ef8bc7d6abe73b72bc4fb00e02da90b366
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Mon Oct 18 23:43:49 2021

Delays PressInteractions if the Compose view is inside a scrolling ViewGroup

Updates ComposeView and other ViewGroups to correctly set shouldDelayChildPressedState to false

Bug:

b/203141462

Test: ClickableInScrollableViewGroupTest.kt
Change-Id: Ib7e000139d2bf39c9453fde38efbe2d84a6eac9a

M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/selection/Toggleable.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt
M compose/foundation/foundation/src/desktopMain/kotlin/androidx/compose/foundation/Clickable.desktop.kt
A compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ClickableInScrollableViewGroupTest.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidViewsHandler.android.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ClickableTest.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/ComposeView.android.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/viewinterop/AndroidViewHolder.android.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

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

il...@google.com <il...@google.com> #4Apr 21, 2020 10:05PM

Accepted by il...@google.com.

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

commit 262448965ac6cc81652e47e0a1534ae14ff7014e
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Mon Oct 18 17:18:06 2021

No longer delays PressInteractions within Modifier.scrollable()

As a result ripples in non-scrollable containers will now instantly start to ripple on a down event, instead of waiting for a delay.

Bug:

b/203141462

Test: tests
Relnote: "Ripples and other indications will now only be delayed if they are inside a Modifier.scrollable() container, instead of always being delayed for a down event."
Change-Id: Ibefe01bcdef89e01b6e9f7edf9fe13622450f487

M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/selection/Toggleable.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/CardTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ClickableTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ToggleableTest.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/ButtonScreenshotTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/IndicationTest.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/SwitchScreenshotTest.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/SurfaceTest.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/RadioButtonScreenshotTest.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/CheckboxScreenshotTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ScrollableTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/SelectableTest.kt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/FloatingActionButtonScreenshotTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/Scrollable.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/Clickable.kt

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

ap...@google.com <ap...@google.com> #5Apr 21, 2020 10:59PM

Project: platform/frameworks/support
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

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

il...@google.com <il...@google.com> #6Apr 22, 2020 12:00AM

Marked as fixed.

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

[Deleted User] <[Deleted User]> #7Apr 23, 2020 05:43PM

The comment on ModifierLocalScrollableContainer says to make the modifier local public - is this going to be done?
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?

an...@google.com <an...@google.com> #8May 26, 2020 09:07PM

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.

il...@google.com <il...@google.com> #9May 27, 2020 05:37PM

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.

dk...@fundrise.com <dk...@fundrise.com> #10Aug 18, 2020 08:39PM

There's some more (internal) discussion in

b/261452022

as to why we reverted the change to make this public before, but the main point raised was that there are actually two sort of use cases here: knowing if you are in a scrollable container, and able to disambiguate between gestures. Some of the use cases such as swipe to dismiss, sheets / drawers, and the use case you mentioned have nothing to do with scrolling: but clickable components inside need to know that a down event might become consumed with some motion later. Currently we only really need gesture disambiguation support, so if we can figure out a way to represent this in the pointer input system, it might make this a lot cleaner / more generically useful.

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?

il...@google.com <il...@google.com> #11Aug 18, 2020 08:47PM

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 http://shortn/_sm4WMJVJ8T.