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

Standardize Pointer Event Names with WinUI #8058

Closed
robloo opened this issue Apr 30, 2022 Discussed in #7907 · 4 comments · Fixed by #8396
Closed

Standardize Pointer Event Names with WinUI #8058

robloo opened this issue Apr 30, 2022 Discussed in #7907 · 4 comments · Fixed by #8396
Labels

Comments

@robloo
Copy link
Contributor

robloo commented Apr 30, 2022

Discussed in #7907

Avalonia has the following primary pointer events:

PointerEnter
PointerLeave
PointerMoved
PointerPressed
PointerReleased

UWP/WinUI, which was the basis for this implementation, has:

PointerEntered
PointerExited
PointerMoved
PointerPressed
PointerReleased
PointerCanceled
PointerCaptureLost

The first observation is a good simplification. UWP has PointerCanceled, PointerCaptureLost and PointerExited which are all used in slightly different situations. UWP apps must actually handle all of these events as PointerExited may never fire after PointerEntered. My assumption is Avalonia cleaned all of this up and fires PointerEnter and PointerLeave consistently doing away with the other two.

However, then we get into naming:

UWP PointerEntered vs Avalonia PointerEnter. Almost all events are past-tense and almost ALWAYS end in 'ed'. This was changed in Avalonia so it no longer matches the other event names or standard convention. In my opinion PointerEnter is misnamed and should be PointerEntered like in UWP.

Then we have PointerLeave. This was probably renamed because it no longer works exactly like UWP -- as mentioned it handles all cases where pointer exits the control for whatever reason. however, again it doesn't end in 'ed' and isn't well paired with the term 'enter'. In English the terms are matched "Enter"/"Exit" (most common) and "Arrive"/"Leave" (slightly less common) as antonyms. So in my opinion this never should have been changed and should have retained the UWP naming PointerExited.


As agreed in the discussion, two events should be renamed as follows:

  1. PointerEnter -> PointerEntered
  2. PointerLeave -> PointerExited

I will submit a PR for this.

@maxkatz6
Copy link
Member

maxkatz6 commented May 1, 2022

UWP apps must actually handle all of these events as PointerExited may never fire after PointerEntered. My assumption is Avalonia cleaned all of this up and fires PointerEnter and PointerLeave consistently doing away with the other two.

Basically, that's how it's handled at least with windows backend, where we have all of these three native events (at least in WM_POINTER PR).

But there is also PointerCaptureLost in avalonia, which can't be easily replaced with PointerExited, if you want to track the capture.

As agreed in the discussion, two events should be renamed as follows

It's annoying for everybody to rename events, but also annoying to keep wrong naming (or not wrong, but unconventional). So it's better to rename now.

There are more API deletions are planned for this release as well (most of obsolete members in Pointer apis).

@robloo
Copy link
Contributor Author

robloo commented Jun 24, 2022

@maxkatz6 @grokys This also should be done by 11.0 and I'm not sure the best time to do it. I would like to wait until compositor and the big styling PRs are done first but it might not matter much.

When do you think is a good time to make this switch? Yes, I know everyone is going to hate me for a while.

@maxkatz6
Copy link
Member

@robloo I think it's not really depended on compositor and control themes, so it can be done in parallel. Anyway, it should be done in first preview version, maybe preview1 or preview2. As these versions should have as much breaking changes as possible.

First preview is planned to be released after new renderer merged. Probably it will wait for control themes too.

@robloo
Copy link
Contributor Author

robloo commented Jun 26, 2022

@maxkatz6 Thanks, I went ahead and created the PR #8396 to be merged when you all think best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants