Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@skreborn
Copy link
Contributor

@skreborn skreborn commented May 2, 2022

This makes onExit work for MouseRegion on the web even if it touches the edges of the window.

Resolves #78280.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label May 2, 2022
@flutter-dashboard
Copy link

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.

@skreborn
Copy link
Contributor Author

@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.

Copy link
Contributor

@dkwingsmt dkwingsmt left a 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.

Copy link
Contributor

@mdebbar mdebbar left a 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.

@skreborn
Copy link
Contributor Author

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.

Copy link
Contributor

@mdebbar mdebbar left a 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)),
Copy link
Contributor

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).

Suggested change
handler: allowInterop((html.Event event) => handler(event)),
handler: (html.Event event) => handler(event),

Copy link
Contributor Author

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.
@mdebbar mdebbar merged commit 6734def into flutter:main Jun 2, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 2, 2022
@skreborn skreborn deleted the pointer-leave branch June 3, 2022 10:13
houhuayong pushed a commit to houhuayong/engine that referenced this pull request Jun 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Web - MouseRegion triggering error outside the window.

3 participants