feat: Expose `buttons` accessor on Tap Details events (TapDownDetails and TapUpDetails) by luanpotter · Pull Request #176968 · flutter/flutter

luanpotter · 2025-10-13T20:56:05Z

Expose buttons accessor on Tap Details events (TapDownDetails and TapUpDetails).

Note: I chose the option of making it optional to avoid breaking anyone creating synthetic events by hand (though I doubt many people would be doing that). I am also perfectly happy with making it required - just let me know. That would be more ergonomic to the end user as the framework can always guarantee it is there but might break those edge cases of people creating these classes "by hand". A compromise would be a default value, though I think that is more misleading than it would be helping. If we are ok with the minor breaking change my preference would be to make it required. But I think adding it as optional is a much safer first step.

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 Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
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.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

luanpotter · 2025-10-15T19:51:26Z

Note: I took a look at the few failing macOS CI steps but after digging into LUCI and the raw outputs, I couldn't quite fine anything connected to this PR, but I might have missed something! Please let me know if there are any specific pointers I could look for and I'd be happy to fix any issues.

luanpotter · 2025-11-09T00:07:33Z

@Renzo-Olivares hey wondering if you were able to take a look; thank you so much! 🙏

Renzo-Olivares · 2025-11-10T17:23:00Z

Hi @luanpotter, thank you for your patience and this contribution! I will take a look this week.

Renzo-Olivares · 2025-11-17T16:12:52Z

Thank you for your patience on this one @luanpotter! I agree that making buttons nullable is the right approach to ensure this change is not breaking. This also allows us to distinguish between an unknown/unset button state(buttons = null) and no buttons being pressed (buttons = 0).

luanpotter · 2025-12-07T22:20:08Z

                    details.globalPosition - getOffset(context),
                    _transform,
                  ),
+                  kind: details.kind,


NOTE: I am also wiring kind on places that I found it missing. Please let me know if the omission was intentional in these places.

kind is also require because the dartdocs we are importing in on every event with the macro references [kind]. so if we don't want to also add kind, we will need to have two versions of that dartdoc

luanpotter · 2025-12-07T22:20:52Z

+
+  /// {@macro flutter.gestures.PointerEvent.buttons}
+  ///
+  /// NOTE: this will always be set by the platform but synthetic events might


didn't feel great to hide a NOTE inside the macro (and it might not apply everywhere) so I left it here, but happy to pivot.

I think the documentation is not clear enough. Instead of focusing on the format of the bitfield, there are more things to clarify. Here's my proposal:

/// The buttons that were pressed when the device first contacted the screen. /// /// For the format of this value, see [PointerEvent.buttons]. /// /// Subsequent changes to the buttons pressed during the same drag sequence /// will be reported in [DragUpdateDetails.buttons], regardless of whether the /// drag has started ([DragStartDetails]) or not. /// /// This property will always be set by the platform but synthetic events /// might not have it. It can be made required on future releases.

Feel free to clarify more details if you feel necessary.

would you still put that into a macro? it seems we are going in the direction of more customized per-event messages (such as referring to drag-specifics or referring to things with the exact name in the context they are). makes sense to just not use a macro at all?

luanpotter · 2025-12-07T22:21:08Z

    properties.add(DiagnosticsProperty<Duration?>('sourceTimeStamp', sourceTimeStamp));
    properties.add(DiagnosticsProperty<Offset>('delta', delta));
    properties.add(DoubleProperty('primaryDelta', primaryDelta));
+    properties.add(EnumProperty<PointerDeviceKind?>('kind', kind));


this was missing but I believe was a mistake

luanpotter · 2025-12-07T22:24:33Z

          .map((DiagnosticsNode node) => node.toString())
          .toList();
-      expect(description, expected);
+      expect(description, expected, reason: 'for ${detail.runtimeType}');


this is an addendum; just making the test easier to debug

if you prefer I can extract that to a separate PR to reduce touchpoints. or just remove it.

btw on this test I just arbitrarily added the parameters to test the toStrings

luanpotter · 2025-12-07T22:43:55Z

@Renzo-Olivares I've added all the other events and addressed your comments :) I did get a bit big but I believe review shouldn't be too hard 🙏

there were a few considerations to make:

