Separate mutators for text and selection #21612
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, TextInputModel's SetEditingState method was a 1:1 mapping of the underlying protocol used on the text input channel between the framework and the engine. This breaks it up into two methods, which allows the selection to be updated independently of the text, and avoids tying the API the the underlying protocol. This will become more important when we add additional state to support composing regions for multi-step input methods such as those used for Japanese. SetText resets the selection rather than making a best-efforts attempt to preserve it. This choice was primarily to keep the code simple and make the API easier to reason about. An alternative would have been to make a best-effort attempt to preserve the selection, potentially clamping one or both to the end of the new string. In all cases where an embedder resets the string, it is expected that they also have the selection, so can call SetSelection with an updated selection if needed.
cbracken
force-pushed
the
break-up-big-state
branch
from
844a3c1 to
136d345
Compare
cbracken
added
the
waiting for tree to go green
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Oct 6, 2020
gspencergoog
pushed a commit
to gspencergoog/engine
that referenced
this pull request
Oct 20, 2020
Previously, TextInputModel's SetEditingState method was a 1:1 mapping of the underlying protocol used on the text input channel between the framework and the engine. This breaks it up into two methods, which allows the selection to be updated independently of the text, and avoids tying the API the the underlying protocol. This will become more important when we add additional state to support composing regions for multi-step input methods such as those used for Japanese. SetText resets the selection rather than making a best-efforts attempt to preserve it. This choice was primarily to keep the code simple and make the API easier to reason about. An alternative would have been to make a best-effort attempt to preserve the selection, potentially clamping one or both to the end of the new string. In all cases where an embedder resets the string, it is expected that they also have the selection, so can call SetSelection with an updated selection if needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Previously,
TextInputModel'sSetEditingStatemethod was a 1:1 mapping of the underlying protocol used on the text input channel between the framework and the engine. This breaks it up into two methods, which allows the selection to be updated independently of the text, and avoids tying the API the the underlying protocol.This will become more important when we add additional state to support composing regions for multi-step input methods such as those used for Japanese.
SetTextresets the selection rather than making a best-efforts attempt to preserve it. This choice was primarily to keep the code simple and make the API easier to reason about. An alternative would have been to make a best-effort attempt to preserve the selection, potentially clamping one or both to the end of the new string. In all cases where an embedder resets the string, it is expected that they also have the selection, so can callSetSelectionwith an updated selection if needed.Related Issues
Tests
I added the following tests: