[SuperEditor] Add more specific SelectionChangeTypes (Resolves #2367) #2589
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[SuperEditor] Add more specific SelectionChangeTypes (Resolves #2367)
Currently, when moving the caret using up or down arrow, the editor generates a selection change event with the type
SelectionChangeType.collapseSelection
. This makes it difficult for reactions to determine if the selection moved up or down.Also, our
SelectionChangeType
s for pushing the caret or extent don't carry the direction of the selection change.This PR removes
SelectionChangeType.pushCaret
andSelectionChangeType.pushExtent
in favor of more specific types:SelectionChangeType.pushDownstream
SelectionChangeType.pushCaretUpstream
SelectionChangeType.pushCaretDown
SelectionChangeType.pushCaretUp
SelectionChangeType.pushExtentDownstream
SelectionChangeType.pushExtentUpstream
SelectionChangeType.pushExtentDown
SelectionChangeType.pushExtentUp
This is a breaking change, but I think it's much more safer to generate a compilation error instead of keeping
SelectionChangeType.pushCaretUp
and possibly silently failing at runtime.The upgrade should be straightforward:
Before
After:
On Android, we are always generating a selection change type of
SelectionChangeType.pushCaret
. This PR changes it toSelectionChangeType.placeCaret
, but it might be better to let that decision to the touch interactor instead.On web, things are a bit trickier. We handle selection changes by IME deltas, so we don't have information about the intent of the selection change. Should we compare the new selection, with the selection that is expected when the user presses up/down/left/right arrow to determine what is the correct selection change type?