Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Changes from 1 commit
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
86885dd
Initial implementation of TextEditingDeltaState for the web
Renzo-Olivares Sep 8, 2021
9ef2c43
Capture composing region through compositionupdate and handle cases w…
Renzo-Olivares Sep 9, 2021
b93b043
clean up unused code
Renzo-Olivares Sep 9, 2021
d3201a1
clean up rest of logs
Renzo-Olivares Sep 9, 2021
9f99a66
Make sure we initialize oldText in beforeInput
Renzo-Olivares Sep 9, 2021
8d69317
Clean up comments
Renzo-Olivares Sep 9, 2021
3ba145c
more defaults
Renzo-Olivares Sep 9, 2021
728055a
Add more comments
Renzo-Olivares Sep 9, 2021
efbdf5b
Move delta inferrence logic to TextEditingDeltaState
Renzo-Olivares Sep 9, 2021
1c330f3
Add new listeners to rest of strategies
Renzo-Olivares Sep 9, 2021
0dd249a
Fix existing tests
Renzo-Olivares Sep 13, 2021
37b757e
Fix tests
Renzo-Olivares Sep 13, 2021
fd1396f
Add lastTextEditingDeltaState to test
Renzo-Olivares Sep 13, 2021
cb6ef96
fix tests
Renzo-Olivares Sep 13, 2021
4f610e0
Add some preliminary tests for TextEditingDeltaState
Renzo-Olivares Sep 13, 2021
f772bf1
Send as list to framework
Renzo-Olivares Sep 13, 2021
f6e4d32
Add composing region test
Renzo-Olivares Sep 13, 2021
704d4f9
Address nits
Renzo-Olivares Sep 13, 2021
577aa59
Update tests
Renzo-Olivares Sep 13, 2021
0dcf1f2
Try to fix tests
Renzo-Olivares Sep 13, 2021
a2e5525
Prefer const with constant constructors
Renzo-Olivares Sep 13, 2021
95fe97b
Clean up comments
Renzo-Olivares Sep 13, 2021
5921215
Specify types
Renzo-Olivares Sep 13, 2021
a2a8b9d
fix tests
Renzo-Olivares Sep 14, 2021
1fbab86
Specify type annotations
Renzo-Olivares Sep 14, 2021
928f186
Merge branch 'main' of github.com:flutter/engine into text_editing_de…
Renzo-Olivares Jan 11, 2022
03d63c3
batchDeltas -> deltas
Renzo-Olivares Jan 12, 2022
51eabc8
Make eventData nullable so we dont compare with a 'null' string
Renzo-Olivares Jan 12, 2022
125580e
Make TextEditingDeltaState mutable to avoid multiple copies
Renzo-Olivares Jan 12, 2022
26668cb
Fix analyzer
Renzo-Olivares Jan 12, 2022
5a33dbf
Fix test
Renzo-Olivares Jan 13, 2022
e47a3ed
Use safe browser api instead of directly accessing js_util
Renzo-Olivares Jan 13, 2022
04020c2
remove last prefix from editingDeltaState
Renzo-Olivares Jan 19, 2022
13ca351
Remove logs
Renzo-Olivares Jan 19, 2022
73c1411
Merge branch 'main' of github.com:flutter/engine into text_editing_de…
Renzo-Olivares Jan 19, 2022
be578c3
fix merge
Renzo-Olivares Jan 22, 2022
2df95b0
fix whitespace
Renzo-Olivares Jan 22, 2022
5e2b294
Merge branch 'main' of github.com:flutter/engine into text_editing_de…
Renzo-Olivares Jan 22, 2022
ac3407b
revert composing changes
Renzo-Olivares Jan 24, 2022
fa1d886
Merge branch 'main' of github.com:flutter/engine into text_editing_de…
Renzo-Olivares Jan 24, 2022
6ced401
update comments
Renzo-Olivares Jan 25, 2022
15868f6
remove trailing whitespace
Renzo-Olivares Jan 25, 2022
059fe74
Add docs for TextEditingDeltaState
Renzo-Olivares Jan 26, 2022
26e676a
Normalize delta naming and use a copy instead of modifying function a…
Renzo-Olivares Jan 26, 2022
9196226
Update selection of delta in inferDeltaState instead of onChange
Renzo-Olivares Jan 26, 2022
38bc244
Fix tests, previously the selection was not set in inferDeltaState, n…
Renzo-Olivares Jan 26, 2022
cc0bb16
Make a copy of delta instead of modifying function arguments
Renzo-Olivares Jan 26, 2022
b7a91da
remove whitespace
Renzo-Olivares Jan 26, 2022
a600e9e
Move some logic into inferDeltaState
Renzo-Olivares Jan 26, 2022
98fad47
whitespace
Renzo-Olivares Jan 26, 2022
786a528
analyzer fix
Renzo-Olivares Jan 26, 2022
3f260ce
Revert "analyzer fix"
Renzo-Olivares Jan 27, 2022
b9db35a
Revert "whitespace"
Renzo-Olivares Jan 27, 2022
41765f8
Revert "Move some logic into inferDeltaState"
Renzo-Olivares Jan 27, 2022
8ff4f2d
pass _editingDeltaState instead of editingDeltaState to onChange for …
Renzo-Olivares Jan 27, 2022
f4a858a
Add docs for beforeinput
Renzo-Olivares Jan 27, 2022
cbb6b6b
Add docs for inferDeltaState
Renzo-Olivares Jan 27, 2022
b596600
whitespace
Renzo-Olivares Jan 27, 2022
5a8a5f7
Add more docs
Renzo-Olivares Jan 27, 2022
6645a0b
update docs
Renzo-Olivares Jan 27, 2022
da90e99
update docs
Renzo-Olivares Jan 27, 2022
6217ea9
Merge branch 'main' of github.com:flutter/engine into text_editing_de…
Renzo-Olivares Jan 27, 2022
b5e0b55
Fix for insertion of a period following a double space within old tex…
Renzo-Olivares Jan 29, 2022
7a5aff9
Fix accent insertion
Jan 31, 2022
c4c9cf6
Merge branch 'main' of github.com:flutter/engine into text_editing_de…
Jan 31, 2022
9d97c88
clean up comments
Jan 31, 2022
0fc279d
Address comments for clarity aand regexp
Feb 9, 2022
fc823e9
Make composing and selection nullable
Feb 10, 2022
6bf2ca7
update docs
Feb 10, 2022
acbc0de
whitespace
Feb 10, 2022
7722380
Merge branch 'main' of github.com:flutter/engine into text_editing_de…
Feb 10, 2022
0bb7f30
address comments
Feb 10, 2022
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
Prev Previous commit
Next Next commit
update comments
  • Loading branch information
