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

Use coercion for MaskedTextBox.Text #17143

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

MrJul
Copy link
Member

@MrJul MrJul commented Sep 26, 2024

What does the pull request do?

This PR uses coercion to change MaskedTextBox.Text whenever Text is externally changed to a value that doesn't match the mask.

What is the current behavior?

The Text property is immediately changed back in OnPropertyChanged, causing reentrancy and property change events to be sent out of order. See #16746 (comment) for more context.

What is the updated/expected behavior with this PR?

Extra property change events aren't raised.
The invalid value is invisible to external listeners.

How was the solution implemented?

The new implementation uses a coercion callback.

The coercion callback from TextBox has been exposed as a virtual CoerceText method, since it's responsible for undo/redo. Without this, using OverrideMetadata(TextProperty) on any TextBox descendant breaks the undo/redo feature.

Checklist

  • Added unit tests (if possible)?
  • Added XML documentation to any related classes?

@avaloniaui-bot
Copy link

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

Copy link
Member

@maxkatz6 maxkatz6 left a comment

Choose a reason for hiding this comment

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

Technically a new API, as this method wasn't protected previously.
But LGTM.

@maxkatz6 maxkatz6 merged commit db10780 into AvaloniaUI:master Sep 30, 2024
11 checks passed
@maxkatz6 maxkatz6 added bug and removed feature labels Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants