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 mouse buttons to mouse event args #85

Merged
merged 1 commit into from Jun 24, 2023
Merged

Conversation

tomrijnbeek
Copy link
Member

✨ What's this?

This PR updates the MouseEventArgs to include the currently held down mouse buttons in the same way as we pass in the modifier keys.

πŸ”— Relationships

Provides a helpful step towards #5

πŸ” Why do we want this?

After looking into how to implement drag and drop, I decided to investigate how WPF does it, which basically lets controls themselves decide when a drag starts (this because very few controls actually need to support it). So what we want to be able to do is in the OnMouseMoved handler check whether the mouse button was pressed while moving the mouse, as this is a pretty good indication we're starting a drag.

Problem is: right now we can't.

This PR makes the change to include this information in each of the MouseEventArgs objects so that this information becomes available.

πŸ— How is it done?

The MouseButtons struct follows the ModifierKeys struct structure very closely, and works in a similar way. This is then passed into the MouseEventArgs, which also acts as base class for all other mouse event args.

πŸ’₯ Breaking changes

The constructors of the mouse event args are public, and they now require an extra parameter, so all previous calls are broken. I considered adding a constructor overload, but I feel it is weird that MouseButtons would be optional.

πŸ”¬ Why not another way?

It may be considered weird that MouseButtons is present in even the MouseClickEventArgs, but it opens up the option of responding to "left and right" click at the same time, and same for scrolling. If we wanted to limit the MouseButtons to the OnMouseMoved event, we would have to either introduce a new MouseMoveEventArgs, which would break all existing overrides of that method (much bigger breaking change), or we would remove the MouseEventArgs as base class of the other two, which would also have unintended side effects.

@tomrijnbeek tomrijnbeek added the breaking change Issues that would lead to breaking change, or PRs that include breaking changes label Jun 21, 2023

namespace Bearded.UI.EventArgs
{
public readonly struct MouseButtons : IEquatable<MouseButtons>
Copy link
Member

Choose a reason for hiding this comment

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

Not a record?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to keep the constructor private (in line with the modifier keys), which is not something you can do with records. The constructor parameters are very non-expressive, so having the builder methods to build the object instead makes much more readable code. So yeah... record would be nice if they allowed private constructors, but unfortunately there will always be a positional constructor.

Copy link
Member

Choose a reason for hiding this comment

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

Understood!

@tomrijnbeek tomrijnbeek merged commit 757a3c7 into main Jun 24, 2023
1 check passed
@tomrijnbeek tomrijnbeek deleted the mouse-buttons branch June 24, 2023 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Issues that would lead to breaking change, or PRs that include breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants