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.

Sign up for GitHub

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

Jump to bottom

[pointer_interceptor_web] Update package APIs and tests. #5785

Merged
merged 8 commits into from
Jan 3, 2024
Merged

[pointer_interceptor_web] Update package APIs and tests. #5785

merged 8 commits into from
Jan 3, 2024

Conversation

ditman
Copy link
Member

@ditman ditman commented Jan 2, 2024 •
edited
Loading

Changes:

  • Updates the web implementation of the pointer_interceptor to package:web, and the latest HtmlElementView APIs available on Flutter.
  • Resolves a timing issue in integration tests by waiting a few frames before doing any assertions from the DOM of a Flutter Web app, that should fix flutter rolls.

Issues:

Tests

  • Tested manually. All integration tests pass locally.

Co-authored-by: balvinderz <b📧2@g📬l.com>

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 [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style].
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@ditman
Copy link
Member Author

ditman commented Jan 2, 2024 •
edited
Loading

/cc @mdebbar migrated the package to the new HtmlElementView.fromTagName API, it simplifies the code a lot (as if the API was added for code like this 😜)!

@ditman ditman mentioned this pull request Jan 2, 2024
Closed
11 tasks
tarrinneal
tarrinneal approved these changes Jan 3, 2024
View reviewed changes
Copy link
Contributor

@tarrinneal tarrinneal left a comment

Choose a reason for hiding this comment

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

Approved, might be worth waiting for @stuartmorgan to take a look though.

packages/pointer_interceptor/pointer_interceptor_web/example/integration_test/widget_test.dart Outdated Show resolved Hide resolved
mdebbar
mdebbar approved these changes Jan 3, 2024
View reviewed changes
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.

Looks good, thanks for the cleanup!

The only thing I would recommend is making onElementCreated a method so we don't keep creating a new closure on every build.

stuartmorgan
stuartmorgan approved these changes Jan 3, 2024
View reviewed changes
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM

packages/pointer_interceptor/pointer_interceptor_web/example/integration_test/widget_test.dart Show resolved Hide resolved
packages/pointer_interceptor/pointer_interceptor_web/example/lib/main.dart Show resolved Hide resolved
packages/pointer_interceptor/pointer_interceptor_web/lib/pointer_interceptor_web.dart Show resolved Hide resolved
@ditman ditman force-pushed the fix-pointer-interceptor-web-tests branch from 7b31df1 to 9ab2360 Compare January 3, 2024 19:06
@ditman ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 3, 2024
@ditman
Copy link
Member Author

ditman commented Jan 3, 2024

Rolling the 🎲 with autosubmit!

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 3, 2024
@auto-submit Auto-submit
Copy link
Contributor

auto-submit bot commented Jan 3, 2024

auto label is removed for flutter/packages/5785, due to - The status or check suite Linux_android android_platform_tests_shard_6 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 3, 2024
@stuartmorgan
Copy link
Contributor

🎲

@stuartmorgan
Copy link
Contributor

Rolling the 🎲 with autosubmit!

🎲

Maybe the problem is that the die keeps coming up "1" 🙂

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 3, 2024
@auto-submit Auto-submit
Copy link
Contributor

auto-submit bot commented Jan 3, 2024

auto label is removed for flutter/packages/5785, due to - The status or check suite Linux_android android_platform_tests_shard_5 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@ditman
Copy link
Member Author

ditman commented Jan 3, 2024 •
edited
Loading

Maybe the problem is that the die keeps coming up "1" 🙂

Those dies are not to spec! RFC 1149.5 specifies 4 as the standard IEEE-vetted random number.

@tarrinneal tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 3, 2024
@tarrinneal
Copy link
Contributor

My turn 🎲 🎲 🎲 🎲 🎲 🎲 (6)

@auto-submit Auto-submit
Copy link
Contributor

auto-submit bot commented Jan 3, 2024

auto label is removed for flutter/packages/5785, due to - The status or check suite Linux_android android_platform_tests_shard_5 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 3, 2024
@ditman ditman force-pushed the fix-pointer-interceptor-web-tests branch from 9ab2360 to df9e305 Compare January 3, 2024 21:26
@ditman
Copy link
Member Author

ditman commented Jan 3, 2024 •
edited
Loading

Removed bump to the master version, it seems other shards were breaking because of it.

🎲

@ditman ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 3, 2024
@auto-submit Auto-submit
Copy link
Contributor

auto-submit bot commented Jan 3, 2024

auto label is removed for flutter/packages/5785, due to - The status or check suite Linux_android android_platform_tests_api_33_shard_2 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 3, 2024
@ditman ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 3, 2024
@auto-submit auto-submit bot merged commit 6b2249f into flutter:main Jan 3, 2024
80 checks passed
@ditman ditman deleted the fix-pointer-interceptor-web-tests branch January 3, 2024 22:48
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 4, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 4, 2024
@engine-flutter-autoroll
6b2249f [pointer_interceptor_web] Update package APIs and tests. (flu…
0856f04 
…tter/packages#5785)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 4, 2024
@engine-flutter-autoroll
Roll Packages from bbb4134 to 31fc7b5 (6 revisions) (#140967)
6ad032b 
flutter/packages@bbb4134...31fc7b5

2024-01-04 christopherfujino@gmail.com bump mockito across repo to 5.4.4 (flutter/packages#5794)
2024-01-03 goderbauer@google.com [various] Sync lints with flutter/flutter (flutter/packages#5717)
2024-01-03 ditman@gmail.com [pointer_interceptor_web] Update package APIs and tests. (flutter/packages#5785)
2024-01-03 stuartmorgan@google.com [google_sign_in] Correct method channel `clearAuthCache` declaration (flutter/packages#5787)
2024-01-03 stuartmorgan@google.com [google_sign_in] Correct clearAuthCache declarations (flutter/packages#5693)
2024-01-03 15619084+vashworth@users.noreply.github.com Unpin mac_toolchain version (flutter/packages#5683)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@reidbaker reidbaker mentioned this pull request Jan 12, 2024
Closed
@reidbaker reidbaker mentioned this pull request Jan 19, 2024
Merged
12 tasks
@ditman ditman mentioned this pull request Jan 25, 2024
Closed
@reidbaker reidbaker mentioned this pull request Feb 19, 2024
Merged
16 tasks
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
@ditman @balvinderz @arc-yong
[pointer_interceptor_web] Update package APIs and tests. (flutter#5785)
1c2f81e 
## Changes:

* Updates the web implementation of the `pointer_interceptor` to `package:web`, and the latest `HtmlElementView` APIs available on Flutter.
* Resolves a timing issue in integration tests by waiting a few frames before doing any assertions from the DOM of a Flutter Web app, that should fix flutter rolls.

## Issues:

* Fixes flutter/flutter#140834
* Fixes flutter/flutter#139753
* Closes flutter#5673 (Credit to @balvinderz!)

## Tests

* Tested manually. All integration tests pass locally.

---

Co-authored-by: balvinderz <balvindersi2@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuartmorgan stuartmorgan stuartmorgan approved these changes

@mdebbar mdebbar mdebbar approved these changes

@tarrinneal tarrinneal tarrinneal approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: pointer_interceptor platform-web
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[pointer_interceptor_web] Breaking the packages roll. [pointer_interceptor] migrate to pkg:web
4 participants
@ditman @stuartmorgan @tarrinneal @mdebbar