[SuperTextField] Fix text capitalization configuration (Resolves #1617) #1619
Conversation
| // Change the brightness to rebuild the widget | ||
| // and re-attach to the IME. | ||
| brightnessNotifier.value = Brightness.light; | ||
| await tester.pumpAndSettle(); |
There was a problem hiding this comment.
Do you need to settle here?
There was a problem hiding this comment.
pump is enough. Updated.
| child: ListenableBuilder( | ||
| listenable: rebuildSignal, | ||
| builder: (context, child) { | ||
| // Explicitly avoid using const to ensure that each build |
There was a problem hiding this comment.
How about instead of the careful non-const approach, you declare two different variables with the same IME config value and you switch from one to the other?
super_editor/test/super_textfield/super_textfield_ime_test.dart
Outdated
Show resolved
Hide resolved
| @@ -35,3 +37,42 @@ enum WebPlatformOverride { | |||
| /// Configuration to run the app as if we are on web. | |||
| web, | |||
| } | |||
|
|
|||
| extension TextInputConfigurationExtensions on TextInputConfiguration { | |||
There was a problem hiding this comment.
Does TextInputConfiguration really not implement a custom ==? If not, can you file a Flutter issue to at least put this on their radar?
Let's name these extensions more specifically. We may never add another extension to them, so let's say what they really do, e.g., TextInputConfigurationEquivalency
Also, let's place these in a location that we don't export from the package. These exist for an internal detail of super_editor. I'd rather not be on the hook for maintaining these extensions, and the Flutter team might eventually implement this behavior directly.
There was a problem hiding this comment.
Filed flutter/flutter#139033
|
@matthew-carroll The test failures are the same as the previous PR. |
| readOnly == other.readOnly && | ||
| obscureText == other.obscureText && | ||
| autocorrect == other.autocorrect && | ||
| autofillConfiguration.isEquivalentTo(other.autofillConfiguration) && |
There was a problem hiding this comment.
I wanted to point one thing out that might be relevant. I checked the API for AutofillConfiguration and it seems to include the current TextEditingValue. That suggests to me that every time the user types a character, deletes a character, or moves the caret, the overall TextInputConfiguration will be seen as unequal. Do you also see it that way? If so, do we really want that configuration getting sent every time those things happen?
There was a problem hiding this comment.
As this is a configuration, I would expect that AutofillConfiguration stores only the initial TextEditingValue without updating it at every keystroke, but the docs aren't clear about that and I've never used the AutofillConfiguration before.
I don't think we want to consider the TextInputConfiguration unequal at every TextEditingValue. Should I remove it from the comparison?
There was a problem hiding this comment.
I think the safest thing is probably to remove it. Please add a comment mentioning that we don't include it in the comparison, and why we don't.
[SuperTextField] Fix text capitalization configuration. Resolves #1617
Passing a
TextInputConfigurationwith atextCapitalizationdifferent fromTextCapitalization.nonehas no effect.Two issues are involved on this:
ImeAttributedTextEditingController.updateTextInputConfigurationdoesn't take aTextCapitalization.When we added support for passing a
TextInputConfigurationtoSuperTextField, we keptupdateTextInputConfigurationtaking individual arguments instead of aTextInputConfigurationto avoid a breaking change.I suggest we make that breaking change and take a
TextInputConfigurationas argument. Otherwise, we need to take allTextInputConfigurationproperties in this method.didUpdateWidgetif we have aimeConfiguration.The reason is that we are comparing only
widget.imeConfiguration != oldWidget.imeConfiguration. This compares the references, not theimeConfigurationproperties. So, if we don't use aconstimeConfiguration, in each build we get a different instance, which causes us to re-attach to the IME.This PR changes the comparison to also compare all
TextInputConfigurationproperties, by using an extension method.