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

Implemented mouse release handling #110

Merged
merged 6 commits into from Jan 8, 2022

Conversation

Kavantix
Copy link

@Kavantix Kavantix commented Dec 26, 2021

This PR adds/fixes a few things:

  • Send mouse button events on mouse down just like the termbox version did
  • Send mouse release event only after all buttons have been released (Closes [BUG] Mouse handling delayed in neovim terminal #99)
  • Track mouse position and expose it to the user to allow for creating drag interactions
  • Add all these features to the example:
    • Clicking on one of the buttons shows the message after down event
    • Message can be dragged around
    • Message is deleted if the user does not move the move and releases the button again
    • Only highlight hovered view (Closes Allow removing view cursor to "un-highlight" all lines #108)
    • Dragging based on the border using the mouseposition
    • Showing mouse down and up anywhere outside the buttons

@Kavantix Kavantix force-pushed the mouse-release branch 2 times, most recently from ccdef6c to 266011f Compare December 27, 2021 09:45
gui.go Show resolved Hide resolved
gui.go Show resolved Hide resolved
@Kavantix Kavantix requested a review from mjarkk December 31, 2021 11:21
@mjarkk mjarkk requested a review from dankox December 31, 2021 17:41
@Kavantix
Copy link
Author

Kavantix commented Jan 2, 2022

@mjarkk I was thinking about this feature a bit more today and I am not sure anymore whether this is the best solution.
Currently the release event is triggered on the view the mouse is at when it is released, however it would be better if we always trigger the release event on the view that triggered the initial down event, this way it will be a lot easier to add keybindings for users.

Also, currently a mouse press can only be detected on a view, it would make more sense if a global mouse key binding would also trigger if no view is under the mouse on the down event.

@dankox
Copy link

dankox commented Jan 7, 2022

I'm sorry for the delay, was a bit off during the holidays so didn't have time to check what's going on.

This looks good.
@Kavantix regarding your concern. I was thinking about it and it kinda makes sense, but on the other hand, it could make sense to send that event to the view which the mouse is over (like a drag where the dragged view is not moving, but would be moved after mouse release) for some cases.
However, both case could be implemented with the current implementation, so I guess it's more up to what is more convenient.

Not really sure though. Maybe there could be an option where the user could choose where the MouseRelease event is sent to (the originate view or the target view like gui.MouseReleaseOrigin bool?).

@Kavantix
Copy link
Author

Kavantix commented Jan 7, 2022

@dankox I agree on having the option.
The only question then is what should be the default?
I still think that having the view from the down event as default makes the most sense

@dankox
Copy link

dankox commented Jan 7, 2022

The release on origin view as default make sense. But I'm not sure from "backward compatibility". I'm not even sure if anybody was using it before the tcell version (because in tcell version the MouseRelease probably never worked as intended), but in the original termbox version the MouseRelease was there and was triggered on the view where it was released (assuming from the code).

Anyway, this is kinda tricky question and not sure. @mjarkk what do you think about it?

@Kavantix
Copy link
Author

Kavantix commented Jan 7, 2022

Hmm good point, from that standpoint it should by default use the old behaviour

@mjarkk
Copy link
Member

mjarkk commented Jan 8, 2022

Not really sure though. Maybe there could be an option where the user could choose where the MouseRelease event is sent to (the originate view or the target view like gui.MouseReleaseOrigin bool?).

I don't think we should add such an option.

How about this.. The mouse down and up are triggered on the views they happen on but the mouse event data add an extra property like MouseOriginView with some good documentation describing the property?

@Kavantix
Copy link
Author

Kavantix commented Jan 8, 2022

Hmm that sounds confusing.
Let’s move this discussion to a new issue though since either way the behavior that is currently here is backwards compatible with the termbox version.
So I think we can merge this.

@mjarkk
Copy link
Member

mjarkk commented Jan 8, 2022

Good point, lets merge this first.

@mjarkk mjarkk merged commit df3d389 into awesome-gocui:master Jan 8, 2022
@Kavantix Kavantix deleted the mouse-release branch January 8, 2022 16:38
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.

Allow removing view cursor to "un-highlight" all lines [BUG] Mouse handling delayed in neovim terminal
3 participants