-
Notifications
You must be signed in to change notification settings - Fork 6k
Detect pointer leaving the window on web #33057
Conversation
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. |
|
@dkwingsmt @mdebbar I'm told you might be able to help out with a review to push this forward. I understand that the interop layer is being changed, and I'm happy to make these commits comply as well, if necessary. |
dkwingsmt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've briefly read through. It looks good! I'll read a second time soon.
mdebbar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to work on this!
I've taken a first look and left some comments.
|
I've made the requested changes, ran the tests, and everything looks good on this side. If there's anything else for me to do, let me know. |
mdebbar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after fixing the double allowInterop.
| _listeners.add(_Listener.registerNative( | ||
| event: 'wheel', | ||
| target: glassPaneElement, | ||
| handler: allowInterop((html.Event event) => handler(event)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The handler is being wrapped twice in allowInterop (here and in _Listener.registerNative).
| handler: allowInterop((html.Event event) => handler(event)), | |
| handler: (html.Event event) => handler(event), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, that was indeed left over by mistake.
This makes `onExit` work for `MouseRegion` on the web even if it touches the edges of the window.
This makes
onExitwork forMouseRegionon the web even if it touches the edges of the window.Resolves #78280.
Pre-launch Checklist
///).