Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split Mouse from Listener #36217

Merged
merged 26 commits into from Aug 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
3af5500
Split MouseRegion out and fix tests
dkwingsmt Jul 13, 2019
e508de3
Renames
dkwingsmt Jul 15, 2019
64b8aba
Merge remote-tracking branch 'upstream/master' into split-mouse-listener
dkwingsmt Jul 16, 2019
0e8ccd4
Move test to deprecated test for ignore
dkwingsmt Jul 16, 2019
5f92daa
Fix deprecation and lint
dkwingsmt Jul 16, 2019
9974693
Rename Mouse -> MouseRegion
dkwingsmt Jul 19, 2019
3b3443d
Merge remote-tracking branch 'upstream/master' into split-mouse-listener
dkwingsmt Jul 19, 2019
eeae80a
Make deprecation a TODO
dkwingsmt Jul 19, 2019
fd68832
Fix sample code
dkwingsmt Jul 19, 2019
b3761f2
Fix deprecation message
dkwingsmt Jul 19, 2019
fbe4385
Rephrase TODO
dkwingsmt Jul 19, 2019
d84af3e
Merge remote-tracking branch 'upstream/master' into split-mouse-listener
dkwingsmt Jul 22, 2019
6cf520c
Merge remote-tracking branch 'upstream/master' into split-mouse-listener
dkwingsmt Jul 23, 2019
80c09ec
Fix unnecessary expectation that broke tests
dkwingsmt Jul 23, 2019
52384b5
Merge remote-tracking branch 'upstream/master' into split-mouse-listener
dkwingsmt Jul 24, 2019
da18a80
Fix a few docs
dkwingsmt Jul 24, 2019
4188a90
Fix docs
dkwingsmt Jul 24, 2019
899e830
Stop using static values
dkwingsmt Jul 24, 2019
b284a80
Changing expectation to adapt with the change
dkwingsmt Jul 25, 2019
0202cfc
Doc proof reading
dkwingsmt Jul 26, 2019
0836349
Merge remote-tracking branch 'upstream/master' into split-mouse-listener
dkwingsmt Jul 26, 2019
466c783
Fix doc because mouse region allows buttons
dkwingsmt Jul 26, 2019
f1d1210
Empty line
dkwingsmt Jul 26, 2019
dbaae0f
Merge remote-tracking branch 'upstream/master' into split-mouse-listener
dkwingsmt Jul 29, 2019
6e0241e
Merge remote-tracking branch 'upstream/master' into split-mouse-listener
dkwingsmt Jul 31, 2019
d569873
Rename to RenderMouseRegion
dkwingsmt Jul 31, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -1,8 +1,6 @@
<<skip until matching line>>
══╡ EXCEPTION CAUGHT BY WIDGETS LIBRARY ╞═══════════════════════════════════════════════════════════
The following assertion was thrown building
RawGestureDetector-\[LabeledGlobalKey<RawGestureDetectorState>#.+\]\(state:
RawGestureDetectorState#.+\(gestures: <none>, behavior: opaque\)\):
The following assertion was thrown building Listener
'package:flutter\/src\/painting\/basic_types\.dart': Failed assertion: line 223 pos 10: 'textDirection
!= null': is not true\.

Expand Down
@@ -1,8 +1,6 @@
<<skip until matching line>>
══╡ EXCEPTION CAUGHT BY WIDGETS LIBRARY ╞═══════════════════════════════════════════════════════════
The following assertion was thrown building
RawGestureDetector-\[LabeledGlobalKey<RawGestureDetectorState>#.+\]\(state:
RawGestureDetectorState#.+\(gestures: <none>, behavior: opaque\)\):
The following assertion was thrown building Listener
'package:flutter\/src\/painting\/basic_types\.dart': Failed assertion: line 223 pos 10: 'textDirection
!= null': is not true\.

Expand Down
11 changes: 5 additions & 6 deletions packages/flutter/lib/src/material/ink_well.dart
Expand Up @@ -685,8 +685,8 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe

bool get enabled => _isWidgetEnabled(widget);

void _handlePointerEnter(PointerEnterEvent event) => _handleHoverChange(true);
void _handlePointerExit(PointerExitEvent event) => _handleHoverChange(false);
void _handleMouseEnter(PointerEnterEvent event) => _handleHoverChange(true);
void _handleMouseExit(PointerExitEvent event) => _handleHoverChange(false);
void _handleHoverChange(bool hovering) {
if (_hovering != hovering) {
_hovering = hovering;
Expand All @@ -702,10 +702,9 @@ class _InkResponseState<T extends InkResponse> extends State<T> with AutomaticKe
_highlights[type]?.color = getHighlightColorForType(type);
}
_currentSplash?.color = widget.splashColor ?? Theme.of(context).splashColor;
return Listener(
onPointerEnter: enabled ? _handlePointerEnter : null,
onPointerExit: enabled ? _handlePointerExit : null,
behavior: HitTestBehavior.translucent,
return MouseRegion(
onEnter: enabled ? _handleMouseEnter : null,
onExit: enabled ? _handleMouseExit : null,
child: GestureDetector(
onTapDown: enabled ? _handleTapDown : null,
onTap: enabled ? () => _handleTap(context) : null,
Expand Down
10 changes: 5 additions & 5 deletions packages/flutter/lib/src/material/text_field.dart
Expand Up @@ -887,8 +887,8 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi
super.deactivate();
}

void _handlePointerEnter(PointerEnterEvent event) => _handleHover(true);
void _handlePointerExit(PointerExitEvent event) => _handleHover(false);
void _handleMouseEnter(PointerEnterEvent event) => _handleHover(true);
void _handleMouseExit(PointerExitEvent event) => _handleHover(false);

void _handleHover(bool hovering) {
if (hovering != _isHovering) {
Expand Down Expand Up @@ -1020,9 +1020,9 @@ class _TextFieldState extends State<TextField> with AutomaticKeepAliveClientMixi
_effectiveController.selection = TextSelection.collapsed(offset: _effectiveController.text.length);
_requestKeyboard();
},
child: Listener(
onPointerEnter: _handlePointerEnter,
onPointerExit: _handlePointerExit,
child: MouseRegion(
onEnter: _handleMouseEnter,
onExit: _handleMouseExit,
child: IgnorePointer(
ignoring: !(widget.enabled ?? widget.decoration?.enabled ?? true),
child: _selectionGestureDetectorBuilder.buildGestureDetector(
Expand Down
6 changes: 3 additions & 3 deletions packages/flutter/lib/src/material/tooltip.dart
Expand Up @@ -399,9 +399,9 @@ class _TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {

// Only check for hovering if there is a mouse connected.
if (_mouseIsConnected) {
result = Listener(
onPointerEnter: (PointerEnterEvent event) => _showTooltip(),
onPointerExit: (PointerExitEvent event) => _hideTooltip(),
result = MouseRegion(
onEnter: (PointerEnterEvent event) => _showTooltip(),
onExit: (PointerExitEvent event) => _hideTooltip(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we use methods in the previous two files and closures here? we should be consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but I'm leaning towards making as few changes in other files as possible since it's a breaking PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why it being a breaking PR matters here?

But if you want to change it in another PR that's fine. :-)

child: result,
);
}
Expand Down
199 changes: 117 additions & 82 deletions packages/flutter/lib/src/rendering/proxy_box.dart
Expand Up @@ -2460,43 +2460,31 @@ typedef PointerCancelEventListener = void Function(PointerCancelEvent event);
/// Used by [Listener] and [RenderPointerListener].
typedef PointerSignalEventListener = void Function(PointerSignalEvent event);

/// Calls callbacks in response to pointer events.
/// Calls callbacks in response to common pointer events.
///
/// It responds to events that can construct gestures, such as when the
/// pointer is pressed, moved, then released or canceled.
///
/// It does not respond to events that are exclusive to mouse, such as when the
/// mouse enters, exits or hovers a region without pressing any buttons. For
/// these events, use [RenderMouseRegion].
///
/// If it has a child, defers to the child for sizing behavior.
///
/// If it does not have a child, grows to fit the parent-provided constraints.
///
/// The [onPointerEnter], [onPointerHover], and [onPointerExit] events are only
/// relevant to and fired by pointers that can hover (e.g. mouse pointers, but
/// not most touch pointers).
class RenderPointerListener extends RenderProxyBoxWithHitTestBehavior {
/// Creates a render object that forwards pointer events to callbacks.
///
/// The [behavior] argument defaults to [HitTestBehavior.deferToChild].
RenderPointerListener({
this.onPointerDown,
this.onPointerMove,
PointerEnterEventListener onPointerEnter,
PointerHoverEventListener onPointerHover,
PointerExitEventListener onPointerExit,
this.onPointerUp,
this.onPointerCancel,
this.onPointerSignal,
HitTestBehavior behavior = HitTestBehavior.deferToChild,
RenderBox child,
}) : _onPointerEnter = onPointerEnter,
_onPointerHover = onPointerHover,
_onPointerExit = onPointerExit,
super(behavior: behavior, child: child) {
if (_onPointerEnter != null || _onPointerHover != null || _onPointerExit != null) {
_hoverAnnotation = MouseTrackerAnnotation(
onEnter: _onPointerEnter,
onHover: _onPointerHover,
onExit: _onPointerExit,
);
}
_mouseIsConnected = RendererBinding.instance.mouseTracker.mouseIsConnected;
}
}) : super(behavior: behavior, child: child);

/// Called when a pointer comes into contact with the screen (for touch
/// pointers), or has its button pressed (for mouse pointers) at this widget's
Expand All @@ -2506,55 +2494,129 @@ class RenderPointerListener extends RenderProxyBoxWithHitTestBehavior {
/// Called when a pointer that triggered an [onPointerDown] changes position.
PointerMoveEventListener onPointerMove;

/// Called when a pointer that triggered an [onPointerDown] is no longer in
/// contact with the screen.
PointerUpEventListener onPointerUp;

/// Called when the input from a pointer that triggered an [onPointerDown] is
/// no longer directed towards this receiver.
PointerCancelEventListener onPointerCancel;

/// Called when a pointer signal occurs over this object.
PointerSignalEventListener onPointerSignal;

@override
void performResize() {
size = constraints.biggest;
}

@override
void handleEvent(PointerEvent event, HitTestEntry entry) {
assert(debugHandleEvent(event, entry));
if (onPointerDown != null && event is PointerDownEvent)
return onPointerDown(event);
if (onPointerMove != null && event is PointerMoveEvent)
return onPointerMove(event);
if (onPointerUp != null && event is PointerUpEvent)
return onPointerUp(event);
if (onPointerCancel != null && event is PointerCancelEvent)
return onPointerCancel(event);
if (onPointerSignal != null && event is PointerSignalEvent)
return onPointerSignal(event);
}

@override
void debugFillProperties(DiagnosticPropertiesBuilder properties) {
super.debugFillProperties(properties);
final List<String> listeners = <String>[];
if (onPointerDown != null)
listeners.add('down');
if (onPointerMove != null)
listeners.add('move');
if (onPointerUp != null)
listeners.add('up');
if (onPointerCancel != null)
listeners.add('cancel');
if (onPointerSignal != null)
listeners.add('signal');
if (listeners.isEmpty)
listeners.add('<none>');
properties.add(IterableProperty<String>('listeners', listeners));
// TODO(jacobr): add raw listeners to the diagnostics data.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jacob314 Is there anything special we need to do to resolve this TODO?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

properties.add(ObjectFlagProperty<PointerDownEventListener>('onPointerDown', onPointerDown, ifPresent: 'down', level: DiagnosticLevel.fine);

Would be an idiomatic way to expose each of these.
That would hide the output from the default display but let the inspector provide links to the actual listeners used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jacob314
The current way allows a clean format with the help of IterableProperty,

'listeners: down, move, up, cancel, signal'

or

'listeners: <none>'

Is there a way to maintain the current format while using ObjectFlagProperty?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not currently. Are there any cases with a subclass that wants to add an additional listener? In that case it could make sense to come up with a scheme that enables defining each listener as an ObjectFlagProperty and then aggregate the properties into a terse summary like listeners: down, move, up, cancel when generating the string output.

}
}

/// Calls callbacks in response to pointer events that are exclusive to mice.
///
/// Simply put, it responds to events that are related to hovering,
/// i.e. when the mouse enters, exits or hovers a region without pressing.
///
/// It does not respond to common events that construct gestures, such as when
/// the pointer is pressed, moved, then released or canceled. For these events,
/// use [RenderPointerListener].
///
/// If it has a child, it defers to the child for sizing behavior.
///
/// If it does not have a child, it grows to fit the parent-provided constraints.
class RenderMouseRegion extends RenderProxyBox {
/// Creates a render object that forwards pointer events to callbacks.
dkwingsmt marked this conversation as resolved.
Show resolved Hide resolved
RenderMouseRegion({
PointerEnterEventListener onEnter,
PointerHoverEventListener onHover,
PointerExitEventListener onExit,
RenderBox child,
}) : _onEnter = onEnter,
_onHover = onHover,
_onExit = onExit,
super(child) {
if (_onEnter != null || _onHover != null || _onExit != null) {
_hoverAnnotation = MouseTrackerAnnotation(
onEnter: _onEnter,
onHover: _onHover,
onExit: _onExit,
);
}
_mouseIsConnected = RendererBinding.instance.mouseTracker.mouseIsConnected;
}

/// Called when a hovering pointer enters the region for this widget.
///
/// If this is a mouse pointer, this will fire when the mouse pointer enters
/// the region defined by this widget.
PointerEnterEventListener get onPointerEnter => _onPointerEnter;
set onPointerEnter(PointerEnterEventListener value) {
if (_onPointerEnter != value) {
_onPointerEnter = value;
PointerEnterEventListener get onEnter => _onEnter;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's too bad we can't now name these MouseEnterEventListener, etc., but I do understand: that would make it a larger breaking change.

set onEnter(PointerEnterEventListener value) {
if (_onEnter != value) {
_onEnter = value;
_updateAnnotations();
}
}
PointerEnterEventListener _onPointerEnter;
PointerEnterEventListener _onEnter;

/// Called when a pointer that has not triggered an [onPointerDown] changes
/// position.
///
/// Typically only triggered for mouse pointers.
PointerHoverEventListener get onPointerHover => _onPointerHover;
set onPointerHover(PointerHoverEventListener value) {
if (_onPointerHover != value) {
_onPointerHover = value;
PointerHoverEventListener get onHover => _onHover;
set onHover(PointerHoverEventListener value) {
if (_onHover != value) {
_onHover = value;
_updateAnnotations();
}
}
PointerHoverEventListener _onPointerHover;
PointerHoverEventListener _onHover;

/// Called when a hovering pointer leaves the region for this widget.
///
/// If this is a mouse pointer, this will fire when the mouse pointer leaves
/// the region defined by this widget.
PointerExitEventListener get onPointerExit => _onPointerExit;
set onPointerExit(PointerExitEventListener value) {
if (_onPointerExit != value) {
_onPointerExit = value;
PointerExitEventListener get onExit => _onExit;
set onExit(PointerExitEventListener value) {
if (_onExit != value) {
_onExit = value;
_updateAnnotations();
}
}
PointerExitEventListener _onPointerExit;

/// Called when a pointer that triggered an [onPointerDown] is no longer in
/// contact with the screen.
PointerUpEventListener onPointerUp;

/// Called when the input from a pointer that triggered an [onPointerDown] is
/// no longer directed towards this receiver.
PointerCancelEventListener onPointerCancel;

/// Called when a pointer signal occurs over this object.
PointerSignalEventListener onPointerSignal;
PointerExitEventListener _onExit;

// Object used for annotation of the layer used for hover hit detection.
MouseTrackerAnnotation _hoverAnnotation;
Expand All @@ -2567,19 +2629,19 @@ class RenderPointerListener extends RenderProxyBoxWithHitTestBehavior {
MouseTrackerAnnotation get hoverAnnotation => _hoverAnnotation;

void _updateAnnotations() {
assert(_hoverAnnotation == null || _onPointerEnter != _hoverAnnotation.onEnter || _onPointerHover != _hoverAnnotation.onHover || _onPointerExit != _hoverAnnotation.onExit,
assert(_hoverAnnotation == null || _onEnter != _hoverAnnotation.onEnter || _onHover != _hoverAnnotation.onHover || _onExit != _hoverAnnotation.onExit,
"Shouldn't call _updateAnnotations if nothing has changed.");
bool changed = false;
final bool hadHoverAnnotation = _hoverAnnotation != null;
if (_hoverAnnotation != null && attached) {
RendererBinding.instance.mouseTracker.detachAnnotation(_hoverAnnotation);
changed = true;
}
if (_onPointerEnter != null || _onPointerHover != null || _onPointerExit != null) {
if (_onEnter != null || _onHover != null || _onExit != null) {
_hoverAnnotation = MouseTrackerAnnotation(
onEnter: _onPointerEnter,
onHover: _onPointerHover,
onExit: _onPointerExit,
onEnter: _onEnter,
onHover: _onHover,
onExit: _onExit,
);
if (attached) {
RendererBinding.instance.mouseTracker.attachAnnotation(_hoverAnnotation);
Expand Down Expand Up @@ -2674,43 +2736,16 @@ class RenderPointerListener extends RenderProxyBoxWithHitTestBehavior {
size = constraints.biggest;
}

@override
void handleEvent(PointerEvent event, HitTestEntry entry) {
assert(debugHandleEvent(event, entry));
// The onPointerEnter, onPointerHover, and onPointerExit events are are
// triggered from within the MouseTracker, not here.
if (onPointerDown != null && event is PointerDownEvent)
return onPointerDown(event);
if (onPointerMove != null && event is PointerMoveEvent)
return onPointerMove(event);
if (onPointerUp != null && event is PointerUpEvent)
return onPointerUp(event);
if (onPointerCancel != null && event is PointerCancelEvent)
return onPointerCancel(event);
if (onPointerSignal != null && event is PointerSignalEvent)
return onPointerSignal(event);
}

@override
void debugFillProperties(DiagnosticPropertiesBuilder properties) {
super.debugFillProperties(properties);
final List<String> listeners = <String>[];
if (onPointerDown != null)
listeners.add('down');
if (onPointerMove != null)
listeners.add('move');
if (onPointerEnter != null)
if (onEnter != null)
listeners.add('enter');
if (onPointerHover != null)
if (onHover != null)
listeners.add('hover');
if (onPointerExit != null)
if (onExit != null)
listeners.add('exit');
if (onPointerUp != null)
listeners.add('up');
if (onPointerCancel != null)
listeners.add('cancel');
if (onPointerSignal != null)
listeners.add('signal');
if (listeners.isEmpty)
listeners.add('<none>');
properties.add(IterableProperty<String>('listeners', listeners));
Expand Down