Merge pull request #2421 from freakboy3742/gtk-menu-sections

Correct handling of menu sections in GTK

android/tests_backend/app.py

@@ -78,6 +78,20 @@ def activate_menu_visit_homepage(self):
     def assert_menu_item(self, path, *, enabled=True):
         assert self._menu_item(path).isEnabled() == enabled
 
+    def assert_menu_order(self, path, expected):
+        item = self._menu_item(path)
+        menu = item.getSubMenu()
+
+        # Android doesn't include separators, so we need to exclude separators from the
+        # length check, and add an offset when a separator is expected.
+        separator_offset = 0
+        assert menu.size() == len([item for item in expected if item != "---"])
+        for i, title in enumerate(expected):
+            if title == "---":
+                separator_offset += 1
+            else:
+                assert menu.getItem(i - separator_offset).getTitle() == title
+
     def assert_system_menus(self):
         self.assert_menu_item(["About Toga Testbed"])
 

changes/2418.bugfix.rst

@@ -0,0 +1 @@
+The placement of menu items relative to submenus was corrected on GTK.

cocoa/tests_backend/app.py

@@ -147,6 +147,16 @@ def assert_menu_item(self, path, enabled):
         item = self._menu_item(path)
         assert item.isEnabled() == enabled
 
+    def assert_menu_order(self, path, expected):
+        menu = self._menu_item(path).submenu
+
+        assert menu.numberOfItems == len(expected)
+        for item, title in zip(menu.itemArray, expected):
+            if title == "---":
+                assert item.isSeparatorItem
+            else:
+                assert item.title == title
+
     def keystroke(self, combination):
         key, modifiers = cocoa_key(combination)
         key_code = {

gtk/src/toga_gtk/app.py

@@ -126,24 +126,28 @@ def create_app_commands(self):
 
     def _submenu(self, group, menubar):
         try:
-            return self._menu_groups[group], False
+            submenu, section = self._menu_groups[group]
         except KeyError:
+            # It's a new menu/group, so it must start a new section.
+            section = Gio.Menu()
             if group is None:
+                # Menu is a top-level menu; so it's a child of the menu bar
                 submenu = menubar
             else:
-                parent_menu, _ = self._submenu(group.parent, menubar)
+                _, parent_section = self._submenu(group.parent, menubar)
                 submenu = Gio.Menu()
-                self._menu_groups[group] = submenu
 
                 text = group.text
                 if text == "*":
                     text = self.interface.formal_name
-                parent_menu.append_submenu(text, submenu)
+                parent_section.append_submenu(text, submenu)
 
-            # Install the item in the group cache.
-            self._menu_groups[group] = submenu
+            # Add the initial section to the submenu,
+            # and install the menu item in the group cache.
+            submenu.append_section(None, section)
+            self._menu_groups[group] = submenu, section
 
-            return submenu, True
+        return submenu, section
 
     def create_menus(self):
         # Only create the menu if the menu item index has been created.
@@ -152,19 +156,13 @@ def create_menus(self):
 
         # Create the menu for the top level menubar.
         menubar = Gio.Menu()
-        section = None
         for cmd in self.interface.commands:
+            submenu, section = self._submenu(cmd.group, menubar)
             if isinstance(cmd, Separator):
-                section = None
+                section = Gio.Menu()
+                submenu.append_section(None, section)
+                self._menu_groups[cmd.group] = (submenu, section)
             else:
-                submenu, created = self._submenu(cmd.group, menubar)
-                if created:
-                    section = None
-
-                if section is None:
-                    section = Gio.Menu()
-                    submenu.append_section(None, section)
-
                 cmd_id = "command-%s" % id(cmd)
                 action = Gio.SimpleAction.new(cmd_id, None)
                 action.connect("activate", cmd._impl.gtk_activate)

gtk/tests_backend/app.py

@@ -79,14 +79,18 @@ def _menu_item(self, path):
         except AttributeError:
             raise AssertionError(f"Menu {' > '.join(orig_path)} not found")
 
-        action_name = item[0].get_item_attribute_value(item[1], "action").get_string()
-        cmd_id = action_name.split(".")[1]
-        action = self.app._impl.native.lookup_action(cmd_id)
-        return action
+        action = item[0].get_item_attribute_value(item[1], "action")
+        if action:
+            action_name = (
+                item[0].get_item_attribute_value(item[1], "action").get_string()
+            )
+            cmd_id = action_name.split(".")[1]
+            action = self.app._impl.native.lookup_action(cmd_id)
+        return item, action
 
     def _activate_menu_item(self, path):
-        item = self._menu_item(path)
-        item.emit("activate", None)
+        _, action = self._menu_item(path)
+        action.emit("activate", None)
 
     def activate_menu_exit(self):
         self._activate_menu_item(["*", "Quit Toga Testbed"])
@@ -117,8 +121,33 @@ def activate_menu_minimize(self):
         pytest.xfail("GTK doesn't have a window management menu items")
 
     def assert_menu_item(self, path, enabled):
-        item = self._menu_item(path)
-        assert item.get_enabled() == enabled
+        _, action = self._menu_item(path)
+        assert action.get_enabled() == enabled
+
+    def assert_menu_order(self, path, expected):
+        item, action = self._menu_item(path)
+        menu = item[0].get_item_link(item[1], "submenu")
+
+        # Loop over the sections
+        actual = []
+        for index in range(menu.get_n_items()):
+            section = menu.get_item_link(index, "section")
+            if section:
+                if actual:
+                    actual.append("---")
+
+                for section_index in range(section.get_n_items()):
+                    actual.append(
+                        section.get_item_attribute_value(
+                            section_index, "label"
+                        ).get_string()
+                    )
+            else:
+                actual.append(
+                    section.get_item_attribute_value(index, "label").get_string()
+                )
+
+        assert actual == expected
 
     def keystroke(self, combination):
         accel = gtk_accel(combination)

iOS/tests_backend/app.py

@@ -56,6 +56,9 @@ def activate_menu_visit_homepage(self):
     def assert_menu_item(self, path, enabled):
         pytest.skip("Menus not implemented on iOS")
 
+    def assert_menu_order(self, path, expected):
+        pytest.skip("Menus not implemented on iOS")
+
     def enter_background(self):
         self.native.delegate.applicationWillResignActive(self.native)
         self.native.delegate.applicationDidEnterBackground(self.native)

testbed/src/testbed/app.py

@@ -70,6 +70,9 @@ def startup(self):
         # Items on submenu2
         self.cmd5 = toga.Command(self.cmd_action, "Jiggle", group=subgroup2)
 
+        # Items on the main group after a submenu
+        self.cmd6 = toga.Command(self.cmd_action, "Wiggle", group=group, section=2)
+
         # Add all the commands
         self.commands.add(
             self.cmd1,
@@ -80,6 +83,7 @@ def startup(self):
             self.no_action_cmd,
             self.deep_cmd,
             self.cmd5,
+            self.cmd6,
         )
 
         self.main_window = toga.MainWindow(title=self.formal_name)

testbed/tests/app/test_app.py

@@ -500,6 +500,27 @@ async def test_menu_items(app, app_probe):
         ["Other", "Submenu1", "Submenu1 menu1", "Deep"],
         enabled=True,
     )
