From c509b0c582934c964fc845a1dcae979e997d1881 Mon Sep 17 00:00:00 2001 From: David Iglesias Date: Tue, 16 Jul 2024 19:08:37 -0700 Subject: [PATCH] [web] Set touch-action:none in embedded views. (#53945) This PR adds `touch-action:none` to `flutter-view` elements, so the browser lets the flutter engine fully handle all touch gestures. This fix is more delicate than the first approach, which broke some merged taps when accessibility/semantics are enabled. ## Issues * Found while testing: https://github.com/flutter/flutter/issues/130950 * "More correct" fix for: https://github.com/flutter/engine/pull/53647 ## Demos * Flutter scroll: https://dit-multiview-scroll.web.app * Semantics: https://dit-tests.web.app * Scrollable platform views: https://dit-multiview-tests.web.app [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- lib/web_ui/lib/src/engine/pointer_binding.dart | 14 -------------- .../custom_element_embedding_strategy.dart | 5 ++++- lib/web_ui/test/engine/pointer_binding_test.dart | 6 +++--- .../custom_element_embedding_strategy_test.dart | 2 ++ 4 files changed, 9 insertions(+), 18 deletions(-) diff --git a/lib/web_ui/lib/src/engine/pointer_binding.dart b/lib/web_ui/lib/src/engine/pointer_binding.dart index b914a37125b74..5bd9b6f7dc5fa 100644 --- a/lib/web_ui/lib/src/engine/pointer_binding.dart +++ b/lib/web_ui/lib/src/engine/pointer_binding.dart @@ -968,20 +968,6 @@ class _PointerAdapter extends _BaseAdapter with _WheelEventListenerMixin { @override void setup() { - // Prevents the browser auto-canceling pointer events. - // Register into `_listener` directly so the event isn't forwarded to semantics. - _listeners.add(Listener.register( - event: 'touchstart', - target: _viewTarget, - handler: (DomEvent event) { - // This is one of the ways I've seen this done. PixiJS does a similar thing. - // ThreeJS seems to subscribe move/leave in the pointerdown handler instead? - if (event.cancelable) { - event.preventDefault(); - } - }, - )); - _addPointerEventListener(_viewTarget, 'pointerdown', (DomPointerEvent event) { final int device = _getPointerId(event); final List pointerData = []; diff --git a/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart b/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart index 802d81341eb57..d6f73faf30d57 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart @@ -34,7 +34,10 @@ class CustomElementEmbeddingStrategy implements EmbeddingStrategy { ..style.height = '100%' ..style.display = 'block' ..style.overflow = 'hidden' - ..style.position = 'relative'; + ..style.position = 'relative' + // This is needed so the browser lets flutter handle all pointer events + // it receives, without canceling them. + ..style.touchAction = 'none'; hostElement.appendChild(rootElement); diff --git a/lib/web_ui/test/engine/pointer_binding_test.dart b/lib/web_ui/test/engine/pointer_binding_test.dart index e8d90c465836a..9e3354eb8d55d 100644 --- a/lib/web_ui/test/engine/pointer_binding_test.dart +++ b/lib/web_ui/test/engine/pointer_binding_test.dart @@ -305,15 +305,15 @@ void testMain() { }, ); - test('prevents default on touchstart events', () async { + test('allows default on touchstart events', () async { final event = createDomEvent('Event', 'touchstart'); rootElement.dispatchEvent(event); expect( event.defaultPrevented, - isTrue, - reason: 'touchstart events should be prevented so pointer events are not cancelled later.', + isFalse, + reason: 'touchstart events should NOT be prevented. That breaks semantic taps!', ); }); diff --git a/lib/web_ui/test/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy_test.dart b/lib/web_ui/test/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy_test.dart index fd28d14fe2ce0..0a954377f10c5 100644 --- a/lib/web_ui/test/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy_test.dart +++ b/lib/web_ui/test/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy_test.dart @@ -76,6 +76,8 @@ void doTests() { reason: 'Should take 100% of the available height'); expect(styleAfter.overflow, 'hidden', reason: 'Should hide the occasional oversized canvas elements.'); + expect(styleAfter.touchAction, 'none', + reason: 'Should disable browser handling of touch events.'); }); }); }