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

Add keyboard modifiers to mouse events #2733

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

edwloef
Copy link
Contributor

@edwloef edwloef commented Jan 18, 2025

This PR makes the current state of keyboard modifiers available in mouse click and scroll events.

Before, implementing mouse+modifier shortcuts meant the need to save the state of the modifier keys in your app or widget state, and then track changes to the current modifier keys via Event::Keyboard(keyboard::Event::ModifiersChanged(modifiers)), which is cumbersome.

https://discourse.iced.rs/t/should-mouse-events-expose-the-state-of-the-modifier-keys/577
#1868
#1914

)),
Event::Mouse(mouse::Event::ButtonPressed {
button: mouse::Button::Left,
modifiers: Modifiers::empty(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure hard-coding the modifiers to be empty here is correct, open to suggestions if it isn't :)

@edwloef edwloef force-pushed the mouse-event-modifiers branch from 35cf791 to 99b2b35 Compare January 18, 2025 10:06
@edwloef
Copy link
Contributor Author

edwloef commented Jan 19, 2025

Another idea I've had recently is to pass the mouse position in the ButtonPressed message as it is in the CursorMoved message, since that seems like a logical thing to want when a mouse button is pressed, but it's considerably less inconvenient to get that so I just haven't done it yet. If anyone thinks that would be a good idea as well I can add it here or make a new PR.

@airstrike
Copy link
Contributor

airstrike commented Jan 19, 2025

Another idea I've had recently is to pass the mouse position in the ButtonPressed message as it is in the CursorMoved message, since that seems like a logical thing to want when a mouse button is pressed, but it's considerably less inconvenient to get that so I just haven't done it yet. If anyone thinks that would be a good idea as well I can add it here or make a new PR.

Sounds to me like that's for a separate PR. I see the appeal and I think the library should default to providing as much information as possible with the messages that are produced, unless there is a meaningful cost associated with it. For example, mouse_area should also report the position in which a click happened, possibly also the Point in which the cursor entered the area, etc. Many won't care about this additional information, but it would be very valuable to applications with very fine grained interactivity without forcing them to create their own widget.

The counter argument is that creating your own widget isn't that hard and you could always make a button which includes this information in a third-party crate or directly in your app...

@edwloef
Copy link
Contributor Author

edwloef commented Jan 19, 2025

The counter argument is that creating your own widget isn't that hard and you could always make a button which includes this information in a third-party crate or directly in your app...

This is exactly why I haven't done this yet, however it shouldn't be much work. I'll probably just implement it in the coming days, then Héctor can merge or close it depending on whether he finds it to be a good fit.

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