diff --git a/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart b/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart
index 4dbcba776..121e85d7a 100644
--- a/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart
+++ b/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart
@@ -123,11 +123,19 @@ class SubmitComposingActionTagCommand extends EditCommand {
void execute(EditContext context, CommandExecutor executor) {
final document = context.document;
final composer = context.find<MutableDocumentComposer>(Editor.composerKey);
- if (composer.selection == null) {
+ final selection = composer.selection;
+
+ if (selection == null) {
+ return;
+ }
+
+ if (!selection.isCollapsed) {
+ // Action tags are composed while the user is typing. Since the
+ // selection is expanded, the user is not typing.
return;
}
- final extent = composer.selection!.extent;
+ final extent = selection.extent;
final extentPosition = extent.nodePosition;
if (extentPosition is! TextNodePosition) {
return;
@@ -135,14 +143,12 @@ class SubmitComposingActionTagCommand extends EditCommand {
final textNode = document.getNodeById(extent.nodeId) as TextNode;
- final normalizedSelection = composer.selection!.normalize(document);
- final tagAroundPosition = _findTag(
+ final tagAroundPosition = _findTagUpstream(
// TODO: deal with these tag rules in requests and commands, should the user really pass them?
tagRule: defaultActionTagRule,
nodeId: composer.selection!.extent.nodeId,
text: textNode.text,
- expansionPosition: extentPosition,
- endPosition: normalizedSelection.end.nodePosition,
+ caretPosition: extentPosition,
isTokenCandidate: (attributions) => !attributions.contains(actionTagCancelledAttribution),
);
@@ -213,32 +219,25 @@ class CancelComposingActionTagCommand extends EditCommand {
return;
}
- // Look for a composing tag at the extent, or the base.
+ if (!selection.isCollapsed) {
+ // Action tags are composed while the user is typing. Since the
+ // selection is expanded, the user is not typing.
+ return;
+ }
+
+ // Look for a composing tag at the extent.
final base = selection.base;
final extent = selection.extent;
TagAroundPosition? composingToken;
TextNode? textNode;
- final normalizedSelection = selection.normalize(document);
- if (base.nodePosition is TextNodePosition) {
- textNode = document.getNodeById(selection.base.nodeId) as TextNode;
- composingToken = _findTag(
- tagRule: _tagRule,
- nodeId: textNode.id,
- text: textNode.text,
- expansionPosition: base.nodePosition as TextNodePosition,
- endPosition: normalizedSelection.end.nodePosition,
- isTokenCandidate: (tokenAttributions) => tokenAttributions.contains(actionTagComposingAttribution),
- );
- }
- if (composingToken == null && extent.nodePosition is TextNodePosition) {
+ if (extent.nodePosition is TextNodePosition) {
textNode = document.getNodeById(selection.extent.nodeId) as TextNode;
- composingToken = _findTag(
+ composingToken = _findTagUpstream(
tagRule: _tagRule,
nodeId: textNode.id,
text: textNode.text,
- expansionPosition: base.nodePosition as TextNodePosition,
- endPosition: normalizedSelection.end.nodePosition,
+ caretPosition: base.nodePosition as TextNodePosition,
isTokenCandidate: (tokenAttributions) => tokenAttributions.contains(actionTagComposingAttribution),
);
}
@@ -293,7 +292,9 @@ class ActionTagComposingReaction extends EditReaction {
_healCancelledTags(requestDispatcher, document, changeList);
- if (composer.selection == null) {
+ if (composer.selection?.isCollapsed != true) {
+ // Action tags are composed while the user is typing. Since the
+ // selection is either null or expanded, the user is not typing.
_cancelComposingTag(requestDispatcher);
editorContext.composingActionTag.value = null;
_onUpdateComposingActionTag(null);
@@ -302,33 +303,18 @@ class ActionTagComposingReaction extends EditReaction {
final selection = composer.selection!;
- // Look for a composing tag at the extent, or the base.
- final base = selection.base;
+ // Look for a composing tag at the extent.
final extent = selection.extent;
TagAroundPosition? tagAroundPosition;
TextNode? textNode;
- final normalizedSelection = selection.normalize(document);
-
- if (base.nodePosition is TextNodePosition) {
- textNode = document.getNodeById(selection.base.nodeId) as TextNode;
- tagAroundPosition = _findTag(
- tagRule: _tagRule,
- nodeId: textNode.id,
- text: textNode.text,
- expansionPosition: base.nodePosition as TextNodePosition,
- endPosition: normalizedSelection.end.nodePosition,
- isTokenCandidate: (attributions) => !attributions.contains(actionTagCancelledAttribution),
- );
- }
- if (tagAroundPosition == null && extent.nodePosition is TextNodePosition) {
+ if (extent.nodePosition is TextNodePosition) {
textNode = document.getNodeById(selection.extent.nodeId) as TextNode;
- tagAroundPosition = _findTag(
+ tagAroundPosition = _findTagUpstream(
tagRule: _tagRule,
nodeId: textNode.id,
text: textNode.text,
- expansionPosition: extent.nodePosition as TextNodePosition,
- endPosition: normalizedSelection.end.nodePosition,
+ caretPosition: extent.nodePosition as TextNodePosition,
isTokenCandidate: (attributions) => !attributions.contains(actionTagCancelledAttribution),
);
}
@@ -467,37 +453,35 @@ class ActionTagComposingReaction extends EditReaction {
}
}
-/// Finds a tag that touches the given [expansionPosition], constaining it
-/// to not cross the [endPosition].
+/// Finds a tag that starts upstream to the [caretPosition] and ends
+/// at the [caretPosition].
///
-/// If [endPosition] is not a `TextNodePosition`, it will be ignored .
-TagAroundPosition? _findTag({
+/// For example, considering the following text, where '|' represents the caret:
+///
+/// "hello/wo|rld"
+///
+/// This method will extract "/wo" as the tag.
+TagAroundPosition? _findTagUpstream({
required TagRule tagRule,
required String nodeId,
required AttributedText text,
- required TextNodePosition expansionPosition,
- required NodePosition endPosition,
+ required TextNodePosition caretPosition,
required bool Function(Set<Attribution> tokenAttributions) isTokenCandidate,
}) {
- final rawText = text.text;
+ final rawText = text.toPlainText();
if (rawText.isEmpty) {
return null;
}
- int splitIndex = min(expansionPosition.offset, rawText.length);
+ int splitIndex = min(caretPosition.offset, rawText.length);
splitIndex = max(splitIndex, 0);
- final endOffset = endPosition is TextNodePosition ? endPosition.offset : null;
-
- // Create 2 splits of characters to navigate upstream and downstream the caret position.
- // ex: "this is a very|long string"
- // -> split around the caret into charactersBefore="this is a very" and charactersAfter="long string"
+ // Extract the text upstream to the caret.
+ // For example: "hello/wor|ld"
+ // -> extracts the text "hello/wor"
final charactersBefore = rawText.substring(0, splitIndex).characters;
final iteratorUpstream = charactersBefore.iteratorAtEnd;
- final charactersAfter = rawText.substring(splitIndex, endOffset).characters;
- final iteratorDownstream = charactersAfter.iterator;
-
if (charactersBefore.isNotEmpty && tagRule.excludedCharacters.contains(charactersBefore.last)) {
// The character where we're supposed to begin our expansion is a
// character that's not allowed in a tag. Therefore, no tag exists
@@ -521,16 +505,8 @@ TagAroundPosition? _findTag({
}
}
- // Move downstream the caret position until we find excluded character or reach the end of the text.
- while (iteratorDownstream.moveNext()) {
- final current = iteratorDownstream.current;
- if (tagRule.excludedCharacters.contains(current)) {
- break;
- }
- }
-
final tokenStartOffset = splitIndex - iteratorUpstream.stringAfterLength;
- final tokenRange = SpanRange(tokenStartOffset, splitIndex + iteratorDownstream.stringBeforeLength);
+ final tokenRange = SpanRange(tokenStartOffset, splitIndex);
final tagText = text.substringInRange(tokenRange);
if (!tagText.startsWith(tagRule.trigger)) {
@@ -548,7 +524,7 @@ TagAroundPosition? _findTag({
nodeId,
tokenStartOffset,
),
- searchOffset: expansionPosition.offset,
+ searchOffset: caretPosition.offset,
);
}
diff --git a/super_editor/test/super_editor/text_entry/tagging/action_tags_test.dart b/super_editor/test/super_editor/text_entry/tagging/action_tags_test.dart
index e1f224f4d..61d1a178d 100644
--- a/super_editor/test/super_editor/text_entry/tagging/action_tags_test.dart
+++ b/super_editor/test/super_editor/text_entry/tagging/action_tags_test.dart
@@ -197,7 +197,7 @@ void main() {
);
});
- testWidgetsOnAllPlatforms("continues when user expands the selection upstream", (tester) async {
+ testWidgetsOnAllPlatforms("does not continue when user expands the selection upstream", (tester) async {
await _pumpTestEditor(
tester,
MutableDocument(
@@ -220,6 +220,13 @@ void main() {
// Compose an action tag.
await tester.typeImeText("/header");
+ // Ensure we're composing a tag.
+ AttributedText text = SuperEditorInspector.findTextInComponent("1");
+ expect(
+ text.getAttributedRange({actionTagComposingAttribution}, 7),
+ const SpanRange(7, 13),
+ );
+
// Expand the selection to "before /heade|r|"
await tester.pressShiftLeftArrow();
expect(
@@ -236,11 +243,14 @@ void main() {
),
);
- // Ensure we're still composing
- AttributedText text = SuperEditorInspector.findTextInComponent("1");
+ // Ensure we're not composing anymore.
+ text = SuperEditorInspector.findTextInComponent("1");
expect(
- text.getAttributedRange({actionTagComposingAttribution}, 7),
- const SpanRange(7, 13),
+ text.getAttributionSpansInRange(
+ attributionFilter: (attribution) => attribution == actionTagComposingAttribution,
+ range: const SpanRange(7, 13),
+ ),
+ isEmpty,
);
// Expand the selection to "before |/header|"
@@ -251,26 +261,32 @@ void main() {
await tester.pressShiftLeftArrow();
await tester.pressShiftLeftArrow();
- // Ensure we're still composing
+ // Ensure we're still not composing.
text = SuperEditorInspector.findTextInComponent("1");
expect(
- text.getAttributedRange({actionTagComposingAttribution}, 7),
- const SpanRange(7, 13),
+ text.getAttributionSpansInRange(
+ attributionFilter: (attribution) => attribution == actionTagComposingAttribution,
+ range: const SpanRange(7, 13),
+ ),
+ isEmpty,
);
// Expand the selection to "befor|e /header|"
await tester.pressShiftLeftArrow();
await tester.pressShiftLeftArrow();
- // Ensure we're still composing
+ // Ensure we're still not composing.
text = SuperEditorInspector.findTextInComponent("1");
expect(
- text.getAttributedRange({actionTagComposingAttribution}, 7),
- const SpanRange(7, 13),
+ text.getAttributionSpansInRange(
+ attributionFilter: (attribution) => attribution == actionTagComposingAttribution,
+ range: const SpanRange(7, 13),
+ ),
+ isEmpty,
);
});
- testWidgetsOnAllPlatforms("continues when user expands the selection downstream", (tester) async {
+ testWidgetsOnAllPlatforms("does not continue when user expands the selection downstream", (tester) async {
await _pumpTestEditor(
tester,
MutableDocument(
@@ -293,6 +309,13 @@ void main() {
// Compose an action tag.
await tester.typeImeText("/header");
+ // Ensure we're composing a tag.
+ AttributedText text = SuperEditorInspector.findTextInComponent("1");
+ expect(
+ text.getAttributedRange({actionTagComposingAttribution}, 7),
+ const SpanRange(7, 13),
+ );
+
// Move the caret to "before /|header".
await tester.pressLeftArrow();
await tester.pressLeftArrow();
@@ -324,11 +347,14 @@ void main() {
),
);
- // Ensure we're still composing
- AttributedText text = SuperEditorInspector.findTextInComponent("1");
+ // Ensure we're not composing anymore.
+ text = SuperEditorInspector.findTextInComponent("1");
expect(
- text.getAttributedRange({actionTagComposingAttribution}, 7),
- const SpanRange(7, 13),
+ text.getAttributionSpansInRange(
+ attributionFilter: (attribution) => attribution == actionTagComposingAttribution,
+ range: const SpanRange(7, 13),
+ ),
+ isEmpty,
);
});
@@ -873,7 +899,7 @@ void main() {
// Ensure that the action tag was removed.
text = SuperEditorInspector.findTextInComponent("1");
- expect(text.text, "before after");
+ expect(text.toPlainText(), "before after");
expect(
text.getAttributionSpansInRange(
attributionFilter: (attribution) => attribution == actionTagComposingAttribution,
@@ -886,7 +912,47 @@ void main() {
});
group("selections >", () {
- testWidgetsOnAllPlatforms("can find tag that surrounds the extent position when the selection is expanded",
+ testWidgetsOnArbitraryDesktop('does not extract a tag when the selection is expanded', (tester) async {
+ await _pumpTestEditor(
+ tester,
+ MutableDocument(nodes: [
+ ParagraphNode(id: '1', text: AttributedText('A paragraph')),
+ // It's important that the second paragraph is longer than the first to ensure
+ // that we don't try to access a character in the first paragraph using an index
+ // from the second paragraph.
+ ParagraphNode(id: '2', text: AttributedText('Another paragraph with longer text')),
+ ]),
+ );
+
+ // Place the caret at the end of the second paragraph.
+ await tester.placeCaretInParagraph('2', 34);
+
+ // Press CMD + SHIFT + ARROW UP to expand the selection to the beginning of
+ // the document.
+ await tester.pressShiftCmdUpArrow();
+
+ // Ensure nothing in the first paragraph is attributed.
+ final firstParagraphText = SuperEditorInspector.findTextInComponent("1");
+ expect(
+ firstParagraphText.getAttributionSpansInRange(
+ attributionFilter: (attribution) => attribution == actionTagComposingAttribution,
+ range: SpanRange(0, firstParagraphText.length),
+ ),
+ isEmpty,
+ );
+
+ // Ensure nothing in the second paragraph is attributed.
+ final secondParagraphText = SuperEditorInspector.findTextInComponent("1");
+ expect(
+ secondParagraphText.getAttributionSpansInRange(
+ attributionFilter: (attribution) => attribution == actionTagComposingAttribution,
+ range: SpanRange(0, secondParagraphText.length),
+ ),
+ isEmpty,
+ );
+ });
+
+ testWidgetsOnAllPlatforms("does not extract a tag when expanding the selection from a non-text node",
(tester) async {
await _pumpTestEditor(
tester,
@@ -901,19 +967,19 @@ void main() {
await tester.pressDownArrow();
await tester.pressRightArrow();
- // Select upstream towards the cancelled action tag
- await expectLater(
- () async {
- await tester.pressShiftLeftArrow();
- await tester.pressShiftUpArrow();
- },
- returnsNormally,
- );
+ // Expand the selection to the first paragraph.
+ await tester.pressShiftLeftArrow();
+ await tester.pressShiftUpArrow();
- // If we reach the end without exception, then ActionTagComposingReaction did not blow up due to the base or extent
- // position, and type of content at those positions.
- //
- // Original bug: https://github.com/superlistapp/super_editor/pull/2201
+ // Ensure nothing in the paragraph is attributed.
+ final text = SuperEditorInspector.findTextInComponent("1");
+ expect(
+ text.getAttributionSpansInRange(
+ attributionFilter: (attribution) => attribution == actionTagComposingAttribution,
+ range: SpanRange(0, text.length),
+ ),
+ isEmpty,
+ );
});
});
}