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

[SuperTextField] Fix text capitalization configuration (Resolves #1617) #1619

Merged
merged 3 commits into from
Nov 27, 2023
Merged

[SuperTextField] Fix text capitalization configuration (Resolves #1617) #1619

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ce566d1
[SuperTextField] Fix text capitalization configuration (Resolves #1617)
angelosilvestre Nov 24, 2023
cfb2683
PR Updates
angelosilvestre Nov 26, 2023
3b0aa2f
Update isEquivalentTo
angelosilvestre Nov 27, 2023
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
41 changes: 41 additions & 0 deletions super_editor/lib/src/infrastructure/flutter/text_input_configuration.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import 'package:collection/collection.dart';
import 'package:flutter/services.dart';

extension TextInputConfigurationEquivalency on TextInputConfiguration {
/// Whether this [TextInputConfiguration] is equivalent to [other].
///
/// Two [TextInputConfiguration]s are considered to be equal
/// if all properties are equal.
bool isEquivalentTo(TextInputConfiguration other) {
return inputType == other.inputType &&
readOnly == other.readOnly &&
obscureText == other.obscureText &&
autocorrect == other.autocorrect &&
autofillConfiguration.isEquivalentTo(other.autofillConfiguration) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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?

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 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.

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.

smartDashesType == other.smartDashesType &&
smartQuotesType == other.smartQuotesType &&
enableSuggestions == other.enableSuggestions &&
enableInteractiveSelection == other.enableInteractiveSelection &&
actionLabel == other.actionLabel &&
inputAction == other.inputAction &&
textCapitalization == other.textCapitalization &&
keyboardAppearance == other.keyboardAppearance &&
enableIMEPersonalizedLearning == other.enableIMEPersonalizedLearning &&
enableDeltaModel == other.enableDeltaModel &&
const DeepCollectionEquality().equals(allowedMimeTypes, other.allowedMimeTypes);
}
}

extension AutofillConfigurationEquivalency on AutofillConfiguration {
/// Whether this [AutofillConfiguration] is equivalent to [other].
///
/// Two [AutofillConfiguration]s are considered to be equal
/// if all properties are equal.
bool isEquivalentTo(AutofillConfiguration other) {
return enabled == other.enabled &&
uniqueIdentifier == other.uniqueIdentifier &&
const DeepCollectionEquality().equals(autofillHints, other.autofillHints) &&
currentEditingValue == other.currentEditingValue &&
hintText == other.hintText;
}
}
3 changes: 3 additions & 0 deletions super_editor/lib/src/super_textfield/android/android_textfield.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:super_editor/src/infrastructure/attributed_text_styles.dart';
import 'package:super_editor/src/infrastructure/flutter/flutter_scheduler.dart';
import 'package:super_editor/src/infrastructure/flutter/text_input_configuration.dart';
import 'package:super_editor/src/infrastructure/focus.dart';
import 'package:super_editor/src/infrastructure/ime_input_owner.dart';
import 'package:super_editor/src/infrastructure/platforms/android/toolbar.dart';
Expand Down Expand Up @@ -220,13 +221,15 @@ class SuperAndroidTextFieldState extends State<SuperAndroidTextField>

if (widget.imeConfiguration != oldWidget.imeConfiguration &&
widget.imeConfiguration != null &&
(oldWidget.imeConfiguration == null || !widget.imeConfiguration!.isEquivalentTo(oldWidget.imeConfiguration!)) &&
_textEditingController.isAttachedToIme) {
_textEditingController.updateTextInputConfiguration(
textInputAction: widget.imeConfiguration!.inputAction,
textInputType: widget.imeConfiguration!.inputType,
autocorrect: widget.imeConfiguration!.autocorrect,
enableSuggestions: widget.imeConfiguration!.enableSuggestions,
keyboardAppearance: widget.imeConfiguration!.keyboardAppearance,
textCapitalization: widget.imeConfiguration!.textCapitalization,
);
}

Expand Down
3 changes: 3 additions & 0 deletions super_editor/lib/src/super_textfield/desktop/desktop_textfield.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:super_editor/src/core/document_layout.dart';
import 'package:super_editor/src/infrastructure/_logging.dart';
import 'package:super_editor/src/infrastructure/attributed_text_styles.dart';
import 'package:super_editor/src/infrastructure/flutter/flutter_scheduler.dart';
import 'package:super_editor/src/infrastructure/flutter/text_input_configuration.dart';
import 'package:super_editor/src/infrastructure/focus.dart';
import 'package:super_editor/src/infrastructure/ime_input_owner.dart';
import 'package:super_editor/src/infrastructure/keyboard.dart';
Expand Down Expand Up @@ -1134,13 +1135,15 @@ class _SuperTextFieldImeInteractorState extends State<SuperTextFieldImeInteracto

if (widget.imeConfiguration != oldWidget.imeConfiguration &&
widget.imeConfiguration != null &&
(oldWidget.imeConfiguration == null || !widget.imeConfiguration!.isEquivalentTo(oldWidget.imeConfiguration!)) &&
_textController.isAttachedToIme) {
_textController.updateTextInputConfiguration(
textInputAction: widget.imeConfiguration!.inputAction,
textInputType: widget.imeConfiguration!.inputType,
autocorrect: widget.imeConfiguration!.autocorrect,
enableSuggestions: widget.imeConfiguration!.enableSuggestions,
keyboardAppearance: widget.imeConfiguration!.keyboardAppearance,
textCapitalization: widget.imeConfiguration!.textCapitalization,
);
}
}
Expand Down
2 changes: 2 additions & 0 deletions super_editor/lib/src/super_textfield/input_method_engine/_ime_text_editing_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ class ImeAttributedTextEditingController extends AttributedTextEditingController
TextInputAction textInputAction = TextInputAction.done,
TextInputType textInputType = TextInputType.text,
Brightness keyboardAppearance = Brightness.light,
TextCapitalization textCapitalization = TextCapitalization.none,
}) {
// Change the keyboard appearance even if we are detached from the IME.
// In the next time we attach to the IME, the keyboard appearance is used.
Expand All @@ -180,6 +181,7 @@ class ImeAttributedTextEditingController extends AttributedTextEditingController
inputAction: textInputAction,
inputType: textInputType,
keyboardAppearance: keyboardAppearance,
textCapitalization: textCapitalization,
);
final inputConnection = _inputConnectionFactory?.call(this, imeConfig) ?? TextInput.attach(this, imeConfig);
inputConnection.show();
Expand Down
3 changes: 3 additions & 0 deletions super_editor/lib/src/super_textfield/ios/ios_textfield.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'package:follow_the_leader/follow_the_leader.dart';
import 'package:super_editor/src/infrastructure/_logging.dart';
import 'package:super_editor/src/infrastructure/attributed_text_styles.dart';
import 'package:super_editor/src/infrastructure/flutter/flutter_scheduler.dart';
import 'package:super_editor/src/infrastructure/flutter/text_input_configuration.dart';
import 'package:super_editor/src/infrastructure/focus.dart';
import 'package:super_editor/src/infrastructure/ime_input_owner.dart';
import 'package:super_editor/src/infrastructure/platforms/ios/toolbar.dart';
Expand Down Expand Up @@ -254,13 +255,15 @@ class SuperIOSTextFieldState extends State<SuperIOSTextField>

