From 88502abbfd8dfbf54fc6546165e16dfb054368ca Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Tue, 26 Nov 2024 16:52:35 -0800 Subject: [PATCH 1/3] [SuperEditor] - Fix: When navigating from screen with keyboard open to screen with no IME connection, KeyboardScaffoldSafeArea pushes the content up above the keyboard even though its closed. (Resolves #2419) (#2421) --- .../example/lib/main_super_editor_chat.dart | 37 +++- .../keyboard_panel_scaffold.dart | 44 +++- .../keyboard_panel_scaffold_test.dart | 190 ++++++++++++++++-- 3 files changed, 243 insertions(+), 28 deletions(-) diff --git a/super_editor/example/lib/main_super_editor_chat.dart b/super_editor/example/lib/main_super_editor_chat.dart index 22e26f6804..16d7fd21b7 100644 --- a/super_editor/example/lib/main_super_editor_chat.dart +++ b/super_editor/example/lib/main_super_editor_chat.dart @@ -25,10 +25,39 @@ void main() { runApp( MaterialApp( - home: Scaffold( - resizeToAvoidBottomInset: false, - body: MobileChatDemo(), - ), + routes: { + "/": (context) => Scaffold( + appBar: AppBar( + actions: [ + IconButton( + onPressed: () { + Navigator.of(context).pushNamed("/second"); + }, + icon: Icon(Icons.settings), + ), + ], + ), + resizeToAvoidBottomInset: false, + body: MobileChatDemo(), + ), + // We include a 2nd screen with navigation so that we can verify + // what happens to the keyboard safe area when navigating from an + // open editor to another screen with a safe area, but no keyboard + // scaffold. See issue #2419 + "/second": (context) => Scaffold( + appBar: AppBar(), + resizeToAvoidBottomInset: false, + body: KeyboardScaffoldSafeArea( + child: ListView.builder( + itemBuilder: (context, index) { + return ListTile( + title: Text("Item $index"), + ); + }, + ), + ), + ), + }, debugShowCheckedModeBanner: false, ), ); diff --git a/super_editor/lib/src/infrastructure/keyboard_panel_scaffold.dart b/super_editor/lib/src/infrastructure/keyboard_panel_scaffold.dart index 7da87e0134..441e53f128 100644 --- a/super_editor/lib/src/infrastructure/keyboard_panel_scaffold.dart +++ b/super_editor/lib/src/infrastructure/keyboard_panel_scaffold.dart @@ -707,6 +707,7 @@ class _KeyboardScaffoldSafeAreaState extends State KeyboardSafeAreaGeometry? _keyboardSafeAreaData; KeyboardScaffoldSafeAreaMutator? _ancestorSafeArea; + bool _isSafeAreaFromMediaQuery = false; @override void didChangeDependencies() { @@ -723,14 +724,46 @@ class _KeyboardScaffoldSafeAreaState extends State // of the editor, not a direct ancestor or descendant. So we need to be able to coordinate // the safe area across independent trees by sharing an ancestor. // - // If there's no existing ancestor KeyboardScaffoldSafeArea, then defer to whatever + // Example: + // KeyboardScaffoldSafeArea + // |- Stack + // |- KeyboardScaffoldSafeArea + // |- Content + // |- SuperEditor + // + // Second, if there's no existing ancestor KeyboardScaffoldSafeArea, then defer to whatever // MediaQuery reports. We only do this for the very first frame because we don't yet // know what our values should be (because that's reported by descendants in the tree). _ancestorSafeArea = KeyboardScaffoldSafeArea.maybeOf(context); - _keyboardSafeAreaData ??= KeyboardSafeAreaGeometry( - bottomInsets: _ancestorSafeArea?.geometry.bottomInsets ?? MediaQuery.viewInsetsOf(context).bottom, - bottomPadding: _ancestorSafeArea?.geometry.bottomPadding ?? MediaQuery.paddingOf(context).bottom, - ); + + if (_keyboardSafeAreaData == null) { + // This is the first call to didChangeDependencies. Initialize our safe area. + _keyboardSafeAreaData = KeyboardSafeAreaGeometry( + bottomInsets: _ancestorSafeArea?.geometry.bottomInsets ?? MediaQuery.viewInsetsOf(context).bottom, + bottomPadding: _ancestorSafeArea?.geometry.bottomPadding ?? MediaQuery.paddingOf(context).bottom, + ); + + // We track whether our safe area is from MediaQuery (instead of an another KeyboardSafeAreaGeometry). + // We do this in case the MediaQuery value changes when we don't have any descendant + // KeyboardPanelScaffold. + // + // For example, you're on Screen 1 with the keyboard up. You navigate to Screen 2, which closes the keyboard. When + // Screen 2 first pumps, it sees that the keyboard is up, so it configures a keyboard safe area. But the keyboard + // immediately closes. Screen 2 is then stuck with a keyboard safe area that never goes away. + // + // By tracking when our safe area comes from MediaQuery, we can continue to honor changing + // MediaQuery values until a descendant explicitly sets our `geometry`. + _isSafeAreaFromMediaQuery = _ancestorSafeArea == null; + } + + if (_isSafeAreaFromMediaQuery) { + // Our current safe area came from MediaQuery, not a descendant. Therefore, + // we want to continue blindly honoring the MediaQuery. + _keyboardSafeAreaData = KeyboardSafeAreaGeometry( + bottomInsets: MediaQuery.viewInsetsOf(context).bottom, + bottomPadding: MediaQuery.paddingOf(context).bottom, + ); + } } @override @@ -738,6 +771,7 @@ class _KeyboardScaffoldSafeAreaState extends State @override set geometry(KeyboardSafeAreaGeometry geometry) { + _isSafeAreaFromMediaQuery = false; if (geometry == _keyboardSafeAreaData) { return; } diff --git a/super_editor/test/infrastructure/keyboard_panel_scaffold_test.dart b/super_editor/test/infrastructure/keyboard_panel_scaffold_test.dart index 652eabaefd..2663dd72e1 100644 --- a/super_editor/test/infrastructure/keyboard_panel_scaffold_test.dart +++ b/super_editor/test/infrastructure/keyboard_panel_scaffold_test.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test_runners/flutter_test_runners.dart'; +import 'package:golden_toolkit/golden_toolkit.dart'; import 'package:super_editor/src/test/super_editor_test/supereditor_robot.dart'; import 'package:super_editor/super_editor.dart'; @@ -10,7 +11,7 @@ void main() { group('Keyboard panel scaffold >', () { group('phones >', () { testWidgetsOnMobilePhone('does not show toolbar upon initialization when IME is disconnected', (tester) async { - await _pumpTestApp(tester); + await _pumpTestAppWithTabs(tester); // Ensure the toolbar isn't visible. expect(find.byKey(_aboveKeyboardToolbarKey), findsNothing); @@ -20,7 +21,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -41,7 +42,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -67,7 +68,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -100,7 +101,7 @@ void main() { ); testWidgetsOnMobilePhone('does not show keyboard panel upon keyboard appearance', (tester) async { - await _pumpTestApp(tester); + await _pumpTestAppWithTabs(tester); // Place the caret at the beginning of the document to show the software keyboard. await tester.placeCaretInParagraph('1', 0); @@ -113,7 +114,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -134,7 +135,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -168,7 +169,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -206,7 +207,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -243,7 +244,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -275,7 +276,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -315,7 +316,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -340,7 +341,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -398,7 +399,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -423,7 +424,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -481,7 +482,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -506,7 +507,7 @@ void main() { final softwareKeyboardController = SoftwareKeyboardController(); final controller = KeyboardPanelController(softwareKeyboardController); - await _pumpTestApp( + await _pumpTestAppWithTabs( tester, controller: controller, softwareKeyboardController: softwareKeyboardController, @@ -536,16 +537,65 @@ void main() { // Ensure that the account tab's content is full height (isn't restricted by safe area). expect(tester.getSize(find.byKey(_accountPageKey)).height, contentHeightWithNoKeyboard); }); + + testWidgetsOnMobilePhone('does not retain bottom insets when closing keyboard during navigation', (tester) async { + final navigatorKey = GlobalKey(); + final softwareKeyboardController = SoftwareKeyboardController(); + final controller = KeyboardPanelController(softwareKeyboardController); + + await _pumpTestAppWithNavigationScreens( + tester, + navigatorKey: navigatorKey, + controller: controller, + softwareKeyboardController: softwareKeyboardController, + simulatedKeyboardHeight: _expandedPhoneKeyboardHeight, + ); + + // Record the height of the content when no keyboard is open. + final contentHeightWithNoKeyboard = tester.getSize(find.byKey(_chatPageKey)).height; + + // Show the keyboard. Don't show the toolbar because it's irrelevant for this test. + controller.toolbarVisibility = KeyboardToolbarVisibility.hidden; + controller.showSoftwareKeyboard(); + await tester.pumpAndSettle(); + + // Record the height of the content now that the keyboard is open. + final contentHeightWithKeyboardPanelOpen = tester.getSize(find.byKey(_chatPageKey)).height; + + // Ensure that the content is pushed up above the keyboard. + expect(contentHeightWithNoKeyboard - contentHeightWithKeyboardPanelOpen, _expandedPhoneKeyboardHeight); + + // Navigate to screen 2, while simultaneously closing the keyboard (which is what + // happens when navigating to a new screen without an IME connection). + navigatorKey.currentState!.pushNamed("/second"); + + // CRITICAL: The reason navigation is a problem is because the first pump of a new + // screen happens before the keyboard starts to close (in a real app). Therefore, we + // pump one frame here to create the new screen and THEN we close the keyboard. + await tester.pump(); + + // Close the keyboard now that the new screen is starting to navigate in. + softwareKeyboardController.close(); + + // Pump and settle to let the navigation animation play. + await tester.pumpAndSettle(); + + // Ensure that the second page body takes up all available space. + expect(tester.getSize(find.byKey(_screen2BodyKey)).height, contentHeightWithNoKeyboard); + }); }); }); } -/// Pumps a tree that displays a panel at the software keyboard position. +/// Pumps a tree that displays a two tab UI, one tab has an editor that shows +/// a keyboard panel, and the other tab has no editor at all, which can be +/// used to verify what happens when navigating from an open editor to another +/// tab. /// /// Simulates the software keyboard appearance and disappearance by animating /// the `MediaQuery` view insets when the app communicates with the IME to show/hide /// the software keyboard. -Future _pumpTestApp( +Future _pumpTestAppWithTabs( WidgetTester tester, { KeyboardPanelController? controller, SoftwareKeyboardController? softwareKeyboardController, @@ -675,6 +725,108 @@ const _chatPageKey = ValueKey("chat_content"); const _accountTabKey = ValueKey("account_tab_button"); const _accountPageKey = ValueKey("account_content"); +const _screen2BodyKey = ValueKey("screen_2_body"); + +/// Pumps a tree that can display two screens - the first screen has an editor +/// with a keyboard panel scaffold, the second screen has a keyboard safe area +/// but no editor or keyboard panel scaffold. +/// +/// Simulates the software keyboard appearance and disappearance by animating +/// the `MediaQuery` view insets when the app communicates with the IME to show/hide +/// the software keyboard. +Future _pumpTestAppWithNavigationScreens( + WidgetTester tester, { + required GlobalKey navigatorKey, + KeyboardPanelController? controller, + SoftwareKeyboardController? softwareKeyboardController, + ValueNotifier? isImeConnected, + double simulatedKeyboardHeight = _expandedPhoneKeyboardHeight, +}) async { + final keyboardController = softwareKeyboardController ?? SoftwareKeyboardController(); + final keyboardPanelController = controller ?? KeyboardPanelController(keyboardController); + final imeConnectionNotifier = isImeConnected ?? ValueNotifier(false); + + await tester // + .createDocument() + .withLongDoc() + .withSoftwareKeyboardController(keyboardController) + .withImeConnectionNotifier(imeConnectionNotifier) + .simulateSoftwareKeyboardInsets( + true, + simulatedKeyboardHeight: simulatedKeyboardHeight, + ) + .withCustomWidgetTreeBuilder( + (superEditor) => MaterialApp( + navigatorKey: navigatorKey, + routes: { + '/': (context) { + return _Screen1( + keyboardPanelController: keyboardPanelController, + imeConnectionNotifier: imeConnectionNotifier, + superEditor: superEditor, + ); + }, + '/second': (context) { + return const _Screen2(); + }, + }, + ), + ) + .pump(); +} + +class _Screen1 extends StatelessWidget { + const _Screen1({ + required this.keyboardPanelController, + required this.imeConnectionNotifier, + required this.superEditor, + }); + + final KeyboardPanelController keyboardPanelController; + final ValueNotifier imeConnectionNotifier; + final Widget superEditor; + + @override + Widget build(BuildContext context) { + return Scaffold( + resizeToAvoidBottomInset: false, + body: KeyboardScaffoldSafeArea( + // ^ This safe area is needed to receive the bottom insets from the + // bottom mounted editor, and then make it available to the subtree + // with the content behind the chat. + child: _buildChatPage( + keyboardPanelController, + imeConnectionNotifier, + superEditor, + ), + ), + ); + } +} + +class _Screen2 extends StatelessWidget { + const _Screen2(); + + @override + Widget build(BuildContext context) { + return Scaffold( + resizeToAvoidBottomInset: false, + body: KeyboardScaffoldSafeArea( + child: Builder(builder: (context) { + return ListView.builder( + key: _screen2BodyKey, + itemBuilder: (context, index) { + return ListTile( + title: Text("Item $index"), + ); + }, + ); + }), + ), + ); + } +} + void testWidgetsOnMobilePhone( String description, WidgetTesterCallback test, { From aa4aeb32378c9033f94a4d3b1156290c12584e37 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 27 Nov 2024 10:02:36 -0800 Subject: [PATCH 2/3] Cherry Pick: [SuperEditor] Fix crashing when deleting all content which contains a non-deletable node at an edge (Resolves #2393) (#2405) --- .../common_editor_operations.dart | 35 +- .../default_editor/multi_node_editing.dart | 65 +- .../supereditor_undeletable_content_test.dart | 1110 +++++++++++++++++ .../lib/src/super_editor_paste_markdown.dart | 6 +- 4 files changed, 1193 insertions(+), 23 deletions(-) diff --git a/super_editor/lib/src/default_editor/common_editor_operations.dart b/super_editor/lib/src/default_editor/common_editor_operations.dart index b67981b517..974590f87a 100644 --- a/super_editor/lib/src/default_editor/common_editor_operations.dart +++ b/super_editor/lib/src/default_editor/common_editor_operations.dart @@ -2,6 +2,7 @@ import 'dart:math'; import 'dart:ui'; import 'package:attributed_text/attributed_text.dart'; +import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; import 'package:linkify/linkify.dart'; @@ -1381,13 +1382,15 @@ class CommonEditorOperations { /// Returns the [DocumentPosition] where the caret should sit after deleting /// the given [selection] from the given [document]. /// + /// Returns `null` if there are no deletable nodes within the [selection]. + /// /// This method doesn't delete any content. Instead, it determines what would /// be deleted if a delete operation was run for the given [selection]. Based /// on the shared understanding of content deletion rules, the resulting caret /// position is returned. // TODO: Move this method to an appropriate place. It was made public and static // because document_keyboard_actions.dart also uses this behavior. - static DocumentPosition getDocumentPositionAfterExpandedDeletion({ + static DocumentPosition? getDocumentPositionAfterExpandedDeletion({ required Document document, required DocumentSelection selection, }) { @@ -1419,15 +1422,32 @@ class CommonEditorOperations { final bottomNodePosition = baseNodeIndex < extentNodeIndex ? extentPosition.nodePosition : basePosition.nodePosition; + final normalizedRange = selection.normalize(document); + final nodes = document.getNodesInside(normalizedRange.start, normalizedRange.end); + final firstDeletableNodeId = nodes.firstWhereOrNull((node) => node.isDeletable)?.id; + DocumentPosition newSelectionPosition; if (baseNodeIndex != extentNodeIndex) { if (topNodePosition == topNode.beginningPosition && bottomNodePosition == bottomNode.endPosition) { - // All nodes in the selection will be deleted. Assume that the start - // node will be retained and converted into a paragraph, if it's not + // All deletable nodes in the selection will be deleted. Assume that one of the + // nodes will be retained and converted into a paragraph, if it's not // already a paragraph. + + final emptyParagraphId = topNode.isDeletable + ? topNode.id + : bottomNode.isDeletable + ? bottomNode.id + : firstDeletableNodeId; + + if (emptyParagraphId == null) { + // There are no deletable nodes in the selection. Fizzle. + // We don't expect this method to be called if there are no deletable nodes. + return null; + } + newSelectionPosition = DocumentPosition( - nodeId: topNode.id, + nodeId: emptyParagraphId, nodePosition: const TextNodePosition(offset: 0), ); } else if (topNodePosition == topNode.beginningPosition) { @@ -2277,7 +2297,7 @@ class CommonEditorOperations { /// moves the caret, it's possible that the clipboard content will be pasted /// at the wrong spot. void paste() { - DocumentPosition pastePosition = composer.selection!.extent; + DocumentPosition? pastePosition = composer.selection!.extent; // Start a transaction so that we can capture both the initial deletion behavior, // and the clipboard content insertion, all as one transaction. @@ -2290,6 +2310,11 @@ class CommonEditorOperations { selection: composer.selection!, ); + if (pastePosition == null) { + // There are no deletable nodes in the selection. Do nothing. + return; + } + // Delete the selected content. editor.execute([ DeleteContentRequest(documentRange: composer.selection!), diff --git a/super_editor/lib/src/default_editor/multi_node_editing.dart b/super_editor/lib/src/default_editor/multi_node_editing.dart index 051eb9e483..6f3297b5cb 100644 --- a/super_editor/lib/src/default_editor/multi_node_editing.dart +++ b/super_editor/lib/src/default_editor/multi_node_editing.dart @@ -753,7 +753,10 @@ class DeleteContentCommand extends EditCommand { if (endNode == null) { throw Exception('Could not locate end node for DeleteSelectionCommand: ${normalizedRange.end}'); } - final endNodeIndex = document.getNodeIndexById(endNode.id); + + // We expect that this command will only be called when the delete range + // contains at least one deletable node. + final firstDeletableNodeId = nodes.firstWhere((node) => node.isDeletable).id; executor.logChanges( _deleteNodesBetweenFirstAndLast( @@ -786,19 +789,40 @@ class DeleteContentCommand extends EditCommand { ); } + final wereAllDeletableNodesInRangeDeleted = nodes.every( + (node) => document.getNodeById(node.id) == null || !node.isDeletable, + ); + final hasNonDeletableNodesInRange = nodes.any((node) => !node.isDeletable); + // If all selected nodes were deleted, e.g., the user selected from // the beginning of the first node to the end of the last node, then // we need insert an empty paragraph node so that there's a place // to position the caret. - if (document.getNodeById(startNode.id) == null && document.getNodeById(endNode.id) == null) { - final insertIndex = min(startNodeIndex, endNodeIndex); + if (wereAllDeletableNodesInRangeDeleted) { + // If there are any non-deletable nodes in the range, insert the new node + // after the last non-deletable node. Otherwise, insert the new node at + // the position where the first selected node was. + final insertIndex = hasNonDeletableNodesInRange // + ? document.getNodeIndexById(nodes.lastWhere((node) => !node.isDeletable).id) + 1 + : startNodeIndex; + + // If one of the edge nodes is deletable, we can use it as the ID for the + // new empty paragraph. Otherwise, use the ID of the first deletable node in the range. + // We expect that this method is never called when there are no deletable nodes + // in the range. + final emptyParagraphId = startNode.isDeletable + ? startNode.id + : endNode.isDeletable + ? endNode.id + : firstDeletableNodeId; + document.insertNodeAt( insertIndex, - ParagraphNode(id: startNode.id, text: AttributedText()), + ParagraphNode(id: emptyParagraphId, text: AttributedText()), ); executor.logChanges([ DocumentEdit( - NodeChangeEvent(startNode.id), + NodeChangeEvent(emptyParagraphId), ) ]); } @@ -1144,12 +1168,12 @@ class DeleteSelectionCommand extends EditCommand { } } - final newSelectionPosition = CommonEditorOperations.getDocumentPositionAfterExpandedDeletion( - document: document, - selection: selection, - ); - final nodes = document.getNodesInside(selection.start, selection.end); + if (nodes.every((node) => !node.isDeletable)) { + // All selected nodes are non-deletable. Do nothing. + return; + } + if (nodes.length == 2) { final normalizedSelection = selection.normalize(document); final nodeAbove = document.getNode(normalizedSelection.start)!; @@ -1197,19 +1221,26 @@ class DeleteSelectionCommand extends EditCommand { } } - executor - ..executeCommand( - DeleteContentCommand( - documentRange: selection, - ), - ) - ..executeCommand( + final newSelectionPosition = CommonEditorOperations.getDocumentPositionAfterExpandedDeletion( + document: document, + selection: selection, + ); + + executor.executeCommand( + DeleteContentCommand( + documentRange: selection, + ), + ); + + if (newSelectionPosition != null) { + executor.executeCommand( ChangeSelectionCommand( DocumentSelection.collapsed(position: newSelectionPosition), SelectionChangeType.deleteContent, SelectionReason.userInteraction, ), ); + } } } diff --git a/super_editor/test/super_editor/supereditor_undeletable_content_test.dart b/super_editor/test/super_editor/supereditor_undeletable_content_test.dart index 85a204c9c0..9bf274a8d8 100644 --- a/super_editor/test/super_editor/supereditor_undeletable_content_test.dart +++ b/super_editor/test/super_editor/supereditor_undeletable_content_test.dart @@ -3,6 +3,7 @@ import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test_robots/flutter_test_robots.dart'; import 'package:flutter_test_runners/flutter_test_runners.dart'; +import 'package:super_editor/src/infrastructure/platforms/platform.dart'; import 'package:super_editor/super_editor.dart'; import 'package:super_editor/super_editor_test.dart'; @@ -695,6 +696,367 @@ void main() { ), ); }); + + testWidgetsOnDesktop('when the whole document is selected and starts with a non-deletable node', + (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + HorizontalRuleNode(id: '1', metadata: { + NodeMetadata.isDeletable: false, + }), + ParagraphNode( + id: '2', + text: AttributedText('This is some text'), + ), + ], + ), + ) + .pump(); + + // Place the caret at the beginning of the paragraph. + await tester.placeCaretInParagraph("2", 0); + + // Select all content. + if (CurrentPlatform.isApple) { + await tester.pressCmdA(); + } else { + await tester.pressCtlA(); + } + + // Ensure everything is selected. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '1', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '2', + nodePosition: TextNodePosition(offset: 17), + ), + ), + ); + + // Delete all content. + await tester.pressBackspace(); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure the horizontal rule was kept, the paragraph was deleted, + // and a new empty paragraph was added to the end of the document. + expect(document.nodeCount, equals(2)); + expect(document.first, isA()); + expect(document.last, isA()); + expect((document.last as TextNode).text.text, equals('')); + + // Ensure the caret was placed at the beginning of the newly inserted paragraph. + expect( + SuperEditorInspector.findDocumentSelection(), + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: document.last.id, + nodePosition: const TextNodePosition(offset: 0), + ), + ), + ); + }); + + testWidgetsOnDesktop('when the whole document is selected and ends with a non-deletable node', (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + ParagraphNode( + id: '1', + text: AttributedText('This is some text'), + ), + HorizontalRuleNode(id: '2', metadata: { + NodeMetadata.isDeletable: false, + }), + ], + ), + ) + .pump(); + + await tester.placeCaretInParagraph("1", 0); + + // Select all content. + if (CurrentPlatform.isApple) { + await tester.pressCmdA(); + } else { + await tester.pressCtlA(); + } + + // Ensure everything is selected. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '1', + nodePosition: TextNodePosition(offset: 0), + ), + extent: DocumentPosition( + nodeId: '2', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + + // Delete all content. + await tester.pressBackspace(); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure the horizontal rule was kept, the paragraph was deleted, + // and a new empty paragraph was added to the end of the document. + expect(document.nodeCount, equals(2)); + expect(document.first, isA()); + expect(document.last, isA()); + expect((document.last as TextNode).text.text, equals('')); + + // Ensure the caret was placed at the beginning of the newly inserted paragraph. + expect( + SuperEditorInspector.findDocumentSelection(), + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: document.last.id, + nodePosition: const TextNodePosition(offset: 0), + ), + ), + ); + }); + + testWidgetsOnDesktop('when the whole document is selected and starts and ends with non-deletable nodes', + (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + HorizontalRuleNode(id: '1', metadata: { + NodeMetadata.isDeletable: false, + }), + ParagraphNode( + id: '2', + text: AttributedText('This is some text'), + ), + HorizontalRuleNode(id: '3', metadata: { + NodeMetadata.isDeletable: false, + }), + ], + ), + ) + .pump(); + + await tester.placeCaretInParagraph("2", 0); + + // Select all content. + if (CurrentPlatform.isApple) { + await tester.pressCmdA(); + } else { + await tester.pressCtlA(); + } + + // Ensure everything is selected. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '1', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '3', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + + // Delete all content. + await tester.pressBackspace(); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure the horizontal rules were kept, the paragraph was deleted, + // and a new empty paragraph was added to the end of the document. + expect(document.nodeCount, equals(3)); + expect(document.getNodeAt(0), isA()); + expect(document.getNodeAt(1), isA()); + expect(document.getNodeAt(2), isA()); + expect((document.last as TextNode).text.text, equals('')); + + // Ensure the caret was placed at the beginning of the newly inserted paragraph. + expect( + SuperEditorInspector.findDocumentSelection(), + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: document.last.id, + nodePosition: const TextNodePosition(offset: 0), + ), + ), + ); + }); + + testWidgetsOnDesktop('when all nodes are non-deletable', (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + HorizontalRuleNode(id: '1', metadata: { + NodeMetadata.isDeletable: false, + }), + HorizontalRuleNode(id: '2', metadata: { + NodeMetadata.isDeletable: false, + }), + HorizontalRuleNode(id: '3', metadata: { + NodeMetadata.isDeletable: false, + }), + ], + ), + ) + .pump(); + + // Select the first horizontal rule. + await tester.tapAtDocumentPosition( + const DocumentPosition( + nodeId: "1", + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + ); + + // Select all content. + if (CurrentPlatform.isApple) { + await tester.pressCmdA(); + } else { + await tester.pressCtlA(); + } + + // Ensure everything is selected. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '1', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '3', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + + // Try to delete all content. + await tester.pressBackspace(); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure nothing was deleted. + expect(document.nodeCount, equals(3)); + expect(document.getNodeAt(0), isA()); + expect(document.getNodeAt(1), isA()); + expect(document.getNodeAt(2), isA()); + + // Ensure the selection was kept. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '1', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '3', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + }); + + testWidgetsOnDesktop('when all nodes in selection are non-deletable and document contains deletable nodes', + (tester) async { + final testContext = await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + ParagraphNode(id: '1', text: AttributedText()), + HorizontalRuleNode(id: '2', metadata: { + NodeMetadata.isDeletable: false, + }), + HorizontalRuleNode(id: '3', metadata: { + NodeMetadata.isDeletable: false, + }), + HorizontalRuleNode(id: '4', metadata: { + NodeMetadata.isDeletable: false, + }), + ParagraphNode(id: '5', text: AttributedText()), + ], + ), + ) + .pump(); + + // Select the first horizontal rule. + await tester.tapAtDocumentPosition( + const DocumentPosition( + nodeId: "2", + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + ); + + // Select all non-deletable nodes. + testContext.editor.execute([ + const ChangeSelectionRequest( + DocumentSelection( + base: DocumentPosition( + nodeId: '2', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '4', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + SelectionChangeType.expandSelection, + SelectionReason.userInteraction, + ) + ]); + await tester.pump(); + + // Try to delete all content. + await tester.pressBackspace(); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure nothing was deleted. + expect(document.nodeCount, equals(5)); + expect(document.getNodeAt(0), isA()); + expect(document.getNodeAt(1), isA()); + expect(document.getNodeAt(2), isA()); + expect(document.getNodeAt(3), isA()); + expect(document.getNodeAt(4), isA()); + + // Ensure the selection was kept. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '2', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '4', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + }); }); group('with delete', () { @@ -1162,6 +1524,367 @@ void main() { // Ensure the selection didn't change. expect(SuperEditorInspector.findDocumentSelection(), selection); }); + + testWidgetsOnDesktop('when the whole document is selected and starts with a non-deletable node', + (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + HorizontalRuleNode(id: '1', metadata: { + NodeMetadata.isDeletable: false, + }), + ParagraphNode( + id: '2', + text: AttributedText('This is some text'), + ), + ], + ), + ) + .pump(); + + // Place the caret at the beginning of the paragraph. + await tester.placeCaretInParagraph("2", 0); + + // Select all content. + if (CurrentPlatform.isApple) { + await tester.pressCmdA(); + } else { + await tester.pressCtlA(); + } + + // Ensure everything is selected. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '1', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '2', + nodePosition: TextNodePosition(offset: 17), + ), + ), + ); + + // Delete all content. + await tester.pressDelete(); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure the horizontal rule was kept, the paragraph was deleted, + // and a new empty paragraph was added to the end of the document. + expect(document.nodeCount, equals(2)); + expect(document.first, isA()); + expect(document.last, isA()); + expect((document.last as TextNode).text.text, equals('')); + + // Ensure the caret was placed at the beginning of the newly inserted paragraph. + expect( + SuperEditorInspector.findDocumentSelection(), + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: document.last.id, + nodePosition: const TextNodePosition(offset: 0), + ), + ), + ); + }); + + testWidgetsOnDesktop('when the whole document is selected and ends with a non-deletable node', (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + ParagraphNode( + id: '1', + text: AttributedText('This is some text'), + ), + HorizontalRuleNode(id: '2', metadata: { + NodeMetadata.isDeletable: false, + }), + ], + ), + ) + .pump(); + + await tester.placeCaretInParagraph("1", 0); + + // Select all content + if (CurrentPlatform.isApple) { + await tester.pressCmdA(); + } else { + await tester.pressCtlA(); + } + + // Ensure everything is selected. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '1', + nodePosition: TextNodePosition(offset: 0), + ), + extent: DocumentPosition( + nodeId: '2', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + + // Delete all content. + await tester.pressDelete(); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure the horizontal rule was kept, the paragraph was deleted, + // and a new empty paragraph was added to the end of the document. + expect(document.nodeCount, equals(2)); + expect(document.first, isA()); + expect(document.last, isA()); + expect((document.last as TextNode).text.text, equals('')); + + // Ensure the caret was placed at the beginning of the newly inserted paragraph. + expect( + SuperEditorInspector.findDocumentSelection(), + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: document.last.id, + nodePosition: const TextNodePosition(offset: 0), + ), + ), + ); + }); + + testWidgetsOnDesktop('when the whole document is selected and starts and ends with non-deletable nodes', + (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + HorizontalRuleNode(id: '1', metadata: { + NodeMetadata.isDeletable: false, + }), + ParagraphNode( + id: '2', + text: AttributedText('This is some text'), + ), + HorizontalRuleNode(id: '3', metadata: { + NodeMetadata.isDeletable: false, + }), + ], + ), + ) + .pump(); + + await tester.placeCaretInParagraph("2", 0); + + // Select all content. + if (CurrentPlatform.isApple) { + await tester.pressCmdA(); + } else { + await tester.pressCtlA(); + } + + // Ensure everything is selected. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '1', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '3', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + + // Delete all content. + await tester.pressDelete(); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure the horizontal rules were kept, the paragraph was deleted, + // and a new empty paragraph was added to the end of the document. + expect(document.nodeCount, equals(3)); + expect(document.getNodeAt(0), isA()); + expect(document.getNodeAt(1), isA()); + expect(document.getNodeAt(2), isA()); + expect((document.last as TextNode).text.text, equals('')); + + // Ensure the caret was placed at the beginning of the newly inserted paragraph. + expect( + SuperEditorInspector.findDocumentSelection(), + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: document.last.id, + nodePosition: const TextNodePosition(offset: 0), + ), + ), + ); + }); + + testWidgetsOnDesktop('when all nodes are non-deletable', (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + HorizontalRuleNode(id: '1', metadata: { + NodeMetadata.isDeletable: false, + }), + HorizontalRuleNode(id: '2', metadata: { + NodeMetadata.isDeletable: false, + }), + HorizontalRuleNode(id: '3', metadata: { + NodeMetadata.isDeletable: false, + }), + ], + ), + ) + .pump(); + + // Select the first horizontal rule. + await tester.tapAtDocumentPosition( + const DocumentPosition( + nodeId: "1", + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + ); + + // Select all content. + if (CurrentPlatform.isApple) { + await tester.pressCmdA(); + } else { + await tester.pressCtlA(); + } + + // Ensure everything is selected. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '1', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '3', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + + // Try to delete all content. + await tester.pressDelete(); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure nothing was deleted. + expect(document.nodeCount, equals(3)); + expect(document.getNodeAt(0), isA()); + expect(document.getNodeAt(1), isA()); + expect(document.getNodeAt(2), isA()); + + // Ensure the selection was kept. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '1', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '3', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + }); + + testWidgetsOnDesktop('when all nodes in selection are non-deletable and document contains deletable nodes', + (tester) async { + final testContext = await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + ParagraphNode(id: '1', text: AttributedText()), + HorizontalRuleNode(id: '2', metadata: { + NodeMetadata.isDeletable: false, + }), + HorizontalRuleNode(id: '3', metadata: { + NodeMetadata.isDeletable: false, + }), + HorizontalRuleNode(id: '4', metadata: { + NodeMetadata.isDeletable: false, + }), + ParagraphNode(id: '5', text: AttributedText()), + ], + ), + ) + .pump(); + + // Select the first horizontal rule. + await tester.tapAtDocumentPosition( + const DocumentPosition( + nodeId: "2", + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + ); + + // Select all non-deletable nodes. + testContext.editor.execute([ + const ChangeSelectionRequest( + DocumentSelection( + base: DocumentPosition( + nodeId: '2', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '4', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + SelectionChangeType.expandSelection, + SelectionReason.userInteraction, + ) + ]); + await tester.pump(); + + // Try to delete all content. + await tester.pressDelete(); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure nothing was deleted. + expect(document.nodeCount, equals(5)); + expect(document.getNodeAt(0), isA()); + expect(document.getNodeAt(1), isA()); + expect(document.getNodeAt(2), isA()); + expect(document.getNodeAt(3), isA()); + expect(document.getNodeAt(4), isA()); + + // Ensure the selection was kept. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '2', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '4', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + }); }); group('when typing', () { @@ -1610,6 +2333,393 @@ void main() { expect(document.getNodeById('hr'), isNotNull); expect(document.getNodeById('hr'), isA()); }); + + testWidgetsOnMobile('when the whole document is selected and starts with a non-deletable node', (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + HorizontalRuleNode(id: '1', metadata: { + NodeMetadata.isDeletable: false, + }), + ParagraphNode( + id: '2', + text: AttributedText('This is some text'), + ), + ], + ), + ) + .pump(); + + // Place the caret at the beginning of the paragraph. + await tester.placeCaretInParagraph("2", 0); + + // Select all content. + if (CurrentPlatform.isApple) { + await tester.pressCmdA(); + } else { + await tester.pressCtlA(); + } + + // Simulate the user pressing backspace. The IME first generates a + // selection change and then a deletion. Each block node is represented by a "~" + // in the IME. + await tester.ime.sendDeltas([ + const TextEditingDeltaNonTextUpdate( + oldText: '. ~\nThis is some text', + selection: TextSelection(baseOffset: 0, extentOffset: 21), + composing: TextRange.empty, + ), + const TextEditingDeltaDeletion( + oldText: '. ~\nThis is some text', + deletedRange: TextSelection(baseOffset: 0, extentOffset: 21), + selection: TextSelection.collapsed(offset: 0), + composing: TextRange.empty, + ), + ], getter: imeClientGetter); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure the horizontal rule was kept, the paragraph was deleted, + // and a new empty paragraph was added to the end of the document. + expect(document.nodeCount, equals(2)); + expect(document.first, isA()); + expect(document.last, isA()); + expect((document.last as TextNode).text.text, equals('')); + + // Ensure the caret was placed at the beginning of the newly inserted paragraph. + expect( + SuperEditorInspector.findDocumentSelection(), + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: document.last.id, + nodePosition: const TextNodePosition(offset: 0), + ), + ), + ); + }); + + testWidgetsOnMobile('when the whole document is selected and ends with a non-deletable node', (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + ParagraphNode( + id: '1', + text: AttributedText('This is some text'), + ), + HorizontalRuleNode(id: '2', metadata: { + NodeMetadata.isDeletable: false, + }), + ], + ), + ) + .pump(); + + // Place the caret at the beginning of the paragraph. + await tester.placeCaretInParagraph("1", 0); + + // Select all content. + if (CurrentPlatform.isApple) { + await tester.pressCmdA(); + } else { + await tester.pressCtlA(); + } + await tester.pump(); + + // Simulate the user pressing backspace. The IME first generates a + // selection change and then a deletion. Each block node is represented by a "~" + // in the IME. + await tester.ime.sendDeltas([ + const TextEditingDeltaNonTextUpdate( + oldText: '. This is some text\n~', + selection: TextSelection(baseOffset: 0, extentOffset: 21), + composing: TextRange.empty, + ), + const TextEditingDeltaDeletion( + oldText: '. This is some text\n~', + deletedRange: TextSelection(baseOffset: 0, extentOffset: 21), + selection: TextSelection.collapsed(offset: 0), + composing: TextRange.empty, + ), + ], getter: imeClientGetter); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure the horizontal rule was kept, the paragraph was deleted, + // and a new empty paragraph was added to the end of the document. + expect(document.nodeCount, equals(2)); + expect(document.first, isA()); + expect(document.last, isA()); + expect((document.last as TextNode).text.text, equals('')); + + // Ensure the caret was placed at the beginning of the newly inserted paragraph. + expect( + SuperEditorInspector.findDocumentSelection(), + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: document.last.id, + nodePosition: const TextNodePosition(offset: 0), + ), + ), + ); + }); + + testWidgetsOnDesktop('when the whole document is selected and starts and ends with non-deletable nodes', + (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + HorizontalRuleNode(id: '1', metadata: { + NodeMetadata.isDeletable: false, + }), + ParagraphNode( + id: '2', + text: AttributedText('This is some text'), + ), + HorizontalRuleNode(id: '3', metadata: { + NodeMetadata.isDeletable: false, + }), + ], + ), + ) + .pump(); + + await tester.placeCaretInParagraph("2", 0); + + // Select all content. + if (CurrentPlatform.isApple) { + await tester.pressCmdA(); + } else { + await tester.pressCtlA(); + } + + // Ensure everything is selected. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '1', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '3', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + + // Simulate the user pressing backspace. The IME first generates a + // selection change and then a deletion. Each block node is represented by a "~" + // in the IME. + await tester.ime.sendDeltas([ + const TextEditingDeltaNonTextUpdate( + oldText: '. ~\nThis is some text\n~', + selection: TextSelection(baseOffset: 0, extentOffset: 23), + composing: TextRange.empty, + ), + const TextEditingDeltaDeletion( + oldText: '. ~\nThis is some text\n~', + deletedRange: TextSelection(baseOffset: 0, extentOffset: 23), + selection: TextSelection.collapsed(offset: 0), + composing: TextRange.empty, + ), + ], getter: imeClientGetter); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure the horizontal rules were kept, the paragraph was deleted, + // and a new empty paragraph was added to the end of the document. + expect(document.nodeCount, equals(3)); + expect(document.getNodeAt(0), isA()); + expect(document.getNodeAt(1), isA()); + expect(document.getNodeAt(2), isA()); + expect((document.last as TextNode).text.text, equals('')); + + // Ensure the caret was placed at the beginning of the newly inserted paragraph. + expect( + SuperEditorInspector.findDocumentSelection(), + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: document.last.id, + nodePosition: const TextNodePosition(offset: 0), + ), + ), + ); + }); + + testWidgetsOnMobile('when all nodes are non-deletable', (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + HorizontalRuleNode(id: '1', metadata: { + NodeMetadata.isDeletable: false, + }), + HorizontalRuleNode(id: '2', metadata: { + NodeMetadata.isDeletable: false, + }), + HorizontalRuleNode(id: '3', metadata: { + NodeMetadata.isDeletable: false, + }), + ], + ), + ) + .pump(); + + // Select the first horizontal rule. + await tester.tapAtDocumentPosition( + const DocumentPosition( + nodeId: "1", + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + ); + + // Select all content. + if (CurrentPlatform.isApple) { + await tester.pressCmdA(); + } else { + await tester.pressCtlA(); + } + + // Simulate the user pressing backspace. The IME first generates a + // selection change and then a deletion. Each block node is represented by a "~" + // in the IME. + await tester.ime.sendDeltas([ + const TextEditingDeltaNonTextUpdate( + oldText: '. ~\n~\n~', + selection: TextSelection(baseOffset: 0, extentOffset: 7), + composing: TextRange.empty, + ), + const TextEditingDeltaDeletion( + oldText: '. ~\n~\n~', + deletedRange: TextSelection(baseOffset: 0, extentOffset: 7), + selection: TextSelection.collapsed(offset: 0), + composing: TextRange.empty, + ), + ], getter: imeClientGetter); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure nothing was deleted. + expect(document.nodeCount, equals(3)); + expect(document.getNodeAt(0), isA()); + expect(document.getNodeAt(1), isA()); + expect(document.getNodeAt(2), isA()); + + // Ensure the selection was kept. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '1', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '3', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + }); + + testWidgetsOnDesktop('when all nodes in selection are non-deletable and document contains deletable nodes', + (tester) async { + final testContext = await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + ParagraphNode(id: '1', text: AttributedText()), + HorizontalRuleNode(id: '2', metadata: { + NodeMetadata.isDeletable: false, + }), + HorizontalRuleNode(id: '3', metadata: { + NodeMetadata.isDeletable: false, + }), + HorizontalRuleNode(id: '4', metadata: { + NodeMetadata.isDeletable: false, + }), + ParagraphNode(id: '5', text: AttributedText()), + ], + ), + ) + .pump(); + + // Select the first horizontal rule. + await tester.tapAtDocumentPosition( + const DocumentPosition( + nodeId: "2", + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + ); + + // Select all non-deletable nodes. + testContext.editor.execute([ + const ChangeSelectionRequest( + DocumentSelection( + base: DocumentPosition( + nodeId: '2', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '4', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + SelectionChangeType.expandSelection, + SelectionReason.userInteraction, + ) + ]); + await tester.pump(); + + // Simulate the user pressing backspace. The IME first generates a + // selection change and then a deletion. Each block node is represented by a "~" + // in the IME. + await tester.ime.sendDeltas([ + const TextEditingDeltaNonTextUpdate( + oldText: '. ~\n~\n~', + selection: TextSelection(baseOffset: 0, extentOffset: 7), + composing: TextRange.empty, + ), + const TextEditingDeltaDeletion( + oldText: '. ~\n~\n~', + deletedRange: TextSelection(baseOffset: 0, extentOffset: 7), + selection: TextSelection.collapsed(offset: 0), + composing: TextRange.empty, + ), + ], getter: imeClientGetter); + + final document = SuperEditorInspector.findDocument()!; + + // Ensure nothing was deleted. + expect(document.nodeCount, equals(5)); + expect(document.getNodeAt(0), isA()); + expect(document.getNodeAt(1), isA()); + expect(document.getNodeAt(2), isA()); + expect(document.getNodeAt(3), isA()); + expect(document.getNodeAt(4), isA()); + + // Ensure the selection was kept. + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection( + base: DocumentPosition( + nodeId: '2', + nodePosition: UpstreamDownstreamNodePosition.upstream(), + ), + extent: DocumentPosition( + nodeId: '4', + nodePosition: UpstreamDownstreamNodePosition.downstream(), + ), + ), + ); + }); }); }); }); diff --git a/super_editor_markdown/lib/src/super_editor_paste_markdown.dart b/super_editor_markdown/lib/src/super_editor_paste_markdown.dart index 8dd59b178b..b131f36022 100644 --- a/super_editor_markdown/lib/src/super_editor_paste_markdown.dart +++ b/super_editor_markdown/lib/src/super_editor_paste_markdown.dart @@ -40,7 +40,7 @@ Future pasteMarkdown({ required Document document, required DocumentComposer composer, }) async { - DocumentPosition pastePosition = composer.selection!.extent; + DocumentPosition? pastePosition = composer.selection!.extent; // Delete all currently selected content. if (!composer.selection!.isCollapsed) { @@ -49,6 +49,10 @@ Future pasteMarkdown({ selection: composer.selection!, ); + if (pastePosition == null) { + return; + } + // Delete the selected content. editor.execute([ DeleteContentRequest(documentRange: composer.selection!), From 0c55c45e9a70f743a191c064c25fecaff8262849 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Wed, 27 Nov 2024 15:11:33 -0300 Subject: [PATCH 3/3] [SuperEditor][Web] Fix option + arrow key selection changes (Resolves #2415) (#2418) --- .../document_keyboard_actions.dart | 3 +- ...pereditor_input_keyboard_actions_test.dart | 122 ++++++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart b/super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart index 59cf22840e..ffca2c033a 100644 --- a/super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart +++ b/super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart @@ -627,7 +627,8 @@ ExecutionInstruction doNothingWithLeftRightArrowKeysAtMiddleOfTextOnWeb({ return ExecutionInstruction.continueExecution; } - if (defaultTargetPlatform == TargetPlatform.windows && HardwareKeyboard.instance.isAltPressed) { + if ((defaultTargetPlatform == TargetPlatform.windows || CurrentPlatform.isApple) && + HardwareKeyboard.instance.isAltPressed) { return ExecutionInstruction.continueExecution; } diff --git a/super_editor/test/super_editor/supereditor_input_keyboard_actions_test.dart b/super_editor/test/super_editor/supereditor_input_keyboard_actions_test.dart index 05668e6675..f199d48f47 100644 --- a/super_editor/test/super_editor/supereditor_input_keyboard_actions_test.dart +++ b/super_editor/test/super_editor/supereditor_input_keyboard_actions_test.dart @@ -523,6 +523,128 @@ void main() { ), ); }); + + testWidgetsOnMacDesktopAndWeb( + 'SHIFT + OPTION + LEFT ARROW: deselects word at end of line after selecting the whole line from start to end', + (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + ParagraphNode( + id: '1', + text: AttributedText('This is a paragraph'), + ), + ], + ), + ) + .pump(); + + // Place caret at the beginning of the paragraph. + await tester.placeCaretInParagraph('1', 0); + + // Press CMD + SHIFT + RIGHT ARROW to expand the selection to the end of the line. + await tester.pressShiftCmdRightArrow(); + + // Ensure that the whole line is selected. + expect( + SuperEditorInspector.findDocumentSelection(), + selectionEquivalentTo( + const DocumentSelection( + base: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 0), + ), + extent: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 19), + ), + ), + ), + ); + + // Press SHIFT + OPTION + LEFT ARROW to remove the last word from the selection. + await tester.pressShiftAltLeftArrow(); + + // Ensure that the last word was removed from the selection. + expect( + SuperEditorInspector.findDocumentSelection(), + selectionEquivalentTo( + const DocumentSelection( + base: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 0), + ), + extent: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 10), + ), + ), + ), + ); + }); + + testWidgetsOnMacDesktopAndWeb( + 'SHIFT + OPTION + RIGHT ARROW: deselects word at start of line after selecting the whole line from end to start', + (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + ParagraphNode( + id: '1', + text: AttributedText('This is a paragraph'), + ), + ], + ), + ) + .pump(); + + // Place caret at the end of the paragraph. + await tester.placeCaretInParagraph('1', 19); + + // Press CMD + SHIFT + LEFT ARROW to expand the selection to the beginning of the line. + await tester.pressShiftCmdLeftArrow(); + + // Ensure that the whole line is selected. + expect( + SuperEditorInspector.findDocumentSelection(), + selectionEquivalentTo( + const DocumentSelection( + base: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 19), + ), + extent: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 0), + ), + ), + ), + ); + + // Press SHIFT + OPTION + RIGHT ARROW to remove the first word from the selection. + await tester.pressShiftAltRightArrow(); + + // Ensure that the first word was removed from the selection. + expect( + SuperEditorInspector.findDocumentSelection(), + selectionEquivalentTo( + const DocumentSelection( + base: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 19), + ), + extent: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 4), + ), + ), + ), + ); + }); }); group("Windows and Linux >", () {