Correct handling of tab addition/removal.

android/src/toga_android/widgets/optioncontainer.py

@@ -9,7 +9,9 @@
 try:
     from com.google.android.material.bottomnavigation import BottomNavigationView
     from com.google.android.material.navigation import NavigationBarView
-except ImportError:
+except ImportError:  # pragma: no cover
+    # If you've got an older project that doesn't include the Material library,
+    # this import will fail. We can't validate that in CI, so it's marked no cover
     BottomNavigationView = None
     NavigationBarView = None
 
@@ -43,10 +45,11 @@ def __init__(self, impl):
         def onNavigationItemSelected(self, item):
             for index, option in enumerate(self.impl.options):
                 if option.menu_item == item:
-                    self.impl.select_option(index)
+                    self.impl.set_current_tab_index(index, programmatic=False)
                     return True
 
-            return False
+            # You shouldn't be able to select an item that isn't isn't selectable.
+            return False  # pragma: no cover
 
 
 class OptionContainer(Widget, Container):
@@ -89,8 +92,9 @@ def create(self):
             ),
         )
 
+        self.onItemSelectedListener = TogaOnItemSelectedListener(self)
         self.native_navigationview.setOnItemSelectedListener(
-            TogaOnItemSelectedListener(self)
+            self.onItemSelectedListener
         )
 
         self.options = []
@@ -102,16 +106,19 @@ def set_bounds(self, x, y, width, height):
             lp.width, lp.height - self.native_navigationview.getHeight()
         )
 
-    def select_option(self, index):
-        option = self.options[index]
-        self.set_content(option.widget)
-        option.widget.interface.refresh()
+    def purge_options(self):
+        for option in self.options:
+            option.menu_item = None
+        self.native_navigationview.getMenu().clear()
 
-    def _populate_menu_item(self, index, option):
-        option.menu_item = self.native_navigationview.getMenu().add(
-            0, 0, index, option.text
-        )
-        self.set_option_icon(index, option.icon)
+    def rebuld_options(self):
+        for index, option in enumerate(self.options):
+            if index < self.max_items:
+                option.menu_item = self.native_navigationview.getMenu().add(
+                    0, 0, index, option.text
+                )
+                self.set_option_icon(index, option.icon)
+                self.set_option_enabled(index, option.enabled)
 
     def add_option(self, index, text, widget, icon=None):
         # Store the details of the new option
@@ -137,25 +144,24 @@ def add_option(self, index, text, widget, icon=None):
                 )
                 last_option.menu_item = None
 
-            self._populate_menu_item(index, option)
+        # Android doesn't let you change the order index of an item after it has been
+        # created, which means there's no way to insert an item into an existing
+        # ordering. As a workaround, rebuild the entire navigation menu on every
+        # insertion.
+        self.purge_options()
+        self.rebuld_options()
 
         # If this is the only option, make sure the content is selected
         if len(self.options) == 1:
-            self.select_option(0)
+            self.set_current_tab_index(0)
 
     def remove_option(self, index):
-        option = self.options[index]
-        if option.menu_item:
-            self.native_navigationview.getMenu().removeItem(
-                option.menu_item.getItemId()
-            )
-
+        # Android doesn't let you change the order index of an item after it has been
+        # created, which means there's no way to insert an item into an existing
+        # ordering. If an item is deleted, rebuild the entire navigation menu.
+        self.purge_options()
         del self.options[index]
-        if len(self.options) >= self.max_items:
-            self._populate_menu_item(
-                self.max_items - 1,
-                self.options[self.max_items - 1],
-            )
+        self.rebuld_options()
 
     def set_option_enabled(self, index, enabled):
         option = self.options[index]
@@ -201,12 +207,17 @@ def get_current_tab_index(self):
         for index, option in enumerate(self.options):
             if option.menu_item.isChecked():
                 return index
-        return None
-
-    def set_current_tab_index(self, current_tab_index):
-        if current_tab_index < self.max_items:
-            option = self.options[current_tab_index]
-            option.menu_item.setChecked(True)
+        # One of the tabs has to be selected
+        return None  # pragma: no cover
+
+    def set_current_tab_index(self, index, programmatic=True):
+        if index < self.max_items:
+            option = self.options[index]
+            self.set_content(option.widget)
+            option.widget.interface.refresh()
+            if programmatic:
+                option.menu_item.setChecked(True)
+            self.interface.on_select()
         else:
             warnings.warn("Tab is outside selectable range")
 

android/tests_backend/widgets/optioncontainer.py

@@ -15,15 +15,30 @@ def __init__(self, widget):
         assert isinstance(self.native_navigationview, BottomNavigationView)
 
     def select_tab(self, index):
