The recommended substitute for clearTask of popUpTo with popUpToInclusive only works from start destination actions [116831650]

Fixed

Bug

Status Update

No update yet.

Description

[Deleted User]

created issue #1

Sep 28, 2018 10:32AM

I was previously using clearTask in my navigation actions to make sure that the user could not navigate back to the previous destination (useful for splash screens and login screens). After it was deprecated I decided to try the recommended approach of instead setting popUpTo pointing to the root of the graph together with setting popUpToInclusive="true", but this didn't have the effect I wanted.

This approach only seem to work on actions originating from the graph's start destination, and fails on actions between any other destinations. I created a sample project which reproduces the issue:

https://github.com/hsson/android-navigation-component-alpha06-bug

Here is the project's README detailing the issue:

# Bug in Android Navigation Component (alpha06)
(Note: I have only verified this in alpha06, I don't know if it exists in
previous versions)

## The problem
Basically the problem is that the NavOption "clearTask" was removed in
alpha02, and the recommended substitute was to use app:popUpTo pointing to
the root of the graph/the graph ID, together with app:popUpToInclusive="true".
However, this method does not work. It only seems to work for actions
going from the graph's start destination. If we have the following graph:

x y
A ---> B ---> C

Where A, B, and C are fragments, A is the start destination of the grapg,
and x, y are actions going from A to B and B to C respectively. Let's say
the graph's ID is "@+id/graph"

Then, if both actions x and y have
app:popUpTo="@+id/graph"
app:popUpToInclusive="true"

Then when navigating from A to B through x, and then hitting the back button
will exit the app (as expected). But if navigating from A to B to C through
x and y, and then hitting the back button we will navigate back to B — this
is not what was expected. The expected behavior with this graph is to always
exit the app when clicking the back button

## Environment
This was verified using:
Android Studio 3.2 RC 3
Build #AI-181.5540.7.32.4987877, built on August 31, 2018
JRE: 1.8.0_152-release-1136-b06 x86_64
JVM: OpenJDK 64-Bit Server VM by JetBrains s.r.o
macOS 10.13.6

Comments

[Deleted User] <[Deleted User]> #2Oct 1, 2018 08:14AM

alpha01? That's so old! :D

We've found the issue in alpha10 and are working on a fix.

il...@google.com <il...@google.com> Oct 1, 2018 10:56PM

Assigned to il...@google.com.

[Deleted User] <[Deleted User]> #3Oct 10, 2018 09:20AM

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

commit 37ed64d4de87bd4a4365f5c0dfee7b33c8043013
Author: Daniel Santiago Rivera <danysantiago@google.com>
Date: Wed Jan 23 15:46:00 2019

Add missing import of parent Directions class in generated Safe Args.

When generating a parent direction action method in a child directions
class,correctly import the parent direction if its located in another
package. Both JavaPoet and KotlinPoet generate import statements but the
NavWriters were incorrectly creating the class name of the parent type,
specifically it missed the package name which caused the import to be
missing.

Bug: 123307342
Test: ./gradlew navigation:navigation-safe-args-gradle-plugin:test \
navigation:navigation-safe-args-generator:test

Change-Id: Ibeda7a7ec6f31bb1c860dfe1f5cd2a7e7ea6d47d

M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/java/JavaNavWriter.kt
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/kotlin/KotlinNavWriter.kt
M navigation/safe-args-generator/src/tests/kotlin/androidx/navigation/safe/args/generator/NavGeneratorTest.kt
M navigation/safe-args-generator/src/tests/kotlin/androidx/navigation/safe/args/generator/NavParserTest.kt
M navigation/safe-args-generator/src/tests/test-data/expected/nav_generator_test/java/nested/LoginFragmentDirections.java
M navigation/safe-args-generator/src/tests/test-data/expected/nav_generator_test/java/nested/RegisterFragmentDirections.java
M navigation/safe-args-generator/src/tests/test-data/expected/nav_generator_test/java/nested_overridden_action/InnerSettingsFragmentDirections.java
M navigation/safe-args-generator/src/tests/test-data/expected/nav_generator_test/java/nested_overridden_action/SettingsFragmentDirections.java
M navigation/safe-args-generator/src/tests/test-data/expected/nav_generator_test/kotlin/nested/LoginFragmentDirections.kt
M navigation/safe-args-generator/src/tests/test-data/expected/nav_generator_test/kotlin/nested/RegisterFragmentDirections.kt
M navigation/safe-args-generator/src/tests/test-data/expected/nav_generator_test/kotlin/nested_overridden_action/InnerSettingsFragmentDirections.kt
M navigation/safe-args-generator/src/tests/test-data/expected/nav_generator_test/kotlin/nested_overridden_action/SettingsFragmentDirections.kt
M navigation/safe-args-generator/src/tests/test-data/nested_login_test.xml
M navigation/safe-args-gradle-plugin/src/test/test-data/app-project-kotlin/src/main/res/navigation/nav_nested_test.xml
A navigation/safe-args-gradle-plugin/src/test/test-data/app-project/src/main/res/navigation/nav_nested_test.xml

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

