From 792706d233c9b47149bf127e2bd056513f5cece8 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 16 Oct 2020 12:23:24 -0700 Subject: [PATCH 1/4] Compute wheel line scroll distance --- .../lib/src/engine/pointer_binding.dart | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/lib/src/engine/pointer_binding.dart b/lib/web_ui/lib/src/engine/pointer_binding.dart index f6bb53110491b..94db4839dd994 100644 --- a/lib/web_ui/lib/src/engine/pointer_binding.dart +++ b/lib/web_ui/lib/src/engine/pointer_binding.dart @@ -229,6 +229,8 @@ abstract class _BaseAdapter { } mixin _WheelEventListenerMixin on _BaseAdapter { + static double? _defaultScrollLineHeight; + List _convertWheelEventToPointerData( html.WheelEvent event ) { @@ -242,8 +244,9 @@ mixin _WheelEventListenerMixin on _BaseAdapter { double deltaY = event.deltaY as double; switch (event.deltaMode) { case domDeltaLine: - deltaX *= 32.0; - deltaY *= 32.0; + _defaultScrollLineHeight ??= _computeDefaultScrollLineHeight(); + deltaX *= _defaultScrollLineHeight!; + deltaY *= _defaultScrollLineHeight!; break; case domDeltaPage: deltaX *= ui.window.physicalSize.width; @@ -253,6 +256,7 @@ mixin _WheelEventListenerMixin on _BaseAdapter { default: break; } + print(deltaY); final List data = []; _pointerDataConverter.convert( data, @@ -287,6 +291,24 @@ mixin _WheelEventListenerMixin on _BaseAdapter { ] ); } + + /// For browsers that report delta line instead of pixels such as FireFox + /// compute line height using the default font size. + double _computeDefaultScrollLineHeight() { + const double kFallbackFontHeight = 16.0; + final html.DivElement probe = html.DivElement(); + probe.style + ..fontSize = 'initial' + ..display = 'none'; + html.document.body!.append(probe); + String fontSize = probe.getComputedStyle().fontSize; + double? res; + if (fontSize.contains('px')) { + fontSize = fontSize.replaceAll('px', ''); + res = double.tryParse(fontSize); + } + return res == null ? kFallbackFontHeight : res! / 4.0; + } } @immutable From b7903b4e706864b44f388837553082d6a8be98f8 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 16 Oct 2020 14:31:30 -0700 Subject: [PATCH 2/4] Fix Firefox wheel scroll delta --- .../lib/scroll_wheel_main.dart | 154 ++++++++++++++++++ .../test_driver/scroll_wheel_integration.dart | 30 ++++ .../scroll_wheel_integration_test.dart | 9 + .../lib/src/engine/pointer_binding.dart | 4 +- 4 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 e2etests/web/regular_integration_tests/lib/scroll_wheel_main.dart create mode 100644 e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration.dart create mode 100644 e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration_test.dart diff --git a/e2etests/web/regular_integration_tests/lib/scroll_wheel_main.dart b/e2etests/web/regular_integration_tests/lib/scroll_wheel_main.dart new file mode 100644 index 0000000000000..76fcf7cf49815 --- /dev/null +++ b/e2etests/web/regular_integration_tests/lib/scroll_wheel_main.dart @@ -0,0 +1,154 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +import 'dart:html' as html; +import 'dart:js_util' as js_util; + +import 'package:flutter/material.dart'; + +void main() => runApp(MyApp()); + +class MyApp extends StatelessWidget { + // This widget is the root of your application. + @override + Widget build(BuildContext context) { + return MaterialApp( + title: 'Flutter Scroll Wheel Test', + theme: ThemeData( + // This is the theme of your application. + // + // Try running your application with "flutter run". You'll see the + // application has a blue toolbar. Then, without quitting the app, try + // changing the primarySwatch below to Colors.green and then invoke + // "hot reload" (press "r" in the console where you ran "flutter run", + // or simply save your changes to "hot reload" in a Flutter IDE). + // Notice that the counter didn't reset back to zero; the application + // is not restarted. + primarySwatch: Colors.blue, + // This makes the visual density adapt to the platform that you run + // the app on. For desktop platforms, the controls will be smaller and + // closer together (more dense) than on mobile platforms. + visualDensity: VisualDensity.adaptivePlatformDensity, + ), + home: MyHomePage(title: 'Flutter Scroll Wheel Test'), + ); + } +} + +class MyHomePage extends StatefulWidget { + MyHomePage({Key key, this.title}) : super(key: key); + + // This widget is the home page of your application. It is stateful, meaning + // that it has a State object (defined below) that contains fields that affect + // how it looks. + + // This class is the configuration for the state. It holds the values (in this + // case the title) provided by the parent (in this case the App widget) and + // used by the build method of the State. Fields in a Widget subclass are + // always marked "final". + + final String title; + + @override + _MyHomePageState createState() => _MyHomePageState(); +} + +class _MyHomePageState extends State { + int _counter = 0; + + void _incrementCounter() { + setState(() { + // This call to setState tells the Flutter framework that something has + // changed in this State, which causes it to rerun the build method below + // so that the display can reflect the updated values. If we changed + // _counter without calling setState(), then the build method would not be + // called again, and so nothing would appear to happen. + _counter++; + }); + } + + @override + Widget build(BuildContext context) { + // This method is rerun every time setState is called, for instance as done + // by the _incrementCounter method above. + // + // The Flutter framework has been optimized to make rerunning build methods + // fast, so that you can just rebuild anything that needs updating rather + // than having to individually change instances of widgets. + return Scaffold( + appBar: AppBar( + // Here we take the value from the MyHomePage object that was created by + // the App.build method, and use it to set our appbar title. + title: Text(widget.title), + ), + body: ListView.builder( + itemCount: 1000, + itemBuilder: (context, index) => Padding( + padding: EdgeInsets.all(20), + child: Container( + height: 100, + color: Colors.lightBlue, + child: Center( + child: Text("Item $index"), + ), + ), + ), + ), + floatingActionButton: + FloatingActionButton.extended( + key: const Key('scroll-button'), + onPressed: () { + final int centerX = 100; //html.window.innerWidth ~/ 2; + final int centerY = 100; //html.window.innerHeight ~/ 2; + dispatchMouseWheelEvent(centerX, centerY, DeltaMode.kLine, 0, 1); + dispatchMouseWheelEvent(centerX, centerY, DeltaMode.kLine, 0, 1); + dispatchMouseWheelEvent(centerX, centerY, DeltaMode.kLine, 0, 1); + dispatchMouseWheelEvent(centerX, centerY, DeltaMode.kLine, 0, 1); + dispatchMouseWheelEvent(centerX, centerY, DeltaMode.kLine, 0, 1); + }, + label: Text('Scroll'), + icon: Icon(Icons.thumb_up), + ), + ); + } +} + + +abstract class DeltaMode { + static const int kPixel = 0x00; + static const int kLine = 0x01; + static const int kPage = 0x02; +} + +html.WheelEvent dispatchMouseWheelEvent(int mouseX, int mouseY, + int deltaMode, double deltaX, double deltaY, + {bool shiftKeyPressed = false}) { + html.EventTarget target = html.document.elementFromPoint(mouseX, mouseY); + + target.dispatchEvent(html.MouseEvent("mouseover", + screenX: mouseX, + screenY: mouseY, + clientX: mouseX, + clientY: mouseY, + )); + + target.dispatchEvent(html.MouseEvent("mousemove", + screenX: mouseX, + screenY: mouseY, + clientX: mouseX, + clientY: mouseY, + )); + + html.WheelEvent event = html.WheelEvent('wheel', + screenX: mouseX, + screenY: mouseY, + clientX: mouseX, + clientY: mouseY, + deltaMode: deltaMode, + deltaX : deltaX, + deltaY : deltaY, + shiftKey: shiftKeyPressed, + ); + target.dispatchEvent(event); + return event; +} diff --git a/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration.dart b/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration.dart new file mode 100644 index 0000000000000..39f6ac85610f9 --- /dev/null +++ b/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration.dart @@ -0,0 +1,30 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:html' as html; +import 'dart:js_util' as js_util; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:regular_integration_tests/scroll_wheel_main.dart' as app; + +import 'package:integration_test/integration_test.dart'; + +void main() { + final IntegrationTestWidgetsFlutterBinding binding = IntegrationTestWidgetsFlutterBinding.ensureInitialized() as IntegrationTestWidgetsFlutterBinding; + + testWidgets('Test mousewheel scroll by line', + (WidgetTester tester) async { + app.main(); + await tester.pumpAndSettle(); + + final Finder finder = find.byKey(const Key('scroll-button')); + expect(finder, findsOneWidget); + await tester.tap(find.byKey(const Key('scroll-button'))); + await tester.pumpAndSettle(); + await tester.tap(find.byKey(const Key('scroll-button'))); + await tester.pumpAndSettle(); + + await binding.takeScreenshot('wheel_scroll_by_line'); + }); +} diff --git a/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration_test.dart b/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration_test.dart new file mode 100644 index 0000000000000..9c2c0fdcadc77 --- /dev/null +++ b/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration_test.dart @@ -0,0 +1,9 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:regular_integration_tests/screenshot_support.dart' as test; + +Future main() async { + await test.runTestWithScreenshots(); +} diff --git a/lib/web_ui/lib/src/engine/pointer_binding.dart b/lib/web_ui/lib/src/engine/pointer_binding.dart index 94db4839dd994..b5d5ec0db480c 100644 --- a/lib/web_ui/lib/src/engine/pointer_binding.dart +++ b/lib/web_ui/lib/src/engine/pointer_binding.dart @@ -256,7 +256,7 @@ mixin _WheelEventListenerMixin on _BaseAdapter { default: break; } - print(deltaY); + final List data = []; _pointerDataConverter.convert( data, @@ -294,6 +294,8 @@ mixin _WheelEventListenerMixin on _BaseAdapter { /// For browsers that report delta line instead of pixels such as FireFox /// compute line height using the default font size. + /// + /// Use Firefox to test this code path. double _computeDefaultScrollLineHeight() { const double kFallbackFontHeight = 16.0; final html.DivElement probe = html.DivElement(); From 29f08e069e83501a1f480b77f7e6d3300c2b5d52 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 16 Oct 2020 15:52:44 -0700 Subject: [PATCH 3/4] Remove ! check --- lib/web_ui/lib/src/engine/pointer_binding.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/pointer_binding.dart b/lib/web_ui/lib/src/engine/pointer_binding.dart index b5d5ec0db480c..0fe36865c60db 100644 --- a/lib/web_ui/lib/src/engine/pointer_binding.dart +++ b/lib/web_ui/lib/src/engine/pointer_binding.dart @@ -309,7 +309,7 @@ mixin _WheelEventListenerMixin on _BaseAdapter { fontSize = fontSize.replaceAll('px', ''); res = double.tryParse(fontSize); } - return res == null ? kFallbackFontHeight : res! / 4.0; + return res == null ? kFallbackFontHeight : res / 4.0; } } From dace332dd14a433151698f2daf093b170ddd6833 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Thu, 29 Oct 2020 09:48:46 -0700 Subject: [PATCH 4/4] address reviewer comments. disable screenshot until infra issue is fixed --- .../web/regular_integration_tests/lib/scroll_wheel_main.dart | 1 + .../test_driver/scroll_wheel_integration.dart | 2 ++ .../test_driver/scroll_wheel_integration_test.dart | 4 +++- lib/web_ui/lib/src/engine/pointer_binding.dart | 1 + 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/e2etests/web/regular_integration_tests/lib/scroll_wheel_main.dart b/e2etests/web/regular_integration_tests/lib/scroll_wheel_main.dart index 76fcf7cf49815..b4eedfe08432c 100644 --- a/e2etests/web/regular_integration_tests/lib/scroll_wheel_main.dart +++ b/e2etests/web/regular_integration_tests/lib/scroll_wheel_main.dart @@ -25,6 +25,7 @@ class MyApp extends StatelessWidget { // Notice that the counter didn't reset back to zero; the application // is not restarted. primarySwatch: Colors.blue, + fontFamily: 'RobotoMono', // This makes the visual density adapt to the platform that you run // the app on. For desktop platforms, the controls will be smaller and // closer together (more dense) than on mobile platforms. diff --git a/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration.dart b/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration.dart index 39f6ac85610f9..a1123e83b8c31 100644 --- a/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration.dart +++ b/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration.dart @@ -25,6 +25,8 @@ void main() { await tester.tap(find.byKey(const Key('scroll-button'))); await tester.pumpAndSettle(); + // TODO: enable screenshot when + // https://github.com/flutter/flutter/issues/68502 is resolved. await binding.takeScreenshot('wheel_scroll_by_line'); }); } diff --git a/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration_test.dart b/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration_test.dart index 9c2c0fdcadc77..e9c65334bb294 100644 --- a/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration_test.dart +++ b/e2etests/web/regular_integration_tests/test_driver/scroll_wheel_integration_test.dart @@ -5,5 +5,7 @@ import 'package:regular_integration_tests/screenshot_support.dart' as test; Future main() async { - await test.runTestWithScreenshots(); + // TODO: switch to screenshot when + // https://github.com/flutter/flutter/issues/68502 is resolved. + // await test.runTestWithScreenshots(); } diff --git a/lib/web_ui/lib/src/engine/pointer_binding.dart b/lib/web_ui/lib/src/engine/pointer_binding.dart index 0fe36865c60db..ff116ec46acfc 100644 --- a/lib/web_ui/lib/src/engine/pointer_binding.dart +++ b/lib/web_ui/lib/src/engine/pointer_binding.dart @@ -309,6 +309,7 @@ mixin _WheelEventListenerMixin on _BaseAdapter { fontSize = fontSize.replaceAll('px', ''); res = double.tryParse(fontSize); } + probe.remove(); return res == null ? kFallbackFontHeight : res / 4.0; } }