-        self.native_navigationview.getMenu().getItem(index).setChecked(True)
+        item = self.native_navigationview.getMenu().getItem(index)
+        # Android will let you programmatically select a disabled tab.
+        if item.isEnabled():
+            item.setChecked(True)
+            self.impl.onItemSelectedListener(item)
 
     def tab_enabled(self, index):
         return self.native_navigationview.getMenu().getItem(index).isEnabled()
 
     def assert_tab_icon(self, index, expected):
-        actual = self.widget.content[index].icon
+        actual = self.impl.options[index].icon
         if expected is None:
             assert actual is None
         else:
             assert actual.path.name == expected
             assert actual._impl.path.name == f"{expected}-android.png"
+
+    def assert_tab_content(self, index, title, enabled):
+        # Get the actual menu items, and sort them by their order index.
+        # This *should* match the actual option order.
+        menu_items = sorted(
+            [option.menu_item for option in self.impl.options if option.menu_item],
+            key=lambda m: m.getOrder(),
+        )
+
+        assert menu_items[index].getTitle() == title
+        assert menu_items[index].isEnabled() == enabled

testbed/pyproject.toml

@@ -97,6 +97,7 @@ test_requires = [
 base_theme = "Theme.MaterialComponents.Light.DarkActionBar"
 
 build_gradle_dependencies = [
+    "androidx.appcompat:appcompat:1.6.1",
     "com.google.android.material:material:1.11.0",
     "androidx.swiperefreshlayout:swiperefreshlayout:1.1.0",
 ]

testbed/tests/widgets/test_optioncontainer.py

@@ -265,11 +265,12 @@ async def test_select_tab_overflow(widget, probe, on_select_handler):
             extra_probes.pop(0)
             widget.content.append("Tab A", extra)
 
+        with pytest.warns(match=r"Additional item will be ignored"):
             extra = extra_widgets.pop(0)
             extra_probes.pop(0)
             widget.content.append("Tab B", extra)
 
-        await probe.redraw("Appended item was ignored")
+        await probe.redraw("Appended items were ignored")
 
         # Excess tab details can still be read and written
         widget.content[probe.max_tabs].text = "Extra Tab"
@@ -284,11 +285,12 @@ async def test_select_tab_overflow(widget, probe, on_select_handler):
         probe.assert_tab_icon(probe.max_tabs + 1, None)
         assert widget.content[probe.max_tabs + 1].enabled
 
-        # Programamtically selecting a non-visible tab raises a warning, doesn't change
+        # Programmatically selecting a non-visible tab raises a warning, doesn't change
         # the tab, and doesn't generate a selection event.
         with pytest.warns(match=r"Tab is outside selectable range"):
             widget.current_tab = probe.max_tabs + 1
 
+        await probe.redraw("Item selection was ignored")
         on_select_handler.assert_not_called()
 
         # Insert a tab at the start. This will bump the last tab into the void
@@ -299,10 +301,19 @@ async def test_select_tab_overflow(widget, probe, on_select_handler):
 
         await probe.redraw("Inserted item bumped the last item")
 
+        # Assert the properties of the last visible item
         assert widget.content[probe.max_tabs - 1].text == f"Tab {probe.max_tabs - 1}"
         probe.assert_tab_icon(probe.max_tabs - 1, None)
         assert widget.content[probe.max_tabs - 1].enabled
 
+        # As the item is visible, also verify the actual widget properties
+        probe.assert_tab_content(
+            probe.max_tabs - 1,
+            f"Tab {probe.max_tabs - 1}",
+            enabled=True,
+        )
+
+        # Assert the properties of the first invisible item
         assert widget.content[probe.max_tabs].text == f"Tab {probe.max_tabs}"
         probe.assert_tab_icon(probe.max_tabs, None)
         assert widget.content[probe.max_tabs].enabled
@@ -317,6 +328,13 @@ async def test_select_tab_overflow(widget, probe, on_select_handler):
         probe.assert_tab_icon(probe.max_tabs - 1, None)
         assert widget.content[probe.max_tabs - 1].enabled
 
+        # As the item is visible, also verify the actual widget properties
+        probe.assert_tab_content(
+            probe.max_tabs - 1,
+            f"Tab {probe.max_tabs}",
+            enabled=True,
+        )
+
         # Remove another visible tab. This will make the first "extra" tab
         # come into view for the first time. It has a custom icon, and
         # was disabled while it wasn't visible.
@@ -328,6 +346,13 @@ async def test_select_tab_overflow(widget, probe, on_select_handler):
         probe.assert_tab_icon(probe.max_tabs - 1, "new-tab")
         assert not widget.content[probe.max_tabs - 1].enabled
 
+        # As the item is visible, also verify the actual widget properties
+        probe.assert_tab_content(
+            probe.max_tabs - 1,
+            "Extra Tab",
+            enabled=False,
+        )
+
 
 async def test_enable_tab(widget, probe, on_select_handler):
     """Tabs of content can be enabled and disabled"""