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

[web] Cleanup touch and mouse event adapters #43697

Merged
merged 3 commits into from
Nov 13, 2023

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Jul 14, 2023

This PR cleans up a big source of complexity in our pointer handling code. It also drops support for Safari versions prior to 13 (caniuse).

Right now, we throw a hard error when running in a browser that doesn't support PointerEvents. Should we turn it into a soft warning, and just disable gesture/pointer handling on those browsers?

For users who need to support older versions of Safari, they can try this polyfill: https://github.com/wessberg/pointer-events (thanks @ditman!)

Fixes flutter/flutter#116141

@mdebbar mdebbar requested review from yjbanov and ditman July 14, 2023 19:28
@github-actions github-actions bot added the platform-web Code specifically for the web engine label Jul 14, 2023
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

From what I can find, this drops support for: iPhone 6 and lower (5S, etc...) (those are stuck forever in iOS 12).

The oldest model that we'd support would be: iPhone 6S (iOS 15.7), released in September, 2018 2015.

Should we deploy an app with this change applied so we can take a look at it with a bunch of devices?

if (_defaultSupportDetector.hasPointerEvents) {
event = createDomPointerEvent('pointermove');
} else if (_defaultSupportDetector.hasTouchEvents) {
event = createDomTouchEvent('touchcancel');
Copy link
Member

Choose a reason for hiding this comment

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

createDomTouchEvent and createDomMouseEvent are now probably unused in dom.dart?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

createDomTouchEvent yes, but createDomMouseEvent is used in other places.

PointerBinding(
this.flutterViewElement,
this._keyboardConverter, [
this._detector = const PointerSupportDetector(),
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this PointerSupportDetector anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was (and still is) useful for testing

@mdebbar
Copy link
Contributor Author

mdebbar commented Jul 14, 2023

From what I can find, this drops support for: iPhone 6 and lower (5S, etc...) (those are stuck forever in iOS 12).

The oldest model that we'd support would be: iPhone 6S (iOS 15.7), released in September, 2018.

Bummer 😞 maybe we should wait a little longer?

Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

LGTM if LGT @ditman

@ditman
Copy link
Member

ditman commented Jul 14, 2023

Bummer 😞 maybe we should wait a little longer?

I'm gathering some data about usage, maybe we can extract some guideline to know when something can be deprecated :P

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

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

(Approving because I didn't have other than nits)

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 13, 2023
@auto-submit auto-submit bot merged commit 77b952f into flutter:main Nov 13, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 13, 2023
fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Nov 14, 2023
…138377)

flutter/engine@046ec85...77b952f

2023-11-13 [email protected] [web] Cleanup touch and mouse event adapters (flutter/engine#43697)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] 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
@mdebbar mdebbar deleted the pointer_adapters branch November 14, 2023 15:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[web] Cleanup: TouchEvents, MouseEvents in Pointer Binding.
3 participants