这是indexloc提供的服务,不要输入任何密码
Skip to content

Fix PointerAddedEvent handling in LiveTestWidgetsFlutterBinding #61102

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

Merged
merged 3 commits into from
Jul 8, 2020
Merged

Fix PointerAddedEvent handling in LiveTestWidgetsFlutterBinding #61102

merged 3 commits into from
Jul 8, 2020

Conversation

CareF
Copy link
Contributor

@CareF CareF commented Jul 8, 2020

Description

See #61100

This was part of #60796

Related Issues

Fixes #61100

Tests

packages/flutter_test/test/live_binding_test.dart

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

  • No, no existing tests failed, so this is not a breaking change.

@CareF CareF requested review from dnfield and liyuqian July 8, 2020 15:54
@fluttergithubbot fluttergithubbot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Jul 8, 2020
@CareF CareF self-assigned this Jul 8, 2020
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the detailed explanation in the bug!

@CareF
Copy link
Contributor Author

CareF commented Jul 8, 2020

I realize one thing I need to solve before merge. Now I'm just treating PointerAddedEvent differently, but PointerHoverEvent and PointerRemovedEvent look like possibly having same issue. I don't have a situation/example to see when these events are produced in real environment. I think I need confirm from someone more familiar with the motivation of these PointerEvents. @dnfield @liyuqian

@dnfield
Copy link
Contributor

dnfield commented Jul 8, 2020

@CareF I think you'd have an easier time getting those events if you tested in web/chrome or on desktop (e.g. flutter run -d macos and then run the mouse cursor over the app).

It's fine to fix those in a separate PR if you don't have good test cases for them yet - or, if you can generate the test case, then you can go ahead and fix it here too.

Copy link
Contributor

@liyuqian liyuqian left a comment

Choose a reason for hiding this comment

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

LGTM. +1 to fix this first, and fix hover and other possible events later with more knowledge and reproducible test cases.

@CareF CareF merged commit 61477b5 into flutter:master Jul 8, 2020
@CareF CareF deleted the fix_pointeradded branch July 8, 2020 17:32
@CareF CareF mentioned this pull request Jul 8, 2020
9 tasks
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LiveTestWidgetsFlutterBinding is not handling some PointerEvent correctly.
5 participants