diff --git a/docs/releases/upcoming/1820.bugfix.rst b/docs/releases/upcoming/1820.bugfix.rst new file mode 100644 index 000000000..34d77ccf6 --- /dev/null +++ b/docs/releases/upcoming/1820.bugfix.rst @@ -0,0 +1 @@ +Fix the KeyBindingEditor. \ No newline at end of file diff --git a/docs/source/traitsui_user_manual/examples/key_bindings.py b/docs/source/traitsui_user_manual/examples/key_bindings.py index e22da0df1..141c16396 100644 --- a/docs/source/traitsui_user_manual/examples/key_bindings.py +++ b/docs/source/traitsui_user_manual/examples/key_bindings.py @@ -28,6 +28,7 @@ KeyBinding( binding1='Ctrl-k', description='Edit key bindings', + # special method name handled internally method_name='edit_bindings', ), ) @@ -42,10 +43,6 @@ def save_file(self, info): def run_script(self, info): info.object.status = "run script" - def edit_bindings(self, info): - info.object.status = "edit bindings" - key_bindings.edit_traits() - class KBCodeExample(HasPrivateTraits): @@ -70,7 +67,7 @@ class KBCodeExample(HasPrivateTraits): @observe('kb') def _edit_key_bindings(self, event): - key_bindings.edit_traits() + key_bindings.edit() if __name__ == '__main__': diff --git a/traitsui/key_bindings.py b/traitsui/key_bindings.py index ecf142bb4..38ec9859b 100644 --- a/traitsui/key_bindings.py +++ b/traitsui/key_bindings.py @@ -30,6 +30,7 @@ from .editors.key_binding_editor import KeyBindingEditor from .editors.list_editor import ListEditor from .group import HGroup +from .handler import ModelView from .item import Item from .toolkit import toolkit from .view import View @@ -116,31 +117,10 @@ class KeyBindings(HasPrivateTraits): #: The child KeyBindings of this object (if any): children = List(transient=True) - #: Control that currently has the focus (if any) - focus_owner = Any(transient=True) - # ------------------------------------------------------------------------- # Traits view definitions: # ------------------------------------------------------------------------- - traits_view = View( - [ - Item( - "bindings", - style="readonly", - show_label=False, - editor=ListEditor(style="custom"), - ), - "|{Click on an entry field, then press the key to " - "assign. Double-click a field to clear it.}<>", - ], - title="Update Key Bindings", - kind="livemodal", - resizable=True, - width=0.4, - height=0.4, - ) - def __init__(self, *bindings, **traits): # initialize bindings if len(bindings) == 1 and isinstance(bindings[0], SequenceTypes): @@ -194,21 +174,12 @@ def dispose(self): del self.children del self.bindings - self.parent = self._root = self.focus_owner = None + self.parent = self._root = None def edit(self): """Edits a possibly hierarchical set of KeyBindings.""" - bindings = list(set(self.root._get_bindings([]))) - bindings.sort(key=lambda x: "%s%02d" % (x.binding1[-1:], x.binding1)) - KeyBindings(bindings).edit_traits() - - def key_binding_for(self, binding, key_name): - """Returns the current binding for a specified key (if any).""" - if key_name != "": - for a_binding in self._match_binding(key_name, skip={binding}): - return a_binding - - return None + model_view = KeyBindingsHandler(model=self) + ui = model_view.edit_traits() # -- Property Implementations --------------------------------------------- @@ -228,11 +199,6 @@ def _binding_updated(self, event): for a_binding in self._match_binding(event.new, skip={event.object}): a_binding.clear_binding(event.new) - def _focus_owner_changed(self, old, new): - """Handles the focus owner being changed.""" - if old is not None: - old.border_size = 0 - @observe("children.items") def _children_modified(self, event): """Handles child KeyBindings being added to the object.""" @@ -294,3 +260,48 @@ def _match_binding(self, binding, skip=frozenset()): for a_binding in self.bindings if a_binding not in skip and a_binding.match(binding) ) + + +class KeyBindingsHandler(ModelView): + + bindings = List(Instance(KeyBinding)) + + def key_binding_for(self, binding, key_name): + """Returns the current binding for a specified key (if any).""" + if key_name != "": + for a_binding in self._match_binding(key_name, skip={binding}): + return a_binding + + return None + + def _match_binding(self, binding, skip=frozenset()): + """Return all KeyBinding instances that match the given binding. + """ + return ( + a_binding + for a_binding in self.bindings + if a_binding not in skip and a_binding.match(binding) + ) + + def _bindings_default(self): + bindings = list(set(self.model.root._get_bindings([]))) + bindings.sort(key=lambda x: (x.binding1[-1:], x.binding1)) + return bindings + + traits_view = View( + [ + Item( + "bindings", + style="readonly", + show_label=False, + editor=ListEditor(style="custom"), + ), + "|{Click on an entry field, then press the key to " + "assign. Double-click a field to clear it.}<>", + ], + title="Update Key Bindings", + kind="livemodal", + resizable=True, + width=0.4, + height=0.4, + ) diff --git a/traitsui/qt4/key_binding_editor.py b/traitsui/qt4/key_binding_editor.py index fc9fcc2a1..09fc15322 100644 --- a/traitsui/qt4/key_binding_editor.py +++ b/traitsui/qt4/key_binding_editor.py @@ -28,6 +28,7 @@ from pyface.qt import QtCore, QtGui +from pyface.api import YES, confirm from traits.api import Bool, Event from .editor import Editor @@ -57,47 +58,34 @@ def init(self, parent): """ self.control = KeyBindingCtrl(self) - def update_object(self, event): - """Handles the user entering input data in the edit control.""" - try: - self.value = value = key_event_to_name(event) - self._binding.text = value - except: - pass - def update_editor(self): """Updates the editor when the object trait changes externally to the editor. """ self.control.setText(self.value) - def update_focus(self, has_focus): - """Updates the current focus setting of the control.""" - if has_focus: - self._binding.border_size = 1 - self.object.owner.focus_owner = self._binding - def _key_changed(self, event): """Handles a keyboard event.""" binding = self.object key_name = key_event_to_name(event) - cur_binding = binding.owner.key_binding_for(binding, key_name) + cur_binding = self.ui.parent.handler.key_binding_for(binding, key_name) if cur_binding is not None: - if ( - QtGui.QMessageBox.question( - self.control, - "Duplicate Key Definition", - "'%s' has already been assigned to '%s'.\n" + result = confirm( + parent=self.control, + message=( + f"{key_name!r} has already been assigned to " + f"'{cur_binding.description}'.\n" "Do you wish to continue?" - % (key_name, cur_binding.description), - QtGui.QMessageBox.StandardButton.Yes | QtGui.QMessageBox.StandardButton.No, - QtGui.QMessageBox.StandardButton.No, - ) - != QtGui.QMessageBox.StandardButton.Yes - ): + ), + title="Duplicate Key Definition", + ) + if result != YES: return self.value = key_name + # Need to manually update editor because the update to the value + # won't get transferred to toolkit object due to loopback protection. + self.update_editor() def _clear_changed(self): """Handles a clear field event.""" diff --git a/traitsui/tests/test_key_bindings.py b/traitsui/tests/test_key_bindings.py index 06b56f0b0..2ae9dff02 100644 --- a/traitsui/tests/test_key_bindings.py +++ b/traitsui/tests/test_key_bindings.py @@ -14,7 +14,7 @@ from traits.testing.api import UnittestTools -from ..key_bindings import KeyBinding, KeyBindings +from ..key_bindings import KeyBinding, KeyBindings, KeyBindingsHandler class Controller1: @@ -116,50 +116,6 @@ def test_merge(self): self.assertEqual(key_binding.binding1, "Ctrl-V") self.assertEqual(key_binding.binding2, "Ctrl-W") - def test_key_binding_for_match(self): - bindings = [ - KeyBinding(binding1="Ctrl-S", binding2="Ctrl-T", method_name="m1"), - KeyBinding(binding1="Ctrl-U", binding2="", method_name="m2"), - ] - key_bindings = KeyBindings(bindings=bindings) - - binding = key_bindings.key_binding_for(bindings[0], "Ctrl-U") - - self.assertIs(binding, bindings[1]) - - def test_key_binding_for_no_match(self): - bindings = [ - KeyBinding(binding1="Ctrl-S", binding2="Ctrl-T", method_name="m1"), - KeyBinding(binding1="Ctrl-U", binding2="", method_name="m2"), - ] - key_bindings = KeyBindings(bindings=bindings) - - binding = key_bindings.key_binding_for(bindings[0], "Ctrl-V") - - self.assertIsNone(binding) - - def test_key_binding_for_match_self(self): - bindings = [ - KeyBinding(binding1="Ctrl-S", binding2="Ctrl-T", method_name="m1"), - KeyBinding(binding1="Ctrl-U", binding2="", method_name="m2"), - ] - key_bindings = KeyBindings(bindings=bindings) - - binding = key_bindings.key_binding_for(bindings[0], "Ctrl-S") - - self.assertIsNone(binding) - - def test_key_binding_for_match_empty(self): - bindings = [ - KeyBinding(binding1="Ctrl-S", binding2="Ctrl-T", method_name="m1"), - KeyBinding(binding1="Ctrl-U", binding2="", method_name="m2"), - ] - key_bindings = KeyBindings(bindings=bindings) - - binding = key_bindings.key_binding_for(bindings[0], "") - - self.assertIsNone(binding) - def test_clear_bindings_match(self): bindings = [ KeyBinding(binding1="Ctrl-S", binding2="Ctrl-T", method_name="m1"), @@ -269,3 +225,54 @@ def test_do_no_match_complete(self): self.assertFalse(result) self.assertIsNone(controller.called) + + +class TestKeyBindingsHandler(unittest.TestCase): + + def test_key_binding_for_match(self): + bindings = [ + KeyBinding(binding1="Ctrl-S", binding2="Ctrl-T", method_name="m1"), + KeyBinding(binding1="Ctrl-U", binding2="", method_name="m2"), + ] + key_bindings = KeyBindings(bindings=bindings) + handler = KeyBindingsHandler(model=key_bindings) + + binding = handler.key_binding_for(bindings[0], "Ctrl-U") + + self.assertIs(binding, bindings[1]) + + def test_key_binding_for_no_match(self): + bindings = [ + KeyBinding(binding1="Ctrl-S", binding2="Ctrl-T", method_name="m1"), + KeyBinding(binding1="Ctrl-U", binding2="", method_name="m2"), + ] + key_bindings = KeyBindings(bindings=bindings) + handler = KeyBindingsHandler(model=key_bindings) + + binding = handler.key_binding_for(bindings[0], "Ctrl-V") + + self.assertIsNone(binding) + + def test_key_binding_for_match_self(self): + bindings = [ + KeyBinding(binding1="Ctrl-S", binding2="Ctrl-T", method_name="m1"), + KeyBinding(binding1="Ctrl-U", binding2="", method_name="m2"), + ] + key_bindings = KeyBindings(bindings=bindings) + handler = KeyBindingsHandler(model=key_bindings) + + binding = handler.key_binding_for(bindings[0], "Ctrl-S") + + self.assertIsNone(binding) + + def test_key_binding_for_match_empty(self): + bindings = [ + KeyBinding(binding1="Ctrl-S", binding2="Ctrl-T", method_name="m1"), + KeyBinding(binding1="Ctrl-U", binding2="", method_name="m2"), + ] + key_bindings = KeyBindings(bindings=bindings) + handler = KeyBindingsHandler(model=key_bindings) + + binding = handler.key_binding_for(bindings[0], "") + + self.assertIsNone(binding) diff --git a/traitsui/wx/key_binding_editor.py b/traitsui/wx/key_binding_editor.py index 7ffcb1095..e2270a38c 100644 --- a/traitsui/wx/key_binding_editor.py +++ b/traitsui/wx/key_binding_editor.py @@ -18,7 +18,7 @@ from traits.api import Bool, Event -from pyface.wx.dialog import confirmation +from pyface.api import YES, confirm from .editor import Editor @@ -51,42 +51,28 @@ def init(self, parent): """ self.control = KeyBindingCtrl(self, parent, size=wx.Size(160, 19)) - def update_object(self, event): - """Handles the user entering input data in the edit control.""" - try: - self.value = value = key_event_to_name(event) - self._binding.text = value - except: - pass - def update_editor(self): """Updates the editor when the object trait changes externally to the editor. """ self.control.Refresh() - def update_focus(self, has_focus): - """Updates the current focus setting of the control.""" - if has_focus: - self._binding.border_size = 1 - self.object.owner.focus_owner = self._binding - def _key_changed(self, event): """Handles a keyboard event.""" binding = self.object key_name = key_event_to_name(event) cur_binding = binding.owner.key_binding_for(binding, key_name) if cur_binding is not None: - if ( - confirmation( - None, - "'%s' has already been assigned to '%s'.\n" + result = confirm( + parent=self.control, + message=( + f"{key_name!r} has already been assigned to " + f"'{cur_binding.description}'.\n" "Do you wish to continue?" - % (key_name, cur_binding.description), - "Duplicate Key Definition", - ) - == 5104 - ): + ), + title="Duplicate Key Definition", + ) + if result != YES: return self.value = key_name