From 3fcf1696e78359216c9907f2920a874108bf348a Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Wed, 31 Jan 2024 09:20:40 +0800 Subject: [PATCH] Correct handling of tab addition/removal. --- .../toga_android/widgets/optioncontainer.py | 75 +++++++++++-------- .../tests_backend/widgets/optioncontainer.py | 19 ++++- testbed/pyproject.toml | 1 + testbed/tests/widgets/test_optioncontainer.py | 29 ++++++- 4 files changed, 88 insertions(+), 36 deletions(-) diff --git a/android/src/toga_android/widgets/optioncontainer.py b/android/src/toga_android/widgets/optioncontainer.py index 6c37b6c590..8f2268c98d 100644 --- a/android/src/toga_android/widgets/optioncontainer.py +++ b/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") diff --git a/android/tests_backend/widgets/optioncontainer.py b/android/tests_backend/widgets/optioncontainer.py index 259e906194..a33864a14d 100644 --- a/android/tests_backend/widgets/optioncontainer.py +++ b/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 diff --git a/testbed/pyproject.toml b/testbed/pyproject.toml index 35a8e97b31..734ecad203 100644 --- a/testbed/pyproject.toml +++ b/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", ] diff --git a/testbed/tests/widgets/test_optioncontainer.py b/testbed/tests/widgets/test_optioncontainer.py index 7add331646..f3d50cb7c5 100644 --- a/testbed/tests/widgets/test_optioncontainer.py +++ b/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"""