Unverified Commit 22b0a62a authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Allow `TapRegion` to consume tap events (#136305)

## Description

In order for `MenuAnchor` menus to be able to not pass on the taps that close their menus, `TapRegion` needed a way to consume them.  This change adds a flag to the `TapRegion`, `consumeOutsideTap` that will consume taps that occur outside of the region if the flag is set (it is false by default). The same flag is added to `MenuAnchor` to allow selecting the behavior for menus.

`TapRegion` consumes the tap event by registering with the gesture arena and immediately resolving the tap as accepted if any regions in a group have `consumeOutsideTap` set to true.

This PR also deprecates `MenuAnchor.anchorTapClosesMenu`, since it is a much more limited version of the same feature that only applied to the anchor itself, and even then only applied to closing the menu, not passing along the tap.  The same functionality can now be implemented by handling a tap on the anchor widget and checking to see if the menu is open before closing it.

## Related Issues
 - https://github.com/flutter/flutter/issues/135327

## Tests
 - Added tests for `TapRegion` to make sure taps are consumed properly.
parent 7ea5536f
......@@ -126,7 +126,6 @@ class _MyContextMenuState extends State<MyContextMenu> {
onSecondaryTapDown: _handleSecondaryTapDown,
child: MenuAnchor(
controller: _menuController,
anchorTapClosesMenu: true,
menuChildren: <Widget>[
MenuItemButton(
child: Text(MenuEntry.about.label),
......@@ -221,6 +220,10 @@ class _MyContextMenuState extends State<MyContextMenu> {
}
void _handleTapDown(TapDownDetails details) {
if (_menuController.isOpen) {
_menuController.close();
return;
}
switch (defaultTargetPlatform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
......
......@@ -129,7 +129,12 @@ class MenuAnchor extends StatefulWidget {
this.style,
this.alignmentOffset = Offset.zero,
this.clipBehavior = Clip.hardEdge,
@Deprecated(
'Use consumeOutsideTap instead. '
'This feature was deprecated after v3.16.0-8.0.pre.',
)
this.anchorTapClosesMenu = false,
this.consumeOutsideTap = false,
this.onOpen,
this.onClose,
this.crossAxisUnconstrained = true,
......@@ -207,8 +212,23 @@ class MenuAnchor extends StatefulWidget {
/// system by the user. In this case [anchorTapClosesMenu] should be true.
///
/// Defaults to false.
@Deprecated(
'Use consumeOutsideTap instead. '
'This feature was deprecated after v3.16.0-8.0.pre.',
)
final bool anchorTapClosesMenu;
/// Whether or not a tap event that closes the menu will be permitted to
/// continue on to the gesture arena.
///
/// If false, then tapping outside of a menu when the menu is open will both
/// close the menu, and allow the tap to participate in the gesture arena. If
/// true, then it will only close the menu, and the tap event will be
/// consumed.
///
/// Defaults to false.
final bool consumeOutsideTap;
/// A callback that is invoked when the menu is opened.
final VoidCallback? onOpen;
......@@ -356,6 +376,7 @@ class _MenuAnchorState extends State<MenuAnchor> {
if (!widget.anchorTapClosesMenu) {
child = TapRegion(
groupId: _root,
consumeOutsideTaps: _root._isOpen && widget.consumeOutsideTap,
onTapOutside: (PointerDownEvent event) {
assert(_debugMenuInfo('Tapped Outside ${widget.controller}'));
_closeChildren();
......@@ -3522,6 +3543,7 @@ class _Submenu extends StatelessWidget {
),
child: TapRegion(
groupId: anchor._root,
consumeOutsideTaps: anchor._root._isOpen && anchor.widget.consumeOutsideTap,
onTapOutside: (PointerDownEvent event) {
anchor._close();
},
......
......@@ -134,8 +134,9 @@ class TapRegionSurface extends SingleChildRenderObjectWidget {
/// A render object that provides notification of a tap inside or outside of a
/// set of registered regions, without participating in the [gesture
/// disambiguation](https://flutter.dev/gestures/#gesture-disambiguation)
/// system.
/// disambiguation](https://flutter.dev/gestures/#gesture-disambiguation) system
/// (other than to consume tap down events if [TapRegion.consumeOutsideTaps] is
/// true).
///
/// The regions are defined by adding [RenderTapRegion] render objects in the
/// render tree around the regions of interest, and they will register with this
......@@ -268,14 +269,26 @@ class RenderTapRegionSurface extends RenderProxyBoxWithHitTestBehavior implement
// If they're not inside, then they're outside.
final Set<RenderTapRegion> outsideRegions = _registeredRegions.difference(insideRegions);
bool consumeOutsideTaps = false;
for (final RenderTapRegion region in outsideRegions) {
assert(_tapRegionDebug('Calling onTapOutside for $region'));
if (region.consumeOutsideTaps) {
assert(_tapRegionDebug('Stopping tap propagation for $region (and all of ${region.groupId})'));
consumeOutsideTaps = true;
}
region.onTapOutside?.call(event);
}
for (final RenderTapRegion region in insideRegions) {
assert(_tapRegionDebug('Calling onTapInside for $region'));
region.onTapInside?.call(event);
}
// If any of the "outside" regions have consumeOutsideTaps set, then stop
// the propagation of the event through the gesture recognizer by adding it
// to the recognizer and immediately resolving it.
if (consumeOutsideTaps) {
GestureBinding.instance.gestureArena.add(event.pointer, _DummyTapRecognizer()).resolve(GestureDisposition.accepted);
}
}
// Returns the registered regions that are in the hit path.
......@@ -291,10 +304,22 @@ class RenderTapRegionSurface extends RenderProxyBoxWithHitTestBehavior implement
}
}
// A dummy tap recognizer so that we don't have to deal with the lifecycle of
// TapGestureRecognizer, since we're just going to immediately resolve it
// anyhow.
class _DummyTapRecognizer extends GestureArenaMember {
@override
void acceptGesture(int pointer) { }
@override
void rejectGesture(int pointer) { }
}
/// A widget that defines a region that can detect taps inside or outside of
/// itself and any group of regions it belongs to, without participating in the
/// [gesture disambiguation](https://flutter.dev/gestures/#gesture-disambiguation)
/// system.
/// [gesture
/// disambiguation](https://flutter.dev/gestures/#gesture-disambiguation) system
/// (other than to consume tap down events if [consumeOutsideTaps] is true).
///
/// This widget indicates to the nearest ancestor [TapRegionSurface] that the
/// region occupied by its child will participate in the tap detection for that
......@@ -316,6 +341,7 @@ class TapRegion extends SingleChildRenderObjectWidget {
this.onTapOutside,
this.onTapInside,
this.groupId,
this.consumeOutsideTaps = false,
String? debugLabel,
}) : debugLabel = kReleaseMode ? null : debugLabel;
......@@ -357,6 +383,19 @@ class TapRegion extends SingleChildRenderObjectWidget {
/// If the group id is null, then only this region is hit tested.
final Object? groupId;
/// If true, then the group that this region belongs to will stop the
/// propagation of the tap down event in the gesture arena.
///
/// This is useful if you want to block the tap down from being given to a
/// [GestureDetector] when [onTapOutside] is called.
///
/// If other [TapRegion]s with the same [groupId] have [consumeOutsideTaps]
/// set to false, but this one is true, then this one will take precedence,
/// and the event will be consumed.
///
/// Defaults to false.
final bool consumeOutsideTaps;
/// An optional debug label to help with debugging in debug mode.
///
/// Will be null in release mode.
......@@ -367,6 +406,7 @@ class TapRegion extends SingleChildRenderObjectWidget {
return RenderTapRegion(
registry: TapRegionRegistry.maybeOf(context),
enabled: enabled,
consumeOutsideTaps: consumeOutsideTaps,
behavior: behavior,
onTapOutside: onTapOutside,
onTapInside: onTapInside,
......@@ -430,6 +470,7 @@ class RenderTapRegion extends RenderProxyBoxWithHitTestBehavior {
RenderTapRegion({
TapRegionRegistry? registry,
bool enabled = true,
bool consumeOutsideTaps = false,
this.onTapOutside,
this.onTapInside,
super.behavior = HitTestBehavior.deferToChild,
......@@ -437,6 +478,7 @@ class RenderTapRegion extends RenderProxyBoxWithHitTestBehavior {
String? debugLabel,
}) : _registry = registry,
_enabled = enabled,
_consumeOutsideTaps = consumeOutsideTaps,
_groupId = groupId,
debugLabel = kReleaseMode ? null : debugLabel;
......@@ -473,6 +515,21 @@ class RenderTapRegion extends RenderProxyBoxWithHitTestBehavior {
}
}
/// Whether or not the tap down even that triggers a call to [onTapOutside]
/// will continue on to participate in the gesture arena.
///
/// If any [RenderTapRegion] in the same group has [consumeOutsideTaps] set to
/// true, then the tap down event will be consumed before other gesture
/// recognizers can process them.
bool get consumeOutsideTaps => _consumeOutsideTaps;
bool _consumeOutsideTaps;
set consumeOutsideTaps(bool value) {
if (_consumeOutsideTaps != value) {
_consumeOutsideTaps = value;
markNeedsLayout();
}
}
/// An optional group ID that groups [RenderTapRegion]s together so that they
/// operate as one region. If any member of a group is hit by a particular
/// tap, then the [onTapOutside] will not be called for any members of the
......@@ -583,6 +640,7 @@ class TextFieldTapRegion extends TapRegion {
super.enabled,
super.onTapOutside,
super.onTapInside,
super.consumeOutsideTaps,
super.debugLabel,
}) : super(groupId: EditableText);
}
......@@ -102,6 +102,111 @@ void main() {
expect(tappedOutside, isEmpty);
});
testWidgetsWithLeakTracking('TapRegionSurface consumes outside taps when asked', (WidgetTester tester) async {
final Set<String> tappedOutside = <String>{};
int propagatedTaps = 0;
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Column(
children: <Widget>[
const Text('Outside Surface'),
TapRegionSurface(
child: Row(
children: <Widget>[
GestureDetector(
onTap: () {
propagatedTaps += 1;
},
child: const Text('Outside'),
),
TapRegion(
consumeOutsideTaps: true,
onTapOutside: (PointerEvent event) {
tappedOutside.add('No Group');
},
child: const Text('No Group'),
),
TapRegion(
groupId: 1,
onTapOutside: (PointerEvent event) {
tappedOutside.add('Group 1 A');
},
child: const Text('Group 1 A'),
),
TapRegion(
groupId: 1,
consumeOutsideTaps: true,
onTapOutside: (PointerEvent event) {
tappedOutside.add('Group 1 B');
},
child: const Text('Group 1 B'),
),
],
),
),
],
),
),
);
await tester.pump();
Future<void> click(Finder finder) async {
final TestGesture gesture = await tester.startGesture(
tester.getCenter(finder),
kind: PointerDeviceKind.mouse,
);
await gesture.up();
await gesture.removePointer();
}
expect(tappedOutside, isEmpty);
expect(propagatedTaps, equals(0));
await click(find.text('No Group'));
expect(
tappedOutside,
unorderedEquals(<String>{
'Group 1 A',
'Group 1 B',
}));
expect(propagatedTaps, equals(0));
tappedOutside.clear();
await click(find.text('Group 1 A'));
expect(
tappedOutside,
equals(<String>{
'No Group',
}));
expect(propagatedTaps, equals(0));
tappedOutside.clear();
await click(find.text('Group 1 B'));
expect(
tappedOutside,
equals(<String>{
'No Group',
}));
expect(propagatedTaps, equals(0));
tappedOutside.clear();
await click(find.text('Outside'));
expect(
tappedOutside,
unorderedEquals(<String>{
'No Group',
'Group 1 A',
'Group 1 B',
}));
expect(propagatedTaps, equals(0));
tappedOutside.clear();
await click(find.text('Outside Surface'));
expect(tappedOutside, isEmpty);
});
testWidgetsWithLeakTracking('TapRegionSurface detects inside taps', (WidgetTester tester) async {
final Set<String> tappedInside = <String>{};
await tester.pumpWidget(
......@@ -206,8 +311,6 @@ void main() {
ConstrainedBox(
constraints: const BoxConstraints.tightFor(width: 100, height: 100),
child: TapRegion(
// ignore: avoid_redundant_argument_values
behavior: HitTestBehavior.deferToChild,
onTapInside: (PointerEvent event) {
tappedInside.add(noGroupKey.value);
},
......@@ -263,7 +366,8 @@ void main() {
await click(find.byKey(group1AKey));
// No hittable children, but set to opaque, so it hits, triggering the
// group.
expect(tappedInside,
expect(
tappedInside,
equals(<String>{
'Group 1 A',
'Group 1 B',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment