From 176552283f0cddfa8fcdf9f1b4f8de24bda667c0 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Sat, 30 Nov 2024 15:59:19 -0800 Subject: [PATCH 1/4] [SuperEditor][SuperReader] - Reduce uses of node index queries in Document (Resolves #2434) --- .../lib/src/core/document_selection.dart | 149 ++++++------------ super_editor/lib/src/core/editor.dart | 2 +- super_editor/lib/src/core/styles.dart | 4 +- .../common_editor_operations.dart | 24 +-- .../document_ime/document_delta_editing.dart | 5 +- .../layout_single_column/_presenter.dart | 7 +- .../default_editor/multi_node_editing.dart | 38 +++-- .../tap_handlers/tap_handlers.dart | 4 +- .../lib/src/default_editor/tasks.dart | 23 +-- super_editor/lib/src/default_editor/text.dart | 25 +-- .../android/drag_handle_selection.dart | 19 ++- 11 files changed, 126 insertions(+), 174 deletions(-) diff --git a/super_editor/lib/src/core/document_selection.dart b/super_editor/lib/src/core/document_selection.dart index cafc3ada0d..15f114d8e5 100644 --- a/super_editor/lib/src/core/document_selection.dart +++ b/super_editor/lib/src/core/document_selection.dart @@ -150,21 +150,8 @@ class DocumentSelection extends DocumentRange { return this; } - final baseNode = document.getNodeById(base.nodeId)!; - final extentNode = document.getNodeById(extent.nodeId)!; - - if (baseNode == extentNode) { - // The selection is expanded, but it sits within a single node. - final upstreamNodePosition = extentNode.selectUpstreamPosition( - base.nodePosition, - extent.nodePosition, - ); - return DocumentSelection.collapsed( - position: extent.copyWith(nodePosition: upstreamNodePosition), - ); - } - - return document.getNodeIndexById(baseNode.id) < document.getNodeIndexById(extentNode.id) + final selectionAffinity = document.getAffinityForSelection(this); + return selectionAffinity == TextAffinity.downstream // ? DocumentSelection.collapsed(position: base) : DocumentSelection.collapsed(position: extent); } @@ -189,23 +176,10 @@ class DocumentSelection extends DocumentRange { return this; } - final baseNode = document.getNodeById(base.nodeId)!; - final extentNode = document.getNodeById(extent.nodeId)!; - - if (baseNode == extentNode) { - // The selection is expanded, but it sits within a single node. - final downstreamNodePosition = extentNode.selectDownstreamPosition( - base.nodePosition, - extent.nodePosition, - ); - return DocumentSelection.collapsed( - position: extent.copyWith(nodePosition: downstreamNodePosition), - ); - } - - return document.getNodeIndexById(baseNode.id) > document.getNodeIndexById(extentNode.id) - ? DocumentSelection.collapsed(position: base) - : DocumentSelection.collapsed(position: extent); + final selectionAffinity = document.getAffinityForSelection(this); + return selectionAffinity == TextAffinity.downstream // + ? DocumentSelection.collapsed(position: extent) + : DocumentSelection.collapsed(position: base); } @override @@ -326,6 +300,21 @@ extension InspectDocumentAffinity on Document { return getAffinityBetween(base: range.start, extent: range.end); } + TextAffinity getAffinityBetweenNodes(DocumentNode base, DocumentNode extent) { + return getAffinityForSelection( + DocumentSelection( + base: DocumentPosition( + nodeId: base.id, + nodePosition: base.beginningPosition, + ), + extent: DocumentPosition( + nodeId: extent.id, + nodePosition: extent.beginningPosition, + ), + ), + ); + } + /// Returns the affinity direction implied by the given [base] and [extent]. // TODO: Replace TextAffinity with a DocumentAffinity to avoid confusion. TextAffinity getAffinityBetween({ @@ -336,19 +325,17 @@ extension InspectDocumentAffinity on Document { if (baseNode == null) { throw Exception('No such position in document: $base'); } - final baseIndex = getNodeIndexById(baseNode.id); final extentNode = getNode(extent); if (extentNode == null) { throw Exception('No such position in document: $extent'); } - final extentIndex = getNodeIndexById(extentNode.id); late TextAffinity affinity; - if (extentIndex > baseIndex) { - affinity = TextAffinity.downstream; - } else if (extentIndex < baseIndex) { - affinity = TextAffinity.upstream; + if (base.nodeId != extent.nodeId) { + affinity = getNodeIndexById(base.nodeId) < getNodeIndexById(extent.nodeId) + ? TextAffinity.downstream + : TextAffinity.upstream; } else { // The selection is within the same node. Ask the node which position // comes first. @@ -384,20 +371,16 @@ extension InspectDocumentSelection on Document { /// Given [docPosition1] and [docPosition2], returns the `DocumentPosition` that /// appears first in the document. DocumentPosition selectUpstreamPosition(DocumentPosition docPosition1, DocumentPosition docPosition2) { - final docPosition1Node = getNodeById(docPosition1.nodeId)!; - final docPosition1NodeIndex = getNodeIndexById(docPosition1Node.id); - final docPosition2Node = getNodeById(docPosition2.nodeId)!; - final docPosition2NodeIndex = getNodeIndexById(docPosition2Node.id); - - if (docPosition1NodeIndex < docPosition2NodeIndex) { - return docPosition1; - } else if (docPosition2NodeIndex < docPosition1NodeIndex) { - return docPosition2; + if (docPosition1.nodeId != docPosition2.nodeId) { + final affinity = getAffinityBetween(base: docPosition1, extent: docPosition2); + return affinity == TextAffinity.downstream // + ? docPosition1 + : docPosition2; } // Both document positions are in the same node. Figure out which // node position comes first. - final theNode = docPosition1Node; + final theNode = getNodeById(docPosition1.nodeId)!; return theNode.selectUpstreamPosition(docPosition1.nodePosition, docPosition2.nodePosition) == docPosition1.nodePosition ? docPosition1 @@ -418,62 +401,18 @@ extension InspectDocumentSelection on Document { return false; } - final baseNode = getNodeById(selection.base.nodeId)!; - final baseNodeIndex = getNodeIndexById(baseNode.id); - final extentNode = getNodeById(selection.extent.nodeId)!; - final extentNodeIndex = getNodeIndexById(extentNode.id); - - final upstreamNode = baseNodeIndex < extentNodeIndex ? baseNode : extentNode; - final upstreamNodeIndex = baseNodeIndex < extentNodeIndex ? baseNodeIndex : extentNodeIndex; - final downstreamNode = baseNodeIndex < extentNodeIndex ? extentNode : baseNode; - final downstreamNodeIndex = baseNodeIndex < extentNodeIndex ? extentNodeIndex : baseNodeIndex; - - final positionNodeIndex = getNodeIndexById(position.nodeId); - - if (upstreamNodeIndex < positionNodeIndex && positionNodeIndex < downstreamNodeIndex) { - // The given position is sandwiched between two other nodes that form - // the bounds of the selection. Therefore, the position is definitely within - // the selection. - return true; - } - - if (positionNodeIndex == upstreamNodeIndex) { - final upstreamPosition = selectUpstreamPosition(selection.base, selection.extent); - final downstreamPosition = upstreamPosition == selection.base ? selection.extent : selection.base; - - // This is the furthest a position could sit in the upstream node - // and still contain the given position. Keep in mind that the - // upstream position, downstream position, and given position may - // all reside in the same node (in fact, they probably do). - final downstreamCap = - upstreamNodeIndex == downstreamNodeIndex ? downstreamPosition.nodePosition : upstreamNode.endPosition; - - // If and only if the given position comes after the upstream position, - // and before the downstream cap, then the position is within the selection. - return upstreamNode.selectDownstreamPosition(upstreamPosition.nodePosition, position.nodePosition) == - upstreamNode.selectUpstreamPosition(position.nodePosition, downstreamCap); - } - - if (positionNodeIndex == downstreamNodeIndex) { - final upstreamPosition = selectUpstreamPosition(selection.base, selection.extent); - final downstreamPosition = upstreamPosition == selection.base ? selection.extent : selection.base; - - // This is the furthest upstream that a position could sit in the - // downstream node and still contain the given position. Keep in - // mind that the upstream position, downstream position, and given - // position may all reside in the same node (in fact, they probably do). - final upstreamCap = - downstreamNodeIndex == upstreamNodeIndex ? upstreamPosition.nodePosition : downstreamNode.beginningPosition; - - // If and only if the given position comes before the downstream position, - // and after the upstream cap, then the position is within the selection. - return downstreamNode.selectDownstreamPosition(upstreamCap, position.nodePosition) == - downstreamNode.selectUpstreamPosition(position.nodePosition, downstreamPosition.nodePosition); - } - - // If we got here, then the position is either before the upstream - // selection boundary, or after the downstream selection boundary. - // Either way, the position is not in the selection. - return false; + final selectionAffinity = getAffinityForSelection(selection); + final upstreamPosition = selectionAffinity == TextAffinity.downstream ? selection.base : selection.extent; + final downstreamPosition = selectionAffinity == TextAffinity.downstream ? selection.extent : selection.base; + + // The selection contains the position if the ordering is as follows: + // + // selection start <= position <= selection end + // + // Another way of stating this relationship is that there's a downstream + // affinity from selection start to the position, and from the position to + // the selection end. + return getAffinityBetween(base: upstreamPosition, extent: position) == TextAffinity.downstream && + getAffinityBetween(base: position, extent: downstreamPosition) == TextAffinity.downstream; } } diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index 2b6fffe153..b14722dafd 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -1291,7 +1291,7 @@ class MutableDocument with Iterable implements Document, Editable } for (int i = 0; i < _nodes.length; ++i) { - if (!_nodes[i].hasEquivalentContent(other.getNodeAt(i)!)) { + if (!_nodes[i].hasEquivalentContent(other.getNodeById(_nodes[i].id)!)) { return false; } } diff --git a/super_editor/lib/src/core/styles.dart b/super_editor/lib/src/core/styles.dart index de979c9a07..8fd4408993 100644 --- a/super_editor/lib/src/core/styles.dart +++ b/super_editor/lib/src/core/styles.dart @@ -207,7 +207,7 @@ class _FirstBlockMatcher implements _BlockMatcher { @override bool matches(Document document, DocumentNode node) { - return document.getNodeIndexById(node.id) == 0; + return document.getNodeById(node.id) == document.firstOrNull; } } @@ -216,7 +216,7 @@ class _LastBlockMatcher implements _BlockMatcher { @override bool matches(Document document, DocumentNode node) { - return document.getNodeIndexById(node.id) == document.nodeCount - 1; + return document.getNodeById(node.id) == document.lastOrNull; } } 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 974590f87a..2f8fe2777b 100644 --- a/super_editor/lib/src/default_editor/common_editor_operations.dart +++ b/super_editor/lib/src/default_editor/common_editor_operations.dart @@ -1404,23 +1404,25 @@ class CommonEditorOperations { if (baseNode == null) { throw Exception('Failed to _getDocumentPositionAfterDeletion because the base node no longer exists.'); } - final baseNodeIndex = document.getNodeIndexById(baseNode.id); final extentPosition = selection.extent; final extentNode = document.getNode(extentPosition); if (extentNode == null) { throw Exception('Failed to _getDocumentPositionAfterDeletion because the extent node no longer exists.'); } - final extentNodeIndex = document.getNodeIndexById(extentNode.id); - final topNodeIndex = min(baseNodeIndex, extentNodeIndex); - final topNode = document.getNodeAt(topNodeIndex)!; - final topNodePosition = baseNodeIndex < extentNodeIndex ? basePosition.nodePosition : extentPosition.nodePosition; + final selectionAffinity = document.getAffinityForSelection(selection); + final topPosition = selectionAffinity == TextAffinity.downstream // + ? selection.base + : selection.extent; + final topNodePosition = topPosition.nodePosition; + final topNode = document.getNodeById(topPosition.nodeId)!; - final bottomNodeIndex = max(baseNodeIndex, extentNodeIndex); - final bottomNode = document.getNodeAt(bottomNodeIndex)!; - final bottomNodePosition = - baseNodeIndex < extentNodeIndex ? extentPosition.nodePosition : basePosition.nodePosition; + final bottomPosition = selectionAffinity == TextAffinity.downstream // + ? selection.extent + : selection.base; + final bottomNodePosition = bottomPosition.nodePosition; + final bottomNode = document.getNodeById(bottomPosition.nodeId)!; final normalizedRange = selection.normalize(document); final nodes = document.getNodesInside(normalizedRange.start, normalizedRange.end); @@ -1428,7 +1430,7 @@ class CommonEditorOperations { DocumentPosition newSelectionPosition; - if (baseNodeIndex != extentNodeIndex) { + if (topPosition.nodeId != bottomPosition.nodeId) { if (topNodePosition == topNode.beginningPosition && bottomNodePosition == bottomNode.endPosition) { // 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 @@ -1469,7 +1471,7 @@ class CommonEditorOperations { // those nodes will remain. // The caret should end up at the base position - newSelectionPosition = baseNodeIndex <= extentNodeIndex ? selection.base : selection.extent; + newSelectionPosition = selectionAffinity == TextAffinity.downstream ? selection.base : selection.extent; } } else { // Selection is within a single node. diff --git a/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart b/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart index 02490b74b2..738692ad53 100644 --- a/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart +++ b/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart @@ -388,9 +388,6 @@ class TextDeltasDocumentEditor { editorImeLog.fine("Doc selection to delete: $docSelectionToDelete"); if (docSelectionToDelete == null) { - final selectedNodeIndex = document.getNodeIndexById( - selection.value!.extent.nodeId, - ); // The user is trying to delete upstream at the start of a node. // This action requires intervention because the IME doesn't know // that there's more content before this node. Instruct the editor @@ -398,7 +395,7 @@ class TextDeltasDocumentEditor { // "backspace" behavior at the start of this node. editor.execute([ DeleteUpstreamAtBeginningOfNodeRequest( - document.getNodeAt(selectedNodeIndex)!, + document.getNodeById(selection.value!.extent.nodeId)!, ), ]); return; diff --git a/super_editor/lib/src/default_editor/layout_single_column/_presenter.dart b/super_editor/lib/src/default_editor/layout_single_column/_presenter.dart index 5201b677db..90827514fe 100644 --- a/super_editor/lib/src/default_editor/layout_single_column/_presenter.dart +++ b/super_editor/lib/src/default_editor/layout_single_column/_presenter.dart @@ -164,17 +164,16 @@ class SingleColumnLayoutPresenter { // The document changed. All view models were invalidated. Create a // new base document view model. final viewModels = []; - for (int i = 0; i < _document.nodeCount; i += 1) { + for (final node in _document) { SingleColumnLayoutComponentViewModel? viewModel; for (final builder in _componentBuilders) { - viewModel = builder.createViewModel(_document, _document.getNodeAt(i)!); + viewModel = builder.createViewModel(_document, node); if (viewModel != null) { break; } } if (viewModel == null) { - throw Exception( - "Couldn't find styler to create component for document node: ${_document.getNodeAt(i)!.runtimeType}"); + throw Exception("Couldn't find styler to create component for document node: ${node.runtimeType}"); } viewModels.add(viewModel); } 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 6f3297b5cb..9c1d731d24 100644 --- a/super_editor/lib/src/default_editor/multi_node_editing.dart +++ b/super_editor/lib/src/default_editor/multi_node_editing.dart @@ -923,28 +923,38 @@ class DeleteContentCommand extends EditCommand { required DocumentNode startNode, required DocumentNode endNode, }) { + if (startNode.id == endNode.id) { + // The start and end nodes are the same. Nothing to delete. + return []; + } + // Delete all nodes between the first node and the last node. - final startIndex = document.getNodeIndexById(startNode.id); - final endIndex = document.getNodeIndexById(endNode.id); + if (document.getAffinityBetweenNodes(startNode, endNode) != TextAffinity.downstream) { + throw Exception( + "Tried to delete the nodes between a start and end node, but the start node doesn't appear before the end node. Start: ${startNode.id}, End: ${endNode.id}.", + ); + } - _log.log('_deleteNodesBetweenFirstAndLast', ' - start node index: $startIndex'); - _log.log('_deleteNodesBetweenFirstAndLast', ' - end node index: $endIndex'); + _log.log('_deleteNodesBetweenFirstAndLast', ' - start node: ${startNode.id}'); + _log.log('_deleteNodesBetweenFirstAndLast', ' - end node: ${endNode.id}'); _log.log('_deleteNodesBetweenFirstAndLast', ' - initially ${document.nodeCount} nodes'); // Remove nodes from last to first so that indices don't get // screwed up during removal. final changes = []; - for (int i = endIndex - 1; i > startIndex; --i) { - _log.log('_deleteNodesBetweenFirstAndLast', ' - deleting node $i: ${document.getNodeAt(i)?.id}'); - final removedNode = document.getNodeAt(i)!; - if (!removedNode.isDeletable) { - // This node is not deletable. Ignore it. - continue; + var nodeToDelete = document.getNodeAfter(startNode); + while (nodeToDelete != null && nodeToDelete != endNode) { + _log.log('_deleteNodesBetweenFirstAndLast', ' - deleting node: ${nodeToDelete.id}'); + if (nodeToDelete.isDeletable) { + // This node is deletable, so delete it. + changes.add(DocumentEdit( + NodeRemovedEvent(nodeToDelete.id, nodeToDelete), + )); + document.deleteNode(nodeToDelete); } - changes.add(DocumentEdit( - NodeRemovedEvent(removedNode.id, removedNode), - )); - document.deleteNodeAt(i); + + // Move to the next node. + nodeToDelete = document.getNodeAfter(nodeToDelete); } return changes; } diff --git a/super_editor/lib/src/default_editor/tap_handlers/tap_handlers.dart b/super_editor/lib/src/default_editor/tap_handlers/tap_handlers.dart index 705497bbd0..4c68c713ca 100644 --- a/super_editor/lib/src/default_editor/tap_handlers/tap_handlers.dart +++ b/super_editor/lib/src/default_editor/tap_handlers/tap_handlers.dart @@ -169,8 +169,8 @@ class SuperEditorAddEmptyParagraphTapHandler extends ContentTapDelegate { final tappedComponent = documentLayout.getComponentByNodeId(nodeId)!; final componentBox = tappedComponent.context.findRenderObject() as RenderBox; final localPosition = componentBox.globalToLocal(globalOffset); - final nodeIndex = document.getNodeIndexById(nodeId); + final node = document.getNodeById(nodeId); - return (nodeIndex == document.nodeCount - 1) && (localPosition.dy > componentBox.size.height); + return (node == document.lastOrNull) && (localPosition.dy > componentBox.size.height); } } diff --git a/super_editor/lib/src/default_editor/tasks.dart b/super_editor/lib/src/default_editor/tasks.dart index 96504bd37e..685f669130 100644 --- a/super_editor/lib/src/default_editor/tasks.dart +++ b/super_editor/lib/src/default_editor/tasks.dart @@ -490,12 +490,11 @@ ExecutionInstruction tabToIndentTask({ return ExecutionInstruction.continueExecution; } - final nodeIndex = editContext.document.getNodeIndexById(node.id); - if (nodeIndex == 0) { + final taskAbove = editContext.document.getNodeBefore(node); + if (taskAbove == null) { // No task above us, so we can't indent. return ExecutionInstruction.continueExecution; } - final taskAbove = editContext.document.getNodeAt(nodeIndex - 1); if (taskAbove is! TaskNode) { // The node above isn't a task. We can't indent. return ExecutionInstruction.continueExecution; @@ -879,12 +878,7 @@ class IndentTaskCommand extends EditCommand { return; } - final taskIndex = document.getNodeIndexById(task.id); - if (taskIndex == 0) { - // There's no task above this task, therefore it can't be indented. - return; - } - final taskAbove = document.getNodeAt(taskIndex - 1); + final taskAbove = document.getNodeBefore(task); if (taskAbove is! TaskNode) { // There's no task above this task, therefore it can't be indented. return; @@ -934,9 +928,9 @@ class UnIndentTaskCommand extends EditCommand { } final subTasks = []; - int index = document.getNodeIndexById(task.id) + 1; - while (index < document.nodeCount) { - final subTask = document.getNodeAt(index); + var nextNode = document.getNodeAfter(task); + while (nextNode != null) { + final subTask = nextNode; if (subTask is! TaskNode) { break; } @@ -945,7 +939,7 @@ class UnIndentTaskCommand extends EditCommand { } subTasks.add(subTask); - index += 1; + nextNode = document.getNodeAfter(nextNode); } final changeLog = []; @@ -1028,8 +1022,7 @@ class UpdateSubTaskIndentAfterTaskDeletionReaction extends EditReaction { final document = editorContext.document; final changeIndentationRequests = []; int maxIndentation = 0; - for (int i = 0; i < document.nodeCount; i += 1) { - final node = document.getNodeAt(i); + for (final node in document) { if (node is! TaskNode) { // This node isn't a task. The first task in a list of tasks // can't have an indent, so reset the max indent back to zero. diff --git a/super_editor/lib/src/default_editor/text.dart b/super_editor/lib/src/default_editor/text.dart index f8a1a8e011..3c9ef4ad2f 100644 --- a/super_editor/lib/src/default_editor/text.dart +++ b/super_editor/lib/src/default_editor/text.dart @@ -2093,6 +2093,8 @@ void _deleteExpandedSelection({ ); } +// FIXME: This method appears to be the same as CommonEditorOperations.getDocumentPositionAfterExpandedDeletion +// De-dup this behavior in an appropriate place DocumentPosition _getDocumentPositionAfterExpandedDeletion({ required Document document, required DocumentSelection selection, @@ -2107,26 +2109,29 @@ DocumentPosition _getDocumentPositionAfterExpandedDeletion({ if (baseNode == null) { throw Exception('Failed to _getDocumentPositionAfterDeletion because the base node no longer exists.'); } - final baseNodeIndex = document.getNodeIndexById(baseNode.id); final extentPosition = selection.extent; final extentNode = document.getNode(extentPosition); if (extentNode == null) { throw Exception('Failed to _getDocumentPositionAfterDeletion because the extent node no longer exists.'); } - final extentNodeIndex = document.getNodeIndexById(extentNode.id); - final topNodeIndex = min(baseNodeIndex, extentNodeIndex); - final topNode = document.getNodeAt(topNodeIndex)!; - final topNodePosition = baseNodeIndex < extentNodeIndex ? basePosition.nodePosition : extentPosition.nodePosition; + final selectionAffinity = document.getAffinityForSelection(selection); + final topPosition = selectionAffinity == TextAffinity.downstream // + ? selection.base + : selection.extent; + final topNodePosition = topPosition.nodePosition; + final topNode = document.getNodeById(topPosition.nodeId)!; - final bottomNodeIndex = max(baseNodeIndex, extentNodeIndex); - final bottomNode = document.getNodeAt(bottomNodeIndex)!; - final bottomNodePosition = baseNodeIndex < extentNodeIndex ? extentPosition.nodePosition : basePosition.nodePosition; + final bottomPosition = selectionAffinity == TextAffinity.downstream // + ? selection.extent + : selection.base; + final bottomNodePosition = bottomPosition.nodePosition; + final bottomNode = document.getNodeById(bottomPosition.nodeId)!; DocumentPosition newSelectionPosition; - if (baseNodeIndex != extentNodeIndex) { + if (topPosition.nodeId != bottomPosition.nodeId) { if (topNodePosition == topNode.beginningPosition && bottomNodePosition == bottomNode.endPosition) { // All nodes in the selection will be deleted. Assume that the base // node will be retained and converted into a paragraph, if it's not @@ -2154,7 +2159,7 @@ DocumentPosition _getDocumentPositionAfterExpandedDeletion({ // those nodes will remain. // The caret should end up at the base position - newSelectionPosition = baseNodeIndex <= extentNodeIndex ? selection.base : selection.extent; + newSelectionPosition = selectionAffinity == TextAffinity.downstream ? selection.base : selection.extent; } } else { // Selection is within a single node. diff --git a/super_editor/lib/src/infrastructure/platforms/android/drag_handle_selection.dart b/super_editor/lib/src/infrastructure/platforms/android/drag_handle_selection.dart index f1ba919a51..52ec41fa6b 100644 --- a/super_editor/lib/src/infrastructure/platforms/android/drag_handle_selection.dart +++ b/super_editor/lib/src/infrastructure/platforms/android/drag_handle_selection.dart @@ -127,14 +127,21 @@ class AndroidTextFieldDragHandleSelectionStrategy { } final nearestPositionTextOffset = (nearestPosition.nodePosition as TextNodePosition).offset; - final nearestPositionNodeIndex = _document.getNodeIndexById(nearestPosition.nodeId); - final previousNearestPositionTextOffset = (_lastFocalPosition!.nodePosition as TextNodePosition).offset; - final previousNearestPositionNodeIndex = _document.getNodeIndexById(_lastFocalPosition!.nodeId); - final didFocalPointMoveToDownstreamNode = nearestPositionNodeIndex > previousNearestPositionNodeIndex; - final didFocalPointMoveToUpstreamNode = nearestPositionNodeIndex < previousNearestPositionNodeIndex; - final didFocalPointStayInSameNode = nearestPositionNodeIndex == previousNearestPositionNodeIndex; + final didFocalPointMoveToDownstreamNode = _document.getAffinityBetween( + base: _lastFocalPosition!, + extent: nearestPosition, + ) == + TextAffinity.downstream; + + final didFocalPointMoveToUpstreamNode = _document.getAffinityBetween( + base: _lastFocalPosition!, + extent: nearestPosition, + ) == + TextAffinity.upstream; + + final didFocalPointStayInSameNode = _lastFocalPosition!.nodeId == nearestPosition.nodeId; final didFocalPointMoveDownstream = didFocalPointMoveToDownstreamNode || (didFocalPointStayInSameNode && nearestPositionTextOffset > previousNearestPositionTextOffset) || From 2c892d812bc4f294e1257ab163e3a6077582a51c Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Sat, 30 Nov 2024 16:07:44 -0800 Subject: [PATCH 2/4] Fixed mobile handle selection test failure --- .../android/drag_handle_selection.dart | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/super_editor/lib/src/infrastructure/platforms/android/drag_handle_selection.dart b/super_editor/lib/src/infrastructure/platforms/android/drag_handle_selection.dart index 52ec41fa6b..791f3e44ad 100644 --- a/super_editor/lib/src/infrastructure/platforms/android/drag_handle_selection.dart +++ b/super_editor/lib/src/infrastructure/platforms/android/drag_handle_selection.dart @@ -129,19 +129,21 @@ class AndroidTextFieldDragHandleSelectionStrategy { final nearestPositionTextOffset = (nearestPosition.nodePosition as TextNodePosition).offset; final previousNearestPositionTextOffset = (_lastFocalPosition!.nodePosition as TextNodePosition).offset; + final didFocalPointStayInSameNode = _lastFocalPosition!.nodeId == nearestPosition.nodeId; + final didFocalPointMoveToDownstreamNode = _document.getAffinityBetween( - base: _lastFocalPosition!, - extent: nearestPosition, - ) == - TextAffinity.downstream; + base: _lastFocalPosition!, + extent: nearestPosition, + ) == + TextAffinity.downstream && + !didFocalPointStayInSameNode; final didFocalPointMoveToUpstreamNode = _document.getAffinityBetween( - base: _lastFocalPosition!, - extent: nearestPosition, - ) == - TextAffinity.upstream; - - final didFocalPointStayInSameNode = _lastFocalPosition!.nodeId == nearestPosition.nodeId; + base: _lastFocalPosition!, + extent: nearestPosition, + ) == + TextAffinity.upstream && + !didFocalPointStayInSameNode; final didFocalPointMoveDownstream = didFocalPointMoveToDownstreamNode || (didFocalPointStayInSameNode && nearestPositionTextOffset > previousNearestPositionTextOffset) || From 78f04c2dcf7fa879674b1e9b62cc2c65c49d2eb3 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Mon, 2 Dec 2024 20:20:04 -0800 Subject: [PATCH 3/4] Fix Document hasEquivalentContent() --- super_editor/lib/src/core/editor.dart | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index b14722dafd..bc583c2fd5 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -1278,7 +1278,7 @@ class MutableDocument with Iterable implements Document, Editable } } - /// Returns [true] if the content of the [other] [Document] is equivalent + /// Returns `true` if the content of the [other] [Document] is equivalent /// to the content of this [Document]. /// /// Content equivalency compares types of content nodes, and the content @@ -1289,11 +1289,21 @@ class MutableDocument with Iterable implements Document, Editable if (_nodes.length != other.nodeCount) { return false; } + if (isEmpty) { + // Both documents are empty, and therefore are equivalent. + return true; + } + + DocumentNode? thisDocNode = first; + DocumentNode? otherDocNode = other.first; - for (int i = 0; i < _nodes.length; ++i) { - if (!_nodes[i].hasEquivalentContent(other.getNodeById(_nodes[i].id)!)) { + while (thisDocNode != null && otherDocNode != null) { + if (!thisDocNode.hasEquivalentContent(otherDocNode)) { return false; } + + thisDocNode = getNodeAfter(thisDocNode); + otherDocNode = other.getNodeAfter(otherDocNode); } return true; From 37d32647fe99091a9e34f87ecdf97bb5220b586c Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Wed, 4 Dec 2024 12:08:38 -0800 Subject: [PATCH 4/4] Fix bad node iteration in _deleteNodesBetweenFirstAndLast() --- super_editor/lib/src/default_editor/multi_node_editing.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 9c1d731d24..5abda1cf71 100644 --- a/super_editor/lib/src/default_editor/multi_node_editing.dart +++ b/super_editor/lib/src/default_editor/multi_node_editing.dart @@ -945,6 +945,7 @@ class DeleteContentCommand extends EditCommand { var nodeToDelete = document.getNodeAfter(startNode); while (nodeToDelete != null && nodeToDelete != endNode) { _log.log('_deleteNodesBetweenFirstAndLast', ' - deleting node: ${nodeToDelete.id}'); + final nextNode = document.getNodeAfter(nodeToDelete); if (nodeToDelete.isDeletable) { // This node is deletable, so delete it. changes.add(DocumentEdit( @@ -954,7 +955,7 @@ class DeleteContentCommand extends EditCommand { } // Move to the next node. - nodeToDelete = document.getNodeAfter(nodeToDelete); + nodeToDelete = nextNode; } return changes; }