Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SuperEditor] Add more specific SelectionChangeTypes (Resolves #2367) #2589

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[SuperEditor] Add more specific SelectionChangeTypes (Resolves #2367) #2589

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce9234d
[SuperEditor] Add more specific SelectionChangeTypes (Resolves #2367)
angelosilvestre Feb 23, 2025
2202f67
Comment updates
angelosilvestre Mar 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions super_editor/lib/src/core/document_composer.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,8 @@ class PushCaretRequest extends ChangeSelectionRequest {
PushCaretRequest(
DocumentPosition newPosition,
this.direction,
) : super(DocumentSelection.collapsed(position: newPosition), SelectionChangeType.pushCaret,
SelectionReason.userInteraction);
SelectionChangeType changeType,
) : super(DocumentSelection.collapsed(position: newPosition), changeType, SelectionReason.userInteraction);

final TextAffinity direction;

Expand Down Expand Up @@ -483,12 +483,33 @@ enum SelectionChangeType {
/// dragging with the mouse.
placeExtent,

/// Place the caret based on a desire to move the previous caret position upstream or downstream.
pushCaret,
/// Place the caret based on a desire to move the previous caret position downstream.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I/we made these descriptions too verbose to begin with. There are so many words that it actually confuses the meaning.

Let's go with: "Move the caret one unit downstream."

Please attempt to adjust the other descriptions to have similar concision for the sake of clarity.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

pushCaretDownstream,

/// Expand/contract a selection by pushing the extent upstream or downstream, such as by pressing
/// Place the caret based on a desire to move the previous caret position upstream.
pushCaretUpstream,

/// Place the caret based on a desire to move the previous caret position down.
pushCaretDown,

/// Place the caret based on a desire to move the previous caret position up.
pushCaretUp,

/// Expand/contract a selection by pushing the extent downstream, such as by pressing
/// SHIFT + RIGHT ARROW.
pushExtentDownstream,

/// Expand/contract a selection by pushing the extent upstream, such as by pressing
/// SHIFT + LEFT ARROW.
pushExtent,
pushExtentUpstream,

/// Expand/contract a selection by pushing the extent down, such as by pressing
/// SHIFT + DOWN ARROW.
pushExtentDown,

/// Expand/contract a selection by pushing the extent up, such as by pressing
/// SHIFT + UP ARROW.
pushExtentUp,

/// Expand a caret to an expanded selection, or move the base or extent of an already expanded selection.
expandSelection,
Expand Down
40 changes: 30 additions & 10 deletions super_editor/lib/src/default_editor/common_editor_operations.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ class CommonEditorOperations {
composer.selection!.expandTo(
newExtent,
),
SelectionChangeType.pushExtent,
SelectionChangeType.pushExtentUpstream,
SelectionReason.userInteraction,
),
]);
Expand All @@ -357,7 +357,7 @@ class CommonEditorOperations {
DocumentSelection.collapsed(
position: newExtent,
),
SelectionChangeType.pushCaret,
SelectionChangeType.pushCaretUpstream,
SelectionReason.userInteraction,
),
]);
Expand Down Expand Up @@ -454,7 +454,7 @@ class CommonEditorOperations {
composer.selection!.expandTo(
newExtent,
),
SelectionChangeType.pushExtent,
SelectionChangeType.pushExtentDownstream,
SelectionReason.userInteraction,
),
]);
Expand All @@ -465,7 +465,7 @@ class CommonEditorOperations {
DocumentSelection.collapsed(
position: newExtent,
),
SelectionChangeType.pushCaret,
SelectionChangeType.pushCaretDownstream,
SelectionReason.userInteraction,
),
]);
Expand Down Expand Up @@ -513,6 +513,8 @@ class CommonEditorOperations {

String newExtentNodeId = nodeId;
NodePosition? newExtentNodePosition = extentComponent.movePositionUp(currentExtent.nodePosition);
SelectionChangeType selectionChangeType =
expand ? SelectionChangeType.pushExtentUp : SelectionChangeType.pushCaretUp;

if (newExtentNodePosition == null) {
// Move to next node
Expand All @@ -530,6 +532,7 @@ class CommonEditorOperations {
// We're at the top of the document. Move the cursor to the
// beginning of the current node.
newExtentNodePosition = extentComponent.getBeginningPosition();
selectionChangeType = expand ? SelectionChangeType.pushExtentUpstream : SelectionChangeType.pushCaretUpstream;
}
}

Expand All @@ -538,7 +541,11 @@ class CommonEditorOperations {
nodePosition: newExtentNodePosition,
);

_updateSelectionExtent(position: newExtent, expandSelection: expand);
_updateSelectionExtent(
position: newExtent,
expandSelection: expand,
selectionChangeType: selectionChangeType,
);

return true;
}
Expand Down Expand Up @@ -582,6 +589,8 @@ class CommonEditorOperations {

String newExtentNodeId = nodeId;
NodePosition? newExtentNodePosition = extentComponent.movePositionDown(currentExtent.nodePosition);
SelectionChangeType selectionChangeType =
expand ? SelectionChangeType.pushExtentDown : SelectionChangeType.pushCaretDown;

if (newExtentNodePosition == null) {
// Move to next node
Expand All @@ -599,6 +608,8 @@ class CommonEditorOperations {
// We're at the bottom of the document. Move the cursor to the
// end of the current node.
newExtentNodePosition = extentComponent.getEndPosition();
selectionChangeType =
expand ? SelectionChangeType.pushExtentDownstream : SelectionChangeType.pushCaretDownstream;
}
}

Expand All @@ -607,7 +618,11 @@ class CommonEditorOperations {
nodePosition: newExtentNodePosition,
);

_updateSelectionExtent(position: newExtent, expandSelection: expand);
_updateSelectionExtent(
position: newExtent,
expandSelection: expand,
selectionChangeType: selectionChangeType,
);

return true;
}
Expand Down Expand Up @@ -658,7 +673,11 @@ class CommonEditorOperations {
nodeId: newNodeId,
nodePosition: newPosition,
);
_updateSelectionExtent(position: newExtent, expandSelection: expand);
_updateSelectionExtent(
position: newExtent,
expandSelection: expand,
selectionChangeType: SelectionChangeType.placeCaret,
);

