From a767a0ccbdec2ff755b9aa5277d2ca9e37e2146d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hannes=20Hulterg=C3=A5rd?= <47989573+Hannnes1@users.noreply.github.com> Date: Mon, 24 Mar 2025 14:14:30 +0100 Subject: [PATCH] Add focus check to onTapUpOutside (#162939) Adds a focus check to `onTapUpOutside` of `EditableText`, so that it doesn't trigger if the `EditableText` doesn't have focus. `onTapOutside` already had this check, but it was missed when `onTapUpOutside` was added. Fixes https://github.com/flutter/flutter/issues/162573 ## 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]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] 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 --- .../lib/src/widgets/editable_text.dart | 42 ++++- .../test/material/text_form_field_test.dart | 33 ---- .../test/widgets/editable_text_test.dart | 147 ++++++++++++++++++ 3 files changed, 182 insertions(+), 40 deletions(-) diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index fa0853c9f1..7a1723b9c5 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -3738,6 +3738,14 @@ class EditableTextState extends State bool get _hasFocus => widget.focusNode.hasFocus; bool get _isMultiline => widget.maxLines != 1; + /// Flag to track whether this [EditableText] was in focus when [onTapOutside] + /// was called. + /// + /// This is used to determine whether [onTapUpOutside] should be called. + /// The reason [_hasFocus] can't be used directly is because [onTapOutside] + /// might unfocus this [EditableText] and block the [onTapUpOutside] call. + bool _hadFocusOnTapDown = false; + // Finds the closest scroll offset to the current scroll offset that fully // reveals the given caret rect. If the given rect's main axis extent is too // large to be fully revealed in `renderEditable`, it will be centered along @@ -5369,6 +5377,31 @@ class EditableTextState extends State return Actions.invoke(context, intent); } + void _onTapOutside(BuildContext context, PointerDownEvent event) { + _hadFocusOnTapDown = true; + + if (widget.onTapOutside != null) { + widget.onTapOutside!(event); + } else { + _defaultOnTapOutside(context, event); + } + } + + void _onTapUpOutside(BuildContext context, PointerUpEvent event) { + if (!_hadFocusOnTapDown) { + return; + } + + // Reset to false so that subsequent events doesn't trigger the callback based on old information. + _hadFocusOnTapDown = false; + + if (widget.onTapUpOutside != null) { + widget.onTapUpOutside!(event); + } else { + _defaultOnTapUpOutside(context, event); + } + } + /// The default behavior used if [EditableText.onTapOutside] is null. /// /// The `event` argument is the [PointerDownEvent] that caused the notification. @@ -5536,13 +5569,8 @@ class EditableTextState extends State return TextFieldTapRegion( groupId: widget.groupId, onTapOutside: - _hasFocus - ? widget.onTapOutside ?? - (PointerDownEvent event) => _defaultOnTapOutside(context, event) - : null, - onTapUpOutside: - widget.onTapUpOutside ?? - (PointerUpEvent event) => _defaultOnTapUpOutside(context, event), + _hasFocus ? (PointerDownEvent event) => _onTapOutside(context, event) : null, + onTapUpOutside: (PointerUpEvent event) => _onTapUpOutside(context, event), debugLabel: kReleaseMode ? null : 'EditableText', child: MouseRegion( cursor: widget.mouseCursor ?? SystemMouseCursors.text, diff --git a/packages/flutter/test/material/text_form_field_test.dart b/packages/flutter/test/material/text_form_field_test.dart index 07d9174a62..6bf0269c04 100644 --- a/packages/flutter/test/material/text_form_field_test.dart +++ b/packages/flutter/test/material/text_form_field_test.dart @@ -737,39 +737,6 @@ void main() { expect(tapOutsideCount, 3); }); - // Regression test for https://github.com/flutter/flutter/issues/134341. - testWidgets('onTapOutside is not called upon tap outside when field is not focused', ( - WidgetTester tester, - ) async { - int tapOutsideCount = 0; - await tester.pumpWidget( - MaterialApp( - home: Material( - child: Center( - child: Column( - children: [ - const Text('Outside'), - TextFormField( - onTapOutside: (PointerEvent event) { - tapOutsideCount += 1; - }, - ), - ], - ), - ), - ), - ), - ); - await tester.pump(); - - expect(tapOutsideCount, 0); - await tester.tap(find.byType(TextFormField)); - await tester.tap(find.text('Outside')); - await tester.tap(find.text('Outside')); - await tester.tap(find.text('Outside')); - expect(tapOutsideCount, 0); - }); - // Regression test for https://github.com/flutter/flutter/issues/127597. testWidgets( 'The second TextFormField is clicked, triggers the onTapOutside callback of the previous TextFormField', diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index 3e77dbb0e3..f812269dcd 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -12038,6 +12038,7 @@ void main() { style: textStyle, cursorColor: Colors.blue, backgroundCursorColor: Colors.grey, + autofocus: true, ), ), ], @@ -16958,6 +16959,152 @@ void main() { variant: const TargetPlatformVariant({TargetPlatform.iOS}), skip: kIsWeb, // [intended] ); + + testWidgets('onTapOutside is called upon tap outside', (WidgetTester tester) async { + int tapOutsideCount = 0; + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Center( + child: Column( + children: [ + const Text('Outside'), + EditableText( + autofocus: true, + controller: controller, + focusNode: focusNode, + style: textStyle, + cursorColor: Colors.blue, + backgroundCursorColor: Colors.grey, + onTapOutside: (PointerEvent event) { + tapOutsideCount += 1; + }, + ), + ], + ), + ), + ), + ), + ); + await tester.pump(); // Wait for autofocus to take effect. + + expect(tapOutsideCount, 0); + await tester.tap(find.byType(EditableText)); + await tester.tap(find.text('Outside')); + await tester.tap(find.text('Outside')); + await tester.tap(find.text('Outside')); + expect(tapOutsideCount, 3); + }); + + // Regression test for https://github.com/flutter/flutter/issues/134341. + testWidgets('onTapOutside is not called upon tap outside when field is not focused', ( + WidgetTester tester, + ) async { + int tapOutsideCount = 0; + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Center( + child: Column( + children: [ + const Text('Outside'), + EditableText( + controller: controller, + focusNode: focusNode, + style: textStyle, + cursorColor: Colors.blue, + backgroundCursorColor: Colors.grey, + onTapOutside: (PointerEvent event) { + tapOutsideCount += 1; + }, + ), + ], + ), + ), + ), + ), + ); + await tester.pump(); + + expect(tapOutsideCount, 0); + await tester.tap(find.text('Outside')); + await tester.tap(find.text('Outside')); + await tester.tap(find.text('Outside')); + expect(tapOutsideCount, 0); + }); + + testWidgets('onTapUpOutside is called upon tap up outside', (WidgetTester tester) async { + int tapOutsideCount = 0; + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Center( + child: Column( + children: [ + const Text('Outside'), + EditableText( + autofocus: true, + controller: controller, + focusNode: focusNode, + style: textStyle, + cursorColor: Colors.blue, + backgroundCursorColor: Colors.grey, + onTapUpOutside: (PointerEvent event) { + tapOutsideCount += 1; + }, + ), + ], + ), + ), + ), + ), + ); + await tester.pump(); // Wait for autofocus to take effect. + + expect(tapOutsideCount, 0); + await tester.tap(find.byType(EditableText)); + await tester.tap(find.text('Outside')); + await tester.tap(find.text('Outside')); + await tester.tap(find.text('Outside')); + expect(tapOutsideCount, 3); + }); + + // Regression test for https://github.com/flutter/flutter/issues/162573 + testWidgets('onTapUpOutside is not called upon tap up outside when field is not focused', ( + WidgetTester tester, + ) async { + int tapOutsideCount = 0; + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Center( + child: Column( + children: [ + const Text('Outside'), + EditableText( + controller: controller, + focusNode: focusNode, + style: textStyle, + cursorColor: Colors.blue, + backgroundCursorColor: Colors.grey, + onTapUpOutside: (PointerEvent event) { + tapOutsideCount += 1; + }, + ), + ], + ), + ), + ), + ), + ); + await tester.pump(); + + expect(tapOutsideCount, 0); + await tester.tap(find.text('Outside')); + await tester.tap(find.text('Outside')); + await tester.tap(find.text('Outside')); + expect(tapOutsideCount, 0); + }); } class UnsettableController extends TextEditingController {