Skip to content

Commit

Permalink
Exclude Tooltip's overlay child from SelectableRegion (#130181)
Browse files Browse the repository at this point in the history
Fixes flutter/flutter#129969 by making tooltip text unselectable (for now). 
Also fixes some other issues uncovered when I was writing the tests.

Currently `getTransformTo` only works on ancestors. I'll try to add a new method that computes the transform from 2 arbitrary render objects in the same render tree in a follow-up PR and make `Selectable` use that method instead.
  • Loading branch information
LongCatIsLooong authored Jul 13, 2023
1 parent dd0b6e3 commit 2da353a
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 24 deletions.
28 changes: 7 additions & 21 deletions packages/flutter/lib/src/material/selection_area.dart
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,7 @@ class SelectionArea extends StatefulWidget {
}

class _SelectionAreaState extends State<SelectionArea> {
FocusNode get _effectiveFocusNode {
if (widget.focusNode != null) {
return widget.focusNode!;
}
_internalNode ??= FocusNode();
return _internalNode!;
}
FocusNode get _effectiveFocusNode => widget.focusNode ?? (_internalNode ??= FocusNode());
FocusNode? _internalNode;

@override
Expand All @@ -121,20 +115,12 @@ class _SelectionAreaState extends State<SelectionArea> {
@override
Widget build(BuildContext context) {
assert(debugCheckHasMaterialLocalizations(context));
TextSelectionControls? controls = widget.selectionControls;
switch (Theme.of(context).platform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
controls ??= materialTextSelectionHandleControls;
case TargetPlatform.iOS:
controls ??= cupertinoTextSelectionHandleControls;
case TargetPlatform.linux:
case TargetPlatform.windows:
controls ??= desktopTextSelectionHandleControls;
case TargetPlatform.macOS:
controls ??= cupertinoDesktopTextSelectionHandleControls;
}

final TextSelectionControls controls = widget.selectionControls ?? switch (Theme.of(context).platform) {
TargetPlatform.android || TargetPlatform.fuchsia => materialTextSelectionHandleControls,
TargetPlatform.linux || TargetPlatform.windows => desktopTextSelectionHandleControls,
TargetPlatform.iOS => cupertinoTextSelectionHandleControls,
TargetPlatform.macOS => cupertinoDesktopTextSelectionHandleControls,
};
return SelectableRegion(
selectionControls: controls,
focusNode: _effectiveFocusNode,
Expand Down
19 changes: 16 additions & 3 deletions packages/flutter/lib/src/material/tooltip.dart
Original file line number Diff line number Diff line change
Expand Up @@ -482,13 +482,16 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
void _scheduleDismissTooltip({ required Duration withDelay }) {
assert(mounted);
assert(
!(_timer?.isActive ?? false) || _controller.status != AnimationStatus.reverse,
!(_timer?.isActive ?? false) || _backingController?.status != AnimationStatus.reverse,
'timer must not be active when the tooltip is fading out',
);

_timer?.cancel();
_timer = null;
switch (_controller.status) {
// Use _backingController instead of _controller to prevent the lazy getter
// from instaniating an AnimationController unnecessarily.
switch (_backingController?.status) {
case null:
case AnimationStatus.reverse:
case AnimationStatus.dismissed:
break;
Expand Down Expand Up @@ -740,7 +743,7 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
};

final TooltipThemeData tooltipTheme = _tooltipTheme;
return _TooltipOverlay(
final _TooltipOverlay overlayChild = _TooltipOverlay(
richMessage: widget.richMessage ?? TextSpan(text: widget.message),
height: widget.height ?? tooltipTheme.height ?? _getDefaultTooltipHeight(),
padding: widget.padding ?? tooltipTheme.padding ?? _getDefaultPadding(),
Expand All @@ -755,13 +758,23 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
verticalOffset: widget.verticalOffset ?? tooltipTheme.verticalOffset ?? _defaultVerticalOffset,
preferBelow: widget.preferBelow ?? tooltipTheme.preferBelow ?? _defaultPreferBelow,
);

return SelectionContainer.maybeOf(context) == null
? overlayChild
: SelectionContainer.disabled(child: overlayChild);
}

@override
void dispose() {
GestureBinding.instance.pointerRouter.removeGlobalRoute(_handleGlobalPointerEvent);
Tooltip._openedTooltips.remove(this);
// _longPressRecognizer.dispose() and _tapRecognizer.dispose() may call
// their registered onCancel callbacks if there's a gesture in progress.
// Remove the onCancel callbacks to prevent the registered callbacks from
// triggering unnecessary side effects (such as animations).
_longPressRecognizer?.onLongPressCancel = null;
_longPressRecognizer?.dispose();
_tapRecognizer?.onTapCancel = null;
_tapRecognizer?.dispose();
_timer?.cancel();
_backingController?.dispose();
Expand Down
1 change: 1 addition & 0 deletions packages/flutter/lib/src/widgets/overlay.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1569,6 +1569,7 @@ class _OverlayPortalState extends State<OverlayPortal> {

@override
void dispose() {
assert(widget.controller._attachTarget == this);
widget.controller._attachTarget = null;
_locationCache?._debugMarkLocationInvalid();
_locationCache = null;
Expand Down
91 changes: 91 additions & 0 deletions packages/flutter/test/material/tooltip_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2278,6 +2278,97 @@ void main() {
await tester.pump(const Duration(seconds: 1));
expect(element.dirty, isFalse);
});

testWidgets('Tooltip does not initialize animation controller in dispose process', (WidgetTester tester) async {
await tester.pumpWidget(
const MaterialApp(
home: Center(
child: Tooltip(
message: tooltipText,
waitDuration: Duration(seconds: 1),
triggerMode: TooltipTriggerMode.longPress,
child: SizedBox.square(dimension: 50),
),
),
),
);

await tester.startGesture(tester.getCenter(find.byType(Tooltip)));
await tester.pumpWidget(const SizedBox());
expect(tester.takeException(), isNull);
});

testWidgets('Tooltip does not crash when showing the tooltip but the OverlayPortal is unmounted, during dispose', (WidgetTester tester) async {
await tester.pumpWidget(
const MaterialApp(
home: SelectionArea(
child: Center(
child: Tooltip(
message: tooltipText,
waitDuration: Duration(seconds: 1),
triggerMode: TooltipTriggerMode.longPress,
child: SizedBox.square(dimension: 50),
),
),
),
),
);

final TooltipState tooltipState = tester.state(find.byType(Tooltip));
await tester.startGesture(tester.getCenter(find.byType(Tooltip)));
tooltipState.ensureTooltipVisible();
await tester.pumpWidget(const SizedBox());
expect(tester.takeException(), isNull);
});

testWidgets('Tooltip is not selectable', (WidgetTester tester) async {
const String tooltipText = 'AAAAAAAAAAAAAAAAAAAAAAA';
String? selectedText;
await tester.pumpWidget(
MaterialApp(
home: SelectionArea(
onSelectionChanged: (SelectedContent? content) { selectedText = content?.plainText; },
child: const Center(
child: Column(
children: <Widget>[
Text('Select Me'),
Tooltip(
message: tooltipText,
waitDuration: Duration(seconds: 1),
triggerMode: TooltipTriggerMode.longPress,
child: SizedBox.square(dimension: 50),
),
],
),
),
),
),
);

final TooltipState tooltipState = tester.state(find.byType(Tooltip));

final Rect textRect = tester.getRect(find.text('Select Me'));
final TestGesture gesture = await tester.startGesture(Alignment.centerLeft.alongSize(textRect.size) + textRect.topLeft);
// Drag from centerLeft to centerRight to select the text.
await tester.pump(const Duration(seconds: 1));
await gesture.moveTo(Alignment.centerRight.alongSize(textRect.size) + textRect.topLeft);
await tester.pump();

tooltipState.ensureTooltipVisible();
await tester.pump();
// Make sure the tooltip becomes visible.
expect(find.text(tooltipText), findsOneWidget);
assert(selectedText != null);

final Rect tooltipTextRect = tester.getRect(find.text(tooltipText));
// Now drag from centerLeft to centerRight to select the tooltip text.
await gesture.moveTo(Alignment.centerLeft.alongSize(tooltipTextRect.size) + tooltipTextRect.topLeft);
await tester.pump();
await gesture.moveTo(Alignment.centerRight.alongSize(tooltipTextRect.size) + tooltipTextRect.topLeft);
await tester.pump();

expect(selectedText, isNot(contains('A')));
});
}

Future<void> setWidgetForTooltipMode(
Expand Down

0 comments on commit 2da353a

Please sign in to comment.