-
-
Notifications
You must be signed in to change notification settings - Fork 307
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
wxGUI/digitizer: fix digitizer - VDigitToolbar.OnTool method was not called #3027
Conversation
gui/wxpython/vdigit/toolbars.py
Outdated
@@ -446,14 +449,15 @@ def OnTool(self, event): | |||
3, | |||
f"VDigitToolbar.OnTool(): id = {event.GetId() if event else event}", | |||
) | |||
if self.toolSwitcher: |
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.
If you try activate Digitize new boundary (Ctrl+B) or Digitize new centroid (Ctrl+C) tool by keyboard shortcut you get error:
Traceback (most recent call last):
File
"/usr/lib64/grass84/gui/wxpython/vdigit/mapwindow.py", line
220, in OnKeyDown
event = self.toolbar.OnTool(tool["event"])
File "/usr/lib64/grass84/gui/wxpython/vdigit/toolbars.py",
line 453, in OnTool
self.toolSwitcher.ToolChanged(event.GetId())
AttributeError
:
'NoneType' object has no attribute 'GetId'
code line should be:
if self.toolSwitcher and event:
gui/wxpython/vdigit/toolbars.py
Outdated
@@ -467,8 +471,7 @@ def OnTool(self, event): | |||
if self.action["id"] == -1: | |||
self.action = {"desc": "", "type": "", "id": -1} | |||
|
|||
# set focus | |||
self.MapWindow.SetFocus() | |||
event.Skip() |
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.
Same as previous comment.
code line should be:
if event:
event.Skip()
All bugs fix patch attached.
|
Thanks, I further simplified your code. |
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.
With your simplification commit ef5d9e7 activation Digitize new boundary (Ctrl+B) and Digitize new centroid (Ctrl+C) by keyboard shortcut is not working as expected.
Steps to reproduce:
- Display some vector map
d.vect geology
- Activate Vector digitizer for geology vector map
- Change focus on actual map display
- Activate and use Pan tool
- Try activate Digitize new point (Ctrl+P) tool by keyboard shortcut
- Try activate Digitize new boundary (Ctrl+B) tool, or Digitize new centroid (Ctrl+C) tool by keyboard shortcut is not working
Thanks, I believe it's all fixed now, I think this is good to be merged. |
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.
Works good on Mac with Python 3.10.10 and wxPython 4.2.0.
It works as expected. |
…called (#3027) Co-authored-by: Tomas Zigo <tomas.zigo@slovanet.sk>
@petrasovaa are you planning also backport to 8.2? |
Yes, will get back to it, it can't be simply cherry-picked due to conflict. |
…called (OSGeo#3027) Co-authored-by: Tomas Zigo <tomas.zigo@slovanet.sk>
…called (OSGeo#3027) Co-authored-by: Tomas Zigo <tomas.zigo@slovanet.sk>
The bug could be reproduced by:
Also g.gui.iclass doesn't work.
The toolbar tools were bound to the ToolbarController.OnTool method, this PR replaces the controller's method in runtime by the specific one in digitizer toolbar. Probably there could be a better solution, but this seems to work and the code is changed minimally.
Unrelated change is removing setting focus, seems to help with using keyboard shortcuts.