https://goto.google.com/android-sha1/37ed64d4de87bd4a4365f5c0dfee7b33c8043013

il...@google.com <il...@google.com> #4Oct 18, 2018 06:05PM

Hi, I'm still having the issue with 1.0.0-alpha11. Generated classes for fragments in nested navigation throws error for global actions.

[Deleted User] <[Deleted User]> #5Oct 19, 2018 11:44AM

I am also having the issue with 1.0.0-alpha11. I'm going to see what the Kotlin generator does with this.

mi...@gmail.com <mi...@gmail.com> #6Oct 19, 2018 12:22PM

Re #4 and #5 - please file a new bug with a sample project that reproduces your issue.

[Deleted User] <[Deleted User]> #7Oct 19, 2018 12:24PM

Re #6 will do. I think it has to do with naming arguments with capital first letters but I'll see if I can make a minimal example app to demonstrate, as a new bug.

il...@google.com <il...@google.com> #8Oct 19, 2018 03:36PM

How do i fix this?

mi...@gmail.com <mi...@gmail.com> #9Oct 19, 2018 03:50PM

If you're working with Jetpack Navigation in Android and the generated classes don't import MainNavDirections, it usually points to an issue with your setup or the https://lillestromflytt.no/lager-lillestrom/ navigation component.If after all these steps the issue persists, consider invalidating caches and restarting Android Studio (File > Invalidate Caches / Restart), as sometimes cached data can cause problems with code generation.

il...@google.com <il...@google.com> #10Oct 19, 2018 04:13PM

It sounds like you are dealing with a coding issue, possibly in a web development or React project. If MainNavDirections isn't being imported in generated classes, you may need to explicitly import it in your files. If you provide more specific details about your setup and the context https://www.exploria.pk/tours/hunza-tour-packages/ in which MainNavDirections is used, I can offer more targeted assistance.

mi...@gmail.com <mi...@gmail.com> #11Oct 19, 2018 04:16PM

deleted

dc...@gmail.com <dc...@gmail.com> #12Oct 23, 2018 05:19PM

About your comment on #8 (mentioned bellow):

>>> - B+C should be handled as conditional navigation from D - D starts B and if you return to D without logging in, D calls finish().

Basically D needs to know the outcome of a task that was accomplished in B, meaning D needs to know if it was reached through opening the app (since it's a startDestination) or through pressing back in B (meaning the user canceled the login process).

Wouldn't this be a break in abstraction? My understanding was that a destination should receive it's arguments and this should be enough for it to work but in this example D needs to detect it was reached through a BACK press in B.

I don't even know how I would implement such a thing unless I start passing arguments between destinations to know about my navigation history.

da...@gmail.com <da...@gmail.com> #13Oct 26, 2018 10:58PM

I made this work, kind of: splash to main activity; starting destination D. It inflates a layout only to save state and go to the back stack as I bounce to B. If I press back, I go through the steps of restoring D--which briefly makes an appearance on screen--only to finish the activity.

I actually like the thought of an incorrect graph if it lets me handle this stuff precisely without some of the overhead needed to make a generic solution.

il...@google.com <il...@google.com> #14Nov 30, 2018 11:21PM

Marked as fixed.

I've confirmed that this is fixed in alpha08 as a result of

https://android-review.googlesource.com/833717 and a number of other internal changes.

app:popUpTo="@+id/graph" should always work now.