Save button fix with Usage dialog popup

ardupilot_methodic_configurator/frontend_tkinter_base_window.py

@@ -301,7 +301,7 @@ def center_window(window: Union[tk.Toplevel, tk.Tk], parent: Union[tk.Toplevel,
         x = parent.winfo_x() + (parent_width // 2) - (window_width // 2)
         y = parent.winfo_y() + (parent_height // 2) - (window_height // 2)
         window.geometry(f"+{x}+{y}")
-        window.update()
+        window.update_idletasks()
 
     @staticmethod
     def center_window_on_screen(window: Union[tk.Toplevel, tk.Tk]) -> None:

ardupilot_methodic_configurator/frontend_tkinter_usage_popup_window.py

@@ -25,6 +25,11 @@
 from ardupilot_methodic_configurator.frontend_tkinter_rich_text import RichText
 
 
+def _is_macos() -> bool:
+    """Return True if the current platform is macOS."""
+    return sys_platform == "darwin"
+
+
 class PopupWindow:
     """
     Base class for creating and managing popup windows in the application.
@@ -86,7 +91,7 @@ def finalize_setup_popupwindow(
     ) -> None:
         """Finalize window setup: center, show, make modal on non-macOS, set close handler."""
         # Only set transient on non-macOS
-        if parent and sys_platform != "darwin":
+        if parent and not _is_macos():
             popup_window.root.transient(parent)
 
         # Resize window height to ensure all widgets are fully visible
@@ -113,7 +118,7 @@ def finalize_setup_popupwindow(
 
             # On macOS, grab_set() causes UI freeze (issue #1264), so skip it
             # On Windows/Linux, make the popup modal and give it focus
-            if sys_platform != "darwin":
+            if not _is_macos():
                 popup_window.root.grab_set()  # Make the popup modal
 
             popup_window.root.protocol("WM_DELETE_WINDOW", close_callback)
@@ -125,7 +130,7 @@ def finalize_setup_popupwindow(
     @staticmethod
     def close(popup_window: BaseWindow, parent: Optional[tk.Tk]) -> None:
         """Close the popup window and re-enable the parent window."""
-        if sys_platform != "darwin":
+        if not _is_macos():
             with contextlib.suppress(tk.TclError):
                 popup_window.root.grab_release()
 

tests/conftest.py

@@ -37,6 +37,11 @@
 
 # ==================== SHARED TKINTER TESTING CONFIGURATION ====================
 
+# deadlock on macOS (issue #1264).
+patch("tkinter.Misc.wait_window", lambda _self, *_args, **_kwargs: None).start()
+patch("tkinter.Misc.wait_visibility", lambda _self, *_args, **_kwargs: None).start()
+patch("tkinter.Misc.grab_set", lambda _self, *_args, **_kwargs: None).start()
+
 
 class MockConfiguration(NamedTuple):
     """Configuration for common mocking patterns in Tkinter tests."""
@@ -99,8 +104,28 @@ def mock_iconphoto(*args, **kwargs) -> None:  # pylint: disable=unused-argument
 @pytest.fixture
 def tk_root() -> Generator[tk.Tk, None, None]:
     """Provide a real Tkinter root for integration tests (legacy name for compatibility)."""
-    # Reuse the existing default root to avoid 'tcl_findLibrary' errors
-    # on Python 3.14 when multiple Tk instances are created.
+    # On macOS, a fresh Tk instance is created per test
+    try:
+        is_macos = tk.Tk.tk_setPalette  # just to get a root to check windowing system
+        temp = tk._default_root  # type: ignore[attr-defined] # pylint: disable=protected-access
+        is_macos = temp is not None and temp.tk.call("tk", "windowingsystem") == "aqua"
+    except (AttributeError, tk.TclError):
+        is_macos = False
+
+    if is_macos:
+        tk_root_instance = tk.Tk()
+        tk_root_instance.withdraw()
+        yield tk_root_instance
+        for child in list(tk_root_instance.winfo_children()):
+            with contextlib.suppress(tk.TclError, Exception):
+                child.protocol("WM_DELETE_WINDOW", lambda: None)
+            with contextlib.suppress(tk.TclError, Exception):
+                child.destroy()
+        with contextlib.suppress(tk.TclError):
+            tk_root_instance.destroy()
+        return
+
+    # Non-macOS: reuse existing root
     try:
         existing_root = tk._default_root  # type: ignore[attr-defined] # pylint: disable=protected-access
         if existing_root is not None:

tests/test_frontend_tkinter_usage_popup_window.py

@@ -26,6 +26,8 @@
 
 # pylint: disable=redefined-outer-name
 
+_IS_MACOS_PATH = "ardupilot_methodic_configurator.frontend_tkinter_usage_popup_window._is_macos"
+
 
 @pytest.fixture
 def popup_window(tk_root) -> BaseWindow:
@@ -145,6 +147,7 @@ def test_closing_popup_returns_focus_to_parent_window(
         """
         # Arrange: Mock window methods
         with (
+            patch(_IS_MACOS_PATH, return_value=False),
             patch.object(popup_window.root, "grab_release") as mock_grab_release,
             patch.object(popup_window.root, "destroy") as mock_destroy,
             patch.object(tk_root, "focus_set") as mock_focus,
@@ -173,6 +176,10 @@ def test_popup_shows_correct_title_and_content(self, tk_root, popup_window, rich
         with (
             patch("tkinter.BooleanVar") as mock_bool_var,
             patch.object(popup_window.root, "grab_set"),
+            patch.object(popup_window.root, "deiconify"),
+            patch.object(popup_window.root, "lift"),
+            patch.object(popup_window.root, "update"),
+            patch.object(popup_window.root, "focus_force"),
         ):
             mock_var_instance = MagicMock()
             mock_bool_var.return_value = mock_var_instance
@@ -220,6 +227,10 @@ def test_user_can_dismiss_popup_with_button(self, tk_root, popup_window, rich_te
         with (
             patch("tkinter.BooleanVar"),
             patch.object(popup_window.root, "grab_set"),
+            patch.object(popup_window.root, "deiconify"),
+            patch.object(popup_window.root, "lift"),
+            patch.object(popup_window.root, "update"),
+            patch.object(popup_window.root, "focus_force"),
         ):
             UsagePopupWindow.display(
                 parent=tk_root,
@@ -253,9 +264,13 @@ def test_popup_prevents_interaction_with_other_windows(self, tk_root, popup_wind
         """
         # Mock grab_set and other methods
         with (
+            patch(_IS_MACOS_PATH, return_value=False),
             patch.object(popup_window.root, "grab_set") as mock_grab_set,
             patch.object(popup_window.root, "withdraw"),
             patch.object(popup_window.root, "deiconify"),
+            patch.object(popup_window.root, "lift"),
+            patch.object(popup_window.root, "update"),
+            patch.object(popup_window.root, "focus_force"),
             patch("tkinter.BooleanVar"),
         ):
             UsagePopupWindow.display(
@@ -280,6 +295,7 @@ def test_closing_popup_returns_focus_to_parent_window(self, tk_root, popup_windo
         """
         # Mock window methods
         with (
+            patch(_IS_MACOS_PATH, return_value=False),
             patch.object(popup_window.root, "grab_release") as mock_grab_release,
             patch.object(popup_window.root, "destroy") as mock_destroy,
             patch.object(tk_root, "focus_set") as mock_focus,
@@ -315,6 +331,10 @@ def test_user_sees_confirmation_popup_with_yes_no_buttons(
             patch.object(tk_root, "wait_window"),
             patch.object(PopupWindow, "close"),
             patch.object(popup_window.root, "grab_set"),
+            patch.object(popup_window.root, "deiconify"),
+            patch.object(popup_window.root, "lift"),
+            patch.object(popup_window.root, "update"),
+            patch.object(popup_window.root, "focus_force"),
         ):
             mock_var_instance = MagicMock()
             mock_bool_var.return_value = mock_var_instance
@@ -362,6 +382,10 @@ def test_confirmation_popup_has_show_again_checkbox(
             patch.object(tk_root, "wait_window"),
             patch.object(PopupWindow, "close"),
             patch.object(popup_window.root, "grab_set"),
+            patch.object(popup_window.root, "deiconify"),
+            patch.object(popup_window.root, "lift"),
+            patch.object(popup_window.root, "update"),
+            patch.object(popup_window.root, "focus_force"),
         ):
             mock_var_instance = MagicMock()
             mock_bool_var.return_value = mock_var_instance
@@ -399,6 +423,10 @@ def test_confirmation_popup_defaults_to_false_when_closed(
             patch.object(tk_root, "wait_window") as mock_wait,
             patch.object(PopupWindow, "close"),
             patch.object(popup_window.root, "grab_set"),
+            patch.object(popup_window.root, "deiconify"),
+            patch.object(popup_window.root, "lift"),
+            patch.object(popup_window.root, "update"),
+            patch.object(popup_window.root, "focus_force"),
         ):
             # Simulate window closing without button click
             mock_wait.return_value = None
@@ -445,10 +473,10 @@ def __init__(self) -> None:
 
     # deiconify was attempted
     fake.root.deiconify.assert_called_once()
-    # attributes/grab_set/protocol should not be called because deiconify failed
+    assert not fake.root.attributes.called
+    fake.root.deiconify.assert_called_once()
     assert not fake.root.attributes.called
     assert not fake.root.grab_set.called
-    assert not fake.root.protocol.called
 
 
 if __name__ == "__main__":