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

Fix editing delta is wrong when using delete key #36616

Merged

Conversation

bleroux
Copy link
Contributor

@bleroux bleroux commented Oct 5, 2022

Description

Before this PR, hitting the delete key causes DeltaTextInputClient to send incorrect delta on Web platform.

Related Issue

Fixes flutter/flutter#112920
Fixes flutter/samples#1424

Tests

Updates 1 test.
Adds 2 tests.

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Oct 5, 2022
@bleroux bleroux requested a review from Renzo-Olivares October 5, 2022 09:40
Copy link
Contributor

@Renzo-Olivares Renzo-Olivares left a comment

Choose a reason for hiding this comment

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

LGTM besides some doc updating. Thanks for fixing this!

@@ -497,7 +500,14 @@ class TextEditingDeltaState {
// 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments might also need some updating.

@@ -497,7 +500,14 @@ class TextEditingDeltaState {
// 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 = newTextEditingDeltaState.oldText.length - newEditingState.text!.length;
newTextEditingDeltaState.deltaStart = newTextEditingDeltaState.deltaEnd - deletedLength;
final bool backwardDeletion = newEditingState.baseOffset != lastEditingState?.baseOffset;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems fine to me, though I wonder if RTL affects this at all.

@bleroux bleroux force-pushed the fix_editing_delta_is_wrong_for_deletion branch from 5016d99 to 7cd2dd5 Compare October 6, 2022 11:31
@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 6, 2022
@auto-submit auto-submit bot merged commit 5b01075 into flutter:main Oct 6, 2022
@amantoux
Copy link

amantoux commented Oct 6, 2022

@bleroux @Renzo-Olivares
Many thanks for the fix
Can we expect to have it as part of next patch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
3 participants