Fix BottomNavigationMenu item selection via Menu object
- Move the logic of handling which item is checked to MenuBuilder instead of
BottomNavigationMenu itself by making all the MenuItems exclusively checked.
- Fixes MenuBuilder#startDispatchingItemsChanged to call onItemsChanged
with a correct value of structureChanged. It now keeps track if structure
changes while dispatching is prevented.
- Added a button to focus the next bottom navigation item to
BottomNavigationViewUsage
Bug: 32336752
Test: Added a new test to BottomNavigationViewTest
Change-Id: Ief3ff356b501373aaabdd5f4990ecf6f9c506f05
diff --git a/design/src/android/support/design/internal/BottomNavigationItemView.java b/design/src/android/support/design/internal/BottomNavigationItemView.java
index ea22399..5403ba1 100644
--- a/design/src/android/support/design/internal/BottomNavigationItemView.java
+++ b/design/src/android/support/design/internal/BottomNavigationItemView.java
@@ -16,6 +16,8 @@
package android.support.design.internal;
+import static android.support.annotation.RestrictTo.Scope.GROUP_ID;
+
import android.content.Context;
import android.content.res.ColorStateList;
import android.content.res.Resources;
@@ -35,8 +37,6 @@
import android.widget.ImageView;
import android.widget.TextView;
-import static android.support.annotation.RestrictTo.Scope.GROUP_ID;
-
/**
* @hide
*/
@@ -131,8 +131,6 @@
@Override
public void setChecked(boolean checked) {
- mItemData.setChecked(checked);
-
ViewCompat.setPivotX(mLargeLabel, mLargeLabel.getWidth() / 2);
ViewCompat.setPivotY(mLargeLabel, mLargeLabel.getBaseline());
ViewCompat.setPivotX(mSmallLabel, mSmallLabel.getWidth() / 2);
diff --git a/design/src/android/support/design/internal/BottomNavigationMenu.java b/design/src/android/support/design/internal/BottomNavigationMenu.java
index 8832097..9862175 100644
--- a/design/src/android/support/design/internal/BottomNavigationMenu.java
+++ b/design/src/android/support/design/internal/BottomNavigationMenu.java
@@ -16,14 +16,15 @@
package android.support.design.internal;
+import static android.support.annotation.RestrictTo.Scope.GROUP_ID;
+
import android.content.Context;
import android.support.annotation.RestrictTo;
import android.support.v7.view.menu.MenuBuilder;
+import android.support.v7.view.menu.MenuItemImpl;
import android.view.MenuItem;
import android.view.SubMenu;
-import static android.support.annotation.RestrictTo.Scope.GROUP_ID;
-
/**
* @hide
*/
@@ -47,6 +48,12 @@
"Maximum number of items supported by BottomNavigationView is " + MAX_ITEM_COUNT
+ ". Limit can be checked with BottomNavigationView#getMaxItemCount()");
}
- return super.addInternal(group, id, categoryOrder, title);
+ stopDispatchingItemsChanged();
+ final MenuItem item = super.addInternal(group, id, categoryOrder, title);
+ if (item instanceof MenuItemImpl) {
+ ((MenuItemImpl) item).setExclusiveCheckable(true);
+ }
+ startDispatchingItemsChanged();
+ return item;
}
}
diff --git a/design/src/android/support/design/internal/BottomNavigationMenuView.java b/design/src/android/support/design/internal/BottomNavigationMenuView.java
index e6ae08f..ba43b5b 100644
--- a/design/src/android/support/design/internal/BottomNavigationMenuView.java
+++ b/design/src/android/support/design/internal/BottomNavigationMenuView.java
@@ -287,6 +287,9 @@
}
for (int i = 0; i < menuSize; i++) {
mPresenter.setUpdateSuspended(true);
+ if (mMenu.getItem(i).isChecked()) {
+ mActiveButton = i;
+ }
mButtons[i].initialize((MenuItemImpl) mMenu.getItem(i), 0);
mPresenter.setUpdateSuspended(false);
}
@@ -297,10 +300,7 @@
mAnimationHelper.beginDelayedTransition(this);
- mPresenter.setUpdateSuspended(true);
- mButtons[mActiveButton].setChecked(false);
- mButtons[newButton].setChecked(true);
- mPresenter.setUpdateSuspended(false);
+ mMenu.getItem(newButton).setChecked(true);
mActiveButton = newButton;
}
diff --git a/design/tests/src/android/support/design/widget/BottomNavigationViewTest.java b/design/tests/src/android/support/design/widget/BottomNavigationViewTest.java
index b716630..09a9c35 100644
--- a/design/tests/src/android/support/design/widget/BottomNavigationViewTest.java
+++ b/design/tests/src/android/support/design/widget/BottomNavigationViewTest.java
@@ -47,7 +47,6 @@
import android.test.suitebuilder.annotation.SmallTest;
import android.view.Menu;
import android.view.MenuItem;
-import android.view.ViewGroup;
import org.junit.Before;
import org.junit.Test;
@@ -212,4 +211,27 @@
onView(allOf(withId(R.id.icon), isDescendantOfA(withId(R.id.destination_people)))).check(
matches(TestUtilsMatchers.drawable(blueFill, allowedComponentVariance)));
}
+
+ @UiThreadTest
+ @Test
+ @SmallTest
+ public void testItemChecking() throws Throwable {
+ final Menu menu = mBottomNavigation.getMenu();
+ assertTrue(menu.getItem(0).isChecked());
+ checkAndVerifyExclusiveItem(menu, R.id.destination_home);
+ checkAndVerifyExclusiveItem(menu, R.id.destination_profile);
+ checkAndVerifyExclusiveItem(menu, R.id.destination_people);
+ }
+
+ private void checkAndVerifyExclusiveItem(final Menu menu, final int id) throws Throwable {
+ menu.findItem(id).setChecked(true);
+ for (int i = 0; i < menu.size(); i++) {
+ final MenuItem item = menu.getItem(i);
+ if (item.getItemId() == id) {
+ assertTrue(item.isChecked());
+ } else {
+ assertFalse(item.isChecked());
+ }
+ }
+ }
}
diff --git a/samples/SupportDesignDemos/res/layout/design_bottom_navigation_view.xml b/samples/SupportDesignDemos/res/layout/design_bottom_navigation_view.xml
index 3add119..b2a4d59 100644
--- a/samples/SupportDesignDemos/res/layout/design_bottom_navigation_view.xml
+++ b/samples/SupportDesignDemos/res/layout/design_bottom_navigation_view.xml
@@ -52,11 +52,18 @@
android:text="@string/bottomnavigation_tint"
android:layout_below="@+id/button_remove"/>
+ <Button
+ android:id="@+id/button_select_next"
+ android:layout_width="wrap_content"
+ android:layout_height="wrap_content"
+ android:text="@string/bottomnavigation_select_next"
+ android:layout_below="@+id/button_tint"/>
+
<TextView
android:id="@+id/selected_item"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
- android:layout_below="@+id/button_tint"/>
+ android:layout_below="@+id/button_select_next"/>
</RelativeLayout>
</ScrollView>
<android.support.design.widget.BottomNavigationView
diff --git a/samples/SupportDesignDemos/res/values/strings.xml b/samples/SupportDesignDemos/res/values/strings.xml
index 95900ca..c257f1d 100644
--- a/samples/SupportDesignDemos/res/values/strings.xml
+++ b/samples/SupportDesignDemos/res/values/strings.xml
@@ -127,4 +127,5 @@
<string name="bottomnavigation_add">Add an item</string>
<string name="bottomnavigation_remove">Remove an item</string>
<string name="bottomnavigation_tint">Toggle tint</string>
+ <string name="bottomnavigation_select_next">Select next item</string>
</resources>
diff --git a/samples/SupportDesignDemos/src/com/example/android/support/design/widget/BottomNavigationViewUsage.java b/samples/SupportDesignDemos/src/com/example/android/support/design/widget/BottomNavigationViewUsage.java
index 3442218..fa9b0b6 100644
--- a/samples/SupportDesignDemos/src/com/example/android/support/design/widget/BottomNavigationViewUsage.java
+++ b/samples/SupportDesignDemos/src/com/example/android/support/design/widget/BottomNavigationViewUsage.java
@@ -76,6 +76,20 @@
}
}
});
+ Button buttonNext = (Button) findViewById(R.id.button_select_next);
+ buttonNext.setOnClickListener(new View.OnClickListener() {
+ @Override
+ public void onClick(View view) {
+ final int menuSize = bottom.getMenu().size();
+ int currentlySelected = 0;
+ for (int i = 0; i < menuSize; i++) {
+ if (bottom.getMenu().getItem(i).isChecked()) {
+ currentlySelected = i;
+ }
+ }
+ bottom.getMenu().getItem((currentlySelected + 1) % menuSize).setChecked(true);
+ }
+ });
final TextView selectedItem = (TextView) findViewById(R.id.selected_item);
bottom.setOnNavigationItemSelectedListener(
new BottomNavigationView.OnNavigationItemSelectedListener() {
diff --git a/v7/appcompat/src/android/support/v7/view/menu/MenuBuilder.java b/v7/appcompat/src/android/support/v7/view/menu/MenuBuilder.java
index 2ee07d8..cff87c5 100644
--- a/v7/appcompat/src/android/support/v7/view/menu/MenuBuilder.java
+++ b/v7/appcompat/src/android/support/v7/view/menu/MenuBuilder.java
@@ -16,6 +16,8 @@
package android.support.v7.view.menu;
+import static android.support.annotation.RestrictTo.Scope.GROUP_ID;
+
import android.content.ComponentName;
import android.content.Context;
import android.content.Intent;
@@ -47,8 +49,6 @@
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
-import static android.support.annotation.RestrictTo.Scope.GROUP_ID;
-
/**
* Implementation of the {@link android.support.v4.internal.view.SupportMenu} interface for creating a
* standard menu UI.
@@ -165,6 +165,8 @@
private boolean mItemsChangedWhileDispatchPrevented = false;
+ private boolean mStructureChangedWhileDispatchPrevented = false;
+
private boolean mOptionalIconsVisible = false;
private boolean mIsClosing = false;
@@ -582,6 +584,7 @@
clearHeader();
mPreventDispatchingItemsChanged = false;
mItemsChangedWhileDispatchPrevented = false;
+ mStructureChangedWhileDispatchPrevented = false;
onItemsChanged(true);
}
@@ -599,6 +602,7 @@
final int group = item.getGroupId();
final int N = mItems.size();
+ stopDispatchingItemsChanged();
for (int i = 0; i < N; i++) {
MenuItemImpl curItem = mItems.get(i);
if (curItem.getGroupId() == group) {
@@ -609,6 +613,7 @@
curItem.setCheckedInt(curItem == item);
}
}
+ startDispatchingItemsChanged();
}
@Override
@@ -1042,6 +1047,9 @@
dispatchPresenterUpdate(structureChanged);
} else {
mItemsChangedWhileDispatchPrevented = true;
+ if (structureChanged) {
+ mStructureChangedWhileDispatchPrevented = true;
+ }
}
}
@@ -1054,6 +1062,7 @@
if (!mPreventDispatchingItemsChanged) {
mPreventDispatchingItemsChanged = true;
mItemsChangedWhileDispatchPrevented = false;
+ mStructureChangedWhileDispatchPrevented = false;
}
}
@@ -1062,7 +1071,7 @@
if (mItemsChangedWhileDispatchPrevented) {
mItemsChangedWhileDispatchPrevented = false;
- onItemsChanged(true);
+ onItemsChanged(mStructureChangedWhileDispatchPrevented);
}
}