return true;
}
Expand Down Expand Up @@ -794,13 +813,14 @@ class CommonEditorOperations {
void _updateSelectionExtent({
required DocumentPosition position,
required bool expandSelection,
required SelectionChangeType selectionChangeType,
}) {
if (expandSelection) {
// Selection should be expanded.
editor.execute([
ChangeSelectionRequest(
composer.selection!.expandTo(position),
SelectionChangeType.expandSelection,
selectionChangeType,
SelectionReason.userInteraction,
),
]);
Expand All @@ -809,7 +829,7 @@ class CommonEditorOperations {
editor.execute([
ChangeSelectionRequest(
DocumentSelection.collapsed(position: position),
SelectionChangeType.collapseSelection,
selectionChangeType,
SelectionReason.userInteraction,
),
]);
Expand Down Expand Up @@ -967,7 +987,7 @@ class CommonEditorOperations {
nodePosition: nodeAfter.beginningPosition,
),
),
SelectionChangeType.pushCaret,
SelectionChangeType.pushCaretDownstream,
SelectionReason.userInteraction,
),
]);
Expand Down
5 changes: 4 additions & 1 deletion super_editor/lib/src/default_editor/composer/composer_reactions.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ class UpdateComposerTextStylesReaction extends EditReaction {

switch (lastSelectionChange.changeType) {
case SelectionChangeType.placeCaret:
case SelectionChangeType.pushCaret:
case SelectionChangeType.pushCaretDownstream:
case SelectionChangeType.pushCaretUpstream:
case SelectionChangeType.pushCaretUp:
case SelectionChangeType.pushCaretDown:
case SelectionChangeType.collapseSelection:
case SelectionChangeType.deleteContent:
_updateComposerStylesAtCaret(editContext);
Expand Down
25 changes: 24 additions & 1 deletion super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ class TextDeltasDocumentEditor {
editor.execute([
ChangeSelectionRequest(
docSelection,
docSelection.isCollapsed ? SelectionChangeType.placeCaret : SelectionChangeType.expandSelection,
_identifySelectionChangeType(selection.value, docSelection),
SelectionReason.userInteraction,
),
ChangeComposingRegionRequest(docComposingRegion),
Expand All @@ -273,6 +273,29 @@ class TextDeltasDocumentEditor {
_previousImeValue = delta.apply(_previousImeValue);
}

SelectionChangeType _identifySelectionChangeType(DocumentSelection? oldSelection, DocumentSelection newSelection) {
if (!newSelection.isCollapsed) {
// The selection is expanded.
return SelectionChangeType.expandSelection;
}

if (oldSelection != null && !oldSelection.isCollapsed) {
// The selection was expanded before, but now it's collapsed.
return SelectionChangeType.collapseSelection;
}

if (oldSelection != null) {
// The selection was collapsed before and still is. Check the direction of the caret movement.
final affinity = document.getAffinityBetween(base: oldSelection.extent, extent: newSelection.extent);
return affinity == TextAffinity.downstream
? SelectionChangeType.pushCaretDownstream
: SelectionChangeType.pushCaretUpstream;
}

// The selection is collapsed and there was no previous selection.
return SelectionChangeType.placeCaret;
}

void insert(DocumentSelection insertionSelection, String textInserted) {
editorImeLog.fine('Inserting "$textInserted" at position "$insertionSelection"');
editorImeLog
Expand Down
2 changes: 1 addition & 1 deletion super_editor/lib/src/default_editor/super_editor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,7 @@ class SuperEditorState extends State<SuperEditor> {
getDocumentLayout: () => _docLayoutKey.currentState as DocumentLayout,
selection: _composer.selectionNotifier,
setSelection: (newSelection) => editContext.editor.execute([
ChangeSelectionRequest(newSelection, SelectionChangeType.pushCaret, SelectionReason.userInteraction),
ChangeSelectionRequest(newSelection, SelectionChangeType.placeCaret, SelectionReason.userInteraction),
]),
scrollChangeSignal: _scrollChangeSignal,
dragHandleAutoScroller: _dragHandleAutoScroller,
Expand Down
2 changes: 1 addition & 1 deletion super_editor/lib/src/default_editor/tasks.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,7 @@ class SplitExistingTaskCommand extends EditCommand {
SelectionChangeEvent(
oldSelection: oldSelection,
newSelection: newSelection,
changeType: SelectionChangeType.pushCaret,
changeType: SelectionChangeType.pushCaretDownstream,
reason: SelectionReason.userInteraction,
),
ComposingRegionChangeEvent(
Expand Down
32 changes: 26 additions & 6 deletions super_editor/lib/src/default_editor/text_tokenizing/stable_tags.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1214,12 +1214,18 @@ class AdjustSelectionAroundTagReaction extends EditReaction {
// Move the caret to the nearest edge of the tag.
_moveCaretToNearestTagEdge(requestDispatcher, selectionChangeEvent, textNode.id, tagAroundCaret);
break;
case SelectionChangeType.pushCaret:
case SelectionChangeType.pushCaretDownstream:
case SelectionChangeType.pushCaretUpstream:
case SelectionChangeType.pushCaretUp:
case SelectionChangeType.pushCaretDown:
// Move the caret to the side of the tag in the direction of push motion.
_pushCaretToOppositeTagEdge(editContext, requestDispatcher, selectionChangeEvent, textNode.id, tagAroundCaret);
break;
case SelectionChangeType.placeExtent:
case SelectionChangeType.pushExtent:
case SelectionChangeType.pushExtentDownstream:
case SelectionChangeType.pushExtentUpstream:
case SelectionChangeType.pushExtentUp:
case SelectionChangeType.pushExtentDown:
case SelectionChangeType.expandSelection:
throw AssertionError(
"A collapsed selection reported a SelectionChangeType for an expanded selection: ${selectionChangeEvent.changeType}\n${selectionChangeEvent.newSelection}");
Expand Down Expand Up @@ -1267,7 +1273,10 @@ class AdjustSelectionAroundTagReaction extends EditReaction {
// Move the caret to the nearest edge of the tag.
_moveCaretToNearestTagEdge(requestDispatcher, selectionChangeEvent, extentNode.id, tagAroundCaret);
break;
case SelectionChangeType.pushExtent:
case SelectionChangeType.pushExtentDownstream:
case SelectionChangeType.pushExtentUpstream:
case SelectionChangeType.pushExtentUp:
case SelectionChangeType.pushExtentDown:
if (tagAroundCaret == null) {
return;
}
Expand Down Expand Up @@ -1299,7 +1308,10 @@ class AdjustSelectionAroundTagReaction extends EditReaction {
);
break;
case SelectionChangeType.placeCaret:
case SelectionChangeType.pushCaret:
case SelectionChangeType.pushCaretDownstream:
case SelectionChangeType.pushCaretUpstream:
case SelectionChangeType.pushCaretUp:
case SelectionChangeType.pushCaretDown:
case SelectionChangeType.collapseSelection:
throw AssertionError(
"An expanded selection reported a SelectionChangeType for a collapsed selection: ${selectionChangeEvent.changeType}\n${selectionChangeEvent.newSelection}");
Expand Down Expand Up @@ -1353,13 +1365,16 @@ class AdjustSelectionAroundTagReaction extends EditReaction {
TagAroundPosition tagAroundCaret,
) {
DocumentSelection? newSelection;
late SelectionChangeType newSelectionChangeType;

editorStableTagsLog.info("oldCaret is null. Pushing caret to end of tag.");
// The caret was placed directly in the token without a previous selection. This might
// be a user tap, or programmatic placement. Move the caret to the nearest edge of the
// token.
if ((tagAroundCaret.searchOffset - tagAroundCaret.indexedTag.startOffset).abs() <
(tagAroundCaret.searchOffset - tagAroundCaret.indexedTag.endOffset).abs()) {
// Move the caret to the start of the tag.
newSelectionChangeType = SelectionChangeType.pushCaretUpstream;
newSelection = DocumentSelection.collapsed(
position: DocumentPosition(
nodeId: textNodeId,
Expand All @@ -1368,6 +1383,7 @@ class AdjustSelectionAroundTagReaction extends EditReaction {
);
} else {
// Move the caret to the end of the tag.
newSelectionChangeType = SelectionChangeType.pushCaretDownstream;
newSelection = DocumentSelection.collapsed(
position: DocumentPosition(
nodeId: textNodeId,
Expand All @@ -1379,7 +1395,7 @@ class AdjustSelectionAroundTagReaction extends EditReaction {
requestDispatcher.execute([
ChangeSelectionRequest(
newSelection,
newSelection.isCollapsed ? SelectionChangeType.pushCaret : SelectionChangeType.expandSelection,
newSelection.isCollapsed ? newSelectionChangeType : SelectionChangeType.expandSelection,
SelectionReason.contentChange,
),
]);
Expand All @@ -1401,15 +1417,19 @@ class AdjustSelectionAroundTagReaction extends EditReaction {
extent: selectionChangeEvent.newSelection!.extent,
);

late SelectionChangeType selectionChangeType;
late int textOffset;
switch (pushDirection) {
case TextAffinity.upstream:
// Move to starting edge.
textOffset = tagAroundCaret.indexedTag.startOffset;
selectionChangeType = expand ? SelectionChangeType.pushExtentUpstream : SelectionChangeType.pushCaretUpstream;
break;
case TextAffinity.downstream:
// Move to ending edge.
textOffset = tagAroundCaret.indexedTag.endOffset;
selectionChangeType =
expand ? SelectionChangeType.pushExtentDownstream : SelectionChangeType.pushCaretDownstream;
break;
}

Expand All @@ -1435,7 +1455,7 @@ class AdjustSelectionAroundTagReaction extends EditReaction {
requestDispatcher.execute([
ChangeSelectionRequest(
newSelection,
SelectionChangeType.pushCaret,
selectionChangeType,
SelectionReason.contentChange,
),
]);
Expand Down
Loading
Loading