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.

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

Framework can receive TextEditingDeltas from engine #88477

Merged
merged 81 commits into from
Sep 16, 2021
Merged
Show file tree
Hide file tree
Changes from 70 commits
Commits
Show all changes
81 commits
Select commit Hold shift + click to select a range
6896c6e
Add TextDelta to TextEditingValue
Renzo-Olivares Aug 9, 2021
535c58b
docs updates
Renzo-Olivares Aug 9, 2021
b1c537e
move TextEditingDeltas out of TextEditingValue and create new channel…
Renzo-Olivares Aug 17, 2021
15cb4f4
updates for deltaManager
Renzo-Olivares Aug 18, 2021
f65518e
remove deltaManager
Renzo-Olivares Aug 18, 2021
1575424
add flag to TextInputConfiguration for choosing between plain text mo…
Renzo-Olivares Aug 22, 2021
2c09999
Add support for autofill with delta model
Renzo-Olivares Aug 22, 2021
a7d4162
updates
Renzo-Olivares Aug 24, 2021
b0de5c9
updates
Renzo-Olivares Aug 24, 2021
c674092
Add constructor from value from DeltaTextEditingController
Renzo-Olivares Aug 25, 2021
1f69ab9
Extend from common TextEditingDelta class
Renzo-Olivares Aug 27, 2021
fc800c7
update DeltaTextEditingController example
Renzo-Olivares Aug 27, 2021
6edb7d3
delta -> deltaType
Renzo-Olivares Aug 27, 2021
b1dbf54
updates
Renzo-Olivares Aug 27, 2021
390c61e
Implement apply method for TextEditingDeltas
Renzo-Olivares Aug 28, 2021
929f89f
Remove DeltaTextEditingController
Renzo-Olivares Aug 28, 2021
5576bb8
Add tests
Renzo-Olivares Aug 28, 2021
c0e952d
update docs
Renzo-Olivares Aug 28, 2021
c2ca0c4
more docs
Renzo-Olivares Aug 28, 2021
c9d0d24
update tests
Renzo-Olivares Aug 29, 2021
8b2e366
fix some tests
Renzo-Olivares Aug 29, 2021
c55debe
fix text_input_test
Renzo-Olivares Aug 29, 2021
99c4b1f
fix more analyzer issues
Renzo-Olivares Aug 29, 2021
3b823ad
More docs and fixes
Renzo-Olivares Aug 29, 2021
8d8f15b
Fix analyzer
Renzo-Olivares Aug 29, 2021
b2e007e
sort constructors first
Renzo-Olivares Aug 29, 2021
41ecd4f
Fix broken test in editable_text_test.dart
Renzo-Olivares Aug 29, 2021
9cd5efa
Docs update
Renzo-Olivares Aug 29, 2021
051678c
Update deltaType format
Renzo-Olivares Aug 30, 2021
bb7d24d
Fix tests
Renzo-Olivares Aug 30, 2021
636b955
review updates
Renzo-Olivares Aug 30, 2021
8aac1ea
fix tests
Renzo-Olivares Aug 30, 2021
5938ae1
updates
Renzo-Olivares Aug 30, 2021
0b4db8f
Updates
Renzo-Olivares Sep 1, 2021
c046c53
fix tests
Renzo-Olivares Sep 1, 2021
9daa2ec
Address review comments
Renzo-Olivares Sep 7, 2021
559a14c
fix tests
Renzo-Olivares Sep 7, 2021
9c3d545
Updating tests
Renzo-Olivares Sep 7, 2021
b70562f
Update docs
Renzo-Olivares Sep 7, 2021
4eeb416
Move inference to framework
Renzo-Olivares Sep 7, 2021
43690fa
clean up logs
Renzo-Olivares Sep 7, 2021
eda6fd6
remove more logs
Renzo-Olivares Sep 7, 2021
55de79b
move infer deltas to TextEditingDelta json constructor
Renzo-Olivares Sep 7, 2021
36b9e60
Update tests
Renzo-Olivares Sep 7, 2021
392ea4a
fix analyzer
Renzo-Olivares Sep 7, 2021
5db542d
fix tests
Renzo-Olivares Sep 7, 2021
aca5de8
Fix analyzer
Renzo-Olivares Sep 7, 2021
f16c9d3
updates
Renzo-Olivares Sep 7, 2021
3024d46
analyzer fixes
Renzo-Olivares Sep 7, 2021
2e2cb29
fix tests
Renzo-Olivares Sep 7, 2021
f0fe443
Address reviewer comments
Renzo-Olivares Sep 8, 2021
995e3d9
Update docs
Renzo-Olivares Sep 8, 2021
30411f9
Move TextEditingDelta to its own file
Renzo-Olivares Sep 8, 2021
c2cc8f3
Make replace method global and private
Renzo-Olivares Sep 8, 2021
a574926
Add unit tests for TextEditingDelta
Renzo-Olivares Sep 8, 2021
db8db42
updates
Renzo-Olivares Sep 8, 2021
fed90a8
Fix tests and add immutable annotations to deltas
Renzo-Olivares Sep 8, 2021
d5318aa
Update docs and address reviewer comments
Renzo-Olivares Sep 9, 2021
1b1241b
remove leftovers
Renzo-Olivares Sep 9, 2021
0627ab2
fix tests
Renzo-Olivares Sep 9, 2021
0f553a1
Don't use triple /// inside a method
Renzo-Olivares Sep 10, 2021
a5e95a1
remove widget tester initializer
Renzo-Olivares Sep 10, 2021
69a0252
Leave apply unimplemented and apply delta to oldText for now to follo…
Renzo-Olivares Sep 10, 2021
505a06f
Address reviewer comments in relation to removing variables from the …
Renzo-Olivares Sep 10, 2021
586a5d6
prefer const
Renzo-Olivares Sep 10, 2021
3c65b82
Merge branch 'master' of github.com:flutter/flutter into text_deltas
Renzo-Olivares Sep 10, 2021
af5f856
Fix test
Renzo-Olivares Sep 10, 2021
feb3990
fix docs
Renzo-Olivares Sep 10, 2021
283ca65
Address reviewer comments in relation to deltaType
Renzo-Olivares Sep 10, 2021
60a718e
Report an error if the editing state is out of sync
Renzo-Olivares Sep 10, 2021
ec31582
Remove prints
Renzo-Olivares Sep 10, 2021
65e8130
Every platform now sends a list of deltas
Renzo-Olivares Sep 13, 2021
967c44c
Update docs
Renzo-Olivares Sep 13, 2021
d4b3e9c
Remove FlutterError from TextEditingDelta
Renzo-Olivares Sep 13, 2021
1546d7d
Documentation is inherited
Renzo-Olivares Sep 13, 2021
ec45d92
update docs
Renzo-Olivares Sep 14, 2021
46f30c2
Remove logs
Renzo-Olivares Sep 14, 2021
a594423
Merge branch 'master' of github.com:flutter/flutter into text_deltas
Renzo-Olivares Sep 14, 2021
79b883a
Update documentation
Renzo-Olivares Sep 14, 2021
4e7b4c8
Use deltas instead of batchDeltas
Renzo-Olivares Sep 15, 2021
5b6183f
Update documentation
Renzo-Olivares Sep 15, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/flutter/lib/services.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export 'src/services/system_chrome.dart';
export 'src/services/system_navigator.dart';
export 'src/services/system_sound.dart';
export 'src/services/text_editing.dart';
export 'src/services/text_editing_delta.dart';
export 'src/services/text_formatter.dart';
export 'src/services/text_input.dart';
export 'src/services/text_layout_metrics.dart';
424 changes: 424 additions & 0 deletions packages/flutter/lib/src/services/text_editing_delta.dart

