iPad Scribble flicker and crash (#159508)
Previously, dragging to select with an Apple Pencil on an iPad (Scribble) caused the context menu to rapidly hide and show. Sometimes this even caused an assertion error when using SystemContextMenu due to showing two context menus in one frame. After this PR, the flicker and crash are gone. The flicker happened on both the Flutter-rendered context menu and SystemContextMenu, but the error only happened with SystemContextMenu due to a safeguard that prevents two from showing at the same time. The flickering is likely a regression caused by https://github.com/flutter/flutter/pull/142463. | Before this PR | After this PR | | --- | --- | | <video src="https://github.com/user-attachments/assets/e35f36f5-350d-41fb-b878-ee7b7820699d" /> | <video src="https://github.com/user-attachments/assets/262cb8d3-6670-4765-ace8-2d9bf61ae112" /> | Flutter's behavior isn't perfect compared to native (below), but it's a major improvement. If we want to match native, I think we might have to mess with the engine and see why it's calling showToolbar so much. I checked and scribbleInProgress is false during this selection gesture, so we can't use that. <details> <summary>Scribble native video</summary> https://github.com/user-attachments/assets/207e208a-ac36-4c9e-a8ed-9e90e6ef9e3a </details> Fixes https://github.com/flutter/flutter/issues/159259
This commit is contained in:
parent
2d2cc5dab9
commit
e7a8dc3a27
@ -4683,6 +4683,9 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
|
|||||||
if (_selectionOverlay == null) {
|
if (_selectionOverlay == null) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
if (_selectionOverlay!.toolbarIsVisible) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
_liveTextInputStatus?.update();
|
_liveTextInputStatus?.update();
|
||||||
clipboardStatus.update();
|
clipboardStatus.update();
|
||||||
_selectionOverlay!.showToolbar();
|
_selectionOverlay!.showToolbar();
|
||||||
|
@ -683,4 +683,61 @@ void main() {
|
|||||||
|
|
||||||
// On web, we should rely on the browser's implementation of Scribble, so we will not send selection rects.
|
// On web, we should rely on the browser's implementation of Scribble, so we will not send selection rects.
|
||||||
}, skip: kIsWeb, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS })); // [intended]
|
}, skip: kIsWeb, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS })); // [intended]
|
||||||
|
|
||||||
|
// Regression test for https://github.com/flutter/flutter/issues/159259.
|
||||||
|
testWidgets('showToolbar does nothing and returns false when already shown during Scribble selection', (WidgetTester tester) async {
|
||||||
|
controller.text = 'Lorem ipsum dolor sit amet';
|
||||||
|
final GlobalKey<EditableTextState> editableTextKey = GlobalKey();
|
||||||
|
|
||||||
|
await tester.pumpWidget(
|
||||||
|
MaterialApp(
|
||||||
|
home: EditableText(
|
||||||
|
key: editableTextKey,
|
||||||
|
controller: controller,
|
||||||
|
backgroundCursorColor: Colors.grey,
|
||||||
|
focusNode: focusNode,
|
||||||
|
style: textStyle,
|
||||||
|
cursorColor: cursorColor,
|
||||||
|
selectionControls: materialTextSelectionHandleControls,
|
||||||
|
contextMenuBuilder: (BuildContext context, EditableTextState editableTextState) {
|
||||||
|
return AdaptiveTextSelectionToolbar.editableText(
|
||||||
|
editableTextState: editableTextState,
|
||||||
|
);
|
||||||
|
},
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(find.byType(AdaptiveTextSelectionToolbar), findsNothing);
|
||||||
|
|
||||||
|
await tester.showKeyboard(find.byType(EditableText));
|
||||||
|
|
||||||
|
await tester.testTextInput.startScribbleInteraction();
|
||||||
|
tester.testTextInput.updateEditingValue(TextEditingValue(
|
||||||
|
text: controller.text,
|
||||||
|
selection: const TextSelection(baseOffset: 3, extentOffset: 4),
|
||||||
|
));
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
|
||||||
|
expect(find.byType(AdaptiveTextSelectionToolbar), findsNothing);
|
||||||
|
|
||||||
|
expect(editableTextKey.currentState!.showToolbar(), isTrue);
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
|
||||||
|
expect(find.byType(AdaptiveTextSelectionToolbar), findsOneWidget);
|
||||||
|
|
||||||
|
expect(editableTextKey.currentState!.showToolbar(), isFalse);
|
||||||
|
await tester.pump();
|
||||||
|
|
||||||
|
expect(find.byType(AdaptiveTextSelectionToolbar), findsOneWidget);
|
||||||
|
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
|
||||||
|
expect(find.byType(AdaptiveTextSelectionToolbar), findsOneWidget);
|
||||||
|
|
||||||
|
await tester.testTextInput.finishScribbleInteraction();
|
||||||
|
},
|
||||||
|
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS }),
|
||||||
|
skip: kIsWeb, // [intended]
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
@ -17212,6 +17212,51 @@ void main() {
|
|||||||
|
|
||||||
expect(tester.takeException(), isNull);
|
expect(tester.takeException(), isNull);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Regression test for https://github.com/flutter/flutter/issues/159259.
|
||||||
|
testWidgets('showToolbar does nothing and returns false when already shown', (WidgetTester tester) async {
|
||||||
|
controller.text = 'Lorem ipsum dolor sit amet';
|
||||||
|
final GlobalKey<EditableTextState> editableTextKey = GlobalKey();
|
||||||
|
|
||||||
|
await tester.pumpWidget(
|
||||||
|
MaterialApp(
|
||||||
|
home: EditableText(
|
||||||
|
key: editableTextKey,
|
||||||
|
autofocus: true,
|
||||||
|
controller: controller,
|
||||||
|
backgroundCursorColor: Colors.grey,
|
||||||
|
focusNode: focusNode,
|
||||||
|
style: textStyle,
|
||||||
|
cursorColor: cursorColor,
|
||||||
|
selectionControls: materialTextSelectionHandleControls,
|
||||||
|
contextMenuBuilder: (BuildContext context, EditableTextState editableTextState) {
|
||||||
|
return AdaptiveTextSelectionToolbar.editableText(
|
||||||
|
editableTextState: editableTextState,
|
||||||
|
);
|
||||||
|
},
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(find.byType(AdaptiveTextSelectionToolbar), findsNothing);
|
||||||
|
|
||||||
|
expect(editableTextKey.currentState!.showToolbar(), isTrue);
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
|
||||||
|
expect(find.byType(AdaptiveTextSelectionToolbar), findsOneWidget);
|
||||||
|
|
||||||
|
expect(editableTextKey.currentState!.showToolbar(), isFalse);
|
||||||
|
await tester.pump();
|
||||||
|
|
||||||
|
expect(find.byType(AdaptiveTextSelectionToolbar), findsOneWidget);
|
||||||
|
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
|
||||||
|
expect(find.byType(AdaptiveTextSelectionToolbar), findsOneWidget);
|
||||||
|
},
|
||||||
|
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS }),
|
||||||
|
skip: kIsWeb, // [intended]
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
class UnsettableController extends TextEditingController {
|
class UnsettableController extends TextEditingController {
|
||||||
|
Loading…
x
Reference in New Issue
Block a user