Skip to content
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

Closed
wants to merge 4 commits into from

Conversation

Tiomat85
Copy link
Contributor

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.

Tiomat85 and others added 3 commits July 22, 2024 10:25
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.
@cmeyer
Copy link
Collaborator

cmeyer commented Jul 30, 2024

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...?

nion/ui/QtUserInterface.py Outdated Show resolved Hide resolved
nion/ui/QtUserInterface.py Outdated Show resolved Hide resolved
Correcting typing issue and whitespace

Removed double-space and adding a False return indicating that the event has not been handled.
Copy link
Collaborator

@cmeyer cmeyer left a 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.

Comment on lines +3426 to +3431
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
Copy link
Collaborator

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

Copy link
Contributor Author

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.

Copy link
Collaborator

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:

nion-software/nionswift#1130

@cmeyer
Copy link
Collaborator

cmeyer commented Aug 14, 2024

Closing in favor of the solution here nion-software/nionswift#1130

See notes here, too nion-software/nionui-tool#63

@cmeyer cmeyer closed this Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants