Fixed
Status Update
Comments
[Deleted User] <[Deleted User]> #2
How should this work "else if (parent instanceof BottomSheetDialogFragment)"?
il...@google.com <il...@google.com>
[Deleted User] <[Deleted User]> #3
Yeah, I just realized that it's not as simple as in the case of the DrawerLayout. When using a DrawerLayout, people mostly have something like this:
<android.support.v4.widget.DrawerLayout xmlns:android="http://schemas.android.com/apk/res/android "
xmlns:app="http://schemas.android.com/apk/res-auto "
android:id="@+id/drawer_layout"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:fitsSystemWindows="true">
<FrameLayout
android:layout_width="match_parent"
android:layout_height="match_parent"
android:id="@+id/header_container"/>
<android.support.design.widget.NavigationView
android:id="@+id/nav_view"
android:layout_width="wrap_content"
android:layout_height="match_parent"
android:layout_gravity="start"
app:headerLayout="@layout/main_nav_header"
android:fitsSystemWindows="true"
app:menu="@menu/main_drawercontent"/>
</android.support.v4.widget.DrawerLayout>
Obviously here the NavigationView is a direct child of the DrawerLayout, which is why the implementation we already have for handling the closing of the navigation drawer works like it does, by checking the NavigationView's parent.
But when using a BottomSheetDialogFragment, we can't rely on the NavigationView being a direct child of the BottomSheetDialogFragment. Since it's mostly a regular Fragment, people may use a layout like this (simplified):
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android "
xmlns:app="http://schemas.android.com/apk/res-auto "
xmlns:tools="http://schemas.android.com/tools "
android:layout_width="match_parent"
android:layout_height="match_parent"
tools:context=".view.BottomDrawerFragment">
<!-- Here we'd have views typically found in the navigation drawer,
such as a user profile picture or their name -->
<com.google.android.material.navigation.NavigationView
android:id="@+id/bottom_drawer_navigation"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_gravity="bottom"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/divider"
app:menu="@menu/bottom_drawer_menu" />
</androidx.constraintlayout.widget.ConstraintLayout>
My suggestion wouldn't work with this, as the NavigationView's parent is not the BottomSheetDialogFragment, but the ConstraintLayout. So we can't use the parent to detect when we should dismiss a BottomSheetDialogFragment.
A potential solution would probably have to use a different approach. Maybe in setupWithNavController() you could pass a reference to the BottomSheetDialogFragment directly, in addition to the NavigationView and NavController. Something similar happens already, for example when using a Toolbar that should be kept in sync also, the method call is NavigationUI.setupWithNavController(collapsingToolbarLayout, toolbar, navController) for that.
So we could end up having an implementation of setupWithNavController just for our case with the BottomSheetDialogFragment that goes something like this:
public static void setupWithNavController(
@NonNull BottomSheetDialogFragment bottomSheetDialogFragment,
@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) {
bottomSheetDialogFragment.dismiss();
}
return handled;
}
});
final WeakReference<NavigationView> weakReference = new WeakReference<>(navigationView);
navController.addOnNavigatedListener(new NavController.OnNavigatedListener() {
@Override
public void onNavigated(@NonNull NavController controller,
@NonNull NavDestination destination) {
NavigationView view = weakReference.get();
if (view == null) {
controller.removeOnNavigatedListener(this);
return;
}
Menu menu = view.getMenu();
for (int h = 0, size = menu.size(); h < size; h++) {
MenuItem item = menu.getItem(h);
item.setChecked(matchDestination(destination, item.getItemId()));
}
}
});
}
And we'd use it in our example like so:
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(this, navigationView, Navigation.findNavController(getActivity(), R.id.nav_host_fragment));
return view;
}
}
<android.support.v4.widget.DrawerLayout xmlns:android="
xmlns:app="
android:id="@+id/drawer_layout"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:fitsSystemWindows="true">
<FrameLayout
android:layout_width="match_parent"
android:layout_height="match_parent"
android:id="@+id/header_container"/>
<android.support.design.widget.NavigationView
android:id="@+id/nav_view"
android:layout_width="wrap_content"
android:layout_height="match_parent"
android:layout_gravity="start"
app:headerLayout="@layout/main_nav_header"
android:fitsSystemWindows="true"
app:menu="@menu/main_drawercontent"/>
</android.support.v4.widget.DrawerLayout>
Obviously here the NavigationView is a direct child of the DrawerLayout, which is why the implementation we already have for handling the closing of the navigation drawer works like it does, by checking the NavigationView's parent.
But when using a BottomSheetDialogFragment, we can't rely on the NavigationView being a direct child of the BottomSheetDialogFragment. Since it's mostly a regular Fragment, people may use a layout like this (simplified):
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="
xmlns:app="
xmlns:tools="
android:layout_width="match_parent"
android:layout_height="match_parent"
tools:context=".view.BottomDrawerFragment">
<!-- Here we'd have views typically found in the navigation drawer,
such as a user profile picture or their name -->
<com.google.android.material.navigation.NavigationView
android:id="@+id/bottom_drawer_navigation"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_gravity="bottom"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/divider"
app:menu="@menu/bottom_drawer_menu" />
</androidx.constraintlayout.widget.ConstraintLayout>
My suggestion wouldn't work with this, as the NavigationView's parent is not the BottomSheetDialogFragment, but the ConstraintLayout. So we can't use the parent to detect when we should dismiss a BottomSheetDialogFragment.
A potential solution would probably have to use a different approach. Maybe in setupWithNavController() you could pass a reference to the BottomSheetDialogFragment directly, in addition to the NavigationView and NavController. Something similar happens already, for example when using a Toolbar that should be kept in sync also, the method call is NavigationUI.setupWithNavController(collapsingToolbarLayout, toolbar, navController) for that.
So we could end up having an implementation of setupWithNavController just for our case with the BottomSheetDialogFragment that goes something like this:
public static void setupWithNavController(
@NonNull BottomSheetDialogFragment bottomSheetDialogFragment,
@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) {
bottomSheetDialogFragment.dismiss();
}
return handled;
}
});
final WeakReference<NavigationView> weakReference = new WeakReference<>(navigationView);
navController.addOnNavigatedListener(new NavController.OnNavigatedListener() {
@Override
public void onNavigated(@NonNull NavController controller,
@NonNull NavDestination destination) {
NavigationView view = weakReference.get();
if (view == null) {
controller.removeOnNavigatedListener(this);
return;
}
Menu menu = view.getMenu();
for (int h = 0, size = menu.size(); h < size; h++) {
MenuItem item = menu.getItem(h);
item.setChecked(matchDestination(destination, item.getItemId()));
}
}
});
}
And we'd use it in our example like so:
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(this, navigationView, Navigation.findNavController(getActivity(), R.id.nav_host_fragment));
return view;
}
}
il...@google.com <il...@google.com> #4
You nailed it man. Thanks for your investigation!
[Deleted User] <[Deleted User]> #5
Are there any cases where you'd have a NavigationView in a BottomSheet of any type and not expect the bottom sheet to be hidden when you tap on a NavigationView item?
If you'd always expect it to be hidden, we could use BottomSheetBehavior.from(navigationView) to find the bottom sheet, avoiding the need to create another setup method.
If you'd always expect it to be hidden, we could use BottomSheetBehavior.from(navigationView) to find the bottom sheet, avoiding the need to create another setup method.
mi...@gmail.com <mi...@gmail.com> #6
Not that I can think of. According to the Material Design spec, there are standard bottom sheets which remain open while the user interacts with the main UI region or the sheet itself, and modal bottom sheets which must be dismissed to interact with the underlying content.
Examples for standard bottom sheets are playback controls inside a music player or the location information in Google Maps, while modal bottom sheets provide a set of choices and are an alternative to inline menus.
This makes it pretty clear that modal bottom sheets should be used for any kind of menu, and modal bottom sheets are always dismissed by tapping a menu item or action. That being said, it's not unlikely that someone is going to use a NavigationView inside a standard bottom sheet at some point. While the spec doesn't explicitly prohibit that, it's clearly not the intention, so I wouldn't consider this case in the implementation.
With that in mind, I think using BottomSheetBehavior.from(navigationView) is a good solution.
Examples for standard bottom sheets are playback controls inside a music player or the location information in Google Maps, while modal bottom sheets provide a set of choices and are an alternative to inline menus.
This makes it pretty clear that modal bottom sheets should be used for any kind of menu, and modal bottom sheets are always dismissed by tapping a menu item or action. That being said, it's not unlikely that someone is going to use a NavigationView inside a standard bottom sheet at some point. While the spec doesn't explicitly prohibit that, it's clearly not the intention, so I wouldn't consider this case in the implementation.
With that in mind, I think using BottomSheetBehavior.from(navigationView) is a good solution.
il...@google.com <il...@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
mi...@gmail.com <mi...@gmail.com> #9
Nicely done, works just as expected. Thank you!
il...@google.com <il...@google.com> #10
Is it possible to see when I can use this code officially?
mi...@gmail.com <mi...@gmail.com> #11
It'll be part of the upcoming 1.0.0-alpha06 release.
dc...@gmail.com <dc...@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".
da...@gmail.com <da...@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.
Description
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:
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