Fixed
Status Update
Comments
gg...@google.com <gg...@google.com>
il...@google.com <il...@google.com>
yb...@google.com <yb...@google.com>
at...@gmail.com <at...@gmail.com> #2
If you need to add a custom Navigator but don't want to subclass NavHost (which we expect will be needed in most cases), the correct approach is to add it before you set your graph:
1) Don't use app:navGraph in your XML
2) Get a reference to your NavController
3) Add your Navigator to the NavController
4) Call setGraph on the NavController
This relies on the fact that NavController waits until the graph is set before restoring state / doing anything at all.
Technically, the createFragmentNavigator() call in NavHostFragment is already called at the same time as your proposed onNavControllerCreated() method so as a workaround, you could override that method, add your Navigator, then return super.createFragmentNavigator().
1) Don't use app:navGraph in your XML
2) Get a reference to your NavController
3) Add your Navigator to the NavController
4) Call setGraph on the NavController
This relies on the fact that NavController waits until the graph is set before restoring state / doing anything at all.
Technically, the createFragmentNavigator() call in NavHostFragment is already called at the same time as your proposed onNavControllerCreated() method so as a workaround, you could override that method, add your Navigator, then return super.createFragmentNavigator().
ma...@gmail.com <ma...@gmail.com> #3
The first solution does not work, because after screen rotation, the graph (probably a restored graph) is set when Activity.setContentView() is called.
The workaround with createFragmentNavigator() works, though it's not very elegant. Maybe the method could be renamed to createNavigators() and return a list or an array of Navigators?
Anyway, it is a good enough solution for now, so it is not very urgent.
In conclusion:
* I think that a better support for custom destinations should be provided before final release.
* I would suggest describing the "Add support for new destination types" section in the documentation in more details.
Thank you
The workaround with createFragmentNavigator() works, though it's not very elegant. Maybe the method could be renamed to createNavigators() and return a list or an array of Navigators?
Anyway, it is a good enough solution for now, so it is not very urgent.
In conclusion:
* I think that a better support for custom destinations should be provided before final release.
* I would suggest describing the "Add support for new destination types" section in the documentation in more details.
Thank you
at...@gmail.com <at...@gmail.com> #4
I suspect the issue you're running into for the first solution is that you're using the setGraph(R.navigation.graph) method, which does indeed cause it to automatically inflate the graph by id on rotation. Using setGraph(navGraph) where navGraph is a navGraph object you've inflated with navController.getNavInflater().inflate(R.navigation.graph) will never automatically be recreated.
Definitely something we can do better on though :)
Definitely something we can do better on though :)
il...@google.com <il...@google.com> #5
Project: platform/frameworks/support
Branch: androidx-master-dev
commit a2e68921841727237528c4ac7076d50af64663c8
Author: Ian Lake <ilake@google.com>
Date: Wed Oct 17 16:46:28 2018
Make setGraph(int) and setGraph(NavGraph) act identically
Remove the auto restore functionality from NavController's
setGraph(int) method so that it functions identically to
setGraph(NavGraph).
This ensures that developers that manually are calling
setGraph() after adding custom navigators don't get
recreated too early.
Test: updated NavController test
BUG: 110763345
Change-Id: I51ae7c74451669d6bfed0fe84df445c2ff272e6e
M navigation/runtime/src/androidTest/java/androidx/navigation/NavControllerTest.kt
M navigation/runtime/src/main/java/androidx/navigation/NavController.java
https://android-review.googlesource.com/792819
https://goto.google.com/android-sha1/a2e68921841727237528c4ac7076d50af64663c8
Branch: androidx-master-dev
commit a2e68921841727237528c4ac7076d50af64663c8
Author: Ian Lake <ilake@google.com>
Date: Wed Oct 17 16:46:28 2018
Make setGraph(int) and setGraph(NavGraph) act identically
Remove the auto restore functionality from NavController's
setGraph(int) method so that it functions identically to
setGraph(NavGraph).
This ensures that developers that manually are calling
setGraph() after adding custom navigators don't get
recreated too early.
Test: updated NavController test
BUG: 110763345
Change-Id: I51ae7c74451669d6bfed0fe84df445c2ff272e6e
M navigation/runtime/src/androidTest/java/androidx/navigation/NavControllerTest.kt
M navigation/runtime/src/main/java/androidx/navigation/NavController.java
ma...@gmail.com <ma...@gmail.com> #6
We've decided to remove the automatic recreation that was unique to setGraph(int) - it now acts identically to setGraph(NavGraph), so the flow in #2 can be used to ensure that you have custom Navigators added before calling setGraph.
This fix will be available in 1.0.0-alpha07
This fix will be available in 1.0.0-alpha07
il...@google.com <il...@google.com>
il...@google.com <il...@google.com> #7
Project: platform/frameworks/support
Branch: androidx-master-dev
commit f40bad0faef05bded2d226711338e489309885ba
Author: Ian Lake <ilake@google.com>
Date: Tue Oct 23 21:03:24 2018
Ensure NavHostFragment calls setGraph on restore
If a graph is set on NavHostFragment via
NavHostFragment.create() or through
app:navGraph, it should be restored immediately
after the call to restoreState.
Test: testapp now works
BUG: 110763345
Change-Id: I7a0c656480e27a66637667519eabe61820b09a6c
M navigation/fragment/src/main/java/androidx/navigation/fragment/NavHostFragment.java
https://android-review.googlesource.com/799721
https://goto.google.com/android-sha1/f40bad0faef05bded2d226711338e489309885ba
Branch: androidx-master-dev
commit f40bad0faef05bded2d226711338e489309885ba
Author: Ian Lake <ilake@google.com>
Date: Tue Oct 23 21:03:24 2018
Ensure NavHostFragment calls setGraph on restore
If a graph is set on NavHostFragment via
NavHostFragment.create() or through
app:navGraph, it should be restored immediately
after the call to restoreState.
Test: testapp now works
BUG: 110763345
Change-Id: I7a0c656480e27a66637667519eabe61820b09a6c
M navigation/fragment/src/main/java/androidx/navigation/fragment/NavHostFragment.java
ap...@google.com <ap...@google.com> #8
Project: platform/frameworks/support
Branch: androidx-master-dev
commit 43e060b8c23a7163673f77ee42e5a1e3cb1f7c14
Author: Ian Lake <ilake@google.com>
Date: Fri Aug 31 14:11:51 2018
Close bottom sheets when tapping a NavigationView
When a NavigationView is contained within a bottom
sheet (such as is the case for the common
BottomAppBar use case), tapping on a menu item that
navigates to a destination will also hide the
bottom sheet.
Test: manual testing
Change-Id: Ia6cf92288988fe160ffd1136de417066b33377f2
Fixes: 112158843
M navigation/ui/src/main/java/androidx/navigation/ui/NavigationUI.java
https://android-review.googlesource.com/743749
https://goto.google.com/android-sha1/43e060b8c23a7163673f77ee42e5a1e3cb1f7c14
Branch: androidx-master-dev
commit 43e060b8c23a7163673f77ee42e5a1e3cb1f7c14
Author: Ian Lake <ilake@google.com>
Date: Fri Aug 31 14:11:51 2018
Close bottom sheets when tapping a NavigationView
When a NavigationView is contained within a bottom
sheet (such as is the case for the common
BottomAppBar use case), tapping on a menu item that
navigates to a destination will also hide the
bottom sheet.
Test: manual testing
Change-Id: Ia6cf92288988fe160ffd1136de417066b33377f2
Fixes: 112158843
M navigation/ui/src/main/java/androidx/navigation/ui/NavigationUI.java
ma...@gmail.com <ma...@gmail.com> #9
Nicely done, works just as expected. Thank you!
at...@gmail.com <at...@gmail.com> #10
Is it possible to see when I can use this code officially?
il...@google.com <il...@google.com> #11
It'll be part of the upcoming 1.0.0-alpha06 release.
at...@gmail.com <at...@gmail.com> #12
Can you show me your CustomBottomSheetDialofFragment and the associated layout please. I get the error message "The view is not a child of CoordinatorLayout".
at...@gmail.com <at...@gmail.com> #13
This is what would work if you use a simple bottom sheet layout with a ConstraintLayout as root view as pointed out in comment #3 :
if (parent instanceof DrawerLayout) {
((DrawerLayout) parent).closeDrawer(navigationView);
} else {
BottomSheetBehavior bottomSheetBehavior =
BottomSheetBehavior.from((View) navigationView.getParent().getParent());
if (bottomSheetBehavior != null) {
bottomSheetBehavior.setState(BottomSheetBehavior.STATE_HIDDEN);
}
}
but only
BottomSheetBehavior bottomSheetBehavior =
BottomSheetBehavior.from(navigationView);
would expect, that the navigationView is a direct child of a CoordinatorLayout and would have the app:layout_behavior="com.google.android.material.bottomsheet.BottomSheetBehavior" attribute. That can't work or did I miss something?
if (parent instanceof DrawerLayout) {
((DrawerLayout) parent).closeDrawer(navigationView);
} else {
BottomSheetBehavior bottomSheetBehavior =
BottomSheetBehavior.from((View) navigationView.getParent().getParent());
if (bottomSheetBehavior != null) {
bottomSheetBehavior.setState(BottomSheetBehavior.STATE_HIDDEN);
}
}
but only
BottomSheetBehavior bottomSheetBehavior =
BottomSheetBehavior.from(navigationView);
would expect, that the navigationView is a direct child of a CoordinatorLayout and would have the app:layout_behavior="com.google.android.material.bottomsheet.BottomSheetBehavior" attribute. That can't work or did I miss something?
il...@google.com <il...@google.com> #14
Yeah, we should avoid BottomSheetBehavior.from()'s overzealous throwing of exceptions and handle cases like BottomSheetDialogFragment where the NavigationView is a few layers below the CoordinatorLayout.
ap...@google.com <ap...@google.com> #15
Project: platform/frameworks/support
Branch: androidx-master-dev
commit 4fb1acaf8ee002979bc161e362c1e83f89195205
Author: Ian Lake <ilake@google.com>
Date: Tue Sep 18 13:38:07 2018
Fix NavigationUI for use with BottomSheetDialogFragment
Replace using BottomSheetBehavior.from() which
throws IllegalArgumentExceptions to a safe
method that searches up the view hierarchy
for a CoordinatorLayout to determine if the
NavigationView is within a bottom sheet.
Test: example added to the integration-tests/testapp
BUG: 112158843
Change-Id: I774991ef1b23085e075e6bac6fa820ee3e001a31
M navigation/integration-tests/testapp/build.gradle
M navigation/integration-tests/testapp/src/main/java/androidx/navigation/testapp/HelpActivity.kt
M navigation/integration-tests/testapp/src/main/res/layout/activity_help.xml
A navigation/integration-tests/testapp/src/main/res/layout/bottom_bar_menu.xml
M navigation/ui/src/main/java/androidx/navigation/ui/NavigationUI.java
https://android-review.googlesource.com/760505
https://goto.google.com/android-sha1/4fb1acaf8ee002979bc161e362c1e83f89195205
Branch: androidx-master-dev
commit 4fb1acaf8ee002979bc161e362c1e83f89195205
Author: Ian Lake <ilake@google.com>
Date: Tue Sep 18 13:38:07 2018
Fix NavigationUI for use with BottomSheetDialogFragment
Replace using BottomSheetBehavior.from() which
throws IllegalArgumentExceptions to a safe
method that searches up the view hierarchy
for a CoordinatorLayout to determine if the
NavigationView is within a bottom sheet.
Test: example added to the integration-tests/testapp
BUG: 112158843
Change-Id: I774991ef1b23085e075e6bac6fa820ee3e001a31
M navigation/integration-tests/testapp/build.gradle
M navigation/integration-tests/testapp/src/main/java/androidx/navigation/testapp/HelpActivity.kt
M navigation/integration-tests/testapp/src/main/res/layout/activity_help.xml
A navigation/integration-tests/testapp/src/main/res/layout/bottom_bar_menu.xml
M navigation/ui/src/main/java/androidx/navigation/ui/NavigationUI.java
il...@google.com <il...@google.com> #16
Thanks for catching the oversight! The new code should work considerably better for other bottom sheet cases without the IllegalArgumentExceptions :)
Description
The Navigation Component has this very useful function to sync up menu items from a NavigationView with the NavController. It makes sure that the selected item is checked, can close the navigation drawer on selection if a DrawerLayout is used, or set the title of an ActionBar/Toolbar according to the current destination.
With the addition of the bottom app bar in Material Design we've also begun using the bottom navigation drawer (
public class BottomDrawerFragment extends BottomSheetDialogFragment {
@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
View view = inflater.inflate(R.layout.fragment_bottom_drawer, container, false);
NavigationView navigationView = view.findViewById(R.id.bottom_drawer_navigation);
NavigationUI.setupWithNavController(navigationView, Navigation.findNavController(getActivity(), R.id.nav_host_fragment));
return view;
}
}
This works well, but because NavigationUI currently doesn't dismiss the BottomSheetDialogFragment when a menu item has been selected like it closes the DrawerLayout, the bottom navigation drawer remains open and needs to be dismissed in another way (which comes with additional code overhead and issues).
Since AndroidX is now part of AOSP, I took a look at the implementation and I think that support for this could be added with two lines of code (see '+' characters at beginning of lines below) by doing for the DialogFragment what is already done for the DrawerLayout (excerpt from androidx-master-dev/navigation/ui/src/main/java/androidx/navigation/ui/NavigationUI.java).
public static void setupWithNavController(@NonNull final NavigationView navigationView,
@NonNull final NavController navController) {
navigationView.setNavigationItemSelectedListener(
new NavigationView.OnNavigationItemSelectedListener() {
@Override
public boolean onNavigationItemSelected(@NonNull MenuItem item) {
boolean handled = onNavDestinationSelected(item, navController, true);
if (handled) {
ViewParent parent = navigationView.getParent();
if (parent instanceof DrawerLayout) {
((DrawerLayout) parent).closeDrawer(navigationView);
+ } else if (parent instanceof BottomSheetDialogFragment) {
+ ((DialogFragment) parent).dismiss();
}
}
return handled;
}
});
navController.addOnNavigatedListener(new NavController.OnNavigatedListener() {
@Override
public void onNavigated(@NonNull NavController controller,
@NonNull NavDestination destination) {
Menu menu = navigationView.getMenu();
for (int h = 0, size = menu.size(); h < size; h++) {
MenuItem item = menu.getItem(h);
item.setChecked(matchDestination(destination, item.getItemId()));
}
}
});
}
The most active owner of the Navigation Component is Ian Lake and it he's been adding support for more and more UI components to the NavigationUI class in the past months, so I think he's the person to talk to about this. Assuming this doesn't introduce any issues, he could probably get it done in a couple of minutes.
Alternatively, I could implement this and the accompanying test myself, contribute it and try to get it reviewed.