-
Notifications
You must be signed in to change notification settings - Fork 15
feat: Update of shortcuts on menu rebuild #258
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
09abb3d
80f298e
ec859d4
285c58d
edf4b80
81c8f92
3bab630
f835e19
94b694c
371027f
18b6b52
2d6a11a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
Action, | ||
CommandRule, | ||
KeyBindingRule, | ||
KeyBindingSource, | ||
KeyCode, | ||
MenuItem, | ||
ToggleRule, | ||
|
@@ -18,7 +19,8 @@ | |
from conftest import FullApp | ||
|
||
|
||
def test_cache_qaction(qapp, full_app: "FullApp") -> None: | ||
@pytest.mark.usefixtures("qapp") | ||
def test_cache_qaction(full_app: "FullApp") -> None: | ||
action = next( | ||
i for k, items in full_app.menus for i in items if isinstance(i, MenuItem) | ||
) | ||
|
@@ -28,7 +30,8 @@ def test_cache_qaction(qapp, full_app: "FullApp") -> None: | |
assert repr(a1).startswith("QMenuItemAction") | ||
|
||
|
||
def test_toggle_qaction(qapp, simple_app: "Application") -> None: | ||
@pytest.mark.usefixtures("qapp") | ||
def test_toggle_qaction(simple_app: "Application") -> None: | ||
mock = Mock() | ||
x = False | ||
|
||
|
@@ -75,6 +78,7 @@ def test_icon_visible_in_menu(qapp, simple_app: "Application") -> None: | |
assert not q_action.isIconVisibleInMenu() | ||
|
||
|
||
@pytest.mark.usefixtures("qapp") | ||
@pytest.mark.parametrize( | ||
("tooltip", "expected_tooltip"), | ||
[ | ||
|
@@ -83,7 +87,7 @@ def test_icon_visible_in_menu(qapp, simple_app: "Application") -> None: | |
], | ||
) | ||
def test_tooltip( | ||
qapp, simple_app: "Application", tooltip: str, expected_tooltip: str | ||
simple_app: "Application", tooltip: str, expected_tooltip: str | ||
) -> None: | ||
action = Action( | ||
id="test.tooltip", title="Test tooltip", tooltip=tooltip, callback=lambda: None | ||
|
@@ -93,6 +97,7 @@ def test_tooltip( | |
assert q_action.toolTip() == expected_tooltip | ||
|
||
|
||
@pytest.mark.usefixtures("qapp") | ||
@pytest.mark.parametrize( | ||
("tooltip", "tooltip_with_keybinding", "tooltip_without_keybinding"), | ||
[ | ||
|
@@ -105,7 +110,6 @@ def test_tooltip( | |
], | ||
) | ||
def test_keybinding_in_tooltip( | ||
qapp, | ||
simple_app: "Application", | ||
tooltip: str, | ||
tooltip_with_keybinding: str, | ||
|
@@ -127,3 +131,36 @@ def test_keybinding_in_tooltip( | |
# check setting tooltip manually removes keybinding info | ||
q_action.setToolTip(tooltip) | ||
assert q_action.toolTip() == tooltip_without_keybinding | ||
|
||
|
||
@pytest.mark.usefixtures("qapp") | ||
def test_update_keybinding_in_tooltip( | ||
simple_app: "Application", | ||
) -> None: | ||
action = Action( | ||
id="test.update.keybinding.tooltip", | ||
title="Test update keybinding tooltip", | ||
callback=lambda: None, | ||
tooltip="Initial tooltip", | ||
keybindings=[KeyBindingRule(primary=KeyCode.KeyK)], | ||
) | ||
dispose1 = simple_app.register_action(action) | ||
|
||
q_action = QCommandRuleAction(action, simple_app) | ||
assert q_action.toolTip() == "Initial tooltip (K)" | ||
|
||
# Update the keybinding | ||
dispose2 = simple_app.keybindings.register_keybinding_rule( | ||
"test.update.keybinding.tooltip", | ||
KeyBindingRule(primary=KeyCode.KeyL, source=KeyBindingSource.USER), | ||
) | ||
q_action._update_keybinding() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a little surprised to see this private method called directly in tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently it is called on a rebuild of the menu. app model is not evented. I do not yet identified other places. |
||
assert q_action.toolTip() == "Initial tooltip (L)" | ||
|
||
dispose2() | ||
q_action._update_keybinding() | ||
assert q_action.toolTip() == "Initial tooltip (K)" | ||
|
||
dispose1() | ||
q_action._update_keybinding() | ||
assert q_action.toolTip() == "Initial tooltip" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code is more or less the same as what's happening in the init at line 52. Can we maybe avoid that duplication by having this be the one place where that logic is held, and call it in init?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is with tooltip. If we call this method from line 52, then, for
QCommandRuleAction
, theself._tooltip
will be undefined on call of_update_keybinding
I think that we may fix this using metaclass and add
__post_init__
where_update_keybinding
will be called.