From efda3fafd1bb698c767d86bf6e055e8c5e2143e9 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Wed, 26 Oct 2022 20:46:01 -0300 Subject: [PATCH 01/15] [SuperEditor] Fix typing lag in large documents (Resolves #751) --- .../lib/demos/example_editor/_task.dart | 2 - .../lib/src/core/document_editor.dart | 103 +++-- .../lib/src/default_editor/blockquote.dart | 2 - .../lib/src/default_editor/list_items.dart | 2 - .../lib/src/default_editor/paragraph.dart | 2 - super_editor/lib/src/default_editor/text.dart | 48 ++- .../test/src/default_editor/text_test.dart | 59 +-- .../super_editor/mutable_document_test.dart | 367 ++++++++++++++++++ .../super_editor/supereditor_style_test.dart | 40 ++ 9 files changed, 554 insertions(+), 71 deletions(-) create mode 100644 super_editor/test/super_editor/supereditor_style_test.dart diff --git a/super_editor/example/lib/demos/example_editor/_task.dart b/super_editor/example/lib/demos/example_editor/_task.dart index 63c892c290..2aa4505369 100644 --- a/super_editor/example/lib/demos/example_editor/_task.dart +++ b/super_editor/example/lib/demos/example_editor/_task.dart @@ -179,7 +179,6 @@ class TaskComponentViewModel extends SingleColumnLayoutComponentViewModel with T isComplete == other.isComplete && setComplete == other.setComplete && text == other.text && - textStyleBuilder == other.textStyleBuilder && textDirection == other.textDirection && textAlignment == other.textAlignment && selection == other.selection && @@ -192,7 +191,6 @@ class TaskComponentViewModel extends SingleColumnLayoutComponentViewModel with T isComplete.hashCode ^ setComplete.hashCode ^ text.hashCode ^ - textStyleBuilder.hashCode ^ textDirection.hashCode ^ textAlignment.hashCode ^ selection.hashCode ^ diff --git a/super_editor/lib/src/core/document_editor.dart b/super_editor/lib/src/core/document_editor.dart index 3ae2f59549..d71ada7a17 100644 --- a/super_editor/lib/src/core/document_editor.dart +++ b/super_editor/lib/src/core/document_editor.dart @@ -1,7 +1,6 @@ import 'dart:math'; import 'package:collection/collection.dart'; -import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:super_editor/src/core/document_selection.dart'; import 'package:super_editor/src/infrastructure/_logging.dart'; @@ -133,22 +132,29 @@ class MutableDocument with ChangeNotifier implements Document { MutableDocument({ List? nodes, }) : _nodes = nodes ?? [] { - // Register listeners for all initial nodes. - for (final node in _nodes) { + // Register listeners for all initial nodes and populates the node maps. + for (int i = 0; i < _nodes.length; i++) { + final node = _nodes[i]; node.addListener(_forwardNodeChange); + _nodeIndicesById[node.id] = i; + _nodesById[node.id] = node; } } final List _nodes; @override - List get nodes => _nodes; + List get nodes => List.unmodifiable(_nodes); + + /// Maps a node id to its index in the node list. + final Map _nodeIndicesById = {}; + + /// Maps a node id to its node. + final Map _nodesById = {}; @override DocumentNode? getNodeById(String nodeId) { - return _nodes.firstWhereOrNull( - (element) => element.id == nodeId, - ); + return _nodesById[nodeId]; } @override @@ -162,7 +168,10 @@ class MutableDocument with ChangeNotifier implements Document { @override int getNodeIndex(DocumentNode node) { - return _nodes.indexOf(node); + // We always update _nodeIndicesById when the node list changes. + // If the id isn't present in the map it means the node isn't present + // in the document. + return _nodeIndicesById[node.id] ?? -1; } @override @@ -180,12 +189,11 @@ class MutableDocument with ChangeNotifier implements Document { @override DocumentNode? getNodeAfter(DocumentNode node) { final nodeIndex = getNodeIndex(node); - return nodeIndex >= 0 && nodeIndex < nodes.length - 1 ? getNodeAt(nodeIndex + 1) : null; + return nodeIndex >= 0 && nodeIndex < _nodes.length - 1 ? getNodeAt(nodeIndex + 1) : null; } @override - DocumentNode? getNode(DocumentPosition position) => - _nodes.firstWhereOrNull((element) => element.id == position.nodeId); + DocumentNode? getNode(DocumentPosition position) => getNodeById(position.nodeId); @override DocumentRange getRangeBetween(DocumentPosition position1, DocumentPosition position2) { @@ -202,13 +210,13 @@ class MutableDocument with ChangeNotifier implements Document { if (node1 == null) { throw Exception('No such position in document: $position1'); } - final index1 = _nodes.indexOf(node1); + final index1 = getNodeIndex(node1); final node2 = getNode(position2); if (node2 == null) { throw Exception('No such position in document: $position2'); } - final index2 = _nodes.indexOf(node2); + final index2 = getNodeIndex(node2); final from = min(index1, index2); final to = max(index1, index2); @@ -218,9 +226,13 @@ class MutableDocument with ChangeNotifier implements Document { /// Inserts the given [node] into the [Document] at the given [index]. void insertNodeAt(int index, DocumentNode node) { - if (index <= nodes.length) { - nodes.insert(index, node); + if (index <= _nodes.length) { + _nodes.insert(index, node); node.addListener(_forwardNodeChange); + + // The node list changed, we need to update the map to consider the new indices. + _updateNodeIdMappings(); + notifyListeners(); } } @@ -230,9 +242,13 @@ class MutableDocument with ChangeNotifier implements Document { required DocumentNode existingNode, required DocumentNode newNode, }) { - final nodeIndex = nodes.indexOf(existingNode); - nodes.insert(nodeIndex, newNode); + final nodeIndex = getNodeIndex(existingNode); + _nodes.insert(nodeIndex, newNode); newNode.addListener(_forwardNodeChange); + + // The node list changed, we need to update the map to consider the new indices. + _updateNodeIdMappings(); + notifyListeners(); } @@ -241,19 +257,27 @@ class MutableDocument with ChangeNotifier implements Document { required DocumentNode existingNode, required DocumentNode newNode, }) { - final nodeIndex = nodes.indexOf(existingNode); - if (nodeIndex >= 0 && nodeIndex < nodes.length) { - nodes.insert(nodeIndex + 1, newNode); + final nodeIndex = getNodeIndex(existingNode); + if (nodeIndex >= 0 && nodeIndex < _nodes.length) { + _nodes.insert(nodeIndex + 1, newNode); newNode.addListener(_forwardNodeChange); + + // The node list changed, we need to update the map to consider the new indices. + _updateNodeIdMappings(); + notifyListeners(); } } /// Deletes the node at the given [index]. void deleteNodeAt(int index) { - if (index >= 0 && index < nodes.length) { - final removedNode = nodes.removeAt(index); + if (index >= 0 && index < _nodes.length) { + final removedNode = _nodes.removeAt(index); removedNode.removeListener(_forwardNodeChange); + + // The node list changed, we need to update the map to consider the new indices. + _updateNodeIdMappings(); + notifyListeners(); } else { editorDocLog.warning('Could not delete node. Index out of range: $index'); @@ -265,7 +289,11 @@ class MutableDocument with ChangeNotifier implements Document { bool isRemoved = false; node.removeListener(_forwardNodeChange); - isRemoved = nodes.remove(node); + + isRemoved = _nodes.remove(node); + + // The node list changed, we need to update the map to consider the new indices. + _updateNodeIdMappings(); notifyListeners(); @@ -282,8 +310,12 @@ class MutableDocument with ChangeNotifier implements Document { throw Exception('Could not find node with nodeId: $nodeId'); } - if (nodes.remove(node)) { - nodes.insert(targetIndex, node); + if (_nodes.remove(node)) { + _nodes.insert(targetIndex, node); + + // The node list changed, we need to update the map to consider the new indices. + _updateNodeIdMappings(); + notifyListeners(); } } @@ -302,6 +334,9 @@ class MutableDocument with ChangeNotifier implements Document { newNode.addListener(_forwardNodeChange); _nodes.insert(index, newNode); + // The node list changed, we need to update the map to consider the new indices. + _updateNodeIdMappings(); + notifyListeners(); } else { throw Exception('Could not find oldNode: ${oldNode.id}'); @@ -320,12 +355,13 @@ class MutableDocument with ChangeNotifier implements Document { /// ignores the runtime type of the [Document], itself. @override bool hasEquivalentContent(Document other) { - if (_nodes.length != other.nodes.length) { + final otherNodes = other.nodes; + if (_nodes.length != otherNodes.length) { return false; } for (int i = 0; i < _nodes.length; ++i) { - if (!_nodes[i].hasEquivalentContent(other.nodes[i])) { + if (!_nodes[i].hasEquivalentContent(otherNodes[i])) { return false; } } @@ -333,6 +369,19 @@ class MutableDocument with ChangeNotifier implements Document { return true; } + /// Updates all the maps which use the node id as the key. + /// + /// All the maps are cleared and re-populated. + void _updateNodeIdMappings() { + _nodeIndicesById.clear(); + _nodesById.clear(); + for (int i = 0; i < _nodes.length; i++) { + final node = _nodes[i]; + _nodeIndicesById[node.id] = i; + _nodesById[node.id] = node; + } + } + @override bool operator ==(Object other) => identical(this, other) || diff --git a/super_editor/lib/src/default_editor/blockquote.dart b/super_editor/lib/src/default_editor/blockquote.dart index cca56a6e01..59b397b0b1 100644 --- a/super_editor/lib/src/default_editor/blockquote.dart +++ b/super_editor/lib/src/default_editor/blockquote.dart @@ -147,7 +147,6 @@ class BlockquoteComponentViewModel extends SingleColumnLayoutComponentViewModel runtimeType == other.runtimeType && nodeId == other.nodeId && text == other.text && - textStyleBuilder == other.textStyleBuilder && textDirection == other.textDirection && textAlignment == other.textAlignment && backgroundColor == other.backgroundColor && @@ -161,7 +160,6 @@ class BlockquoteComponentViewModel extends SingleColumnLayoutComponentViewModel super.hashCode ^ nodeId.hashCode ^ text.hashCode ^ - textStyleBuilder.hashCode ^ textDirection.hashCode ^ textAlignment.hashCode ^ backgroundColor.hashCode ^ diff --git a/super_editor/lib/src/default_editor/list_items.dart b/super_editor/lib/src/default_editor/list_items.dart index 8a0cff52da..3052f8ecd1 100644 --- a/super_editor/lib/src/default_editor/list_items.dart +++ b/super_editor/lib/src/default_editor/list_items.dart @@ -231,7 +231,6 @@ class ListItemComponentViewModel extends SingleColumnLayoutComponentViewModel wi ordinalValue == other.ordinalValue && indent == other.indent && text == other.text && - textStyleBuilder == other.textStyleBuilder && textDirection == other.textDirection && selection == other.selection && selectionColor == other.selectionColor; @@ -244,7 +243,6 @@ class ListItemComponentViewModel extends SingleColumnLayoutComponentViewModel wi ordinalValue.hashCode ^ indent.hashCode ^ text.hashCode ^ - textStyleBuilder.hashCode ^ textDirection.hashCode ^ selection.hashCode ^ selectionColor.hashCode; diff --git a/super_editor/lib/src/default_editor/paragraph.dart b/super_editor/lib/src/default_editor/paragraph.dart index 969ae4b938..6d95042e4d 100644 --- a/super_editor/lib/src/default_editor/paragraph.dart +++ b/super_editor/lib/src/default_editor/paragraph.dart @@ -162,7 +162,6 @@ class ParagraphComponentViewModel extends SingleColumnLayoutComponentViewModel w nodeId == other.nodeId && blockType == other.blockType && text == other.text && - textStyleBuilder == other.textStyleBuilder && textDirection == other.textDirection && textAlignment == other.textAlignment && selection == other.selection && @@ -175,7 +174,6 @@ class ParagraphComponentViewModel extends SingleColumnLayoutComponentViewModel w nodeId.hashCode ^ blockType.hashCode ^ text.hashCode ^ - textStyleBuilder.hashCode ^ textDirection.hashCode ^ textAlignment.hashCode ^ selection.hashCode ^ diff --git a/super_editor/lib/src/default_editor/text.dart b/super_editor/lib/src/default_editor/text.dart index 9f6fd34dbf..fd0e7f147a 100644 --- a/super_editor/lib/src/default_editor/text.dart +++ b/super_editor/lib/src/default_editor/text.dart @@ -887,9 +887,19 @@ class AddTextAttributionsCommand implements EditorCommand { final node = entry.key; final range = entry.value.toSpanRange(); editorDocLog.info(' - adding attribution: $attribution. Range: $range'); - node.text.addAttribution( - attribution, - range, + + // During style phase, we check which components changed, so we only re-style those components. + // If we just add the attribution on the same instance of AttributedText, we can't detect the change, + // because comparing the same instance will always evaluate to true. + final spans = node.text.spans.copy(); + spans.addAttribution( + newAttribution: attribution, + start: range.start, + end: range.end, + ); + node.text = AttributedText( + text: node.text.text, + spans: spans, ); } } @@ -975,9 +985,19 @@ class RemoveTextAttributionsCommand implements EditorCommand { final node = entry.key; final range = entry.value.toSpanRange(); editorDocLog.info(' - removing attribution: $attribution. Range: $range'); - node.text.removeAttribution( - attribution, - range, + + // During style phase, we check which components changed, so we only re-style those components. + // If we just remove the attribution on the same instance of AttributedText, we can't detect the change, + // because comparing the same instance will always evaluate to true. + final spans = node.text.spans.copy(); + spans.removeAttribution( + attributionToRemove: attribution, + start: range.start, + end: range.end, + ); + node.text = AttributedText( + text: node.text.text, + spans: spans, ); } } @@ -1073,9 +1093,19 @@ class ToggleTextAttributionsCommand implements EditorCommand { final node = entry.key; final range = entry.value; editorDocLog.info(' - toggling attribution: $attribution. Range: $range'); - node.text.toggleAttribution( - attribution, - range, + + // During style phase, we check which components changed, so we only re-style those components. + // If we just toggle the attribution on the same instance of AttributedText, we can't detect the change, + // because comparing the same instance will always evaluate to true. + final spans = node.text.spans.copy(); + spans.toggleAttribution( + attribution: attribution, + start: range.start, + end: range.end, + ); + node.text = AttributedText( + text: node.text.text, + spans: spans, ); } } diff --git a/super_editor/test/src/default_editor/text_test.dart b/super_editor/test/src/default_editor/text_test.dart index 075f03742b..9f1ba6c54f 100644 --- a/super_editor/test/src/default_editor/text_test.dart +++ b/super_editor/test/src/default_editor/text_test.dart @@ -107,12 +107,13 @@ void main() { final editContext = _createEditContext(); // Add a paragraph to the document. - (editContext.editor.document as MutableDocument).nodes.add( - ParagraphNode( - id: 'paragraph', - text: AttributedText(text: 'This is some text'), - ), - ); + (editContext.editor.document as MutableDocument).insertNodeAt( + 0, + ParagraphNode( + id: 'paragraph', + text: AttributedText(text: 'This is some text'), + ), + ); // Select multiple characters in the paragraph editContext.composer.selection = const DocumentSelection( @@ -145,9 +146,10 @@ void main() { final editContext = _createEditContext(); // Add a non-text node to the document. - (editContext.editor.document as MutableDocument).nodes.add( - HorizontalRuleNode(id: 'horizontal_rule'), - ); + (editContext.editor.document as MutableDocument).insertNodeAt( + 0, + HorizontalRuleNode(id: 'horizontal_rule'), + ); // Select the horizontal rule node. editContext.composer.selection = const DocumentSelection.collapsed( @@ -176,12 +178,13 @@ void main() { final editContext = _createEditContext(); // Add a paragraph to the document. - (editContext.editor.document as MutableDocument).nodes.add( - ParagraphNode( - id: 'paragraph', - text: AttributedText(text: 'This is some text'), - ), - ); + (editContext.editor.document as MutableDocument).insertNodeAt( + 0, + ParagraphNode( + id: 'paragraph', + text: AttributedText(text: 'This is some text'), + ), + ); // Select multiple characters in the paragraph editContext.composer.selection = const DocumentSelection.collapsed( @@ -227,12 +230,13 @@ void main() { final editContext = _createEditContext(); // Add a paragraph to the document. - (editContext.editor.document as MutableDocument).nodes.add( - ParagraphNode( - id: 'paragraph', - text: AttributedText(text: 'This is some text'), - ), - ); + (editContext.editor.document as MutableDocument).insertNodeAt( + 0, + ParagraphNode( + id: 'paragraph', + text: AttributedText(text: 'This is some text'), + ), + ); // Select multiple characters in the paragraph editContext.composer.selection = const DocumentSelection.collapsed( @@ -266,12 +270,13 @@ void main() { final editContext = _createEditContext(); // Add a paragraph to the document. - (editContext.editor.document as MutableDocument).nodes.add( - ParagraphNode( - id: 'paragraph', - text: AttributedText(text: 'This is some text'), - ), - ); + (editContext.editor.document as MutableDocument).insertNodeAt( + 0, + ParagraphNode( + id: 'paragraph', + text: AttributedText(text: 'This is some text'), + ), + ); // Select multiple characters in the paragraph editContext.composer.selection = const DocumentSelection.collapsed( diff --git a/super_editor/test/super_editor/mutable_document_test.dart b/super_editor/test/super_editor/mutable_document_test.dart index 5f422c13cb..aa78e46647 100644 --- a/super_editor/test/super_editor/mutable_document_test.dart +++ b/super_editor/test/super_editor/mutable_document_test.dart @@ -1,6 +1,8 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:super_editor/super_editor.dart'; +import 'test_documents.dart'; + void main() { group("MutableDocument", () { test("calculates a range from an upstream selection within a single node", () { @@ -68,5 +70,370 @@ void main() { ), ); }); + + group("getNodeIndex returns the correct index", () { + test("when creating a document", () { + final document = _createThreeParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + final thirdNode = document.nodes[2]; + + // Ensure the indices are correct when creating the document. + expect(document.getNodeIndex(firstNode), 0); + expect(document.getNodeIndex(secondNode), 1); + expect(document.getNodeIndex(thirdNode), 2); + }); + + test("when inserting a node at the beginning by index", () { + final document = _createTwoParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + + // Insert a new node at the beginning. + final thirdNode = ParagraphNode( + id: "3", + text: AttributedText(text: "This is the third paragraph."), + ); + document.insertNodeAt(0, thirdNode); + + // Ensure the indices are correct. + expect(document.getNodeIndex(thirdNode), 0); + expect(document.getNodeIndex(firstNode), 1); + expect(document.getNodeIndex(secondNode), 2); + }); + + test("when inserting a node at the middle by index", () { + final document = _createTwoParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + + // Insert a new node between firstNode and secondNode. + final thirdNode = ParagraphNode( + id: "3", + text: AttributedText(text: "This is the third paragraph."), + ); + document.insertNodeAt(1, thirdNode); + + // Ensure the indices are correct. + expect(document.getNodeIndex(firstNode), 0); + expect(document.getNodeIndex(thirdNode), 1); + expect(document.getNodeIndex(secondNode), 2); + }); + + test("when inserting a node at the end by index", () { + final document = _createTwoParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + + // Insert a new node at the end. + final thirdNode = ParagraphNode( + id: "3", + text: AttributedText(text: "This is the third paragraph."), + ); + document.insertNodeAt(2, thirdNode); + + // Ensure the indices are correct. + expect(document.getNodeIndex(firstNode), 0); + expect(document.getNodeIndex(secondNode), 1); + expect(document.getNodeIndex(thirdNode), 2); + }); + + test("when inserting a node before the first node", () { + final document = _createTwoParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + + // Insert a new node at the beginning. + final thirdNode = ParagraphNode( + id: "3", + text: AttributedText(text: "This is the third paragraph."), + ); + document.insertNodeBefore( + existingNode: firstNode, + newNode: thirdNode, + ); + + // Ensure the indices are correct. + expect(document.getNodeIndex(thirdNode), 0); + expect(document.getNodeIndex(firstNode), 1); + expect(document.getNodeIndex(secondNode), 2); + }); + + test("when inserting a node before the last node", () { + final document = _createTwoParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + + // Insert a new node between the two nodes. + final thirdNode = ParagraphNode( + id: "3", + text: AttributedText(text: "This is the third paragraph."), + ); + document.insertNodeBefore( + existingNode: secondNode, + newNode: thirdNode, + ); + + // Ensure the indices are correct. + expect(document.getNodeIndex(firstNode), 0); + expect(document.getNodeIndex(thirdNode), 1); + expect(document.getNodeIndex(secondNode), 2); + }); + + test("when inserting a node after the first node", () { + final document = _createTwoParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + + // Insert a new node between the two nodes. + final thirdNode = ParagraphNode( + id: "3", + text: AttributedText(text: "This is the third paragraph."), + ); + document.insertNodeAfter( + existingNode: firstNode, + newNode: thirdNode, + ); + + // Ensure the indices are correct. + expect(document.getNodeIndex(firstNode), 0); + expect(document.getNodeIndex(thirdNode), 1); + expect(document.getNodeIndex(secondNode), 2); + }); + + test("when inserting a node after the last node", () { + final document = _createTwoParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + + // Insert a new node at the end. + final thirdNode = ParagraphNode( + id: "3", + text: AttributedText(text: "This is the third paragraph."), + ); + document.insertNodeAfter( + existingNode: secondNode, + newNode: thirdNode, + ); + + // Ensure the indices are correct. + expect(document.getNodeIndex(firstNode), 0); + expect(document.getNodeIndex(secondNode), 1); + expect(document.getNodeIndex(thirdNode), 2); + }); + + test("when moving a node from the beginning to the middle", () { + final document = _createThreeParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + final thirdNode = document.nodes[2]; + + document.moveNode( + nodeId: firstNode.id, + targetIndex: 1, + ); + + // Ensure the indices are correct. + expect(document.getNodeIndex(secondNode), 0); + expect(document.getNodeIndex(firstNode), 1); + expect(document.getNodeIndex(thirdNode), 2); + }); + + test("when moving a node from the middle to the end", () { + final document = _createThreeParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + final thirdNode = document.nodes[2]; + + document.moveNode( + nodeId: secondNode.id, + targetIndex: 2, + ); + + // Ensure the indices are correct. + expect(document.getNodeIndex(firstNode), 0); + expect(document.getNodeIndex(thirdNode), 1); + expect(document.getNodeIndex(secondNode), 2); + }); + + test("when moving a node from the end to the middle", () { + final document = _createThreeParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + final thirdNode = document.nodes[2]; + + document.moveNode( + nodeId: thirdNode.id, + targetIndex: 1, + ); + + // Ensure the indices are correct. + expect(document.getNodeIndex(firstNode), 0); + expect(document.getNodeIndex(thirdNode), 1); + expect(document.getNodeIndex(secondNode), 2); + }); + + test("when moving a node from the middle to the beginning", () { + final document = _createThreeParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + final thirdNode = document.nodes[2]; + + document.moveNode( + nodeId: secondNode.id, + targetIndex: 0, + ); + + // Ensure the indices are correct. + expect(document.getNodeIndex(secondNode), 0); + expect(document.getNodeIndex(firstNode), 1); + expect(document.getNodeIndex(thirdNode), 2); + }); + + test("when deleting a node at the beginning", () { + final document = _createThreeParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + final thirdNode = document.nodes[2]; + + document.deleteNode(firstNode); + + // Ensure the indices are correct. + expect(document.getNodeIndex(firstNode), -1); + expect(document.getNodeIndex(secondNode), 0); + expect(document.getNodeIndex(thirdNode), 1); + }); + + test("when deleting a node at the middle", () { + final document = _createThreeParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + final thirdNode = document.nodes[2]; + + document.deleteNode(secondNode); + + // Ensure the indices are correct. + expect(document.getNodeIndex(secondNode), -1); + expect(document.getNodeIndex(firstNode), 0); + expect(document.getNodeIndex(thirdNode), 1); + }); + + test("when deleting a node at the end", () { + final document = _createThreeParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + final thirdNode = document.nodes[2]; + + document.deleteNode(thirdNode); + + // Ensure the indices are correct. + expect(document.getNodeIndex(thirdNode), -1); + expect(document.getNodeIndex(firstNode), 0); + expect(document.getNodeIndex(secondNode), 1); + }); + + test("when replacing a node at the beginning", () { + final document = _createThreeParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + final thirdNode = document.nodes[2]; + + final fourthNode = ParagraphNode( + id: "4", + text: AttributedText(text: "This is the third paragraph."), + ); + + document.replaceNode( + oldNode: firstNode, + newNode: fourthNode, + ); + + // Ensure the indices are correct. + expect(document.getNodeIndex(firstNode), -1); + expect(document.getNodeIndex(fourthNode), 0); + expect(document.getNodeIndex(secondNode), 1); + expect(document.getNodeIndex(thirdNode), 2); + }); + + test("when replacing a node at the middle", () { + final document = _createThreeParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + final thirdNode = document.nodes[2]; + + final fourthNode = ParagraphNode( + id: "4", + text: AttributedText(text: "This is the third paragraph."), + ); + + document.replaceNode( + oldNode: secondNode, + newNode: fourthNode, + ); + + // Ensure the indices are correct. + expect(document.getNodeIndex(secondNode), -1); + expect(document.getNodeIndex(firstNode), 0); + expect(document.getNodeIndex(fourthNode), 1); + expect(document.getNodeIndex(thirdNode), 2); + }); + + test("when replacing a node at the end", () { + final document = _createThreeParagraphDoc(); + final firstNode = document.nodes[0]; + final secondNode = document.nodes[1]; + final thirdNode = document.nodes[2]; + + final fourthNode = ParagraphNode( + id: "4", + text: AttributedText(text: "This is the third paragraph."), + ); + + document.replaceNode( + oldNode: thirdNode, + newNode: fourthNode, + ); + + // Ensure the indices are correct. + expect(document.getNodeIndex(thirdNode), -1); + expect(document.getNodeIndex(firstNode), 0); + expect(document.getNodeIndex(secondNode), 1); + expect(document.getNodeIndex(fourthNode), 2); + }); + }); }); } + +MutableDocument _createTwoParagraphDoc() { + return MutableDocument( + nodes: [ + ParagraphNode( + id: "1", + text: AttributedText(text: "This is the first paragraph."), + ), + ParagraphNode( + id: "2", + text: AttributedText(text: "This is the second paragraph."), + ), + ], + ); +} + +MutableDocument _createThreeParagraphDoc() { + return MutableDocument( + nodes: [ + ParagraphNode( + id: "1", + text: AttributedText(text: "This is the first paragraph."), + ), + ParagraphNode( + id: "2", + text: AttributedText(text: "This is the second paragraph."), + ), + ParagraphNode( + id: "3", + text: AttributedText(text: "This is the third paragraph."), + ), + ], + ); +} diff --git a/super_editor/test/super_editor/supereditor_style_test.dart b/super_editor/test/super_editor/supereditor_style_test.dart new file mode 100644 index 0000000000..1f4db29dff --- /dev/null +++ b/super_editor/test/super_editor/supereditor_style_test.dart @@ -0,0 +1,40 @@ +import 'package:flutter/cupertino.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:super_editor/src/default_editor/attributions.dart'; +import 'package:super_editor/super_editor_test.dart'; +import 'package:super_text_layout/super_text_layout.dart'; + +import '../test_tools.dart'; +import 'document_test_tools.dart'; + +void main() { + group('SuperEditor', () { + testWidgetsOnArbitraryDesktop('applies style when toggling attributions', (tester) async { + final testContext = await tester + .createDocument() // + .withSingleParagraph() + .pump(); + + // Double tap to select the first word. + await tester.doubleTapInParagraph('1', 0); + + // Apply italic to the word. + testContext.editContext.commonOps.toggleAttributionsOnSelection({italicsAttribution}); + await tester.pump(); + + // Ensure italic was applied. + expect( + _findSpanAtOffset(tester, offset: 0).style!.fontStyle, + FontStyle.italic, + ); + }); + }); +} + +InlineSpan _findSpanAtOffset( + WidgetTester tester, { + required int offset, +}) { + final superTextWithSelection = tester.widget(find.byType(SuperTextWithSelection)); + return superTextWithSelection.richText.getSpanForPosition(TextPosition(offset: offset))!; +} From 350b644837076ee6fdeb3c09050288d37bdf4a45 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Wed, 26 Oct 2022 22:15:20 -0300 Subject: [PATCH 02/15] Attempt to optimize mouse selection --- .../src/default_editor/layout_single_column/_layout.dart | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/super_editor/lib/src/default_editor/layout_single_column/_layout.dart b/super_editor/lib/src/default_editor/layout_single_column/_layout.dart index a021527fbc..eb089b3bfd 100644 --- a/super_editor/lib/src/default_editor/layout_single_column/_layout.dart +++ b/super_editor/lib/src/default_editor/layout_single_column/_layout.dart @@ -367,6 +367,13 @@ class _SingleColumnDocumentLayoutState extends State bottomNodeId = _nodeIdsToComponentKeys.entries.firstWhere((element) => element.value == componentKey).key; bottomNodeBasePosition = _getNodePositionForComponentOffset(component, componentBaseOffset); bottomNodeExtentPosition = _getNodePositionForComponentOffset(component, componentExtentOffset); + } else if (topNodeId != null) { + // We already found an overlapping component and the current component doesn't + // overlap with the region. + // Because we're iterating through components from top to bottom, + // it means that there isn't any other component which will overlap, + // so we can skip the rest of the list. + break; } } @@ -605,7 +612,7 @@ class _SingleColumnDocumentLayoutState extends State DocumentPosition? findLastSelectablePosition() { NodePosition? nodePosition; String? nodeId; - + for (int i = _topToBottomComponentKeys.length - 1; i >= 0; i--) { final componentKey = _topToBottomComponentKeys[i]; final component = componentKey.currentState as DocumentComponent; From 92d9d8eef3b13d5e9c522066ff659fc253bdda37 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Thu, 27 Oct 2022 18:27:50 -0300 Subject: [PATCH 03/15] PR Update --- super_editor/lib/src/core/document_editor.dart | 3 --- super_editor/test/super_editor/supereditor_style_test.dart | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/super_editor/lib/src/core/document_editor.dart b/super_editor/lib/src/core/document_editor.dart index d71ada7a17..3d94f115c6 100644 --- a/super_editor/lib/src/core/document_editor.dart +++ b/super_editor/lib/src/core/document_editor.dart @@ -168,9 +168,6 @@ class MutableDocument with ChangeNotifier implements Document { @override int getNodeIndex(DocumentNode node) { - // We always update _nodeIndicesById when the node list changes. - // If the id isn't present in the map it means the node isn't present - // in the document. return _nodeIndicesById[node.id] ?? -1; } diff --git a/super_editor/test/super_editor/supereditor_style_test.dart b/super_editor/test/super_editor/supereditor_style_test.dart index 1f4db29dff..68ed7cfbb3 100644 --- a/super_editor/test/super_editor/supereditor_style_test.dart +++ b/super_editor/test/super_editor/supereditor_style_test.dart @@ -9,7 +9,7 @@ import 'document_test_tools.dart'; void main() { group('SuperEditor', () { - testWidgetsOnArbitraryDesktop('applies style when toggling attributions', (tester) async { + testWidgetsOnArbitraryDesktop('changes visual text style when attributions change', (tester) async { final testContext = await tester .createDocument() // .withSingleParagraph() From e1d47d64adbe2c685f2b4ccde08f9f1cccdce304 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Thu, 27 Oct 2022 18:49:25 -0300 Subject: [PATCH 04/15] Replace List.unmodifiable with UnmodifiableListView. --- super_editor/lib/src/core/document_editor.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/super_editor/lib/src/core/document_editor.dart b/super_editor/lib/src/core/document_editor.dart index 3d94f115c6..5e233c9048 100644 --- a/super_editor/lib/src/core/document_editor.dart +++ b/super_editor/lib/src/core/document_editor.dart @@ -144,7 +144,7 @@ class MutableDocument with ChangeNotifier implements Document { final List _nodes; @override - List get nodes => List.unmodifiable(_nodes); + List get nodes => UnmodifiableListView(_nodes); /// Maps a node id to its index in the node list. final Map _nodeIndicesById = {}; From c65a7243054577b5aa0103c22f3265058a779bbe Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Fri, 28 Oct 2022 21:38:24 -0300 Subject: [PATCH 05/15] Mouse dragging performance improvement --- .../layout_single_column/_layout.dart | 78 ++++++++++++++++--- 1 file changed, 66 insertions(+), 12 deletions(-) diff --git a/super_editor/lib/src/default_editor/layout_single_column/_layout.dart b/super_editor/lib/src/default_editor/layout_single_column/_layout.dart index eb089b3bfd..c71cdc0e5f 100644 --- a/super_editor/lib/src/default_editor/layout_single_column/_layout.dart +++ b/super_editor/lib/src/default_editor/layout_single_column/_layout.dart @@ -53,6 +53,7 @@ class SingleColumnDocumentLayout extends StatefulWidget { class _SingleColumnDocumentLayoutState extends State implements DocumentLayout { final Map _nodeIdsToComponentKeys = {}; + final Map _componentKeysToNodeIds = {}; // Keys are cached in top-to-bottom order so that we can visually // traverse components without repeatedly querying a `Document` @@ -174,7 +175,7 @@ class _SingleColumnDocumentLayoutState extends State } final selectionAtOffset = DocumentPosition( - nodeId: _nodeIdsToComponentKeys.entries.firstWhere((element) => element.value == componentKey).key, + nodeId: _componentKeysToNodeIds[componentKey]!, nodePosition: componentPosition, ); editorLayoutLog.info(' - selection at offset: $selectionAtOffset'); @@ -300,11 +301,7 @@ class _SingleColumnDocumentLayoutState extends State final bottomNodeIndex = topNodeIndex == baseComponentIndex ? extentComponentIndex : baseComponentIndex; final componentsInside = _topToBottomComponentKeys.sublist(topNodeIndex, bottomNodeIndex + 1); - return componentsInside.map((componentKey) { - return _nodeIdsToComponentKeys.entries.firstWhere((entry) { - return entry.value == componentKey; - }).key; - }).toList(); + return componentsInside.map((componentKey) => _componentKeysToNodeIds[componentKey]!).toList(); } @override @@ -320,7 +317,11 @@ class _SingleColumnDocumentLayoutState extends State dynamic bottomNodeBasePosition; dynamic bottomNodeExtentPosition; - for (final componentKey in _topToBottomComponentKeys) { + final startingOffset = min(baseOffset.dy, extentOffset.dy); + final startIndex = max(_findComponentIndexAtOffset(startingOffset), 0); + + for (int i = startIndex; i < _topToBottomComponentKeys.length; i++) { + final componentKey = _topToBottomComponentKeys[i]; editorLayoutLog.info(' - considering component "$componentKey"'); if (componentKey.currentState is! DocumentComponent) { editorLayoutLog.info(' - found unknown component: ${componentKey.currentState}'); @@ -356,7 +357,7 @@ class _SingleColumnDocumentLayoutState extends State // Because we're iterating through components from top to bottom, the // first intersecting component that we find must be the top node of // the selected area. - topNodeId = _nodeIdsToComponentKeys.entries.firstWhere((element) => element.value == componentKey).key; + topNodeId = _componentKeysToNodeIds[componentKey]; topNodeBasePosition = _getNodePositionForComponentOffset(component, componentBaseOffset); topNodeExtentPosition = _getNodePositionForComponentOffset(component, componentExtentOffset); } @@ -364,7 +365,7 @@ class _SingleColumnDocumentLayoutState extends State // intersection that we find. This way, when the iteration ends, // the last bottom node that we assigned must be the actual bottom // node within the selected area. - bottomNodeId = _nodeIdsToComponentKeys.entries.firstWhere((element) => element.value == componentKey).key; + bottomNodeId = _componentKeysToNodeIds[componentKey]; bottomNodeBasePosition = _getNodePositionForComponentOffset(component, componentBaseOffset); bottomNodeExtentPosition = _getNodePositionForComponentOffset(component, componentExtentOffset); } else if (topNodeId != null) { @@ -519,7 +520,7 @@ class _SingleColumnDocumentLayoutState extends State final component = componentKey.currentState as DocumentComponent; return DocumentPosition( - nodeId: _nodeIdsToComponentKeys.entries.firstWhere((element) => element.value == componentKey).key, + nodeId: _componentKeysToNodeIds[componentKey]!, nodePosition: component.getBeginningPosition(), ); } @@ -534,7 +535,7 @@ class _SingleColumnDocumentLayoutState extends State final component = componentKey.currentState as DocumentComponent; return DocumentPosition( - nodeId: _nodeIdsToComponentKeys.entries.firstWhere((element) => element.value == componentKey).key, + nodeId: _componentKeysToNodeIds[componentKey]!, nodePosition: component.getEndPosition(), ); } @@ -619,7 +620,7 @@ class _SingleColumnDocumentLayoutState extends State if (component.isVisualSelectionSupported()) { nodePosition = component.getEndPosition(); - nodeId = _nodeIdsToComponentKeys.entries.firstWhere((element) => element.value == componentKey).key; + nodeId = _componentKeysToNodeIds[componentKey]; break; } } @@ -652,6 +653,7 @@ class _SingleColumnDocumentLayoutState extends State final docComponents = []; final newComponentKeys = {}; + final newNodeIds = {}; _topToBottomComponentKeys.clear(); final viewModel = widget.presenter.viewModel; @@ -661,6 +663,7 @@ class _SingleColumnDocumentLayoutState extends State newComponentKeyMap: newComponentKeys, nodeId: componentViewModel.nodeId, ); + newNodeIds[componentKey] = componentViewModel.nodeId; editorLayoutLog.finer('Node -> Key: ${componentViewModel.nodeId} -> $componentKey'); _topToBottomComponentKeys.add(componentKey); @@ -687,6 +690,10 @@ class _SingleColumnDocumentLayoutState extends State ..clear() ..addAll(newComponentKeys); + _componentKeysToNodeIds + ..clear() + ..addAll(newNodeIds); + editorLayoutLog.finer(' - keys -> IDs after building all components:'); _nodeIdsToComponentKeys.forEach((key, value) { editorLayoutLog.finer(' - $key: $value'); @@ -712,6 +719,53 @@ class _SingleColumnDocumentLayoutState extends State } return newComponentKeyMap[nodeId]!; } + + int _findComponentIndexAtOffset(double dy) { + return _binarySearchComponentIndexAtOffset(dy, 0, _topToBottomComponentKeys.length); + } + + int _binarySearchComponentIndexAtOffset(double dy, int minIndex, int maxIndex) { + if (minIndex > maxIndex) { + return -1; + } + + final midleIndex = ((minIndex + maxIndex) / 2).floor(); + final componentBounds = _getComponentBoundsByIndex(midleIndex); + + if (dy >= componentBounds.top && dy <= componentBounds.bottom) { + return midleIndex; + } + + if (dy > componentBounds.bottom) { + if (midleIndex + 1 < _topToBottomComponentKeys.length) { + // Check the gap between two components. + final nextComponentBounds = _getComponentBoundsByIndex(midleIndex + 1); + if (dy > componentBounds.bottom && dy < nextComponentBounds.top) { + return midleIndex; + } + } + return _binarySearchComponentIndexAtOffset(dy, midleIndex + 1, maxIndex); + } else { + if (midleIndex - 1 >= 0) { + // Check the gap between two components. + final previousComponentBounds = _getComponentBoundsByIndex(midleIndex - 1); + if (dy < componentBounds.top && dy > previousComponentBounds.bottom) { + return midleIndex; + } + } + return _binarySearchComponentIndexAtOffset(dy, minIndex, midleIndex - 1); + } + } + + /// Gets the component bounds of the component at [componentIndex] from top to bottom order. + Rect _getComponentBoundsByIndex(int componentIndex) { + final componentKey = _topToBottomComponentKeys[componentIndex]; + final component = componentKey.currentState as DocumentComponent; + + final componentBox = component.context.findRenderObject() as RenderBox; + final contentOffset = componentBox.localToGlobal(Offset.zero, ancestor: context.findRenderObject()); + return contentOffset & componentBox.size; + } } class _PresenterComponentBuilder extends StatefulWidget { From d9660d7899f622a5e37cb72c8aa0295f75c3fa8a Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Sat, 29 Oct 2022 12:36:39 -0300 Subject: [PATCH 06/15] Add Tests --- .../super_editor/supereditor_style_test.dart | 88 ++++++++++++++++++- 1 file changed, 86 insertions(+), 2 deletions(-) diff --git a/super_editor/test/super_editor/supereditor_style_test.dart b/super_editor/test/super_editor/supereditor_style_test.dart index 68ed7cfbb3..27946302c7 100644 --- a/super_editor/test/super_editor/supereditor_style_test.dart +++ b/super_editor/test/super_editor/supereditor_style_test.dart @@ -1,6 +1,9 @@ -import 'package:flutter/cupertino.dart'; +import 'dart:math'; + +import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:super_editor/src/default_editor/attributions.dart'; +import 'package:flutter_test_robots/flutter_test_robots.dart'; +import 'package:super_editor/super_editor.dart'; import 'package:super_editor/super_editor_test.dart'; import 'package:super_text_layout/super_text_layout.dart'; @@ -28,6 +31,58 @@ void main() { FontStyle.italic, ); }); + + testWidgetsOnArbitraryDesktop('changes visual text style when style rule changes', (tester) async { + final testContext = await tester // + .createDocument() + .withTwoEmptyParagraphs() + .useStylesheet(_stylesheet) + .pump(); + + final doc = testContext.editContext.editor.document; + + final firstParagraphId = doc.nodes[0].id; + final secondParagraphId = doc.nodes[1].id; + + // Ensure the rule for paragraph is applied. + expect(SuperEditorInspector.findParagraphStyle(firstParagraphId)!.color, Colors.red); + + // Remove the second paragraph. + testContext.editContext.editor.executeCommand( + DeleteNodeCommand(nodeId: secondParagraphId), + ); + await tester.pump(); + + // The first paragraph is now the only paragraph in the document. + // Therefore, the rule for "last paragraph" should be applied. + expect(SuperEditorInspector.findParagraphStyle(firstParagraphId)!.color, Colors.blue); + }); + + testWidgetsOnArbitraryDesktop('rebuilds only changed nodes', (tester) async { + int componentChangedCount = 0; + + await tester + .createDocument() // + .withLongTextContent() + .pump(); + + await tester.placeCaretInParagraph(SuperEditorInspector.findDocument()!.nodes.last.id, 0); + + final presenter = tester.state(find.byType(SuperEditor)).presenter; + presenter.addChangeListener(SingleColumnLayoutPresenterChangeListener( + onViewModelChange: ({required addedComponents, required changedComponents, required removedComponents}) { + // The listener is called two times. The first one for the text change, + // and the second one for the selection change. + componentChangedCount = max(componentChangedCount, changedComponents.length); + }, + )); + + // Type text, changing only one node. + await tester.typeKeyboardText('a'); + + // Ensure only the changed component was marked as dirty. + expect(componentChangedCount, 1); + }); }); } @@ -38,3 +93,32 @@ InlineSpan _findSpanAtOffset( final superTextWithSelection = tester.widget(find.byType(SuperTextWithSelection)); return superTextWithSelection.richText.getSpanForPosition(TextPosition(offset: offset))!; } + +final _stylesheet = Stylesheet( + inlineTextStyler: inlineTextStyler, + rules: [ + StyleRule( + const BlockSelector("paragraph"), + (doc, docNode) { + return { + "textStyle": const TextStyle( + color: Colors.red, + ), + }; + }, + ), + StyleRule( + const BlockSelector("paragraph").last(), + (doc, docNode) { + return { + "textStyle": const TextStyle( + color: Colors.blue, + ) + }; + }, + ), + ], +); +TextStyle inlineTextStyler(Set attributions, TextStyle base) { + return base; +} From f5a76ca7de2f8e79e3e3ed8bd650b1d2ec5f076c Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Sat, 29 Oct 2022 12:59:13 -0300 Subject: [PATCH 07/15] File missing from last commit --- super_editor/lib/src/default_editor/super_editor.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/super_editor/lib/src/default_editor/super_editor.dart b/super_editor/lib/src/default_editor/super_editor.dart index 95c7175828..451841f8cf 100644 --- a/super_editor/lib/src/default_editor/super_editor.dart +++ b/super_editor/lib/src/default_editor/super_editor.dart @@ -297,6 +297,9 @@ class SuperEditorState extends State { late SoftwareKeyboardHandler _softwareKeyboardHandler; final _floatingCursorController = FloatingCursorController(); + @visibleForTesting + SingleColumnLayoutPresenter get presenter => _docLayoutPresenter!; + @override void initState() { super.initState(); From 1ba252667a21e2ef16eea5efaffa736fa56ff3cc Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Mon, 31 Oct 2022 18:50:18 -0300 Subject: [PATCH 08/15] Add dart doc --- .../default_editor/layout_single_column/_layout.dart | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/super_editor/lib/src/default_editor/layout_single_column/_layout.dart b/super_editor/lib/src/default_editor/layout_single_column/_layout.dart index c71cdc0e5f..1a88671aa2 100644 --- a/super_editor/lib/src/default_editor/layout_single_column/_layout.dart +++ b/super_editor/lib/src/default_editor/layout_single_column/_layout.dart @@ -317,8 +317,8 @@ class _SingleColumnDocumentLayoutState extends State dynamic bottomNodeBasePosition; dynamic bottomNodeExtentPosition; - final startingOffset = min(baseOffset.dy, extentOffset.dy); - final startIndex = max(_findComponentIndexAtOffset(startingOffset), 0); + final startOffset = min(baseOffset.dy, extentOffset.dy); + final startIndex = max(_findComponentIndexAtOffset(startOffset), 0); for (int i = startIndex; i < _topToBottomComponentKeys.length; i++) { final componentKey = _topToBottomComponentKeys[i]; @@ -720,10 +720,17 @@ class _SingleColumnDocumentLayoutState extends State return newComponentKeyMap[nodeId]!; } + /// Finds the component whose vertical bounds contains the offset [dy]. + /// + /// Returns the index of the component, from top to bottom order. int _findComponentIndexAtOffset(double dy) { return _binarySearchComponentIndexAtOffset(dy, 0, _topToBottomComponentKeys.length); } + /// Performs a binary search starting from [minIndex] to [maxIndex] to find + /// a component whose bounds contains the offset [dy]. + /// + /// Returns the index of the component, from top to bottom order. int _binarySearchComponentIndexAtOffset(double dy, int minIndex, int maxIndex) { if (minIndex > maxIndex) { return -1; From 1268ae641ad0179ddbc7bef7f041acba5cdc8bf3 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Mon, 31 Oct 2022 20:55:26 -0300 Subject: [PATCH 09/15] PR updates --- .../lib/src/core/document_editor.dart | 31 +++++++++---- .../layout_single_column/_layout.dart | 46 +++++++++++-------- .../test/src/default_editor/text_test.dart | 15 ++---- .../super_editor/supereditor_style_test.dart | 12 +++-- 4 files changed, 60 insertions(+), 44 deletions(-) diff --git a/super_editor/lib/src/core/document_editor.dart b/super_editor/lib/src/core/document_editor.dart index 5e233c9048..d4270c7503 100644 --- a/super_editor/lib/src/core/document_editor.dart +++ b/super_editor/lib/src/core/document_editor.dart @@ -228,7 +228,7 @@ class MutableDocument with ChangeNotifier implements Document { node.addListener(_forwardNodeChange); // The node list changed, we need to update the map to consider the new indices. - _updateNodeIdMappings(); + _refreshNodeIdCaches(); notifyListeners(); } @@ -244,7 +244,7 @@ class MutableDocument with ChangeNotifier implements Document { newNode.addListener(_forwardNodeChange); // The node list changed, we need to update the map to consider the new indices. - _updateNodeIdMappings(); + _refreshNodeIdCaches(); notifyListeners(); } @@ -260,12 +260,23 @@ class MutableDocument with ChangeNotifier implements Document { newNode.addListener(_forwardNodeChange); // The node list changed, we need to update the map to consider the new indices. - _updateNodeIdMappings(); + _refreshNodeIdCaches(); notifyListeners(); } } + /// Inserts [node] at the end of the document. + void insertNode(DocumentNode node) { + _nodes.insert(_nodes.length, node); + node.addListener(_forwardNodeChange); + + // The node list changed, we need to update the map to consider the new indices. + _refreshNodeIdCaches(); + + notifyListeners(); + } + /// Deletes the node at the given [index]. void deleteNodeAt(int index) { if (index >= 0 && index < _nodes.length) { @@ -273,7 +284,7 @@ class MutableDocument with ChangeNotifier implements Document { removedNode.removeListener(_forwardNodeChange); // The node list changed, we need to update the map to consider the new indices. - _updateNodeIdMappings(); + _refreshNodeIdCaches(); notifyListeners(); } else { @@ -290,7 +301,7 @@ class MutableDocument with ChangeNotifier implements Document { isRemoved = _nodes.remove(node); // The node list changed, we need to update the map to consider the new indices. - _updateNodeIdMappings(); + _refreshNodeIdCaches(); notifyListeners(); @@ -310,8 +321,8 @@ class MutableDocument with ChangeNotifier implements Document { if (_nodes.remove(node)) { _nodes.insert(targetIndex, node); - // The node list changed, we need to update the map to consider the new indices. - _updateNodeIdMappings(); + // An existing node's index changed. Update our Node ID -> Node Index cache. + _refreshNodeIdCaches(); notifyListeners(); } @@ -331,8 +342,8 @@ class MutableDocument with ChangeNotifier implements Document { newNode.addListener(_forwardNodeChange); _nodes.insert(index, newNode); - // The node list changed, we need to update the map to consider the new indices. - _updateNodeIdMappings(); + // An existing node's index changed. Update our Node ID -> Node Index cache. + _refreshNodeIdCaches(); notifyListeners(); } else { @@ -369,7 +380,7 @@ class MutableDocument with ChangeNotifier implements Document { /// Updates all the maps which use the node id as the key. /// /// All the maps are cleared and re-populated. - void _updateNodeIdMappings() { + void _refreshNodeIdCaches() { _nodeIndicesById.clear(); _nodesById.clear(); for (int i = 0; i < _nodes.length; i++) { diff --git a/super_editor/lib/src/default_editor/layout_single_column/_layout.dart b/super_editor/lib/src/default_editor/layout_single_column/_layout.dart index 1a88671aa2..e4e68cda8d 100644 --- a/super_editor/lib/src/default_editor/layout_single_column/_layout.dart +++ b/super_editor/lib/src/default_editor/layout_single_column/_layout.dart @@ -317,10 +317,12 @@ class _SingleColumnDocumentLayoutState extends State dynamic bottomNodeBasePosition; dynamic bottomNodeExtentPosition; - final startOffset = min(baseOffset.dy, extentOffset.dy); - final startIndex = max(_findComponentIndexAtOffset(startOffset), 0); - - for (int i = startIndex; i < _topToBottomComponentKeys.length; i++) { + // Find the top and bottom nodes in the selection region. We do this by finding the component + // at the top of the selection, then we iterate down the document until we find the bottom + // component in the selection region. We obtain the document nodes from the components. + final selectionRegionTopOffset = min(baseOffset.dy, extentOffset.dy); + final componentSearchStartIndex = max(_findComponentIndexAtOffset(selectionRegionTopOffset), 0); + for (int i = componentSearchStartIndex; i < _topToBottomComponentKeys.length; i++) { final componentKey = _topToBottomComponentKeys[i]; editorLayoutLog.info(' - considering component "$componentKey"'); if (componentKey.currentState is! DocumentComponent) { @@ -724,7 +726,10 @@ class _SingleColumnDocumentLayoutState extends State /// /// Returns the index of the component, from top to bottom order. int _findComponentIndexAtOffset(double dy) { - return _binarySearchComponentIndexAtOffset(dy, 0, _topToBottomComponentKeys.length); + if (_topToBottomComponentKeys.isEmpty) { + return -1; + } + return _binarySearchComponentIndexAtOffset(dy, 0, _topToBottomComponentKeys.length - 1); } /// Performs a binary search starting from [minIndex] to [maxIndex] to find @@ -736,31 +741,34 @@ class _SingleColumnDocumentLayoutState extends State return -1; } - final midleIndex = ((minIndex + maxIndex) / 2).floor(); - final componentBounds = _getComponentBoundsByIndex(midleIndex); + final middleIndex = ((minIndex + maxIndex) / 2).floor(); + final componentBounds = _getComponentBoundsByIndex(middleIndex); - if (dy >= componentBounds.top && dy <= componentBounds.bottom) { - return midleIndex; + if (componentBounds.top <= dy && dy <= componentBounds.bottom) { + // The component in the middle of the search region is the one we're looking for. Return its index. + return middleIndex; } if (dy > componentBounds.bottom) { - if (midleIndex + 1 < _topToBottomComponentKeys.length) { + if (middleIndex + 1 < _topToBottomComponentKeys.length) { // Check the gap between two components. - final nextComponentBounds = _getComponentBoundsByIndex(midleIndex + 1); - if (dy > componentBounds.bottom && dy < nextComponentBounds.top) { - return midleIndex; + final nextComponentBounds = _getComponentBoundsByIndex(middleIndex + 1); + if (componentBounds.bottom < dy && dy < nextComponentBounds.top) { + // The component we're looking for is somewhere in the bottom half of the current search region. + return middleIndex; } } - return _binarySearchComponentIndexAtOffset(dy, midleIndex + 1, maxIndex); + return _binarySearchComponentIndexAtOffset(dy, middleIndex + 1, maxIndex); } else { - if (midleIndex - 1 >= 0) { + if (middleIndex - 1 >= 0) { // Check the gap between two components. - final previousComponentBounds = _getComponentBoundsByIndex(midleIndex - 1); - if (dy < componentBounds.top && dy > previousComponentBounds.bottom) { - return midleIndex; + final previousComponentBounds = _getComponentBoundsByIndex(middleIndex - 1); + if (previousComponentBounds.bottom < dy && dy < componentBounds.top) { + // The component we're looking for is somewhere in the top half of the current search region. + return middleIndex; } } - return _binarySearchComponentIndexAtOffset(dy, minIndex, midleIndex - 1); + return _binarySearchComponentIndexAtOffset(dy, minIndex, middleIndex - 1); } } diff --git a/super_editor/test/src/default_editor/text_test.dart b/super_editor/test/src/default_editor/text_test.dart index 9f1ba6c54f..56f232c027 100644 --- a/super_editor/test/src/default_editor/text_test.dart +++ b/super_editor/test/src/default_editor/text_test.dart @@ -107,8 +107,7 @@ void main() { final editContext = _createEditContext(); // Add a paragraph to the document. - (editContext.editor.document as MutableDocument).insertNodeAt( - 0, + (editContext.editor.document as MutableDocument).insertNode( ParagraphNode( id: 'paragraph', text: AttributedText(text: 'This is some text'), @@ -146,8 +145,7 @@ void main() { final editContext = _createEditContext(); // Add a non-text node to the document. - (editContext.editor.document as MutableDocument).insertNodeAt( - 0, + (editContext.editor.document as MutableDocument).insertNode( HorizontalRuleNode(id: 'horizontal_rule'), ); @@ -178,8 +176,7 @@ void main() { final editContext = _createEditContext(); // Add a paragraph to the document. - (editContext.editor.document as MutableDocument).insertNodeAt( - 0, + (editContext.editor.document as MutableDocument).insertNode( ParagraphNode( id: 'paragraph', text: AttributedText(text: 'This is some text'), @@ -230,8 +227,7 @@ void main() { final editContext = _createEditContext(); // Add a paragraph to the document. - (editContext.editor.document as MutableDocument).insertNodeAt( - 0, + (editContext.editor.document as MutableDocument).insertNode( ParagraphNode( id: 'paragraph', text: AttributedText(text: 'This is some text'), @@ -270,8 +266,7 @@ void main() { final editContext = _createEditContext(); // Add a paragraph to the document. - (editContext.editor.document as MutableDocument).insertNodeAt( - 0, + (editContext.editor.document as MutableDocument).insertNode( ParagraphNode( id: 'paragraph', text: AttributedText(text: 'This is some text'), diff --git a/super_editor/test/super_editor/supereditor_style_test.dart b/super_editor/test/super_editor/supereditor_style_test.dart index 27946302c7..a56c21e88f 100644 --- a/super_editor/test/super_editor/supereditor_style_test.dart +++ b/super_editor/test/super_editor/supereditor_style_test.dart @@ -1,5 +1,3 @@ -import 'dart:math'; - import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test_robots/flutter_test_robots.dart'; @@ -71,9 +69,13 @@ void main() { final presenter = tester.state(find.byType(SuperEditor)).presenter; presenter.addChangeListener(SingleColumnLayoutPresenterChangeListener( onViewModelChange: ({required addedComponents, required changedComponents, required removedComponents}) { - // The listener is called two times. The first one for the text change, - // and the second one for the selection change. - componentChangedCount = max(componentChangedCount, changedComponents.length); + if (componentChangedCount != 0) { + // The listener is called two times. The first one for the text change, which is the one + // we care about, and the second one for the selection change. + // Return early to avoid overriding the value. + return; + } + componentChangedCount = changedComponents.length; }, )); From 423ae1d4f0aa105b6d43549b1d0aa8b45673fa58 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Mon, 31 Oct 2022 21:46:43 -0300 Subject: [PATCH 10/15] PR updates --- .../src/default_editor/layout_single_column/_layout.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/super_editor/lib/src/default_editor/layout_single_column/_layout.dart b/super_editor/lib/src/default_editor/layout_single_column/_layout.dart index e4e68cda8d..1d048df53e 100644 --- a/super_editor/lib/src/default_editor/layout_single_column/_layout.dart +++ b/super_editor/lib/src/default_editor/layout_single_column/_layout.dart @@ -753,7 +753,8 @@ class _SingleColumnDocumentLayoutState extends State if (middleIndex + 1 < _topToBottomComponentKeys.length) { // Check the gap between two components. final nextComponentBounds = _getComponentBoundsByIndex(middleIndex + 1); - if (componentBounds.bottom < dy && dy < nextComponentBounds.top) { + final gap = nextComponentBounds.top - componentBounds.bottom; + if (componentBounds.bottom < dy && dy < (componentBounds.bottom + gap / 2)) { // The component we're looking for is somewhere in the bottom half of the current search region. return middleIndex; } @@ -763,7 +764,8 @@ class _SingleColumnDocumentLayoutState extends State if (middleIndex - 1 >= 0) { // Check the gap between two components. final previousComponentBounds = _getComponentBoundsByIndex(middleIndex - 1); - if (previousComponentBounds.bottom < dy && dy < componentBounds.top) { + final gap = componentBounds.top - previousComponentBounds.bottom; + if ((componentBounds.top - gap / 2) < dy && dy < componentBounds.top) { // The component we're looking for is somewhere in the top half of the current search region. return middleIndex; } From 301bf93e66f607d84c5ac83b93888d864878936f Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Mon, 31 Oct 2022 22:09:17 -0300 Subject: [PATCH 11/15] Rename method --- super_editor/lib/src/core/document_editor.dart | 4 ++-- super_editor/test/src/default_editor/text_test.dart | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/super_editor/lib/src/core/document_editor.dart b/super_editor/lib/src/core/document_editor.dart index d4270c7503..08ced62988 100644 --- a/super_editor/lib/src/core/document_editor.dart +++ b/super_editor/lib/src/core/document_editor.dart @@ -266,8 +266,8 @@ class MutableDocument with ChangeNotifier implements Document { } } - /// Inserts [node] at the end of the document. - void insertNode(DocumentNode node) { + /// Adds [node] to the end of the document. + void add(DocumentNode node) { _nodes.insert(_nodes.length, node); node.addListener(_forwardNodeChange); diff --git a/super_editor/test/src/default_editor/text_test.dart b/super_editor/test/src/default_editor/text_test.dart index 56f232c027..9484ad28bb 100644 --- a/super_editor/test/src/default_editor/text_test.dart +++ b/super_editor/test/src/default_editor/text_test.dart @@ -107,7 +107,7 @@ void main() { final editContext = _createEditContext(); // Add a paragraph to the document. - (editContext.editor.document as MutableDocument).insertNode( + (editContext.editor.document as MutableDocument).add( ParagraphNode( id: 'paragraph', text: AttributedText(text: 'This is some text'), @@ -145,7 +145,7 @@ void main() { final editContext = _createEditContext(); // Add a non-text node to the document. - (editContext.editor.document as MutableDocument).insertNode( + (editContext.editor.document as MutableDocument).add( HorizontalRuleNode(id: 'horizontal_rule'), ); @@ -176,7 +176,7 @@ void main() { final editContext = _createEditContext(); // Add a paragraph to the document. - (editContext.editor.document as MutableDocument).insertNode( + (editContext.editor.document as MutableDocument).add( ParagraphNode( id: 'paragraph', text: AttributedText(text: 'This is some text'), @@ -227,7 +227,7 @@ void main() { final editContext = _createEditContext(); // Add a paragraph to the document. - (editContext.editor.document as MutableDocument).insertNode( + (editContext.editor.document as MutableDocument).add( ParagraphNode( id: 'paragraph', text: AttributedText(text: 'This is some text'), @@ -266,7 +266,7 @@ void main() { final editContext = _createEditContext(); // Add a paragraph to the document. - (editContext.editor.document as MutableDocument).insertNode( + (editContext.editor.document as MutableDocument).add( ParagraphNode( id: 'paragraph', text: AttributedText(text: 'This is some text'), From d0488fc0b4038f21d966ea95e28c14a760776528 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Tue, 1 Nov 2022 19:07:19 -0300 Subject: [PATCH 12/15] Change attribution commands --- super_editor/lib/src/default_editor/text.dart | 54 +++++++++---------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/super_editor/lib/src/default_editor/text.dart b/super_editor/lib/src/default_editor/text.dart index fd0e7f147a..3c73271f51 100644 --- a/super_editor/lib/src/default_editor/text.dart +++ b/super_editor/lib/src/default_editor/text.dart @@ -888,18 +888,16 @@ class AddTextAttributionsCommand implements EditorCommand { final range = entry.value.toSpanRange(); editorDocLog.info(' - adding attribution: $attribution. Range: $range'); - // During style phase, we check which components changed, so we only re-style those components. - // If we just add the attribution on the same instance of AttributedText, we can't detect the change, - // because comparing the same instance will always evaluate to true. - final spans = node.text.spans.copy(); - spans.addAttribution( - newAttribution: attribution, - start: range.start, - end: range.end, - ); + // Create a new AttributedText with updated attribution spans, so that the presentation system can + // see that we made a change, and re-renders the text in the document. node.text = AttributedText( text: node.text.text, - spans: spans, + spans: node.text.spans.copy() + ..addAttribution( + newAttribution: attribution, + start: range.start, + end: range.end, + ), ); } } @@ -986,18 +984,16 @@ class RemoveTextAttributionsCommand implements EditorCommand { final range = entry.value.toSpanRange(); editorDocLog.info(' - removing attribution: $attribution. Range: $range'); - // During style phase, we check which components changed, so we only re-style those components. - // If we just remove the attribution on the same instance of AttributedText, we can't detect the change, - // because comparing the same instance will always evaluate to true. - final spans = node.text.spans.copy(); - spans.removeAttribution( - attributionToRemove: attribution, - start: range.start, - end: range.end, - ); + // Create a new AttributedText with updated attribution spans, so that the presentation system can + // see that we made a change, and re-renders the text in the document. node.text = AttributedText( text: node.text.text, - spans: spans, + spans: node.text.spans.copy() + ..removeAttribution( + attributionToRemove: attribution, + start: range.start, + end: range.end, + ), ); } } @@ -1094,18 +1090,16 @@ class ToggleTextAttributionsCommand implements EditorCommand { final range = entry.value; editorDocLog.info(' - toggling attribution: $attribution. Range: $range'); - // During style phase, we check which components changed, so we only re-style those components. - // If we just toggle the attribution on the same instance of AttributedText, we can't detect the change, - // because comparing the same instance will always evaluate to true. - final spans = node.text.spans.copy(); - spans.toggleAttribution( - attribution: attribution, - start: range.start, - end: range.end, - ); + // Create a new AttributedText with updated attribution spans, so that the presentation system can + // see that we made a change, and re-renders the text in the document. node.text = AttributedText( text: node.text.text, - spans: spans, + spans: node.text.spans.copy() + ..toggleAttribution( + attribution: attribution, + start: range.start, + end: range.end, + ), ); } } From b9cc1b2fdd6167b494a5d164845e718a43cd3a6b Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Tue, 1 Nov 2022 20:06:34 -0300 Subject: [PATCH 13/15] Add equality comparison in getNodeIndex --- super_editor/lib/src/core/document_editor.dart | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/super_editor/lib/src/core/document_editor.dart b/super_editor/lib/src/core/document_editor.dart index 08ced62988..ce960b93e7 100644 --- a/super_editor/lib/src/core/document_editor.dart +++ b/super_editor/lib/src/core/document_editor.dart @@ -168,7 +168,17 @@ class MutableDocument with ChangeNotifier implements Document { @override int getNodeIndex(DocumentNode node) { - return _nodeIndicesById[node.id] ?? -1; + final index = _nodeIndicesById[node.id] ?? -1; + if (index < 0) { + return -1; + } + + if (_nodes[index] != node) { + // We found a node by id, but the contents aren't equal. + return -1; + } + + return index; } @override From 018840a8bd15a92048e474a0654f4025e5b1d82d Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Tue, 1 Nov 2022 21:32:54 -0300 Subject: [PATCH 14/15] Update comment --- super_editor/lib/src/core/document_editor.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/super_editor/lib/src/core/document_editor.dart b/super_editor/lib/src/core/document_editor.dart index ce960b93e7..2f9d057293 100644 --- a/super_editor/lib/src/core/document_editor.dart +++ b/super_editor/lib/src/core/document_editor.dart @@ -174,7 +174,7 @@ class MutableDocument with ChangeNotifier implements Document { } if (_nodes[index] != node) { - // We found a node by id, but the contents aren't equal. + // We found a node by id, but it wasn't the node we expected. Therefore, we couldn't find the requested node. return -1; } From 20ddc067fdcee2d0376a73c513ba1008703448ba Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Wed, 2 Nov 2022 20:13:09 -0300 Subject: [PATCH 15/15] Deprecate getNodeIndex --- super_editor/lib/src/core/document.dart | 1 + .../lib/src/core/document_editor.dart | 16 +-- .../lib/src/core/document_selection.dart | 22 ++-- super_editor/lib/src/core/styles.dart | 6 +- .../common_editor_operations.dart | 6 +- .../default_editor/document_input_ime.dart | 18 +-- .../default_editor/multi_node_editing.dart | 8 +- .../super_editor/mutable_document_test.dart | 116 +++++++++--------- .../src/document_to_markdown_serializer.dart | 4 +- 9 files changed, 99 insertions(+), 98 deletions(-) diff --git a/super_editor/lib/src/core/document.dart b/super_editor/lib/src/core/document.dart index 3b692f5810..8131752423 100644 --- a/super_editor/lib/src/core/document.dart +++ b/super_editor/lib/src/core/document.dart @@ -33,6 +33,7 @@ abstract class Document with ChangeNotifier { /// Returns the index of the given [node], or [-1] if the [node] /// does not exist within this [Document]. + @Deprecated("Use getNodeIndexById() instead") int getNodeIndex(DocumentNode node); /// Returns the index of the `DocumentNode` in this `Document` that diff --git a/super_editor/lib/src/core/document_editor.dart b/super_editor/lib/src/core/document_editor.dart index 2f9d057293..3696cdc595 100644 --- a/super_editor/lib/src/core/document_editor.dart +++ b/super_editor/lib/src/core/document_editor.dart @@ -167,6 +167,7 @@ class MutableDocument with ChangeNotifier implements Document { } @override + @Deprecated("Use getNodeIndexById() instead") int getNodeIndex(DocumentNode node) { final index = _nodeIndicesById[node.id] ?? -1; if (index < 0) { @@ -183,19 +184,18 @@ class MutableDocument with ChangeNotifier implements Document { @override int getNodeIndexById(String nodeId) { - final node = getNodeById(nodeId); - return node != null ? getNodeIndex(node) : -1; + return _nodeIndicesById[nodeId] ?? -1; } @override DocumentNode? getNodeBefore(DocumentNode node) { - final nodeIndex = getNodeIndex(node); + final nodeIndex = getNodeIndexById(node.id); return nodeIndex > 0 ? getNodeAt(nodeIndex - 1) : null; } @override DocumentNode? getNodeAfter(DocumentNode node) { - final nodeIndex = getNodeIndex(node); + final nodeIndex = getNodeIndexById(node.id); return nodeIndex >= 0 && nodeIndex < _nodes.length - 1 ? getNodeAt(nodeIndex + 1) : null; } @@ -217,13 +217,13 @@ class MutableDocument with ChangeNotifier implements Document { if (node1 == null) { throw Exception('No such position in document: $position1'); } - final index1 = getNodeIndex(node1); + final index1 = getNodeIndexById(node1.id); final node2 = getNode(position2); if (node2 == null) { throw Exception('No such position in document: $position2'); } - final index2 = getNodeIndex(node2); + final index2 = getNodeIndexById(node2.id); final from = min(index1, index2); final to = max(index1, index2); @@ -249,7 +249,7 @@ class MutableDocument with ChangeNotifier implements Document { required DocumentNode existingNode, required DocumentNode newNode, }) { - final nodeIndex = getNodeIndex(existingNode); + final nodeIndex = getNodeIndexById(existingNode.id); _nodes.insert(nodeIndex, newNode); newNode.addListener(_forwardNodeChange); @@ -264,7 +264,7 @@ class MutableDocument with ChangeNotifier implements Document { required DocumentNode existingNode, required DocumentNode newNode, }) { - final nodeIndex = getNodeIndex(existingNode); + final nodeIndex = getNodeIndexById(existingNode.id); if (nodeIndex >= 0 && nodeIndex < _nodes.length) { _nodes.insert(nodeIndex + 1, newNode); newNode.addListener(_forwardNodeChange); diff --git a/super_editor/lib/src/core/document_selection.dart b/super_editor/lib/src/core/document_selection.dart index d7cc9c10f9..336a928c05 100644 --- a/super_editor/lib/src/core/document_selection.dart +++ b/super_editor/lib/src/core/document_selection.dart @@ -132,7 +132,7 @@ class DocumentSelection { ); } - return document.getNodeIndex(baseNode) < document.getNodeIndex(extentNode) + return document.getNodeIndexById(baseNode.id) < document.getNodeIndexById(extentNode.id) ? DocumentSelection.collapsed(position: base) : DocumentSelection.collapsed(position: extent); } @@ -171,7 +171,7 @@ class DocumentSelection { ); } - return document.getNodeIndex(baseNode) > document.getNodeIndex(extentNode) + return document.getNodeIndexById(baseNode.id) > document.getNodeIndexById(extentNode.id) ? DocumentSelection.collapsed(position: base) : DocumentSelection.collapsed(position: extent); } @@ -223,13 +223,13 @@ extension InspectDocumentAffinity on Document { if (baseNode == null) { throw Exception('No such position in document: $base'); } - final baseIndex = getNodeIndex(getNode(base)!); + final baseIndex = getNodeIndexById(baseNode.id); final extentNode = getNode(extent); if (extentNode == null) { throw Exception('No such position in document: $extent'); } - final extentIndex = getNodeIndex(extentNode); + final extentIndex = getNodeIndexById(extentNode.id); late TextAffinity affinity; if (extentIndex > baseIndex) { @@ -251,9 +251,9 @@ extension InspectDocumentSelection on Document { /// from upstream to downstream. List getNodesInContentOrder(DocumentSelection selection) { final upstreamPosition = selectUpstreamPosition(selection.base, selection.extent); - final upstreamIndex = getNodeIndex(getNode(upstreamPosition)!); + final upstreamIndex = getNodeIndexById(upstreamPosition.nodeId); final downstreamPosition = selectDownstreamPosition(selection.base, selection.extent); - final downstreamIndex = getNodeIndex(getNode(downstreamPosition)!); + final downstreamIndex = getNodeIndexById(downstreamPosition.nodeId); return nodes.sublist(upstreamIndex, downstreamIndex + 1); } @@ -262,9 +262,9 @@ extension InspectDocumentSelection on Document { /// appears first in the document. DocumentPosition selectUpstreamPosition(DocumentPosition docPosition1, DocumentPosition docPosition2) { final docPosition1Node = getNodeById(docPosition1.nodeId)!; - final docPosition1NodeIndex = getNodeIndex(docPosition1Node); + final docPosition1NodeIndex = getNodeIndexById(docPosition1Node.id); final docPosition2Node = getNodeById(docPosition2.nodeId)!; - final docPosition2NodeIndex = getNodeIndex(docPosition2Node); + final docPosition2NodeIndex = getNodeIndexById(docPosition2Node.id); if (docPosition1NodeIndex < docPosition2NodeIndex) { return docPosition1; @@ -296,16 +296,16 @@ extension InspectDocumentSelection on Document { } final baseNode = getNodeById(selection.base.nodeId)!; - final baseNodeIndex = getNodeIndex(baseNode); + final baseNodeIndex = getNodeIndexById(baseNode.id); final extentNode = getNodeById(selection.extent.nodeId)!; - final extentNodeIndex = getNodeIndex(extentNode); + 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 = getNodeIndex(getNodeById(position.nodeId)!); + final positionNodeIndex = getNodeIndexById(position.nodeId); if (upstreamNodeIndex < positionNodeIndex && positionNodeIndex < downstreamNodeIndex) { // The given position is sandwiched between two other nodes that form diff --git a/super_editor/lib/src/core/styles.dart b/super_editor/lib/src/core/styles.dart index 729cc063dd..c6a932ef9d 100644 --- a/super_editor/lib/src/core/styles.dart +++ b/super_editor/lib/src/core/styles.dart @@ -183,7 +183,7 @@ class _FirstBlockMatcher implements _BlockMatcher { @override bool matches(Document document, DocumentNode node) { - return document.getNodeIndex(node) == 0; + return document.getNodeIndexById(node.id) == 0; } } @@ -192,7 +192,7 @@ class _LastBlockMatcher implements _BlockMatcher { @override bool matches(Document document, DocumentNode node) { - return document.getNodeIndex(node) == document.nodes.length - 1; + return document.getNodeIndexById(node.id) == document.nodes.length - 1; } } @@ -203,7 +203,7 @@ class _IndexBlockMatcher implements _BlockMatcher { @override bool matches(Document document, DocumentNode node) { - return document.getNodeIndex(node) == _index; + return document.getNodeIndexById(node.id) == _index; } } 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 ced67bbe65..49fd7fc62f 100644 --- a/super_editor/lib/src/default_editor/common_editor_operations.dart +++ b/super_editor/lib/src/default_editor/common_editor_operations.dart @@ -1110,14 +1110,14 @@ class CommonEditorOperations { if (baseNode == null) { throw Exception('Failed to _getDocumentPositionAfterDeletion because the base node no longer exists.'); } - final baseNodeIndex = document.getNodeIndex(baseNode); + 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.getNodeIndex(extentNode); + final extentNodeIndex = document.getNodeIndexById(extentNode.id); final topNodeIndex = min(baseNodeIndex, extentNodeIndex); final topNode = document.getNodeAt(topNodeIndex)!; @@ -1477,7 +1477,7 @@ class CommonEditorOperations { editorOpsLog.fine('Paragraph has an HR match'); // Insert an HR before this paragraph and then clear the // paragraph's content. - final paragraphNodeIndex = editor.document.getNodeIndex(node); + final paragraphNodeIndex = editor.document.getNodeIndexById(node.id); editor.executeCommand( EditorCommandFunction((document, transaction) { diff --git a/super_editor/lib/src/default_editor/document_input_ime.dart b/super_editor/lib/src/default_editor/document_input_ime.dart index f212d3940e..ee5b22cea9 100644 --- a/super_editor/lib/src/default_editor/document_input_ime.dart +++ b/super_editor/lib/src/default_editor/document_input_ime.dart @@ -504,7 +504,7 @@ class DocumentImeSerializer { // the IME would assume that there's no content before the current node and // therefore it wouldn't report the backspace button. final selectedNode = _doc.getNode(_selection.extent)!; - final selectedNodeIndex = _doc.getNodeIndex(selectedNode); + final selectedNodeIndex = _doc.getNodeIndexById(selectedNode.id); return selectedNodeIndex > 0 && _selection.isCollapsed && _selection.extent.nodePosition == selectedNode.beginningPosition; @@ -653,16 +653,16 @@ class DocumentImeSerializer { /// If there is no text content within the [selection], `null` is returned. DocumentSelection? _constrictToTextSelectionEndCaps(DocumentSelection selection) { final baseNode = _doc.getNodeById(selection.base.nodeId)!; - final baseNodeIndex = _doc.getNodeIndex(baseNode); + final baseNodeIndex = _doc.getNodeIndexById(baseNode.id); final extentNode = _doc.getNodeById(selection.extent.nodeId)!; - final extentNodeIndex = _doc.getNodeIndex(extentNode); + final extentNodeIndex = _doc.getNodeIndexById(extentNode.id); final startNode = baseNodeIndex <= extentNodeIndex ? baseNode : extentNode; - final startNodeIndex = _doc.getNodeIndex(startNode); + final startNodeIndex = _doc.getNodeIndexById(startNode.id); final startPosition = baseNodeIndex <= extentNodeIndex ? selection.base.nodePosition : selection.extent.nodePosition; final endNode = baseNodeIndex <= extentNodeIndex ? extentNode : baseNode; - final endNodeIndex = _doc.getNodeIndex(endNode); + final endNodeIndex = _doc.getNodeIndexById(endNode.id); final endPosition = baseNodeIndex <= extentNodeIndex ? selection.extent.nodePosition : selection.base.nodePosition; if (startNodeIndex == endNodeIndex) { @@ -737,16 +737,16 @@ class DocumentImeSerializer { /// so that the IME sends us the delete delta. String _getMinimumTextForIME(DocumentSelection selection) { final baseNode = _doc.getNodeById(selection.base.nodeId)!; - final baseNodeIndex = _doc.getNodeIndex(baseNode); + final baseNodeIndex = _doc.getNodeIndexById(baseNode.id); final extentNode = _doc.getNodeById(selection.extent.nodeId)!; - final extentNodeIndex = _doc.getNodeIndex(extentNode); + final extentNodeIndex = _doc.getNodeIndexById(extentNode.id); final selectionStartNode = baseNodeIndex <= extentNodeIndex ? baseNode : extentNode; - final selectionStartNodeIndex = _doc.getNodeIndex(selectionStartNode); + final selectionStartNodeIndex = _doc.getNodeIndexById(selectionStartNode.id); final startNodeIndex = max(selectionStartNodeIndex - 1, 0); final selectionEndNode = baseNodeIndex <= extentNodeIndex ? extentNode : baseNode; - final selectionEndNodeIndex = _doc.getNodeIndex(selectionEndNode); + final selectionEndNodeIndex = _doc.getNodeIndexById(selectionEndNode.id); final endNodeIndex = min(selectionEndNodeIndex + 1, _doc.nodes.length - 1); final buffer = StringBuffer(); 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 70594e1823..5a2483461f 100644 --- a/super_editor/lib/src/default_editor/multi_node_editing.dart +++ b/super_editor/lib/src/default_editor/multi_node_editing.dart @@ -48,7 +48,7 @@ class DeleteSelectionCommand implements EditorCommand { final startNodePosition = startNode.id == documentSelection.base.nodeId ? documentSelection.base.nodePosition : documentSelection.extent.nodePosition; - final startNodeIndex = document.getNodeIndex(startNode); + final startNodeIndex = document.getNodeIndexById(startNode.id); final endNode = document.getNode(range.end); if (endNode == null) { @@ -57,7 +57,7 @@ class DeleteSelectionCommand implements EditorCommand { final endNodePosition = startNode.id == documentSelection.base.nodeId ? documentSelection.extent.nodePosition : documentSelection.base.nodePosition; - final endNodeIndex = document.getNodeIndex(endNode); + final endNodeIndex = document.getNodeIndexById(endNode.id); _deleteNodesBetweenFirstAndLast( document: document, @@ -163,8 +163,8 @@ class DeleteSelectionCommand implements EditorCommand { required DocumentEditorTransaction transaction, }) { // Delete all nodes between the first node and the last node. - final startIndex = document.getNodeIndex(startNode); - final endIndex = document.getNodeIndex(endNode); + final startIndex = document.getNodeIndexById(startNode.id); + final endIndex = document.getNodeIndexById(endNode.id); _log.log('_deleteNodesBetweenFirstAndLast', ' - start node index: $startIndex'); _log.log('_deleteNodesBetweenFirstAndLast', ' - end node index: $endIndex'); diff --git a/super_editor/test/super_editor/mutable_document_test.dart b/super_editor/test/super_editor/mutable_document_test.dart index aa78e46647..d9a3228cd2 100644 --- a/super_editor/test/super_editor/mutable_document_test.dart +++ b/super_editor/test/super_editor/mutable_document_test.dart @@ -71,7 +71,7 @@ void main() { ); }); - group("getNodeIndex returns the correct index", () { + group("getNodeIndexById returns the correct index", () { test("when creating a document", () { final document = _createThreeParagraphDoc(); final firstNode = document.nodes[0]; @@ -79,9 +79,9 @@ void main() { final thirdNode = document.nodes[2]; // Ensure the indices are correct when creating the document. - expect(document.getNodeIndex(firstNode), 0); - expect(document.getNodeIndex(secondNode), 1); - expect(document.getNodeIndex(thirdNode), 2); + expect(document.getNodeIndexById(firstNode.id), 0); + expect(document.getNodeIndexById(secondNode.id), 1); + expect(document.getNodeIndexById(thirdNode.id), 2); }); test("when inserting a node at the beginning by index", () { @@ -97,9 +97,9 @@ void main() { document.insertNodeAt(0, thirdNode); // Ensure the indices are correct. - expect(document.getNodeIndex(thirdNode), 0); - expect(document.getNodeIndex(firstNode), 1); - expect(document.getNodeIndex(secondNode), 2); + expect(document.getNodeIndexById(thirdNode.id), 0); + expect(document.getNodeIndexById(firstNode.id), 1); + expect(document.getNodeIndexById(secondNode.id), 2); }); test("when inserting a node at the middle by index", () { @@ -115,9 +115,9 @@ void main() { document.insertNodeAt(1, thirdNode); // Ensure the indices are correct. - expect(document.getNodeIndex(firstNode), 0); - expect(document.getNodeIndex(thirdNode), 1); - expect(document.getNodeIndex(secondNode), 2); + expect(document.getNodeIndexById(firstNode.id), 0); + expect(document.getNodeIndexById(thirdNode.id), 1); + expect(document.getNodeIndexById(secondNode.id), 2); }); test("when inserting a node at the end by index", () { @@ -133,9 +133,9 @@ void main() { document.insertNodeAt(2, thirdNode); // Ensure the indices are correct. - expect(document.getNodeIndex(firstNode), 0); - expect(document.getNodeIndex(secondNode), 1); - expect(document.getNodeIndex(thirdNode), 2); + expect(document.getNodeIndexById(firstNode.id), 0); + expect(document.getNodeIndexById(secondNode.id), 1); + expect(document.getNodeIndexById(thirdNode.id), 2); }); test("when inserting a node before the first node", () { @@ -154,9 +154,9 @@ void main() { ); // Ensure the indices are correct. - expect(document.getNodeIndex(thirdNode), 0); - expect(document.getNodeIndex(firstNode), 1); - expect(document.getNodeIndex(secondNode), 2); + expect(document.getNodeIndexById(thirdNode.id), 0); + expect(document.getNodeIndexById(firstNode.id), 1); + expect(document.getNodeIndexById(secondNode.id), 2); }); test("when inserting a node before the last node", () { @@ -175,9 +175,9 @@ void main() { ); // Ensure the indices are correct. - expect(document.getNodeIndex(firstNode), 0); - expect(document.getNodeIndex(thirdNode), 1); - expect(document.getNodeIndex(secondNode), 2); + expect(document.getNodeIndexById(firstNode.id), 0); + expect(document.getNodeIndexById(thirdNode.id), 1); + expect(document.getNodeIndexById(secondNode.id), 2); }); test("when inserting a node after the first node", () { @@ -196,9 +196,9 @@ void main() { ); // Ensure the indices are correct. - expect(document.getNodeIndex(firstNode), 0); - expect(document.getNodeIndex(thirdNode), 1); - expect(document.getNodeIndex(secondNode), 2); + expect(document.getNodeIndexById(firstNode.id), 0); + expect(document.getNodeIndexById(thirdNode.id), 1); + expect(document.getNodeIndexById(secondNode.id), 2); }); test("when inserting a node after the last node", () { @@ -217,9 +217,9 @@ void main() { ); // Ensure the indices are correct. - expect(document.getNodeIndex(firstNode), 0); - expect(document.getNodeIndex(secondNode), 1); - expect(document.getNodeIndex(thirdNode), 2); + expect(document.getNodeIndexById(firstNode.id), 0); + expect(document.getNodeIndexById(secondNode.id), 1); + expect(document.getNodeIndexById(thirdNode.id), 2); }); test("when moving a node from the beginning to the middle", () { @@ -234,9 +234,9 @@ void main() { ); // Ensure the indices are correct. - expect(document.getNodeIndex(secondNode), 0); - expect(document.getNodeIndex(firstNode), 1); - expect(document.getNodeIndex(thirdNode), 2); + expect(document.getNodeIndexById(secondNode.id), 0); + expect(document.getNodeIndexById(firstNode.id), 1); + expect(document.getNodeIndexById(thirdNode.id), 2); }); test("when moving a node from the middle to the end", () { @@ -251,9 +251,9 @@ void main() { ); // Ensure the indices are correct. - expect(document.getNodeIndex(firstNode), 0); - expect(document.getNodeIndex(thirdNode), 1); - expect(document.getNodeIndex(secondNode), 2); + expect(document.getNodeIndexById(firstNode.id), 0); + expect(document.getNodeIndexById(thirdNode.id), 1); + expect(document.getNodeIndexById(secondNode.id), 2); }); test("when moving a node from the end to the middle", () { @@ -268,9 +268,9 @@ void main() { ); // Ensure the indices are correct. - expect(document.getNodeIndex(firstNode), 0); - expect(document.getNodeIndex(thirdNode), 1); - expect(document.getNodeIndex(secondNode), 2); + expect(document.getNodeIndexById(firstNode.id), 0); + expect(document.getNodeIndexById(thirdNode.id), 1); + expect(document.getNodeIndexById(secondNode.id), 2); }); test("when moving a node from the middle to the beginning", () { @@ -285,9 +285,9 @@ void main() { ); // Ensure the indices are correct. - expect(document.getNodeIndex(secondNode), 0); - expect(document.getNodeIndex(firstNode), 1); - expect(document.getNodeIndex(thirdNode), 2); + expect(document.getNodeIndexById(secondNode.id), 0); + expect(document.getNodeIndexById(firstNode.id), 1); + expect(document.getNodeIndexById(thirdNode.id), 2); }); test("when deleting a node at the beginning", () { @@ -299,9 +299,9 @@ void main() { document.deleteNode(firstNode); // Ensure the indices are correct. - expect(document.getNodeIndex(firstNode), -1); - expect(document.getNodeIndex(secondNode), 0); - expect(document.getNodeIndex(thirdNode), 1); + expect(document.getNodeIndexById(firstNode.id), -1); + expect(document.getNodeIndexById(secondNode.id), 0); + expect(document.getNodeIndexById(thirdNode.id), 1); }); test("when deleting a node at the middle", () { @@ -313,9 +313,9 @@ void main() { document.deleteNode(secondNode); // Ensure the indices are correct. - expect(document.getNodeIndex(secondNode), -1); - expect(document.getNodeIndex(firstNode), 0); - expect(document.getNodeIndex(thirdNode), 1); + expect(document.getNodeIndexById(secondNode.id), -1); + expect(document.getNodeIndexById(firstNode.id), 0); + expect(document.getNodeIndexById(thirdNode.id), 1); }); test("when deleting a node at the end", () { @@ -327,9 +327,9 @@ void main() { document.deleteNode(thirdNode); // Ensure the indices are correct. - expect(document.getNodeIndex(thirdNode), -1); - expect(document.getNodeIndex(firstNode), 0); - expect(document.getNodeIndex(secondNode), 1); + expect(document.getNodeIndexById(thirdNode.id), -1); + expect(document.getNodeIndexById(firstNode.id), 0); + expect(document.getNodeIndexById(secondNode.id), 1); }); test("when replacing a node at the beginning", () { @@ -349,10 +349,10 @@ void main() { ); // Ensure the indices are correct. - expect(document.getNodeIndex(firstNode), -1); - expect(document.getNodeIndex(fourthNode), 0); - expect(document.getNodeIndex(secondNode), 1); - expect(document.getNodeIndex(thirdNode), 2); + expect(document.getNodeIndexById(firstNode.id), -1); + expect(document.getNodeIndexById(fourthNode.id), 0); + expect(document.getNodeIndexById(secondNode.id), 1); + expect(document.getNodeIndexById(thirdNode.id), 2); }); test("when replacing a node at the middle", () { @@ -372,10 +372,10 @@ void main() { ); // Ensure the indices are correct. - expect(document.getNodeIndex(secondNode), -1); - expect(document.getNodeIndex(firstNode), 0); - expect(document.getNodeIndex(fourthNode), 1); - expect(document.getNodeIndex(thirdNode), 2); + expect(document.getNodeIndexById(secondNode.id), -1); + expect(document.getNodeIndexById(firstNode.id), 0); + expect(document.getNodeIndexById(fourthNode.id), 1); + expect(document.getNodeIndexById(thirdNode.id), 2); }); test("when replacing a node at the end", () { @@ -395,10 +395,10 @@ void main() { ); // Ensure the indices are correct. - expect(document.getNodeIndex(thirdNode), -1); - expect(document.getNodeIndex(firstNode), 0); - expect(document.getNodeIndex(secondNode), 1); - expect(document.getNodeIndex(fourthNode), 2); + expect(document.getNodeIndexById(thirdNode.id), -1); + expect(document.getNodeIndexById(firstNode.id), 0); + expect(document.getNodeIndexById(secondNode.id), 1); + expect(document.getNodeIndexById(fourthNode.id), 2); }); }); }); diff --git a/super_editor_markdown/lib/src/document_to_markdown_serializer.dart b/super_editor_markdown/lib/src/document_to_markdown_serializer.dart index 6382cd3dba..8ab1fa0ab1 100644 --- a/super_editor_markdown/lib/src/document_to_markdown_serializer.dart +++ b/super_editor_markdown/lib/src/document_to_markdown_serializer.dart @@ -113,7 +113,7 @@ class ListItemNodeSerializer extends NodeTypedDocumentNodeMarkdownSerializer