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

Validate all width/height properties of Layoutable when they are set #15753

Merged

Conversation

TomEdwardsEnscape
Copy link
Contributor

It is currently possible to set the Width, Height and associated min/max properties of Layoutable to values which result in a InvalidOperationException when Measure is called.

If the control is visible, then changing the properties will immediately trigger a new measure and the exception above will be thrown. But if the control is not visible, then the exception will not be thrown until later on, and the point at which the invalid value was set will not be known.

This behaviour makes validation and debugging harder. Users of Avalonia must validate the values they set manually, which means duplicating logic from inside Avalonia's private methods.

What is the updated/expected behavior with this PR?

The relevant properties now have validation methods which immediately block invalid values, no matter what state the control is in.

An exception will be thrown when an invalid value is set. This allows immediate error handling with a try/catch block.

The exception message will be something like -10 is not a valid value for 'Width', which is considerably more useful than Invalid size returned for Measure, which is the message of the exception that would otherwise be thrown by Measure.

Breaking changes

It's possible that a custom control somewhere supports values that the validation methods in this PR reject. This can happen if its overridden MeasureOverride method continues to return a valid size.

This weird situation could be worked around by providing new properties on the custom control which don't block negative or infinite values.

Obsoletions / Deprecations

None

@avaloniaui-bot
Copy link

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

@TomEdwardsEnscape TomEdwardsEnscape force-pushed the fixes/validate-layoutable-dimensions branch from 5c9f8ae to 1fcf803 Compare May 25, 2024 11:28
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0048610-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.

LGTM.

@maxkatz6 maxkatz6 merged commit 3b1c1f4 into AvaloniaUI:master Aug 19, 2024
9 of 11 checks passed
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