-
-
Notifications
You must be signed in to change notification settings - Fork 800
input gestures dialog improvements #20443
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
base: master
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -64,6 +64,7 @@ class _GestureVM: | |
| displayName: str #: How the gesture should be displayed | ||
| normalizedGestureIdentifier: str #: As per items in inputCore.AllGesturesScriptInfo.gestures | ||
| canAdd = False #: adding children is not supported. | ||
| canChange = True #: gestures can be changed | ||
| canRemove = True #: gestures can be removed | ||
|
|
||
| def __init__(self, normalizedGestureIdentifier: str): | ||
|
|
@@ -78,6 +79,7 @@ class _PendingGesture: | |
| # Translators: The prompt to enter a gesture in the Input Gestures dialog. | ||
| displayName = _("Enter input gesture:") | ||
| canAdd = False | ||
| canChange = False | ||
| canRemove = False | ||
|
|
||
| def __repr__(self): | ||
|
|
@@ -89,6 +91,7 @@ class _ScriptVM: | |
| scriptInfo: inputCore.AllGesturesScriptInfo | ||
| gestures: List[Union[_GestureVM, _PendingGesture]] | ||
| canAdd = True #: able to add gestures that trigger this script | ||
| canChange = False #: Scripts can not be Changed | ||
| canRemove = False #: Scripts can not be removed | ||
| addedGestures: List[_GestureVM] #: These will also be in self.gestures | ||
| #: These will not be in self.gestures anymore. Key is the normalized Gesture Identifier. | ||
|
|
@@ -143,6 +146,7 @@ class _CategoryVM: | |
| displayName: str #: Translated display name for the category | ||
| scripts: List[_ScriptVM] | ||
| canAdd = False #: not able to add Scripts | ||
| canChange = False #: categories can not be changed | ||
| canRemove = False #: categories can not be removed | ||
|
|
||
| def __init__(self, displayName: str, scripts: _ScriptsModel): | ||
|
|
@@ -189,6 +193,7 @@ class _PendingEmulatedGestureVM: | |
| # Translators: The prompt to enter an emulated gesture in the Input Gestures dialog. | ||
| displayName = _("Enter gesture to emulate:") | ||
| canAdd = False | ||
| canChange = False | ||
| canRemove = False | ||
|
|
||
| def __repr__(self): | ||
|
|
@@ -199,6 +204,7 @@ class _EmuCategoryVM: | |
| displayName = inputCore.SCRCAT_KBEMU #: Translated display name for the gesture emulation category | ||
| scripts: List[Union[_ScriptVM, _EmulatedGestureVM, _PendingEmulatedGestureVM]] | ||
| canAdd = True #: Can add new emulated gestures | ||
| canChange = False #: categories can not be changed | ||
| canRemove = False #: categories can not be removed | ||
| addedKbEmulation: List[_EmulatedGestureVM] #: These will also be in self.scripts | ||
| #: These will not be in self.scripts anymore. Key is the scriptInfo display name. | ||
|
|
@@ -605,6 +611,9 @@ def makeSettings(self, settingsSizer): | |
| self.gesturesVM = _InputGesturesViewModel() | ||
| tree = self.tree = _GesturesTree(self, self.gesturesVM) | ||
| tree.Bind(wx.EVT_TREE_SEL_CHANGED, self.onTreeSelect) | ||
| tree.Bind(wx.EVT_CHAR_HOOK, self.onCharHook) | ||
| tree.Bind(wx.EVT_CONTEXT_MENU, self.onContextMenu) | ||
| tree.Bind(wx.EVT_TREE_ITEM_ACTIVATED, self.onContextMenu) | ||
| settingsSizer.Add(tree, proportion=1, flag=wx.EXPAND) | ||
|
|
||
| settingsSizer.AddSpacer(guiHelper.SPACE_BETWEEN_ASSOCIATED_CONTROL_VERTICAL) | ||
|
|
@@ -616,6 +625,11 @@ def makeSettings(self, settingsSizer): | |
| self.addButton.Bind(wx.EVT_BUTTON, self.onAdd) | ||
| self.addButton.Disable() | ||
|
|
||
| # Translators: The label of a button to change a gesture in the Input Gestures dialog. | ||
| self.changeButton = bHelper.addButton(self, label=_("C&hange")) | ||
| self.changeButton.Bind(wx.EVT_BUTTON, self.onChange) | ||
| self.changeButton.Disable() | ||
|
|
||
| # Translators: The label of a button to remove a gesture in the Input Gestures dialog. | ||
| self.removeButton = bHelper.addButton(self, label=_("&Remove")) | ||
| self.removeButton.Bind(wx.EVT_BUTTON, self.onRemove) | ||
|
|
@@ -630,6 +644,48 @@ def makeSettings(self, settingsSizer): | |
| settingsSizer.Add(bHelper.sizer, flag=wx.EXPAND) | ||
| self.tree.Bind(wx.EVT_WINDOW_DESTROY, self._onDestroyTree) | ||
|
|
||
| def onCharHook(self, evt): | ||
|
Member
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. type hints |
||
| selectedItems = self.tree.getSelectedItemData() | ||
| key = evt.GetKeyCode() | ||
| if selectedItems is None: | ||
| item = None | ||
| else: | ||
| # get the leaf of the selection | ||
| item = next((item for item in reversed(selectedItems) if item is not None), None) | ||
| pendingAdd = self.gesturesVM.isExpectingNewEmuGesture or self.gesturesVM.isExpectingNewGesture | ||
| if item and not pendingAdd: | ||
| if key == wx.WXK_DELETE and item.canRemove: | ||
| self.onRemove(None) | ||
| evt.Skip() | ||
|
|
||
| def onContextMenu(self, evt): | ||
|
Member
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. type hints |
||
| selectedItems = self.tree.getSelectedItemData() | ||
| if selectedItems is None: | ||
| item = None | ||
| else: | ||
| # get the leaf of the selection | ||
| item = next((item for item in reversed(selectedItems) if item is not None), None) | ||
| pendingAdd = self.gesturesVM.isExpectingNewEmuGesture or self.gesturesVM.isExpectingNewGesture | ||
| menu = wx.Menu() | ||
| if item and not pendingAdd: | ||
| if item.canAdd: | ||
| # Translators: Context menu item label to add a new gesture | ||
| addItem = menu.Append(wx.ID_ANY, _("&Add")) | ||
| self.Bind(wx.EVT_MENU, self.onAdd, addItem) | ||
| if item.canChange: | ||
| # Translators: Context menu item label to change a gesture | ||
| changeItem = menu.Append(wx.ID_ANY, _("C&hange")) | ||
| self.Bind(wx.EVT_MENU, self.onChange, changeItem) | ||
| if item.canRemove: | ||
| # Translators: Context menu item label to remove a gesture | ||
| removeItem = menu.Append(wx.ID_ANY, _("&Remove")) | ||
| self.Bind(wx.EVT_MENU, self.onRemove, removeItem) | ||
| # Translators: Context menu item label to reset factory to defaults | ||
| resetItem = menu.Append(wx.ID_ANY, _("Reset to factory &defaults")) | ||
|
Contributor
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 think that "Remove all" should not be part of context menu. Indeed, context menu should only contain actions which apply to the currently selected item, not to the whole list of shortcuts of the window in general. It's worth noting that there is already a visual distinction between buttons that perform an action on the selected item and the button which Applies to the whole list of gestures: "Add", "Remove" and now "Edit" are grouped together while "Remove all" is located apart. We have already discussed this topic (and disagreed with each other) in #20437. I'm commenting here so that NV Access can make a decision on this point.
Member
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. agreed that it should not be part of the context menu |
||
| self.Bind(wx.EVT_MENU, self.onReset, resetItem) | ||
| self.PopupMenu(menu) | ||
| menu.Destroy() | ||
|
|
||
| def postInit(self): | ||
| self.tree.RefreshItems() | ||
| self.tree.SetFocus() | ||
|
|
@@ -664,6 +720,7 @@ def _refreshButtonState(self): | |
| item = next((item for item in reversed(selectedItems) if item is not None), None) | ||
| pendingAdd = self.gesturesVM.isExpectingNewEmuGesture or self.gesturesVM.isExpectingNewGesture | ||
| self.addButton.Enabled = bool(item and item.canAdd and not pendingAdd) | ||
| self.changeButton.Enabled = bool(item and item.canChange and not pendingAdd) | ||
| self.removeButton.Enabled = bool(item and item.canRemove and not pendingAdd) | ||
|
|
||
| def onAdd(self, evt): | ||
|
|
@@ -712,6 +769,53 @@ def addGestureCaptor(gesture: inputCore.InputGesture): | |
| else: | ||
| log.error("unable to do 'add' action for selected item") | ||
|
|
||
| def onChange(self, evt): | ||
|
Member
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. please add type hints |
||
| if inputCore.manager._captureFunc: | ||
| # don't change while already in process of add/change. | ||
| return | ||
|
|
||
| selectedItems = self.tree.getSelectedItemData() | ||
| assert selectedItems is not None | ||
| catVM, scriptVM, gestureVM = selectedItems | ||
| log.debug(f"selection: {catVM}, {scriptVM}, {gestureVM}") | ||
|
|
||
| if isinstance(scriptVM, _EmulatedGestureVM) and isinstance(catVM, _EmuCategoryVM): | ||
| catVM.removeEmulation(scriptVM) | ||
| self.gesturesVM.isExpectingNewEmuGesture = catVM | ||
| pending = catVM.createPendingEmuGesture() | ||
| self.tree.doRefresh(focus=(catVM, pending, None)) | ||
| self._refreshButtonState() | ||
|
|
||
| def changeKbEmuGestureCaptor(gesture: inputCore.InputGesture): | ||
|
Member
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. please move this function out so it's not nested and add a return type |
||
| if not isinstance(gesture, keyboardHandler.KeyboardInputGesture) or gesture.isModifier: | ||
| return False | ||
| inputCore.manager._captureFunc = None | ||
| wx.CallAfter(self._addCapturedKbEmu, gesture, catVM) | ||
| return False | ||
|
|
||
| inputCore.manager._captureFunc = changeKbEmuGestureCaptor | ||
| elif gestureVM is not None and isinstance(scriptVM, _ScriptVM): | ||
| scriptVM.removeGesture(gestureVM) | ||
| self.gesturesVM.isExpectingNewGesture = scriptVM | ||
| pendingGesture = scriptVM.createPendingGesture() | ||
| self.tree.doRefresh(focus=(catVM, scriptVM, pendingGesture)) | ||
| self._refreshButtonState() | ||
|
|
||
| def changeGestureCaptor(gesture: inputCore.InputGesture): | ||
|
Member
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. please move this function out so it's not nested and add a return type |
||
| if gesture.isModifier: | ||
| return False | ||
| if isinstance(catVM, _EmuCategoryVM): | ||
| gesName = keyLabels.getKeyCombinationLabel(gesture.normalizedIdentifiers[-1][3:]) | ||
| if gesName == scriptVM.scriptInfo.displayName: | ||
| return False | ||
| inputCore.manager._captureFunc = None | ||
| wx.CallAfter(self._addCaptured, catVM, scriptVM, gesture) | ||
| return False | ||
|
|
||
| inputCore.manager._captureFunc = changeGestureCaptor | ||
| else: | ||
| log.error(f"unable to do 'change' action for selected item: {catVM}, {scriptVM}, {gestureVM}") | ||
|
|
||
| def _addCaptured(self, catVM: _CategoryVMTypes, scriptVM: _ScriptVMTypes, gesture): | ||
| gids = gesture.normalizedIdentifiers | ||
| if len(gids) > 1: | ||
|
|
||
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.
adding the ability to change gestures should be tackled in a separate PR to adding context menus