if (widget.imeConfiguration != oldWidget.imeConfiguration &&
widget.imeConfiguration != null &&
(oldWidget.imeConfiguration == null || !widget.imeConfiguration!.isEquivalentTo(oldWidget.imeConfiguration!)) &&
_textEditingController.isAttachedToIme) {
_textEditingController.updateTextInputConfiguration(
textInputAction: widget.imeConfiguration!.inputAction,
textInputType: widget.imeConfiguration!.inputType,
autocorrect: widget.imeConfiguration!.autocorrect,
enableSuggestions: widget.imeConfiguration!.enableSuggestions,
keyboardAppearance: widget.imeConfiguration!.keyboardAppearance,
textCapitalization: widget.imeConfiguration!.textCapitalization,
);
}

Expand Down
152 changes: 151 additions & 1 deletion super_editor/test/super_textfield/super_textfield_ime_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter_test_robots/flutter_test_robots.dart';
import 'package:flutter_test_runners/flutter_test_runners.dart';
import 'package:super_editor/src/infrastructure/platforms/mac/mac_ime.dart';
import 'package:super_editor/super_editor.dart';
import 'package:super_editor/super_editor_test.dart';

Expand Down Expand Up @@ -529,6 +528,157 @@ void main() {
});
});

testWidgetsOnAllPlatforms('updates IME configuration when it changes', (tester) async {
final brightnessNotifier = ValueNotifier(Brightness.dark);

// Pump a SuperTextField with an IME configuration with values
// that differ from the defaults.
await tester.pumpWidget(
_buildScaffold(
child: ValueListenableBuilder(
valueListenable: brightnessNotifier,
builder: (context, brightness, child) {
return SuperTextField(
inputSource: TextInputSource.ime,
imeConfiguration: TextInputConfiguration(
enableSuggestions: false,
autocorrect: false,
inputAction: TextInputAction.search,
keyboardAppearance: brightness,
inputType: TextInputType.number,
enableDeltaModel: false,
textCapitalization: TextCapitalization.characters,
),
);
},
),
),
);

// Holds the IME configuration values passed to the platform.
String? inputAction;
String? inputType;
bool? autocorrect;
bool? enableSuggestions;
String? keyboardAppearance;
bool? enableDeltaModel;
String? textCapitalization;

// Intercept the setClient message sent to the platform to check the configuration.
tester
.interceptChannel(SystemChannels.textInput.name) //
.interceptMethod(
'TextInput.setClient',
(methodCall) {
final params = methodCall.arguments[1] as Map;
inputAction = params['inputAction'];
autocorrect = params['autocorrect'];
enableSuggestions = params['enableSuggestions'];
keyboardAppearance = params['keyboardAppearance'];
enableDeltaModel = params['enableDeltaModel'];
textCapitalization = params['textCapitalization'];

final inputTypeConfig = params['inputType'] as Map;
inputType = inputTypeConfig['name'];

return null;
},
);

// Tap to focus the text field and attach to the IME.
await tester.placeCaretInSuperTextField(0);

// Ensure we use the values from the configuration.
expect(inputAction, 'TextInputAction.search');
expect(inputType, 'TextInputType.number');
expect(autocorrect, false);
expect(enableSuggestions, false);
expect(enableDeltaModel, true);
expect(textCapitalization, 'TextCapitalization.characters');
expect(keyboardAppearance, 'Brightness.dark');

// Change the brightness to rebuild the widget
// and re-attach to the IME.
brightnessNotifier.value = Brightness.light;
await tester.pump();

// Ensure we use the values from the configuration,
// updating only the keyboard appearance.
expect(inputAction, 'TextInputAction.search');
expect(inputType, 'TextInputType.number');
expect(autocorrect, false);
expect(enableSuggestions, false);
expect(enableDeltaModel, true);
expect(textCapitalization, 'TextCapitalization.characters');
expect(keyboardAppearance, 'Brightness.light');
});