I did a bit of hijacking of the tap_and_drag_test.dart - I changed so tests operate with different button combinations. however there is a default behaviour on BaseTapAndDragGestureRecognizer in which the default isPointerAllowed implementation, regardless of allowedButtonsFilter, will only accept primary buttons. So in order to test the change in these cases, I have added a "permissive" version of the recognizer on this test. It just extends the base and overrides that one method. If you think that is too much hijacking, I can make a similar test file testing similar things (maybe a subset) with a custom recognizer and test the button propagation there, and leave this file testing only primary with the default implementation (and thus touching the og allowedButtonsFilter). I pondered about this and went with this approach as it reduces some duplication.
there is a similar issue on LongPressGestureRecognizer where the buttons is treat as an exact match. as far as I understand it, it should be read as a bit mask (for multi-valued buttons). so I think this is an actual existing bug with the implementation, but I didn't want to bloat this PR. if desired, I can followup with proper bitmask support there (can also avoid some duplication on each handler, possibly).

    switch (_initialButtons) {
      case kPrimaryButton: ...

other than that, the PR should be basically just adding buttons (and sometimes missing kind) and wiring them up. tests seem to be catching the checks as I missed a few places that the tested help me find.

please lmk if this looks good or if you prefer I'd split it in any way, or if there are some events or places we don't want the buttons (or kind) to be added. thank you so much for the thorough reviews!

luanpotter · 2025-12-10T15:32:46Z

@Renzo-Olivares I'm not sure the remaining failure is related, I see it is due to:

Retry: group retry flag allows test to run multiple times the group with retry flag the test inside it
00:29 +613 ~3 -4: /b/s/w/ir/x/w/flutter/packages/flutter_test/test/widget_tester_test.dart: hasRunningAnimations control test
00:29 +614 ~3 -4: /b/s/w/ir/x/w/flutter/packages/flutter_test/test/widget_tester_test.dart: pumping pumpFrames
00:29 +615 ~3 -4: /b/s/w/ir/x/w/flutter/packages/flutter_test/test/widget_tester_test.dart: pumping pumping
00:29 +616 ~3 -4: /b/s/w/ir/x/w/flutter/packages/flutter_test/test/widget_tester_test.dart: verifyTickersWereDisposed control test
00:29 +617 ~3 -4: /b/s/w/ir/x/w/flutter/packages/flutter_test/test/widget_tester_test.dart: can pass a View to pumpWidget when wrapWithView: false
00:29 +618 ~3 -4: /b/s/w/ir/x/w/flutter/packages/flutter_test/test/widget_tester_test.dart: wrapWithView: false does not include View
00:29 +619 ~3 -4: Some tests failed.

But that doesn't seem related; the tests pass locally for me. Wondering if this is some well-known or CI issue.

luanpotter · 2025-12-10T15:34:47Z

          'globalPosition: Offset(0.0, 0.0)',
          'localPosition: Offset(0.0, 0.0)',
          'velocity: Velocity(0.0, 0.0)',
+          'kind: null',


intentionally left some as null to test that as well

luanpotter · 2025-12-10T15:36:19Z

    properties.add(DiagnosticsProperty<Offset>('delta', delta));
    properties.add(DoubleProperty('primaryDelta', primaryDelta));
-    properties.add(EnumProperty<PointerDeviceKind?>('kind', kind));
+    properties.add(EnumProperty<PointerDeviceKind>('kind', kind));


the value is already T?

… enhance comments; remove comment macro

…tons with default values

luanpotter · 2026-01-17T19:36:35Z

@dkwingsmt @Renzo-Olivares I think I addressed all comments but I realize this PR is getting a bit out of hand in terms of size; in particular this change drove me over the fence and made me prefer to split it up. I am keeping this PR with all the changes for now so you can analyze as a whole package (I will rebase later), but I think it would be safer to merge some pieces first (since I was biting the bullet and extracting the scrollbar change, I went ahead and extracted a couple more). The bulk of the changes will still be this PR as they are in a way inseparable unless we want to land some form of "in-between" state, which I would rather not.

But these changes are very tangential, and I think make sense separately:

refactor: Centralize copying of drag events when handling scrollbar scrolling #181110 - addressing the comment about the scrollbars
test: Clarify failure messages on gestures/debug_test.dart #181109 - pass-by improvement on error messages
refactor: Save event data by pointer in an extensible _RecognizerEventData structure #181113 - addresses the data class comment

I also started to separate a change to make all EnumProperty of Foo and not Foo?, though I think this is much more of an open-ended philosophical question and totally not necessary to merge this PR; so I am listing it separately but entirely up to you guys what you prefer there.

refactor: Standardize nullability of the EnumProperty type parameter on event diagnostic messages [proposal] #181112

Hopefully this will make it easier as you can review these 3 (or 4) much easier PRs and then I can rebase this one and make it at least a bit smaller (will still be definitely 10+ files though). I think we are honing in the solution thanks to your comments and guidance so I think I have much better picture of everything in my head now.

Thanks again for all the reviews and sorry for the back and forth 🙏

Clarify failure messages on `gestures/debug_test.dart`. Extracted from #176968 in an attempt to keep it simpler. This just adds a custom error message to all asserts on the `Gesture details debugFillProperties` test, making it clear _which_ event broke the test. While we don't expect tests to break, while developing, this can save several minutes of guesswork. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] 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]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md Co-authored-by: Victor Sanni <victorsanniay@gmail.com>

…81109) Clarify failure messages on `gestures/debug_test.dart`. Extracted from flutter#176968 in an attempt to keep it simpler. This just adds a custom error message to all asserts on the `Gesture details debugFillProperties` test, making it clear _which_ event broke the test. While we don't expect tests to break, while developing, this can save several minutes of guesswork. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] 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]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md Co-authored-by: Victor Sanni <victorsanniay@gmail.com>

dkwingsmt · 2026-03-24T21:02:55Z

