Skip to content

Commit

Permalink
[web] Notify engine of handled PointerScrollEvents. (#145500)
Browse files Browse the repository at this point in the history
Notifies the engine when `PointerSignalEvents` have been ignored by the framework, through the `ui.PointerData.respond` method.

This allows the web to "preventDefault" (or not) on `wheel` events.

## Issues

* Fixes (partially): flutter/flutter#139263

## Tests

* Added tests to ensure `respond` is called at the right time, with the right value.

## Demo

* https://dit-multiview-scroll.web.app

<details>
<summary>

## Previous versions

</summary>

1. Modified `PointerScrollEvent`, not shippable.
2. Modified when events were handled, instead of the opposite.

</details>
  • Loading branch information
ditman authored Jun 10, 2024
1 parent 5efb67b commit 0c2ee84
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 4 deletions.
1 change: 1 addition & 0 deletions packages/flutter/lib/src/gestures/converter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ abstract final class PointerEventConverter {
position: position,
scrollDelta: scrollDelta,
embedderId: datum.embedderId,
onRespond: datum.respond,
);
case ui.PointerSignalKind.scrollInertiaCancel:
return PointerScrollInertiaCancelEvent(
Expand Down
49 changes: 45 additions & 4 deletions packages/flutter/lib/src/gestures/events.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1717,7 +1717,7 @@ class _TransformedPointerUpEvent extends _TransformedPointerEvent with _CopyPoin
/// events in a widget tree.
/// * [PointerSignalResolver], which provides an opt-in mechanism whereby
/// participating agents may disambiguate an event's target.
abstract class PointerSignalEvent extends PointerEvent {
abstract class PointerSignalEvent extends PointerEvent with _RespondablePointerEvent {
/// Abstract const constructor. This constructor enables subclasses to provide
/// const constructors so that they can be used in const expressions.
const PointerSignalEvent({
Expand All @@ -1731,6 +1731,27 @@ abstract class PointerSignalEvent extends PointerEvent {
});
}

/// A function that implements the [PointerSignalEvent.respond] method.
typedef RespondPointerEventCallback = void Function({required bool allowPlatformDefault});

mixin _RespondablePointerEvent on PointerEvent {
/// Sends a response to the native embedder for the [PointerSignalEvent].
///
/// The parameter [allowPlatformDefault] allows the platform to perform the
/// default action associated with the native event when it's set to `true`.
///
/// This method can be called any number of times, but once `allowPlatformDefault`
/// is set to `true`, it can't be set to `false` again.
///
/// The implementation of this method is configured through the `onRespond`
/// parameter of the [PointerSignalEvent] constructor.
///
/// See also [RespondPointerEventCallback].
void respond({
required bool allowPlatformDefault,
}) {}
}

mixin _CopyPointerScrollEvent on PointerEvent {
/// The amount to scroll, in logical pixels.
Offset get scrollDelta;
Expand Down Expand Up @@ -1760,6 +1781,7 @@ mixin _CopyPointerScrollEvent on PointerEvent {
double? tilt,
bool? synthesized,
int? embedderId,
RespondPointerEventCallback? onRespond,
}) {
return PointerScrollEvent(
viewId: viewId ?? this.viewId,
Expand All @@ -1769,6 +1791,7 @@ mixin _CopyPointerScrollEvent on PointerEvent {
position: position ?? this.position,
scrollDelta: scrollDelta,
embedderId: embedderId ?? this.embedderId,
onRespond: onRespond ?? (this as PointerScrollEvent).respond,
).transformed(transform);
}
}
Expand All @@ -1794,7 +1817,8 @@ class PointerScrollEvent extends PointerSignalEvent with _PointerEventDescriptio
super.position,
this.scrollDelta = Offset.zero,
super.embedderId,
});
RespondPointerEventCallback? onRespond,
}) : _onRespond = onRespond;

@override
final Offset scrollDelta;
Expand All @@ -1812,6 +1836,15 @@ class PointerScrollEvent extends PointerSignalEvent with _PointerEventDescriptio
super.debugFillProperties(properties);
properties.add(DiagnosticsProperty<Offset>('scrollDelta', scrollDelta));
}

