[web] Notify engine of handled PointerScrollEvents. #145500
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
framework
8 tasks
ditman
changed the title
yjbanov
reviewed
Mar 20, 2024
goderbauer
reviewed
Mar 20, 2024
ditman
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
ditman
force-pushed
the
mutable-pointer-data-packet
branch
2 times, most recently
from
7522da2 to
33cfcaf
Compare
ditman
changed the title
Member
Author
|
I simplified this a good bunch, so it's easy to rip out when we come up with a better (bi-directional) event system. PTAL @goderbauer @yjbanov @mdebbar! |
ditman
commented
May 11, 2024
ditman
force-pushed
the
mutable-pointer-data-packet
branch
4 times, most recently
from
27da7ff to
b1efa0b
Compare
ditman
force-pushed
the
mutable-pointer-data-packet
branch
from
678fec2 to
e61b766
Compare
goderbauer
reviewed
May 29, 2024
Member
There was a problem hiding this comment.
Just wondering why this is implemented as a mixin instead of just defining the respond method directly on PointerSignalEvent?
Member
Author
There was a problem hiding this comment.
This is implemented as a mixin, because the "_TransformedT" subclasses of T extends PointerSignalEvent are implements (not extends), so I'd have to duplicate the empty method a few times.
(PS: Not a fan of the OOP design of this file, it's starting to show its age!)
(PPS: If there's a cleaner way of doing this, I'm all ears. I just didn't want to add more cruft to the base class, having a specific PointerSignalEvent class.
github-actions
bot
added
the
a: tests
ditman
changed the title
Member
Author
|
I think I addressed all the comments @goderbauer, PTAL! PS: I have a change in:
Do I need to split this PR in two, so I can land and version the change to packages/flutter_test, or is that handled by the sdk-dependency on flutter? |
Member
I don't think so. This can all land in one PR. |
auto-submit bot
pushed a commit
to flutter/engine
that referenced
this pull request
Jun 5, 2024
Adds a function to each 'wheel' DataPacket sent to the framework so it can signal whether to `allowPlatformDefault` or not. The current default is to always `preventDefault` on browser events that get sent to the framework. This PR enables the framework to call a method on the `DataPacket`s to `allowPlatformDefault: true`, if the framework won't handle the Signal (signals are handled synchronously on the framework). This lets the engine "wait" for the framework to decide whether to `preventDefault` on a `wheel` event or not. ## Issues * Needed for: flutter/flutter#139263 ## Tests * Added unit tests for the feature in the engine repo, veryfing whether the event has had its `defaultPrevented` or not. * Manually tested in a demo app (see below) ## Demo * https://dit-multiview-scroll.web.app <details> <summary> ## Previous approaches </summary> 1. Add a `handled` bool property to `PointerDataPacket` that the framework can write to (brittle) 2. Modifications to the `PlatformDispatcher` so the framework can `acknowledgePointerData` with a `PointerDataResponse` (fffffatttt change) 3. `acknowledge` function in `PointerDataPacket` </details> > [!IMPORTANT] > * Related: flutter/flutter#145500 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This reverts commit bde00ea.
This reverts commit 33cfcaf206f7f407b18360388fc63f7fd735f267.
The framework now controls when the scroll events on the web are stopped. The PointerEvent now have a respond method that forward a call to the underlying PointerData.respond implementation. Some cleanup in the class hierarchy.
ditman
force-pushed
the
mutable-pointer-data-packet
branch
from
4c749b5 to
f34eef4
Compare
Member
Author
|
This has been rebased to the latest |
Member
Author
|
I've checked the Which makes sense, because I guess my changes to the engine haven't rolled into Google yet. I'll check again in a couple of days. |
Member
Author
|
(Retrying) |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 11, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 11, 2024
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Jun 11, 2024
flutter/flutter@32081aa...14df7be 2024-06-11 [email protected] Revert "Add tests for scaffold drawer and end drawer" (flutter/flutter#150045) 2024-06-11 [email protected] Add tests for scaffold drawer and end drawer (flutter/flutter#149383) 2024-06-11 [email protected] Add high-contrast theme (flutter/flutter#149779) 2024-06-11 [email protected] Manual Pub Roll (flutter/flutter#150025) 2024-06-10 [email protected] [docs] Per-platform desktop triage instructions (flutter/flutter#150019) 2024-06-10 [email protected] Fix copy-paste-o in MethodChannel.invokeListMethod doc (flutter/flutter#149976) 2024-06-10 [email protected] Unpin `camera_android` and remove its only usage (flutter/flutter#150017) 2024-06-10 [email protected] Fixes a bug where NavigatorState.pop does not consider any possible s� (flutter/flutter#150014) 2024-06-10 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland: [CupertinoActionSheet] Match colors to native (#149568) (#150015)" (flutter/flutter#150021) 2024-06-10 [email protected] Reland: [CupertinoActionSheet] Match colors to native (#149568) (flutter/flutter#150015) 2024-06-10 [email protected] Temporarily run Mac_arm64 framework_tests_misc on only Mac-13 (flutter/flutter#150009) 2024-06-10 [email protected] Fixes TextField hinttext in a11y_assessment (flutter/flutter#150007) 2024-06-10 [email protected] Use const bool.fromEnvironment("dart.tool.dart2wasm") to detect dart2wasm (flutter/flutter#149996) 2024-06-10 [email protected] Roll Packages from 8a2c4e4 to e95fe4a (3 revisions) (flutter/flutter#149997) 2024-06-10 [email protected] [web] Notify engine of handled PointerScrollEvents. (flutter/flutter#145500) 2024-06-10 [email protected] Cut no-longer-accurate microtask reference in finalizeTree doc (flutter/flutter#149941) 2024-06-10 [email protected] Update hasTrailingSpaces (flutter/flutter#149698) 2024-06-10 [email protected] [web] Change `--web-renderer` default from `auto` to `canvaskit` (flutter/flutter#149773) 2024-06-10 [email protected] Retain the toString method for subclasses of Key in profile/release mode (flutter/flutter#149926) 2024-06-10 [email protected] Remove package:platform from issue template (flutter/flutter#149995) 2024-06-10 [email protected] Revert "[CupertinoActionSheet] Match colors to native (#149568)" (flutter/flutter#149998) 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 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 Packages: 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
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Notifies the engine when
PointerSignalEventshave been ignored by the framework, through theui.PointerData.respondmethod.This allows the web to "preventDefault" (or not) on
wheelevents.Issues
Tests
respondis called at the right time, with the right value.Demo
Previous versions
PointerScrollEvent, not shippable.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.