I'm converting this to a draft since it is blocked by a few PRs. We'll resume this when all blockers are landed.

dkwingsmt · 2026-04-08T17:47:32Z

I'm going to temporarily close this PR due to being blocked by #181110 and #181113 . Also be aware that code freeze on material and cupertino has been activated (#184093), although the review can continue. :)

…tData structure (flutter#181113) Save event data by pointer in an extensible `_RecognizerEventData` structure instead of just `PointerDeviceKind` This was extracted from flutter#176968 The goal is to use a more flexible structure when storing event data by pointer on the `GestureRecognizer` class. Instead of storing just the `kind`, this also stores the `buttons` property and allows for adding more properties in the future if ever expanded. This supports the next step to add `buttons` to all events. Note: this PR has no tests as it is a no-op change that should not affect any behaviour. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] 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]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Tong Mu <dkwingsmt@users.noreply.github.com>

github-actions Bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. f: gestures flutter/packages/flutter/gestures repository. labels Oct 13, 2025

luanpotter force-pushed the luan.buttons branch 2 times, most recently from 28538a8 to 4ccd7d0 Compare October 13, 2025 21:26

luanpotter marked this pull request as ready for review October 14, 2025 01:37

justinmc requested a review from Renzo-Olivares October 14, 2025 22:25

renancaraujo reviewed Oct 20, 2025

View reviewed changes

Comment thread packages/flutter/lib/src/widgets/text_selection.dart Outdated

luanpotter requested a review from renancaraujo November 4, 2025 18:23

Renzo-Olivares reviewed Nov 17, 2025

View reviewed changes

Comment thread packages/flutter/lib/src/widgets/text_selection.dart Outdated

Comment thread packages/flutter/lib/src/gestures/recognizer.dart Outdated

Comment thread packages/flutter/lib/src/gestures/tap.dart Outdated

github-actions Bot added a: tests "flutter test", flutter_test, or one of our tests f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. f: cupertino flutter/packages/flutter/cupertino repository labels Dec 7, 2025

luanpotter force-pushed the luan.buttons branch from 721a764 to d686448 Compare December 7, 2025 22:19

luanpotter commented Dec 7, 2025

View reviewed changes

Comment thread packages/flutter/lib/src/gestures/multidrag.dart

luanpotter commented Dec 7, 2025

View reviewed changes

luanpotter commented Dec 10, 2025

View reviewed changes

luanpotter requested a review from Renzo-Olivares December 10, 2025 16:32

dkwingsmt removed the request for review from renancaraujo December 10, 2025 19:11

luanpotter added 11 commits January 17, 2026 13:26

Address comments; make kind and button mandatory with default values;…

4cdc17a

… enhance comments; remove comment macro

Fix analyzer

b65b616

Fix consequences of making kind not-null

c75f904

Fix consequences of making kind not-null

913c283

Fix inconsistent test

da4ef9a

Fix inconsistent tests

b4db640

Fix tests

1bac24d

Fix format

2941162

Fix formatting on more files

05b7f45

Address more comments; keep existing kinds nullable and new kinds/but…

9bdb4fa

…tons with default values

Address remaining comments

f3f11ff

luanpotter force-pushed the luan.buttons branch from 1b723c0 to f3f11ff Compare January 17, 2026 18:26

This was referenced Jan 17, 2026

test: Clarify failure messages on gestures/debug_test.dart #181109

Merged

refactor: Centralize copying of drag events when handling scrollbar scrolling #181110

Closed

luanpotter added 2 commits January 17, 2026 14:00

Fix rebase on text_selection.dart

2a74f51

Fix rebase on packages/flutter/lib/src/widgets/selectable_region.dart

b8a230d

This was referenced Jan 17, 2026

refactor: Standardize nullability of the EnumProperty type parameter on event diagnostic messages [proposal] #181112

Closed

refactor: Save event data by pointer in an extensible _RecognizerEventData structure #181113

Merged

Fix lint

9693e85

dkwingsmt self-requested a review January 21, 2026 19:17

justinmc requested a review from Renzo-Olivares January 29, 2026 21:50

dkwingsmt marked this pull request as draft March 24, 2026 21:02

dkwingsmt closed this Apr 8, 2026

luanpotter mentioned this pull request May 18, 2026

refactor: Centralize copying of drag events when handling scrollbar scrolling #186677

Open

9 tasks

Uh oh!

Conversation

luanpotter commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

luanpotter commented Oct 15, 2025

Uh oh!

Uh oh!

luanpotter commented Nov 9, 2025

Uh oh!

Renzo-Olivares commented Nov 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Renzo-Olivares commented Nov 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luanpotter Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luanpotter commented Dec 7, 2025

Uh oh!

luanpotter commented Dec 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luanpotter Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luanpotter commented Jan 17, 2026

Uh oh!

dkwingsmt commented Mar 24, 2026

Uh oh!

dkwingsmt commented Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

luanpotter commented Oct 13, 2025 •

edited

Loading

luanpotter Dec 7, 2025 •

edited

Loading

luanpotter Dec 10, 2025 •

edited

Loading