-
Notifications
You must be signed in to change notification settings - Fork 15
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
Iss983 - Set handled flag on mouse released #77
Conversation
Added extra handling code for the canvas, such that if the Pointer (Selection Tool) is selected to select a display item even if the clicked area is not the display item's toolbar and instead is somewhere like the background of the control.
Propagate further the result of the mouse release handler, and made sure that if a handler had successfully handled the event to set the correct flag on the event to stop Qt re-raising it to one element further up in the display heirarchy.
Is this fixing a bug? If so, can you reference the issue or file an issue describing the problem? This looks like a valid change to fix the problem of multiple fields being highlighted at once...? |
Correcting typing issue and whitespace Removed double-space and adding a False return indicating that the event has not been handled.
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.
nionui
should not know about nionswift
- so needs some work here.
selection_mode = True | ||
if (self.__mouse_canvas_item and | ||
hasattr(self.__mouse_canvas_item, 'delegate') and | ||
self.__mouse_canvas_item.delegate.tool_mode and | ||
self.__mouse_canvas_item.delegate.tool_mode != 'pointer'): | ||
selection_mode = False |
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 is a significant break of encapsulation and needs to be rewritten in a different way. It is accessing a special case in the nionswift
project of the tool_mode
being set to pointer
. nionui
intentionally does not know about anything in nionswift
, as it is a lower level library.
Generally the mouse tracking code is difficult to understand, but can you explain what you're doing here and then we can come up with a solution?
Does the mouse canvas item need something like "handles first clicks"? Something like this https://developer.apple.com/documentation/appkit/nsview/1483410-acceptsfirstmouse
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.
So this was to effectively differentiate between the software (cursor specifically) being in 'selection' mode rather than any other mode such as graphic drawing etc. What I found was that the handlers for the events were not always returning a value indicating successful handling of the event. So specifically the MouseHandler mouse_released function just passes the event on but it has no idea if that event is then being handled by whatever is looping and reading those events.
Since that feedback isn't available at that point of whether or not the event was handled, the event was being re-triggered and handled twice (the drawing of the graphic, and the selection) unless there is a way to identify whether the software is currently in a 'selecting' mode or not.
That is the bit that is needed here, that the ui layer is in a selection mode and it was a relatively simple way that I got to work pretty easily. If there is something else already in nionui that has that information please point me towards it and I can refactor it to that.
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.
I think the only way to go forward is to have the panel in Swift make the decisions and return the correct result. I can't think of a super-clean solution, but, based on your ideas, here is a Swift-only solution:
Closing in favor of the solution here nion-software/nionswift#1130 See notes here, too nion-software/nionui-tool#63 |
When a mouse-released event is handled, set the accepted flag on the event so it doesn't propagate up to the parent control and trigger a selection event.