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 TextBox sync issues #14960

Merged
merged 4 commits into from
Mar 15, 2024
Merged

Conversation

Gillibald
Copy link
Contributor

What does the pull request do?

  • Make sure text is synced directly with TextPresenter before updating the caret index
  • Make sure caret is moved to the new location on text input

What is the current behavior?

What is the updated/expected behavior with this PR?

How was the solution implemented (if it's not obvious)?

Checklist

Breaking changes

Obsoletions / Deprecations

Fixed issues

Fixes #13648
Fixes #13886
Fixes #14563

@Gillibald Gillibald added backport-candidate-11.0.x Consider this PR for backporting to 11.0 branch backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch labels Mar 13, 2024
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0046153-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

//Make sure updated text is in sync
_presenter?.SetCurrentValue(TextPresenter.TextProperty, text);

caretIndex += input.Length;
Copy link
Member

Choose a reason for hiding this comment

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

input.Length is added twice, here and on SetCurrentValue(CaretIndexProperty..., ) below, moving the effective caret twice (the visual one is correct).

Shouldn't we add some small unit test ensuring that TextBox.CaretIndex == _presenter.CaretIndex in the bugged cases after input?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. These unit tests tend to be hard to write because of the property sync. I am having a look. All these issues are caused by the fact that some state is synced via bindings.

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0046205-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul MrJul added this pull request to the merge queue Mar 15, 2024
Merged via the queue into AvaloniaUI:master with commit 1249a5f Mar 15, 2024
6 checks passed
maxkatz6 pushed a commit that referenced this pull request Apr 6, 2024
* Sync change text and changed caret index with the TextPresenter when text input is handled

* Do not add length again
maxkatz6 pushed a commit that referenced this pull request Apr 6, 2024
* Sync change text and changed caret index with the TextPresenter when text input is handled

* Do not add length again
@maxkatz6 maxkatz6 added backported-11.1.x and removed backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch labels Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-candidate-11.0.x Consider this PR for backporting to 11.0 branch backported-11.1.x
Projects
None yet
4 participants