Unverified Commit 0903bf70 authored by Renzo Olivares's avatar Renzo Olivares Committed by GitHub

TextField context menu should fade on scroll on mobile devices (#138313)

This change affects Android and iOS devices using the TextField's context menu. After this change the context menu will fade out when scrolling the text and fade in when the scroll ends. 

If the scroll ends and the selection is outside of the view, then the toolbar will be scheduled to show in a future scroll end. This toolbar scheduling can be invalidated if the `TextEditingValue` changed anytime between the scheduling and when the toolbar is ready to be shown.

This change also fixes a regression where the TextField context menu would not fade when the selection handles where not visible.

When using the native browser context menu this behavior is not controlled by Flutter.

https://github.com/flutter/flutter/assets/948037/3f46bcbb-ba6f-456c-8473-e42919b9d572

Fixes #52425
Fixes #105804
Fixes #52426
parent 6322fef7
...@@ -163,6 +163,14 @@ class _CupertinoTextFieldSelectionGestureDetectorBuilder extends TextSelectionGe ...@@ -163,6 +163,14 @@ class _CupertinoTextFieldSelectionGestureDetectorBuilder extends TextSelectionGe
/// ///
/// {@macro flutter.widgets.editableText.showCaretOnScreen} /// {@macro flutter.widgets.editableText.showCaretOnScreen}
/// ///
/// ## Scrolling Considerations
///
/// If this [CupertinoTextField] is not a descendant of [Scaffold] and is being
/// used within a [Scrollable] or nested [Scrollable]s, consider placing a
/// [ScrollNotificationObserver] above the root [Scrollable] that contains this
/// [CupertinoTextField] to ensure proper scroll coordination for
/// [CupertinoTextField] and its components like [TextSelectionOverlay].
///
/// See also: /// See also:
/// ///
/// * <https://developer.apple.com/documentation/uikit/uitextfield> /// * <https://developer.apple.com/documentation/uikit/uitextfield>
......
...@@ -202,6 +202,14 @@ class _SelectableTextSelectionGestureDetectorBuilder extends TextSelectionGestur ...@@ -202,6 +202,14 @@ class _SelectableTextSelectionGestureDetectorBuilder extends TextSelectionGestur
/// To make [SelectableText] react to touch events, use callback [onTap] to achieve /// To make [SelectableText] react to touch events, use callback [onTap] to achieve
/// the desired behavior. /// the desired behavior.
/// ///
/// ## Scrolling Considerations
///
/// If this [SelectableText] is not a descendant of [Scaffold] and is being used
/// within a [Scrollable] or nested [Scrollable]s, consider placing a
/// [ScrollNotificationObserver] above the root [Scrollable] that contains this
/// [SelectableText] to ensure proper scroll coordination for [SelectableText]
/// and its components like [TextSelectionOverlay].
///
/// See also: /// See also:
/// ///
/// * [Text], which is the non selectable version of this widget. /// * [Text], which is the non selectable version of this widget.
......
...@@ -186,6 +186,14 @@ class _TextFieldSelectionGestureDetectorBuilder extends TextSelectionGestureDete ...@@ -186,6 +186,14 @@ class _TextFieldSelectionGestureDetectorBuilder extends TextSelectionGestureDete
/// ** See code in examples/api/lib/material/text_field/text_field.2.dart ** /// ** See code in examples/api/lib/material/text_field/text_field.2.dart **
/// {@end-tool} /// {@end-tool}
/// ///
/// ## Scrolling Considerations
///
/// If this [TextField] is not a descendant of [Scaffold] and is being used
/// within a [Scrollable] or nested [Scrollable]s, consider placing a
/// [ScrollNotificationObserver] above the root [Scrollable] that contains this
/// [TextField] to ensure proper scroll coordination for [TextField] and its
/// components like [TextSelectionOverlay].
///
/// See also: /// See also:
/// ///
/// * [TextFormField], which integrates with the [Form] widget. /// * [TextFormField], which integrates with the [Form] widget.
......
...@@ -1432,6 +1432,7 @@ class SelectionOverlay { ...@@ -1432,6 +1432,7 @@ class SelectionOverlay {
context: context, context: context,
contextMenuBuilder: (BuildContext context) { contextMenuBuilder: (BuildContext context) {
return _SelectionToolbarWrapper( return _SelectionToolbarWrapper(
visibility: toolbarVisible,
layerLink: toolbarLayerLink, layerLink: toolbarLayerLink,
offset: -renderBox.localToGlobal(Offset.zero), offset: -renderBox.localToGlobal(Offset.zero),
child: contextMenuBuilder(context), child: contextMenuBuilder(context),
...@@ -2228,8 +2229,6 @@ class TextSelectionGestureDetectorBuilder { ...@@ -2228,8 +2229,6 @@ class TextSelectionGestureDetectorBuilder {
switch (defaultTargetPlatform) { switch (defaultTargetPlatform) {
case TargetPlatform.android: case TargetPlatform.android:
case TargetPlatform.fuchsia: case TargetPlatform.fuchsia:
// On mobile platforms the selection is set on tap up.
editableText.hideToolbar(false);
case TargetPlatform.iOS: case TargetPlatform.iOS:
// On mobile platforms the selection is set on tap up. // On mobile platforms the selection is set on tap up.
break; break;
...@@ -2352,6 +2351,7 @@ class TextSelectionGestureDetectorBuilder { ...@@ -2352,6 +2351,7 @@ class TextSelectionGestureDetectorBuilder {
break; break;
// On desktop platforms the selection is set on tap down. // On desktop platforms the selection is set on tap down.
case TargetPlatform.android: case TargetPlatform.android:
editableText.hideToolbar(false);
if (isShiftPressedValid) { if (isShiftPressedValid) {
_extendSelection(details.globalPosition, SelectionChangedCause.tap); _extendSelection(details.globalPosition, SelectionChangedCause.tap);
return; return;
...@@ -2359,6 +2359,7 @@ class TextSelectionGestureDetectorBuilder { ...@@ -2359,6 +2359,7 @@ class TextSelectionGestureDetectorBuilder {
renderEditable.selectPosition(cause: SelectionChangedCause.tap); renderEditable.selectPosition(cause: SelectionChangedCause.tap);
editableText.showSpellCheckSuggestionsToolbar(); editableText.showSpellCheckSuggestionsToolbar();
case TargetPlatform.fuchsia: case TargetPlatform.fuchsia:
editableText.hideToolbar(false);
if (isShiftPressedValid) { if (isShiftPressedValid) {
_extendSelection(details.globalPosition, SelectionChangedCause.tap); _extendSelection(details.globalPosition, SelectionChangedCause.tap);
return; return;
......
...@@ -10034,7 +10034,7 @@ void main() { ...@@ -10034,7 +10034,7 @@ void main() {
skip: kIsWeb, // [intended] skip: kIsWeb, // [intended]
); );
testWidgets('text selection toolbar is hidden on tap down', (WidgetTester tester) async { testWidgets('text selection toolbar is hidden on tap down on desktop platforms', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController( final TextEditingController controller = TextEditingController(
text: 'blah1 blah2', text: 'blah1 blah2',
); );
...@@ -10077,7 +10077,7 @@ void main() { ...@@ -10077,7 +10077,7 @@ void main() {
expect(find.byType(CupertinoAdaptiveTextSelectionToolbar), findsNothing); expect(find.byType(CupertinoAdaptiveTextSelectionToolbar), findsNothing);
}, },
skip: isContextMenuProvidedByPlatform, // [intended] only applies to platforms where we supply the context menu. skip: isContextMenuProvidedByPlatform, // [intended] only applies to platforms where we supply the context menu.
variant: TargetPlatformVariant.all(excluding: <TargetPlatform>{ TargetPlatform.iOS }), variant: TargetPlatformVariant.all(excluding: TargetPlatformVariant.mobile().values),
); );
testWidgets('Does not shrink in height when enters text when there is large single-line placeholder', (WidgetTester tester) async { testWidgets('Does not shrink in height when enters text when there is large single-line placeholder', (WidgetTester tester) async {
......
...@@ -6149,17 +6149,16 @@ void main() { ...@@ -6149,17 +6149,16 @@ void main() {
scrollable.controller!.jumpTo(50.0); scrollable.controller!.jumpTo(50.0);
await tester.pumpAndSettle(); await tester.pumpAndSettle();
// Find the toolbar fade transition after the toolbar has been hidden. // Try to find the toolbar fade transition after the toolbar has been hidden
// as a result of a scroll. This removes the toolbar overlay entry so no fade
// transition should be found.
final List<FadeTransition> transitionsAfter = find.descendant( final List<FadeTransition> transitionsAfter = find.descendant(
of: find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_SelectionToolbarWrapper'), of: find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_SelectionToolbarWrapper'),
matching: find.byType(FadeTransition), matching: find.byType(FadeTransition),
).evaluate().map((Element e) => e.widget).cast<FadeTransition>().toList(); ).evaluate().map((Element e) => e.widget).cast<FadeTransition>().toList();
expect(transitionsAfter.length, 0);
expect(transitionsAfter.length, 1); expect(state.selectionOverlay, isNotNull);
expect(state.selectionOverlay!.toolbarIsVisible, false);
final FadeTransition toolbarAfter = transitionsAfter[0];
expect(toolbarAfter.opacity.value, 0.0);
// On web, we don't show the Flutter toolbar and instead rely on the browser // On web, we don't show the Flutter toolbar and instead rely on the browser
// toolbar. Until we change that, this test should remain skipped. // toolbar. Until we change that, this test should remain skipped.
...@@ -9564,8 +9563,8 @@ void main() { ...@@ -9564,8 +9563,8 @@ void main() {
), ),
); );
expect(scrollController1.attached, isTrue); final EditableTextState state = tester.state<EditableTextState>(find.byType(EditableText));
expect(scrollController2.attached, isFalse); expect(state.widget.scrollController, scrollController1);
// Change scrollController to controller 2. // Change scrollController to controller 2.
await tester.pumpWidget( await tester.pumpWidget(
...@@ -9581,8 +9580,8 @@ void main() { ...@@ -9581,8 +9580,8 @@ void main() {
), ),
); );
expect(scrollController1.attached, isFalse); expect(state.widget.scrollController, scrollController2);
expect(scrollController2.attached, isTrue);
// Changing scrollController to null. // Changing scrollController to null.
await tester.pumpWidget( await tester.pumpWidget(
...@@ -9597,8 +9596,7 @@ void main() { ...@@ -9597,8 +9596,7 @@ void main() {
), ),
); );
expect(scrollController1.attached, isFalse); expect(state.widget.scrollController, isNull);
expect(scrollController2.attached, isFalse);
// Change scrollController to back controller 2. // Change scrollController to back controller 2.
await tester.pumpWidget( await tester.pumpWidget(
...@@ -9614,8 +9612,7 @@ void main() { ...@@ -9614,8 +9612,7 @@ void main() {
), ),
); );
expect(scrollController1.attached, isFalse); expect(state.widget.scrollController, scrollController2);
expect(scrollController2.attached, isTrue);
}); });
testWidgets('getLocalRectForCaret does not throw when it sees an infinite point', (WidgetTester tester) async { testWidgets('getLocalRectForCaret does not throw when it sees an infinite point', (WidgetTester tester) async {
......
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