From fd33a9a4aeb4e41abee9fd0639ec7cbc04e76a52 Mon Sep 17 00:00:00 2001
From: Jatin K Malik <jatinkrmalik@gmail.com>
Date: Fri, 27 Mar 2026 11:29:18 -0700
Subject: [PATCH] fix(settings): add Close button to settings dialog for WM
 compatibility (fixes #323) (#356)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Some window managers (on both X11 and Wayland) hide the title-bar close
button for Gtk.Dialog windows that have no action buttons. The previous
fix (PR #333) added set_decorated(True) which was not sufficient.

This adds an explicit Close action button to the dialog so users always
have a visible way to dismiss it, regardless of WM behavior. The
instant-apply pattern is preserved — settings still apply immediately
on change.
---
 src/vocalinux/ui/settings_dialog.py |  9 +++++++--
 tests/test_settings_dialog.py       | 17 ++++++++++++-----
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/vocalinux/ui/settings_dialog.py b/src/vocalinux/ui/settings_dialog.py
index 8797728..8b12ba0 100644
--- a/src/vocalinux/ui/settings_dialog.py
+++ b/src/vocalinux/ui/settings_dialog.py
@@ -8,7 +8,7 @@ UX Design Notes:
 - Follows GNOME Human Interface Guidelines (HIG) for modern desktop look
 - Uses preference-page style layout with clearly grouped sections
 - Settings apply immediately when changed (instant-apply pattern)
-- No action buttons needed - use title bar close button
+- Close button provided for WM compatibility (some WMs hide title bar close)
 - Provides real-time progress feedback for recognition state
 - Multi-modal feedback (text + icon + audio level) for accessibility
 - Modal dialog for model downloads (explicit confirmation for large downloads)
@@ -727,6 +727,11 @@ class SettingsDialog(Gtk.Dialog):
     ):
         super().__init__(title="Vocalinux Settings", transient_for=parent, flags=0)
         self.set_decorated(True)  # Force window decorations (close button) on all WMs
+
+        # Add a Close action button so the dialog always has a visible way to
+        # dismiss it, even on window managers that hide the title-bar close
+        # button for Gtk.Dialog windows without action buttons (fixes #323).
+        self.add_button("Close", Gtk.ResponseType.CLOSE)
         self.config_manager = config_manager
         self.speech_engine = speech_engine
         self.shortcut_update_callback = shortcut_update_callback
@@ -742,7 +747,7 @@ class SettingsDialog(Gtk.Dialog):
         # Setup CSS styling
         _setup_css()
 
-        # Dialog configuration - no action buttons needed (use title bar close)
+        # Dialog configuration - Close button added above for WM compatibility
         # Calculate dialog size
         display = Gdk.Display.get_default()
         if display:
diff --git a/tests/test_settings_dialog.py b/tests/test_settings_dialog.py
index 9798c68..f27216b 100644
--- a/tests/test_settings_dialog.py
+++ b/tests/test_settings_dialog.py
@@ -262,8 +262,14 @@ class TestSettingsDialogInstantApply(unittest.TestCase):
 
         self.assertIn("def _auto_apply_settings(self", source_code)
 
-    def test_settings_dialog_no_action_buttons(self):
-        """Test that SettingsDialog uses no action buttons (title bar close only)."""
+    def test_settings_dialog_has_close_button_only(self):
+        """Test that SettingsDialog has a Close button but no Apply button.
+
+        A Close button is required for window managers that hide the title bar
+        close button on Gtk.Dialog windows (fixes #323). The instant-apply
+        pattern means settings are applied immediately, so no Apply button is
+        needed.
+        """
         import os
 
         source_path = os.path.join(
@@ -277,11 +283,12 @@ class TestSettingsDialogInstantApply(unittest.TestCase):
         with open(source_path, "r") as f:
             source_code = f.read()
 
-        # Should NOT have action buttons - uses title bar close (instant-apply pattern)
-        self.assertNotIn("_Close", source_code)
+        # Should have a Close button for WM compatibility
+        self.assertIn("ResponseType.CLOSE", source_code)
+
+        # Should NOT have Apply button - uses instant-apply pattern
         self.assertNotIn("_Apply", source_code)
         self.assertNotIn("ResponseType.APPLY", source_code)
-        self.assertNotIn("ResponseType.CLOSE", source_code)
 
     def test_settings_dialog_no_revert_settings(self):
         """Test that SettingsDialog does NOT have _revert_settings (removed)."""