+    app_probe.assert_menu_item(
+        ["Other", "Wiggle"],
+        enabled=True,
+    )
+
+    app_probe.assert_menu_order(
+        ["Other"],
+        ["Full command", "---", "Submenu1", "Submenu2", "Wiggle"],
+    )
+    app_probe.assert_menu_order(
+        ["Other", "Submenu1"],
+        ["Disabled", "No Action", "Submenu1 menu1"],
+    )
+    app_probe.assert_menu_order(
+        ["Other", "Submenu1", "Submenu1 menu1"],
+        ["Deep"],
+    )
+    app_probe.assert_menu_order(
+        ["Other", "Submenu2"],
+        ["Jiggle"],
+    )
 
     app_probe.assert_menu_item(
         ["Commands", "No Tooltip"],

winforms/tests_backend/app.py

@@ -5,7 +5,7 @@
 import pytest
 from System import EventArgs
 from System.Drawing import Point
-from System.Windows.Forms import Application, Cursor
+from System.Windows.Forms import Application, Cursor, ToolStripSeparator
 
 from toga_winforms.keys import toga_to_winforms_key, winforms_to_toga_key
 
@@ -150,6 +150,16 @@ def assert_menu_item(self, path, *, enabled=True):
         else:
             assert item.ShortcutKeyDisplayString == shortcut
 
+    def assert_menu_order(self, path, expected):
+        menu = self._menu_item(path)
+
+        assert len(menu.DropDownItems) == len(expected)
+        for item, title in zip(menu.DropDownItems, expected):
+            if title == "---":
+                assert isinstance(item, ToolStripSeparator)
+            else:
+                assert item.Text == title
+
     def assert_system_menus(self):
         self.assert_menu_item(["File", "Preferences"], enabled=False)
         self.assert_menu_item(["File", "Exit"])