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

Adding more click functionality #275

Closed
wants to merge 3 commits into from

Conversation

TheTacBanana
Copy link

Adds the ability to detect Right and Middle click on widgets.

Changes are breaking due to changes of Enum names. But relatively easy to fix existing code.

Existing:

if let EventType::Click(..) = event.event_type {
    ...
}       

New:

if let EventType::LeftClick(..) = event.event_type {
    ...
}       

@ethereumdegen
Copy link

ethereumdegen commented Jul 25, 2023

IMO this could be cleaner and more backwards compatible to add a 'button type' to CursorEvent struct and leave the EventType Enum the way it was.

This way, if i want to do something on a left OR right click, i wont have to implement 2 different children in a match statement. I can just check to see if the button type in cursorevent inside of 'MouseDown' is LEFT or RIGHT.

im going to make a PR based on yours with this alternative topology so hold off until we can look at both and compare pls.

@ethereumdegen
Copy link

ethereumdegen commented Jul 26, 2023

okay please see PR #276 it is built off of this one. That PR offers the same functionality, using many of your lines of code but instead of adding new EventTypes it just adds a 'button type' prop to the CursorEvent struct. This way the examples don't even have to change and it doesn't break anyones existing codebases but still offers all the same benefits of being able to detect which mouse button has been pressed.

@TheTacBanana
Copy link
Author

Definitely a better way of implementing it, not sure why I didn't do that myself. I'm in favor of your PR, much simpler.

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