testWidgetsOnAllPlatforms('doesn\'t re-attach to IME if the configuration doesn\'t change', (tester) async {
// Keeps track of how many times TextInput.setClient was called.
int imeConnectionCount = 0;

// Explicitly avoid using const to ensure that we have two
// TextInputConfiguration instances with the same values.
//
// ignore: prefer_const_constructors
final configuration1 = TextInputConfiguration(
enableSuggestions: false,
autocorrect: false,
inputAction: TextInputAction.search,
keyboardAppearance: Brightness.dark,
inputType: TextInputType.number,
enableDeltaModel: false,
);
// ignore: prefer_const_constructors
final configuration2 = TextInputConfiguration(
enableSuggestions: false,
autocorrect: false,
inputAction: TextInputAction.search,
keyboardAppearance: Brightness.dark,
inputType: TextInputType.number,
enableDeltaModel: false,
);

final inputConfigurationNotifier = ValueNotifier(configuration1);

// Pump a SuperTextField with an IME configuration with values
// that differ from the defaults.
await tester.pumpWidget(
_buildScaffold(
child: ValueListenableBuilder(
valueListenable: inputConfigurationNotifier,
builder: (context, inputConfiguration, child) {
return SuperTextField(
inputSource: TextInputSource.ime,
imeConfiguration: inputConfiguration,
);
},
),
),
);

// Intercept the setClient message sent to the platform.
tester
.interceptChannel(SystemChannels.textInput.name) //
.interceptMethod(
'TextInput.setClient',
(methodCall) {
imeConnectionCount += 1;
return null;
},
);

// Tap to focus the text field and attach to the IME.
await tester.placeCaretInSuperTextField(0);

// Change the configuration instance to trigger a rebuild.
inputConfigurationNotifier.value = configuration2;
await tester.pump();

// Ensure the connection was performed only once.
expect(imeConnectionCount, 1);
});

group('SuperTextField on some bad Android software keyboards', () {
testWidgetsOnAndroid('handles BACKSPACE key event instead of deletion for a collapsed selection (on Android)',
(tester) async {
Expand Down
Loading