Large diffs are not rendered by default.

49 changes: 48 additions & 1 deletion packages/flutter/lib/src/services/text_input.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import 'platform_channel.dart';
import 'system_channels.dart';
import 'system_chrome.dart';
import 'text_editing.dart';
import 'text_editing_delta.dart';

export 'dart:ui' show TextAffinity;

Expand Down Expand Up @@ -467,6 +468,7 @@ class TextInputConfiguration {
this.textCapitalization = TextCapitalization.none,
this.autofillConfiguration = AutofillConfiguration.disabled,
this.enableIMEPersonalizedLearning = true,
this.enableDeltaModel = false,
}) : assert(inputType != null),
assert(obscureText != null),
smartDashesType = smartDashesType ?? (obscureText ? SmartDashesType.disabled : SmartDashesType.enabled),
Expand All @@ -476,7 +478,8 @@ class TextInputConfiguration {
assert(keyboardAppearance != null),
assert(inputAction != null),
assert(textCapitalization != null),
assert(enableIMEPersonalizedLearning != null);
assert(enableIMEPersonalizedLearning != null),
assert(enableDeltaModel != null);

/// The type of information for which to optimize the text input control.
final TextInputType inputType;
Expand Down Expand Up @@ -622,6 +625,7 @@ class TextInputConfiguration {
TextCapitalization? textCapitalization,
bool? enableIMEPersonalizedLearning,
AutofillConfiguration? autofillConfiguration,
bool? enableDeltaModel,
}) {
return TextInputConfiguration(
inputType: inputType ?? this.inputType,
Expand All @@ -636,8 +640,22 @@ class TextInputConfiguration {
keyboardAppearance: keyboardAppearance ?? this.keyboardAppearance,
enableIMEPersonalizedLearning: enableIMEPersonalizedLearning?? this.enableIMEPersonalizedLearning,
autofillConfiguration: autofillConfiguration ?? this.autofillConfiguration,
enableDeltaModel: enableDeltaModel ?? this.enableDeltaModel,
);
}

/// Whether to enable that the engine sends text input updates to the
/// framework as [TextEditingDelta]'s or as one [TextEditingValue].
///
/// When this is enabled platform text input updates will
/// come through [TextInputClient.updateEditingValueWithDeltas].
Copy link
Contributor

Choose a reason for hiding this comment

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

May want to also mention that using this results in more granular explicit set of changes rather than batched together general updates to the editing value. We want to help developers find/decide if this feature is right for them.

///
/// When this is disabled platform text input updates will come through
/// [TextInputClient.updateEditingValue].
///
/// Defaults to false. Cannot be null.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Should be an empty line above this one:

///
/// Defaults to false. Cannot be null.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, is this the place where we should thoroughly explain what this is for? You could mention updateEditingValueWithDeltas and updateEditingValue, and maybe mention that updateEditingValue doesn't work when this is true (is that right?).

Long term it would also be cool to have an example of a simple rich text editor or something in the docs somewhere. Or maybe your demo of just showing the current change.

final bool enableDeltaModel;

/// Returns a representation of this object as a JSON object.
Map<String, dynamic> toJson() {
final Map<String, dynamic>? autofill = autofillConfiguration.toJson();
Expand All @@ -655,6 +673,7 @@ class TextInputConfiguration {
'keyboardAppearance': keyboardAppearance.toString(),
'enableIMEPersonalizedLearning': enableIMEPersonalizedLearning,
if (autofill != null) 'autofill': autofill,
'enableDeltaModel' : enableDeltaModel,
};
}
}
Expand Down Expand Up @@ -956,6 +975,13 @@ abstract class TextInputClient {
/// formatting.
void updateEditingValue(TextEditingValue value);

/// Requests that this client update its editing state by applying the delta
/// received from the engine.
///
/// The [TextEditingDelta] is treated as a change that will be applied to the client's
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this different from the first paragraph?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh emphasis on "change" to explain what "delta" means?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the docs.

/// value.
void updateEditingValueWithDeltas(List<TextEditingDelta> textEditingDeltas);

/// Requests that this client perform the given action.
void performAction(TextInputAction action);

Expand Down Expand Up @@ -1447,6 +1473,27 @@ class TextInput {
case 'TextInputClient.updateEditingState':
_currentConnection!._client.updateEditingValue(TextEditingValue.fromJSON(args[1] as Map<String, dynamic>));
break;
case 'TextInputClient.updateEditingStateWithDeltas':
final List<TextEditingDelta> batchDeltas = <TextEditingDelta>[];

final Map<String, dynamic> encoded = args[1] as Map<String, dynamic>;

// On Android there could be a situation where multiple edits done to
// an editing state are accumulated before being sent to the framework.
// This is called batch editing on Android, and on other platforms the
// TextInputPlugin does not adhere to this type of editing.
if (encoded['batchDeltas'] != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm interested in what other reviewers think of this. In some cases multiple deltas are sent in one updateEditingValueWithDeltas, and in others just one is sent. Is this the cleanest way to do that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I have the same question. Since framework is not consuming the "isBatched" information, would it be OK if we always send a list of deltas?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Renzo-Olivares Was there ever any more discussion about this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am ok with always sending a list, even if only one is included. The complexity of a boolean check here is more confusing than just a static batch model.

Copy link
Contributor

Choose a reason for hiding this comment

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

On a different thought, the json probably needs to be a map anyways. We need to added a global timestamp to the message in the future to help with synchronization: #89394.

Copy link
Contributor

Choose a reason for hiding this comment

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

So let's always send it as a list, where encoded would always look something like this?

{
  deltas: [ ... ], // One or many, always an array.
  timestamp: ..., // Whenever we add this.
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

{ 
    deltas: [ ... ], // One or many, always an array. 
    timestamp: ..., // Whenever we add this.
 }

and for platforms without batch edits

{ 
   oldText:,
   deltaText:,
   deltaStart:,
   deltaEnd:,
   selectionStart:,
   selectionEnd:,
   composingStart:,
   composingEnd:,
   timestamp: ..., // Whenever we add this.
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought the always sending as a list was in relation to sending the deltas as a list to updateEditingValueWithDeltas. Is it regarding the encoded json format?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah sorry I digressed. I was suggesting the json should be a map with a list of deltas.

  • a map so we can add a fields in the future.
  • a list of deltas so we don't have to write separately logic for handling batched/non-batched updates .

for (final dynamic encodedDelta in encoded['batchDeltas']) {
final TextEditingDelta delta = TextEditingDelta.fromJSON(encodedDelta as Map<String, dynamic>);
batchDeltas.add(delta);
}
} else {
final TextEditingDelta delta = TextEditingDelta.fromJSON(encoded);
batchDeltas.add(delta);
}

_currentConnection!._client.updateEditingValueWithDeltas(batchDeltas);
break;
case 'TextInputClient.performAction':
_currentConnection!._client.performAction(_toTextInputAction(args[1] as String));
break;
Expand Down
34 changes: 34 additions & 0 deletions packages/flutter/lib/src/widgets/editable_text.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1795,6 +1795,39 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
@override
TextEditingValue get currentTextEditingValue => _value;

@override
void updateEditingValueWithDeltas(List<TextEditingDelta> textEditingDeltas) {
TextEditingValue value = _value;
for (final TextEditingDelta delta in textEditingDeltas) {
print('Delta class type: ' + delta.runtimeType.toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

Prints to remove before this is final.

print('Delta old text: ' + delta.oldText);
if (delta is TextEditingDeltaInsertion) {
print('inserted text: ' + (delta as TextEditingDeltaInsertion).textInserted);
print('insertion offset: ' + (delta as TextEditingDeltaInsertion).insertionOffset.toString());
} else if (delta is TextEditingDeltaDeletion) {
print('Deleted text: ' + (delta as TextEditingDeltaDeletion).textDeleted);
print(
'Beginning of deleted range: ' + (delta as TextEditingDeltaDeletion).deletedRange.start.toString());
print('End of deleted range: ' + (delta as TextEditingDeltaDeletion).deletedRange.end.toString());

} else if (delta is TextEditingDeltaReplacement) {
print('text being replaced: ' + (delta as TextEditingDeltaReplacement).textReplaced);
print('replacement source: ' + (delta as TextEditingDeltaReplacement).replacementText);
print(
'beginning of replaced range: ' + (delta as TextEditingDeltaReplacement).replacedRange.start.toString());
print('end of replaced range: ' + (delta as TextEditingDeltaReplacement).replacedRange.end.toString());
}
print('Delta beginning of new selection: ' +
delta.selection.start.toString());
print('Delta end of new selection: ' + delta.selection.end.toString());
print('Delta beginning of new composing: ' +
delta.composing.start.toString());
print('Delta end of new composing: ' + delta.composing.end.toString());
value = delta.apply(value);
}
updateEditingValue(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Smart, just call through to updateEditingValue... For a rich text editor author, could they still disable this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A rich text editor author at this low level I think would just implement updateEditingValueWithDeltas themselves.

}

@override
void updateEditingValue(TextEditingValue value) {
// This method handles text editing state updates from the platform text
Expand Down Expand Up @@ -2687,6 +2720,7 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
keyboardAppearance: widget.keyboardAppearance,
autofillConfiguration: autofillConfiguration,
enableIMEPersonalizedLearning: widget.enableIMEPersonalizedLearning,
enableDeltaModel: true,
);
}

Expand Down
13 changes: 13 additions & 0 deletions packages/flutter/test/services/autofill_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,19 @@ class FakeAutofillClient implements TextInputClient, AutofillClient {
latestMethodCall = 'updateEditingValue';
}

@override
void updateEditingValueWithDeltas(List<TextEditingDelta> textEditingDeltas) {
TextEditingValue newEditingValue = currentTextEditingValue;

for (final TextEditingDelta delta in textEditingDeltas) {
newEditingValue = delta.apply(newEditingValue);
}

currentTextEditingValue = newEditingValue;

latestMethodCall = 'updateEditingValueWithDeltas';
}

@override
AutofillScope? currentAutofillScope;

Expand Down