From b994559845bbd11e0cf88de005080b31e13cc7d5 Mon Sep 17 00:00:00 2001
From: dalthviz <16781833+dalthviz@users.noreply.github.com>
Date: Thu, 15 Aug 2024 10:59:30 -0500
Subject: [PATCH] Simplify and add possibility to op-out keybinding in tooltip.
 Fix usage of single key as key combination with Qt6

---
 src/app_model/backends/qt/_qaction.py | 12 ++++--------
 src/app_model/backends/qt/_qkeymap.py |  4 +++-
 tests/test_qt/test_qactions.py        | 19 +++++++++++++++----
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/src/app_model/backends/qt/_qaction.py b/src/app_model/backends/qt/_qaction.py
index b7f5efb..ccf1175 100644
--- a/src/app_model/backends/qt/_qaction.py
+++ b/src/app_model/backends/qt/_qaction.py
@@ -56,11 +56,6 @@ def _on_triggered(self, checked: bool) -> None:
         # to raise any exceptions.
         self._app.commands.execute_command(self._command_id).result()
 
-    def setToolTip(self, tooltip: str) -> None:
-        """Override Qt method to append keybinding information to tooltip."""
-        tooltip_with_keybinding = f"{tooltip} {self._keybinding_tooltip}".rstrip()
-        super().setToolTip(tooltip_with_keybinding)
-
 
 class QCommandRuleAction(QCommandAction):
     """QAction for a CommandRule.
@@ -91,9 +86,6 @@ def __init__(
             self.setText(command_rule.short_title)  # pragma: no cover
         else:
             self.setText(command_rule.title)
-        # Since the default tooltip value is the action `text`, call `setToolTip`
-        # logic to  include keybinding information.
-        self.setToolTip(self.text())
         if command_rule.icon:
             self.setIcon(to_qicon(command_rule.icon))
         self.setIconVisibleInMenu(command_rule.icon_visible_in_menu)
@@ -104,6 +96,10 @@ def __init__(
         if command_rule.toggled is not None:
             self.setCheckable(True)
             self._refresh()
+        tooltip_with_keybinding = (
+            f"{self.toolTip()} {self._keybinding_tooltip}".rstrip()
+        )
+        self.setToolTip(tooltip_with_keybinding)
 
     def update_from_context(self, ctx: Mapping[str, object]) -> None:
         """Update the enabled state of this menu item from `ctx`."""
diff --git a/src/app_model/backends/qt/_qkeymap.py b/src/app_model/backends/qt/_qkeymap.py
index 8d17d49..d2b0d58 100644
--- a/src/app_model/backends/qt/_qkeymap.py
+++ b/src/app_model/backends/qt/_qkeymap.py
@@ -64,7 +64,9 @@ def simple_keybinding_to_qint(skb: SimpleKeyBinding) -> int:
         )
         key = modelkey2qkey(skb.key) if skb.key else Qt.Key.Key_unknown
         mods = (v for k, v in lookup.items() if getattr(skb, k))
-        combo = QKeyCombination(reduce(operator.or_, mods), key)
+        combo = QKeyCombination(
+            reduce(operator.or_, mods, Qt.KeyboardModifier.NoModifier), key
+        )
         return int(combo.toCombined())
 
 else:
diff --git a/tests/test_qt/test_qactions.py b/tests/test_qt/test_qactions.py
index 4b084e9..3c8a160 100644
--- a/tests/test_qt/test_qactions.py
+++ b/tests/test_qt/test_qactions.py
@@ -94,17 +94,22 @@ def test_tooltip(
 
 
 @pytest.mark.parametrize(
-    ("tooltip", "expected_tooltip"),
+    ("tooltip", "tooltip_with_keybinding", "tooltip_without_keybinding"),
     [
-        ("", "Test keybinding tooltip (K)"),
+        ("", "Test keybinding tooltip (K)", "Test keybinding tooltip"),
         (
             "Test action with a tooltip and a keybinding",
             "Test action with a tooltip and a keybinding (K)",
+            "Test action with a tooltip and a keybinding",
         ),
     ],
 )
 def test_keybinding_in_tooltip(
-    qapp, simple_app: "Application", tooltip: str, expected_tooltip: str
+    qapp,
+    simple_app: "Application",
+    tooltip: str,
+    tooltip_with_keybinding: str,
+    tooltip_without_keybinding: str,
 ) -> None:
     action = Action(
         id="test.keybinding.tooltip",
@@ -114,5 +119,11 @@ def test_keybinding_in_tooltip(
         keybindings=[KeyBindingRule(primary=KeyCode.KeyK)],
     )
     simple_app.register_action(action)
+
+    # check initial action instance shows keybinding info in its tooltip if available
     q_action = QCommandRuleAction(action, simple_app)
-    assert q_action.toolTip() == expected_tooltip
+    assert q_action.toolTip() == tooltip_with_keybinding
+
+    # check setting tooltip manually removes keybinding info
+    q_action.setToolTip(tooltip)
+    assert q_action.toolTip() == tooltip_without_keybinding