final RespondPointerEventCallback? _onRespond;

@override
void respond({required bool allowPlatformDefault}) {
if (_onRespond != null) {
_onRespond!(allowPlatformDefault: allowPlatformDefault);
}
}
}

class _TransformedPointerScrollEvent extends _TransformedPointerEvent with _CopyPointerScrollEvent implements PointerScrollEvent {
Expand All @@ -1834,6 +1867,14 @@ class _TransformedPointerScrollEvent extends _TransformedPointerEvent with _Copy
super.debugFillProperties(properties);
properties.add(DiagnosticsProperty<Offset>('scrollDelta', scrollDelta));
}

@override
RespondPointerEventCallback? get _onRespond => original._onRespond;

@override
void respond({required bool allowPlatformDefault}) {
original.respond(allowPlatformDefault: allowPlatformDefault);
}
}

mixin _CopyPointerScrollInertiaCancelEvent on PointerEvent {
Expand Down Expand Up @@ -1905,7 +1946,7 @@ class PointerScrollInertiaCancelEvent extends PointerSignalEvent with _PointerEv
}
}

class _TransformedPointerScrollInertiaCancelEvent extends _TransformedPointerEvent with _CopyPointerScrollInertiaCancelEvent implements PointerScrollInertiaCancelEvent {
class _TransformedPointerScrollInertiaCancelEvent extends _TransformedPointerEvent with _CopyPointerScrollInertiaCancelEvent, _RespondablePointerEvent implements PointerScrollInertiaCancelEvent {
_TransformedPointerScrollInertiaCancelEvent(this.original, this.transform);

@override
Expand Down Expand Up @@ -1996,7 +2037,7 @@ class PointerScaleEvent extends PointerSignalEvent with _PointerEventDescription
}
}

