Make TextInputClient a mixin #104291
Make TextInputClient a mixin #104291
Conversation
There was a problem hiding this comment.
Does it make sense to provide a default implementation for void updateEditingValueWithDeltas(List<TextEditingDelta> textEditingDeltas);, since this implements TextInputClient thus has access to updateEditingValue?
There was a problem hiding this comment.
I think maybe not? Since that would need to set the TextInputClient's current text editing value which I don't believe we access to here. Or maybe i'm misunderstanding how the default implementation would look.
There was a problem hiding this comment.
There is. https://master-api.flutter.dev/flutter/services/TextInputClient/currentTextEditingValue.html
We can even deprecate DeltaTextInputClient if we move the update method to TextInputClient and provide a default implementation.
There was a problem hiding this comment.
This could open up opportunities for EditableText to use deltas since it would only require the TextInputConfiguration to set enableDeltaModel to true, maybe through a flag thats piped through TextField?. Another option could also be that EditableText mixins both TextInputClient and DeltaTextInputClient. I'm leaning towards the former since deltas seem like a core function of text input. Though the latter would avoid any breakages.
There was a problem hiding this comment.
That's exciting, I'll move forward with this PR and we can go from there in another PR. Deltas would be pretty core especially if we were using it in EditableText.
Renzo-Olivares
left a comment
There was a problem hiding this comment.
LGTM though I do wonder in what cases we would provide a default implementation since this is usually used/implemented by some type of Editable widget which extends State. And the methods in this TextInputClient sometimes act on this state like the TextEditingValue.
|
(Triage) @justinmc is this ready to merge? |
|
I think Google testing is stuck, I'll push a merge commit. Otherwise this is ready to go, thanks for the reminder. |
bdc182d to
6afd3f1
Compare
TextInputClient and DeltaTextInputClient are now mixins, which helps with breaking changes and future deltas work
This PR makes TextInputClient a mixin instead of an abstract class. It does the same for DeltaTextInputClient.
The reason for this is to discourage users from using
implementson these abstract classes, which causes a breaking change when changes are made to the abstract class. This is a common occurrence on these classes related to platform APIs.Migration guide
Users of TextInputClient and DeltaTextInputClient should mix them into their own classes via the
withkeyword, similar to how this PR changes EditableTextState to use TextInputClient viawith.✔️
If users were using TextInputClient or DeltaTextInputClient via the
implementskeyword, they will not be broken by this change, but the above approach withwithis still recommended. The advantage ofwithis thatimplementsrequires implementations of all methods, even if the mixin has one, so future additions to the mixin will be a breaking change.If any user were using TextInputClient or DeltaTextInputClient via the
extendskeyword, they would be broken, and should use thewithkeyword instead as above.❌
See also