Skip to content

Fix: On the Web, cannot support multiline inputting when registering customized TextInputControl #139446

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

Closed
Satsrag wants to merge 4 commits into from
Closed

Fix: On the Web, cannot support multiline inputting when registering customized TextInputControl #139446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9ecd1cd
fix: custom input control cannot support multiline
Satsrag Dec 3, 2023
54b3531
fix: remove unuseful space
Satsrag Dec 3, 2023
9ee679a
fix: removed redundant argument
Satsrag Dec 3, 2023
416f3d6
feat: added test for forceMultiline
Satsrag Dec 4, 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
34 changes: 27 additions & 7 deletions packages/flutter/lib/src/services/text_input.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ enum SmartQuotesType {
class TextInputType {
const TextInputType._(this.index)
: signed = null,
decimal = null;
decimal = null,
Copy link
Contributor

@LongCatIsLooong LongCatIsLooong Dec 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

On looking at the _configurationToJson change, this doesn't have to change the public interface at all I think?

Copy link
Contributor Author

@Satsrag Satsrag Dec 13, 2023

Choose a reason for hiding this comment

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

I agree with you. I will try it on _configurationToJson and just change the JSON that is sent to the engine.

Copy link
Contributor Author

@Satsrag Satsrag Dec 19, 2023

Choose a reason for hiding this comment

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

I open a new PR. Just added forceMultiline flag to JSON. @Renzo-Olivares @LongCatIsLooong please have a review.

forceMultiline = null;

/// Optimize for numerical information.
///
Expand All @@ -98,7 +99,15 @@ class TextInputType {
const TextInputType.numberWithOptions({
this.signed = false,
this.decimal = false,
}) : index = 2;
}) : index = 2, forceMultiline = null;

/// Optimize for none information.
///
/// Requests a none keyboard with additional settings.
/// The [forceMultiline] parameter is optional.
Copy link
Contributor

@Renzo-Olivares Renzo-Olivares Dec 12, 2023

Choose a reason for hiding this comment

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

Consider adding a comment about how forceMultiline only applies in a web context.

Copy link
Contributor

@Renzo-Olivares Renzo-Olivares Dec 12, 2023

Choose a reason for hiding this comment

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

Also we should probably explain why someone might use this i.e. the custom text input control use-case.

const TextInputType.noneWithOptions({
this.forceMultiline = false,
}) : index = 10, signed = null, decimal = null;

/// Enum value index, corresponds to one of the [values].
final int index;
Expand All @@ -115,6 +124,12 @@ class TextInputType {
/// Use `const TextInputType.numberWithOptions(decimal: true)` to set this.
final bool? decimal;

/// The none is force multiline, allowing a multiline text input.
///
/// This flag is only used for the [none] input type, otherwise `null`.
/// Use `const TextInputType.noneWithOptions(forceMultiline: true)` to set this.
final bool? forceMultiline;
Copy link
Contributor

@Renzo-Olivares Renzo-Olivares Dec 12, 2023

Choose a reason for hiding this comment

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

https://github.com/flutter/flutter/pull/139446/files#r1424500145


/// Optimize for textual information.
///
/// Requests the default platform keyboard.
Expand Down Expand Up @@ -182,7 +197,7 @@ class TextInputType {
static const TextInputType streetAddress = TextInputType._(9);

/// Prevent the OS from showing the on-screen virtual keyboard.
Copy link
Contributor

@Renzo-Olivares Renzo-Olivares Dec 12, 2023

Choose a reason for hiding this comment

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

Consider adding a comment similar to how TextInputType number has in this file.

  /// Additional options, such as decimal point and/or positive/negative
  /// signs, can be requested using [TextInputType.numberWithOptions].

static const TextInputType none = TextInputType._(10);
static const TextInputType none = TextInputType.noneWithOptions();

/// All possible enum values.
static const List<TextInputType> values = <TextInputType>[
Expand All @@ -203,6 +218,7 @@ class TextInputType {
'name': _name,
'signed': signed,
'decimal': decimal,
'forceMultiline': forceMultiline,
};
}

Expand All @@ -211,19 +227,21 @@ class TextInputType {
return '${objectRuntimeType(this, 'TextInputType')}('
'name: $_name, '
'signed: $signed, '
'decimal: $decimal)';
'decimal: $decimal, '
'forceMultiline: $forceMultiline)';
}

@override
bool operator ==(Object other) {
return other is TextInputType
&& other.index == index
&& other.signed == signed
&& other.decimal == decimal;
&& other.decimal == decimal
&& other.forceMultiline == forceMultiline;
}

@override
int get hashCode => Object.hash(index, signed, decimal);
int get hashCode => Object.hash(index, signed, decimal, forceMultiline);
}

/// An action the user has requested the text input control to perform.
Expand Down Expand Up @@ -2226,7 +2244,9 @@ class _PlatformTextInputControl with TextInputControl {
Map<String, dynamic> _configurationToJson(TextInputConfiguration configuration) {
final Map<String, dynamic> json = configuration.toJson();
if (TextInput._instance._currentControl != _PlatformTextInputControl.instance) {
json['inputType'] = TextInputType.none.toJson();
json['inputType'] = TextInputType.noneWithOptions(
forceMultiline: configuration.inputType == TextInputType.multiline,
Copy link
Contributor

@Renzo-Olivares Renzo-Olivares Dec 8, 2023

Choose a reason for hiding this comment

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

From what I understand we should still continue to send TextInputType.none in both the multi-line case and the single-line case of custom text input controls because that prevents the keyboard from being shown. Won't this make the keyboard show again?

Copy link
Contributor

@Renzo-Olivares Renzo-Olivares Dec 8, 2023

Choose a reason for hiding this comment

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

Maybe the forceMultiline flag can be processed by the web embedder to decide whether to use textarea or input.

Copy link
Contributor Author

@Satsrag Satsrag Dec 9, 2023

Choose a reason for hiding this comment

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

@Renzo-Olivares Yes, this won't show the keyboard again. This still sends TextInputType.none to the engine. And also, send forceMultiline stated in TextInputType.none.

Copy link
Contributor Author

@Satsrag Satsrag Dec 9, 2023

Choose a reason for hiding this comment

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

I have updated the PR in the web engine to handle forceMultiline . But have not reviewed it yet.

);
}
return json;
}
Expand Down
67 changes: 52 additions & 15 deletions packages/flutter/test/services/text_input_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ void main() {
'name': 'TextInputType.text',
'signed': null,
'decimal': null,
'forceMultiline': null,
});
expect(json['readOnly'], true);
expect(json['obscureText'], true);
Expand All @@ -317,6 +318,27 @@ void main() {
'name': 'TextInputType.number',
'signed': false,
'decimal': true,
'forceMultiline': null,
});
expect(json['readOnly'], false);
expect(json['obscureText'], true);
expect(json['autocorrect'], false);
expect(json['actionLabel'], 'xyzzy');
});