Renzo-Olivares committed Jan 25, 2022
commit 6ced401f02affb8fcc061ae60a5b546dfba4d0b8
11 changes: 6 additions & 5 deletions lib/web_ui/lib/src/engine/text_editing/text_editing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,6 @@ class TextEditingDeltaState {
TextEditingDeltaState({
this.oldText = '',
this.deltaText = '',
this.inputType = '',
this.deltaStart = -1,
this.deltaEnd = -1,
this.baseOffset = -1,
Expand All @@ -471,10 +470,15 @@ class TextEditingDeltaState {

if (lastTextEditingDeltaState.deltaText.isEmpty && lastTextEditingDeltaState.deltaEnd != -1) {
// We are removing text.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: consider extracting complex conditions into named variables. Less needs to be explained this way. E.g.:

final bool isTextBeingRemoved = newTextEditingDeltaState.deltaText.isEmpty && newTextEditingDeltaState.deltaEnd != -1;
if (isTextBeingRemoved) {
  ...

// When text is deleted outside of the composing region or is cut using the native toolbar,
// we calculate the length of the deleted text by comparing the new and old editing state lengths.
// This value is then subtracted from the end position of the delta to capture the deleted range.
final int deletedLength = lastTextEditingDeltaState.oldText.length - newEditingState.text!.length;
lastTextEditingDeltaState.deltaStart = lastTextEditingDeltaState.deltaEnd - deletedLength;
Copy link
Contributor

Choose a reason for hiding this comment

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

Related to a comment I'm leaving below where inferDeltaState is called: What do you think of not modifying the lastTextEditingDelta argument? Maybe just make a copy if that makes sense.

I'm not sure if this is in the Flutter style guide, but I personally try not to modify function arguments unless I have to. Seeing this makes me think that the variable passed in as lastTextEditingDelta where inferDeltaState is called is expecting to be modified and will be reused for something.

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 see what you mean. I think it makes sense to not modify the lastTextEditingDelta argument since we are constructing a new delta with the new information we have at this point in the flow.

} else if (lastTextEditingDeltaState.deltaText.isNotEmpty && !previousSelectionWasCollapsed) {
// We are replacing text at a selection.
// When a selection of text is replaced by a copy/paste operation we set the starting range
// of the delta to be the beginning of the selection of the previous editing state.
lastTextEditingDeltaState.deltaStart = lastEditingState!.baseOffset!;
}

Expand Down Expand Up @@ -547,7 +551,6 @@ class TextEditingDeltaState {

String oldText;
String deltaText;
String inputType;
int deltaStart;
int deltaEnd;
Copy link
Contributor

Choose a reason for hiding this comment

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

If empty strings and -1 are meaningful values for these fields, let's explain what they mean. Something tells me that the 3 fields above could be combined into a TextEditingDelta(text, start, end) struct and the default here expressed as TextEditingDelta? delta;. I may be missing something though.

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 think since the selection and composing region change when the text is mutated they should be together with deltaStart,deltaEnd, and deltaText in one class.

int baseOffset;
Expand Down Expand Up @@ -1190,15 +1193,13 @@ abstract class DefaultTextEditingStrategy implements TextEditingStrategy {
final String? inputType = getJsProperty<void>(event, 'inputType') as String?;

if (inputType != null) {
editingDelta.inputType = inputType;

if (inputType.contains('delete')) {
// The deltaStart is set in handleChange because there is where we get access
// to the new selection baseOffset which is our new deltaStart.
editingDelta.deltaText = '';
editingDelta.deltaEnd = lastEditingState!.extentOffset!;
} else if (inputType == 'insertLineBreak'){
// event.data is null, so we manually set deltaText as a line break by setting it to '\n'.
// event.data is null on a line break, so we manually set deltaText as a line break by setting it to '\n'.
editingDelta.deltaText = '\n';
editingDelta.deltaStart = lastEditingState!.extentOffset!;
editingDelta.deltaEnd = lastEditingState!.extentOffset!;
Expand Down