class _TransformedPointerScaleEvent extends _TransformedPointerEvent with _CopyPointerScaleEvent implements PointerScaleEvent {
class _TransformedPointerScaleEvent extends _TransformedPointerEvent with _CopyPointerScaleEvent, _RespondablePointerEvent implements PointerScaleEvent {
_TransformedPointerScaleEvent(this.original, this.transform);

@override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ class PointerSignalResolver {
void resolve(PointerSignalEvent event) {
if (_firstRegisteredCallback == null) {
assert(_currentEvent == null);
// Nothing in the framework/app wants to handle the `event`. Allow the
// platform to trigger any default native actions.
event.respond(
allowPlatformDefault: true
);
return;
}
assert(_isSameEvent(_currentEvent!, event));
Expand Down
7 changes: 7 additions & 0 deletions packages/flutter/lib/src/widgets/scrollable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -916,14 +916,21 @@ class ScrollableState extends State<Scrollable> with TickerProviderStateMixin, R
void _receivedPointerSignal(PointerSignalEvent event) {
if (event is PointerScrollEvent && _position != null) {
if (_physics != null && !_physics!.shouldAcceptUserOffset(position)) {
// The handler won't use the `event`, so allow the platform to trigger
// any default native actions.
event.respond(allowPlatformDefault: true);
return;
}
final double delta = _pointerSignalEventDelta(event);
final double targetScrollOffset = _targetScrollOffsetForPointerScroll(delta);
// Only express interest in the event if it would actually result in a scroll.
if (delta != 0.0 && targetScrollOffset != position.pixels) {
GestureBinding.instance.pointerSignalResolver.register(event, _handlePointerScroll);
return;
}
// The `event` won't result in a scroll, so allow the platform to trigger
// any default native actions.
event.respond(allowPlatformDefault: true);
} else if (event is PointerScrollInertiaCancelEvent) {
position.pointerScroll(0);
// Don't use the pointer signal resolver, all hit-tested scrollables should stop.
Expand Down
13 changes: 13 additions & 0 deletions packages/flutter/test/gestures/pointer_signal_resolver_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,19 @@ void main() {
tester.resolver.resolve(tester.event);
});

test('Resolving with no entries should notify engine of no-op', () {
bool allowedPlatformDefault = false;
final PointerSignalTester tester = PointerSignalTester();
tester.event = PointerScrollEvent(
onRespond: ({required bool allowPlatformDefault}) {
allowedPlatformDefault = allowPlatformDefault;
},
);
tester.resolver.resolve(tester.event);
expect(allowedPlatformDefault, isTrue,
reason: 'Should have called respond with allowPlatformDefault: true');
});

test('First entry should always win', () {
final PointerSignalTester tester = PointerSignalTester();
final TestPointerSignalListener first = tester.addListener();
Expand Down
57 changes: 57 additions & 0 deletions packages/flutter/test/widgets/scrollable_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,63 @@ void main() {
expect(getScrollOffset(tester), 0.0);
});

testWidgets('Engine is notified of ignored pointer signals (no scroll physics)', (WidgetTester tester) async {
await pumpTest(tester, debugDefaultTargetPlatformOverride, scrollable: false);
final Offset scrollEventLocation = tester.getCenter(find.byType(Viewport));
final TestPointer testPointer = TestPointer(1, ui.PointerDeviceKind.mouse);
// Create a hover event so that |testPointer| has a location when generating the scroll.
testPointer.hover(scrollEventLocation);

bool allowedPlatformDefault = false;
await tester.sendEventToBinding(
testPointer.scroll(
const Offset(0.0, 20.0),
onRespond: ({required bool allowPlatformDefault}) {
allowedPlatformDefault = allowPlatformDefault;
},
));

expect(allowedPlatformDefault, isTrue,
reason: 'Engine should be notified of ignored scroll pointer signals.');
}, variant: TargetPlatformVariant.all());

testWidgets('Engine is notified of rejected scroll events (wrong direction)', (WidgetTester tester) async {
await pumpTest(
tester,
debugDefaultTargetPlatformOverride,
scrollDirection: Axis.horizontal,
);

final Offset scrollEventLocation = tester.getCenter(find.byType(Viewport));
final TestPointer testPointer = TestPointer(1, ui.PointerDeviceKind.mouse);
// Create a hover event so that |testPointer| has a location when generating the scroll.
testPointer.hover(scrollEventLocation);

// Horizontal input is accepted
await tester.sendKeyDownEvent(LogicalKeyboardKey.shift);
await tester.sendEventToBinding(
testPointer.scroll(
const Offset(0.0, 10.0),
onRespond: ({required bool allowPlatformDefault}) {
fail('The engine should not be notified when the scroll is accepted.');
},
));
await tester.sendKeyUpEvent(LogicalKeyboardKey.shift);
await tester.pump();

// Vertical input not accepted
bool allowedPlatformDefault = false;
await tester.sendEventToBinding(
testPointer.scroll(
const Offset(0.0, 20.0),
onRespond: ({required bool allowPlatformDefault}) {
allowedPlatformDefault = allowPlatformDefault;
},
));
expect(allowedPlatformDefault, isTrue,
reason: 'Engine should be notified when scroll is rejected by the scrollable.');
}, variant: TargetPlatformVariant.all());

testWidgets('Holding scroll and Scroll pointer signal will update ScrollDirection.forward / ScrollDirection.reverse', (WidgetTester tester) async {
ScrollDirection? lastUserScrollingDirection;

Expand Down
2 changes: 2 additions & 0 deletions packages/flutter_test/lib/src/test_pointer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ class TestPointer {
PointerScrollEvent scroll(
Offset scrollDelta, {
Duration timeStamp = Duration.zero,
RespondPointerEventCallback? onRespond,
}) {
assert(kind != PointerDeviceKind.touch, "Touch pointers can't generate pointer signal events");
assert(location != null);
Expand All @@ -297,6 +298,7 @@ class TestPointer {
device: _device,
position: location!,
scrollDelta: scrollDelta,
onRespond: onRespond,
);
}

Expand Down

0 comments on commit 0c2ee84

Please sign in to comment.