test('none serializes to JSON', () async {
const TextInputConfiguration configuration = TextInputConfiguration(
inputType: TextInputType.noneWithOptions(forceMultiline: true),
obscureText: true,
autocorrect: false,
actionLabel: 'xyzzy',
);
final Map<String, dynamic> json = configuration.toJson();
expect(json['inputType'], <String, dynamic>{
'name': 'TextInputType.none',
'signed': null,
'decimal': null,
'forceMultiline': true,
});
expect(json['readOnly'], false);
expect(json['obscureText'], true);
Expand All @@ -333,21 +355,26 @@ void main() {
const TextInputType decimal = TextInputType.numberWithOptions(decimal: true);
const TextInputType signedDecimal =
TextInputType.numberWithOptions(signed: true, decimal: true);

expect(text.toString(), 'TextInputType(name: TextInputType.text, signed: null, decimal: null)');
expect(number.toString(), 'TextInputType(name: TextInputType.number, signed: false, decimal: false)');
expect(signed.toString(), 'TextInputType(name: TextInputType.number, signed: true, decimal: false)');
expect(decimal.toString(), 'TextInputType(name: TextInputType.number, signed: false, decimal: true)');
expect(signedDecimal.toString(), 'TextInputType(name: TextInputType.number, signed: true, decimal: true)');
expect(TextInputType.multiline.toString(), 'TextInputType(name: TextInputType.multiline, signed: null, decimal: null)');
expect(TextInputType.phone.toString(), 'TextInputType(name: TextInputType.phone, signed: null, decimal: null)');
expect(TextInputType.datetime.toString(), 'TextInputType(name: TextInputType.datetime, signed: null, decimal: null)');
expect(TextInputType.emailAddress.toString(), 'TextInputType(name: TextInputType.emailAddress, signed: null, decimal: null)');
expect(TextInputType.url.toString(), 'TextInputType(name: TextInputType.url, signed: null, decimal: null)');
expect(TextInputType.visiblePassword.toString(), 'TextInputType(name: TextInputType.visiblePassword, signed: null, decimal: null)');
expect(TextInputType.name.toString(), 'TextInputType(name: TextInputType.name, signed: null, decimal: null)');
expect(TextInputType.streetAddress.toString(), 'TextInputType(name: TextInputType.address, signed: null, decimal: null)');
expect(TextInputType.none.toString(), 'TextInputType(name: TextInputType.none, signed: null, decimal: null)');
const TextInputType none = TextInputType.none;
const TextInputType none2 = TextInputType.none;
const TextInputType multilineNone = TextInputType.noneWithOptions(forceMultiline: true);
const TextInputType multilineNone2 = TextInputType.noneWithOptions(forceMultiline: true);

expect(text.toString(), 'TextInputType(name: TextInputType.text, signed: null, decimal: null, forceMultiline: null)');
expect(number.toString(), 'TextInputType(name: TextInputType.number, signed: false, decimal: false, forceMultiline: null)');
expect(signed.toString(), 'TextInputType(name: TextInputType.number, signed: true, decimal: false, forceMultiline: null)');
expect(decimal.toString(), 'TextInputType(name: TextInputType.number, signed: false, decimal: true, forceMultiline: null)');
expect(signedDecimal.toString(), 'TextInputType(name: TextInputType.number, signed: true, decimal: true, forceMultiline: null)');
expect(none.toString(), 'TextInputType(name: TextInputType.none, signed: null, decimal: null, forceMultiline: false)');
expect(multilineNone.toString(), 'TextInputType(name: TextInputType.none, signed: null, decimal: null, forceMultiline: true)');
expect(TextInputType.multiline.toString(), 'TextInputType(name: TextInputType.multiline, signed: null, decimal: null, forceMultiline: null)');
expect(TextInputType.phone.toString(), 'TextInputType(name: TextInputType.phone, signed: null, decimal: null, forceMultiline: null)');
expect(TextInputType.datetime.toString(), 'TextInputType(name: TextInputType.datetime, signed: null, decimal: null, forceMultiline: null)');
expect(TextInputType.emailAddress.toString(), 'TextInputType(name: TextInputType.emailAddress, signed: null, decimal: null, forceMultiline: null)');
expect(TextInputType.url.toString(), 'TextInputType(name: TextInputType.url, signed: null, decimal: null, forceMultiline: null)');
expect(TextInputType.visiblePassword.toString(), 'TextInputType(name: TextInputType.visiblePassword, signed: null, decimal: null, forceMultiline: null)');
expect(TextInputType.name.toString(), 'TextInputType(name: TextInputType.name, signed: null, decimal: null, forceMultiline: null)');
expect(TextInputType.streetAddress.toString(), 'TextInputType(name: TextInputType.address, signed: null, decimal: null, forceMultiline: null)');

expect(text == number, false);
expect(number == number2, true);
Expand All @@ -357,6 +384,11 @@ void main() {
expect(signed == signedDecimal, false);
expect(decimal == signedDecimal, false);

expect(text == none2, false);
expect(none == none2, true);
expect(none == multilineNone, false);
expect(multilineNone == multilineNone2, true);

expect(text.hashCode == number.hashCode, false);
expect(number.hashCode == number2.hashCode, true);
expect(number.hashCode == signed.hashCode, false);
Expand All @@ -365,6 +397,11 @@ void main() {
expect(signed.hashCode == signedDecimal.hashCode, false);
expect(decimal.hashCode == signedDecimal.hashCode, false);

expect(text.hashCode == none2.hashCode, false);
expect(none.hashCode == none2.hashCode, true);
expect(none.hashCode == multilineNone.hashCode, false);
expect(multilineNone.hashCode == multilineNone2.hashCode, true);

expect(TextInputType.text.index, 0);
expect(TextInputType.multiline.index, 1);
expect(TextInputType.number.index, 2);
Expand Down