Fixed
Status Update
No update yet.
Comments
ap...@google.com
Activity destinations should be considered exit points for your navigation graph - they are not part of the NavController's back stack and therefore not being sent to OnNavigatedListeners is working as intended.
ap...@google.com
Thank you for your explanation, but although that is the intended behavior, it is still confusing.
In my opinion, the developer expects to be notified when any navigation action defined in the graph is performed, regardless the destination.
Although that action is considered an exit point, it is an action defined within the graph and it is indeed a navigation action happening in that graph, so I don't understand why it is not notified to the "OnNavigated" listener when it has indeed navigated, regardless if it affects or not the back stack.
I that is the intended behaviour, then it should be rather called something like "NavController.OnBackStackChangedListener".
In my opinion, the developer expects to be notified when any navigation action defined in the graph is performed, regardless the destination.
Although that action is considered an exit point, it is an action defined within the graph and it is indeed a navigation action happening in that graph, so I don't understand why it is not notified to the "OnNavigated" listener when it has indeed navigated, regardless if it affects or not the back stack.
I that is the intended behaviour, then it should be rather called something like "NavController.OnBackStackChangedListener".
ap...@google.com
Yeah, naming is hard :)
Right now, NavController.OnNavigatedListener gives you a NavDestination that always equals navController.getCurrentDestination(). It doesn't give you what action you triggered, arguments, or anything else (including operations that don't change getCurrentDestination(), such as singleTop actions or activity destinations), so I could definitely see this more as a OnDestinationChangedListener.
I'll reopen this to track renaming the listener to something more appropriate to what it actually is doing or updating the documentation to make its role more clear.
Right now, NavController.OnNavigatedListener gives you a NavDestination that always equals navController.getCurrentDestination(). It doesn't give you what action you triggered, arguments, or anything else (including operations that don't change getCurrentDestination(), such as singleTop actions or activity destinations), so I could definitely see this more as a OnDestinationChangedListener.
I'll reopen this to track renaming the listener to something more appropriate to what it actually is doing or updating the documentation to make its role more clear.
jb...@google.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit fcc0c6059e31f7e53396db8cce0ac62d76a1c2d8
Author: Ian Lake <ilake@google.com>
Date: Fri Nov 30 13:14:58 2018
Rename OnNavigatedListener
The callback to onNavigated() is not only
called in result to a navigate() call, but
is actually called whenever any navigation
event happens, be it single top navigation
events or popBackStack events.
Therefore the name should reflect what is
actually happening.
Test: existing tests still pass
BUG: 118670572
Change-Id: Iad45d646e859428a4f744992821916906ca2d34f
M navigation/integration-tests/testapp/src/main/java/androidx/navigation/testapp/NavigationActivity.kt
M navigation/runtime/api/1.0.0-alpha08.txt
M navigation/runtime/api/current.txt
M navigation/runtime/src/main/java/androidx/navigation/NavController.java
M navigation/ui/src/main/java/androidx/navigation/ui/AbstractAppBarOnDestinationChangedListener.java
M navigation/ui/src/main/java/androidx/navigation/ui/ActionBarOnDestinationChangedListener.java
M navigation/ui/src/main/java/androidx/navigation/ui/CollapsingToolbarOnDestinationChangedListener.java
M navigation/ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/ui/src/main/java/androidx/navigation/ui/ToolbarOnDestinationChangedListener.java
https://android-review.googlesource.com/838931
https://goto.google.com/android-sha1/fcc0c6059e31f7e53396db8cce0ac62d76a1c2d8
Branch: androidx-master-dev
commit fcc0c6059e31f7e53396db8cce0ac62d76a1c2d8
Author: Ian Lake <ilake@google.com>
Date: Fri Nov 30 13:14:58 2018
Rename OnNavigatedListener
The callback to onNavigated() is not only
called in result to a navigate() call, but
is actually called whenever any navigation
event happens, be it single top navigation
events or popBackStack events.
Therefore the name should reflect what is
actually happening.
Test: existing tests still pass
BUG: 118670572
Change-Id: Iad45d646e859428a4f744992821916906ca2d34f
M navigation/integration-tests/testapp/src/main/java/androidx/navigation/testapp/NavigationActivity.kt
M navigation/runtime/api/1.0.0-alpha08.txt
M navigation/runtime/api/current.txt
M navigation/runtime/src/main/java/androidx/navigation/NavController.java
M navigation/ui/src/main/java/androidx/navigation/ui/AbstractAppBarOnDestinationChangedListener.java
M navigation/ui/src/main/java/androidx/navigation/ui/ActionBarOnDestinationChangedListener.java
M navigation/ui/src/main/java/androidx/navigation/ui/CollapsingToolbarOnDestinationChangedListener.java
M navigation/ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/ui/src/main/java/androidx/navigation/ui/ToolbarOnDestinationChangedListener.java
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit 1a5551e0e3145b1e309577a1282d28fa1b14e956
Author: Ian Lake <ilake@google.com>
Date: Mon Dec 03 13:22:31 2018
Correct naming of add/removeOnDestinationChangedListener
Ensure that the add and remove methods
use the same naming convention as the
listener class itself
Test: ./gradlew bOS
BUG: 118670572
Change-Id: I71dd174f7abd3a870c12b26c0aef64ff1512ceea
M navigation/integration-tests/testapp/src/main/java/androidx/navigation/testapp/NavigationActivity.kt
M navigation/runtime/api/1.0.0-alpha08.txt
M navigation/runtime/api/current.txt
M navigation/runtime/src/main/java/androidx/navigation/NavController.java
M navigation/ui/src/main/java/androidx/navigation/ui/AbstractAppBarOnDestinationChangedListener.java
M navigation/ui/src/main/java/androidx/navigation/ui/CollapsingToolbarOnDestinationChangedListener.java
M navigation/ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/ui/src/main/java/androidx/navigation/ui/ToolbarOnDestinationChangedListener.java
https://android-review.googlesource.com/839441
https://goto.google.com/android-sha1/1a5551e0e3145b1e309577a1282d28fa1b14e956
Branch: androidx-master-dev
commit 1a5551e0e3145b1e309577a1282d28fa1b14e956
Author: Ian Lake <ilake@google.com>
Date: Mon Dec 03 13:22:31 2018
Correct naming of add/removeOnDestinationChangedListener
Ensure that the add and remove methods
use the same naming convention as the
listener class itself
Test: ./gradlew bOS
BUG: 118670572
Change-Id: I71dd174f7abd3a870c12b26c0aef64ff1512ceea
M navigation/integration-tests/testapp/src/main/java/androidx/navigation/testapp/NavigationActivity.kt
M navigation/runtime/api/1.0.0-alpha08.txt
M navigation/runtime/api/current.txt
M navigation/runtime/src/main/java/androidx/navigation/NavController.java
M navigation/ui/src/main/java/androidx/navigation/ui/AbstractAppBarOnDestinationChangedListener.java
M navigation/ui/src/main/java/androidx/navigation/ui/CollapsingToolbarOnDestinationChangedListener.java
M navigation/ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/ui/src/main/java/androidx/navigation/ui/ToolbarOnDestinationChangedListener.java
ap...@google.com
We've renamed the listener to OnDestinationChangedListener to make it more clear that it is only called when NavController's getCurrentDestination() or its arguments change, precluding activity destinations.
ap...@google.com
Project: platform/frameworks/support
Branch: snap-temp-L69500000699869549
commit 0330f7a7a613a290675d78b6e2d0f78cf15521d6
Author: Jeremy Woods <jbwoods@google.com>
Date: Fri Sep 11 15:58:27 2020
Add nav default animator resources
Animators are recommeneded over Animations for doing effects in fragment
and Navigation. We need to make default animator resources for
Navigation.
Test: Tested in sample apps
Bug: 167430145
Change-Id: I904d78268d8b8574c799ae1b0f5887e518fdb5f6
(cherry picked from commit 1e9ed84322fddd9228df1f96bc8cb76748e0a4fe)
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/navigation-ui/src/main/res-public/values/public_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_exit_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_exit_anim.xml
https://android-review.googlesource.com/1436912
Branch: snap-temp-L69500000699869549
commit 0330f7a7a613a290675d78b6e2d0f78cf15521d6
Author: Jeremy Woods <jbwoods@google.com>
Date: Fri Sep 11 15:58:27 2020
Add nav default animator resources
Animators are recommeneded over Animations for doing effects in fragment
and Navigation. We need to make default animator resources for
Navigation.
Test: Tested in sample apps
Bug: 167430145
Change-Id: I904d78268d8b8574c799ae1b0f5887e518fdb5f6
(cherry picked from commit 1e9ed84322fddd9228df1f96bc8cb76748e0a4fe)
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/navigation-ui/src/main/res-public/values/public_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_exit_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_exit_anim.xml
ap...@google.com
Project: platform/frameworks/support
Branch: snap-temp-L31300000699869852
commit ae16020f57114d8e9f07544dc57e9c00c4153d26
Author: Jeremy Woods <jbwoods@google.com>
Date: Fri Sep 11 15:58:27 2020
Add nav default animator resources
Animators are recommeneded over Animations for doing effects in fragment
and Navigation. We need to make default animator resources for
Navigation.
Test: Tested in sample apps
Bug: 167430145
Change-Id: I904d78268d8b8574c799ae1b0f5887e518fdb5f6
(cherry picked from commit 1e9ed84322fddd9228df1f96bc8cb76748e0a4fe)
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/navigation-ui/src/main/res-public/values/public_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_exit_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_exit_anim.xml
https://android-review.googlesource.com/1436916
Branch: snap-temp-L31300000699869852
commit ae16020f57114d8e9f07544dc57e9c00c4153d26
Author: Jeremy Woods <jbwoods@google.com>
Date: Fri Sep 11 15:58:27 2020
Add nav default animator resources
Animators are recommeneded over Animations for doing effects in fragment
and Navigation. We need to make default animator resources for
Navigation.
Test: Tested in sample apps
Bug: 167430145
Change-Id: I904d78268d8b8574c799ae1b0f5887e518fdb5f6
(cherry picked from commit 1e9ed84322fddd9228df1f96bc8cb76748e0a4fe)
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/navigation-ui/src/main/res-public/values/public_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_exit_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_exit_anim.xml
ap...@google.com
Project: platform/frameworks/support
Branch: snap-temp-L53700000699921463
commit adb93346bcf4e8b019ec009e499e85536c223415
Author: Jeremy Woods <jbwoods@google.com>
Date: Fri Sep 11 15:58:27 2020
Add nav default animator resources
Animators are recommeneded over Animations for doing effects in fragment
and Navigation. We need to make default animator resources for
Navigation.
Test: Tested in sample apps
Bug: 167430145
Change-Id: I904d78268d8b8574c799ae1b0f5887e518fdb5f6
(cherry picked from commit 1e9ed84322fddd9228df1f96bc8cb76748e0a4fe)
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/navigation-ui/src/main/res-public/values/public_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_exit_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_exit_anim.xml
https://android-review.googlesource.com/1436919
Branch: snap-temp-L53700000699921463
commit adb93346bcf4e8b019ec009e499e85536c223415
Author: Jeremy Woods <jbwoods@google.com>
Date: Fri Sep 11 15:58:27 2020
Add nav default animator resources
Animators are recommeneded over Animations for doing effects in fragment
and Navigation. We need to make default animator resources for
Navigation.
Test: Tested in sample apps
Bug: 167430145
Change-Id: I904d78268d8b8574c799ae1b0f5887e518fdb5f6
(cherry picked from commit 1e9ed84322fddd9228df1f96bc8cb76748e0a4fe)
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/navigation-ui/src/main/res-public/values/public_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_exit_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_exit_anim.xml
ap...@google.com
Project: platform/frameworks/support
Branch: snap-temp-L25200000699921867
commit 868747fcf7ed98039c7a75125a540b348e1b8e9f
Author: Jeremy Woods <jbwoods@google.com>
Date: Fri Sep 11 15:58:27 2020
Add nav default animator resources
Animators are recommeneded over Animations for doing effects in fragment
and Navigation. We need to make default animator resources for
Navigation.
Test: Tested in sample apps
Bug: 167430145
Change-Id: I904d78268d8b8574c799ae1b0f5887e518fdb5f6
(cherry picked from commit 1e9ed84322fddd9228df1f96bc8cb76748e0a4fe)
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/navigation-ui/src/main/res-public/values/public_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_exit_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_exit_anim.xml
https://android-review.googlesource.com/1436923
Branch: snap-temp-L25200000699921867
commit 868747fcf7ed98039c7a75125a540b348e1b8e9f
Author: Jeremy Woods <jbwoods@google.com>
Date: Fri Sep 11 15:58:27 2020
Add nav default animator resources
Animators are recommeneded over Animations for doing effects in fragment
and Navigation. We need to make default animator resources for
Navigation.
Test: Tested in sample apps
Bug: 167430145
Change-Id: I904d78268d8b8574c799ae1b0f5887e518fdb5f6
(cherry picked from commit 1e9ed84322fddd9228df1f96bc8cb76748e0a4fe)
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/navigation-ui/src/main/res-public/values/public_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_exit_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_exit_anim.xml
ap...@google.com
Project: platform/frameworks/support
Branch: snap-temp-L07700000699933785
commit 57ed08fa1034139dbb2bbabd6d4b87299481bd4a
Author: Jeremy Woods <jbwoods@google.com>
Date: Fri Sep 11 15:58:27 2020
Add nav default animator resources
Animators are recommeneded over Animations for doing effects in fragment
and Navigation. We need to make default animator resources for
Navigation.
Test: Tested in sample apps
Bug: 167430145
Change-Id: I904d78268d8b8574c799ae1b0f5887e518fdb5f6
(cherry picked from commit 1e9ed84322fddd9228df1f96bc8cb76748e0a4fe)
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/navigation-ui/src/main/res-public/values/public_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_exit_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_exit_anim.xml
https://android-review.googlesource.com/1436928
Branch: snap-temp-L07700000699933785
commit 57ed08fa1034139dbb2bbabd6d4b87299481bd4a
Author: Jeremy Woods <jbwoods@google.com>
Date: Fri Sep 11 15:58:27 2020
Add nav default animator resources
Animators are recommeneded over Animations for doing effects in fragment
and Navigation. We need to make default animator resources for
Navigation.
Test: Tested in sample apps
Bug: 167430145
Change-Id: I904d78268d8b8574c799ae1b0f5887e518fdb5f6
(cherry picked from commit 1e9ed84322fddd9228df1f96bc8cb76748e0a4fe)
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/NavigationUI.java
M navigation/navigation-ui/src/main/res-public/values/public_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_exit_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_enter_anim.xml
A navigation/navigation-ui/src/main/res/animator/nav_default_pop_exit_anim.xml
Description
The current Navigation default effects are not only
Animations, but they are also inside of sets. This causes issues (likeAnimator,Transition, orAndroidx Transition.Animatorwas built as a follow up toAnimation, specifically to handle its short comings. We should be usingAnimatoras our default effects instead ofAnimation.Because these resources are part of the public API and we don't want to remove or change their directory, we will attempt to have them remain in the same
R.animres location, and just change their underlying implementation.