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

Added nullable annotations to Avalonia.Controls. #7477

Merged
merged 10 commits into from
Feb 1, 2022

Conversation

grokys
Copy link
Member

@grokys grokys commented Jan 30, 2022

What does the pull request do?

Adds nullable annotations to Avalonia.Controls and fixes resulting errors in Avalonia.Diagnostics.

This is a big one! I'm not sure I've got everything right here, so if you're responsible for a control/class here and the annotations look wrong to you, please simply push a fix to this branch if you have write access, otherwise comment and I'll fix.

@avaloniaui-team
Copy link
Contributor

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

@maxkatz6
Copy link
Member

maxkatz6 commented Feb 1, 2022

if you're responsible for a control/class here and the annotations look wrong to you, please simply push a fix to this branch if you have write access

I am pretty sure we will come back to this after merging this PR anyway.

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.

Two sits review done. If I haven't missed anything (I probably have), it looks good.
Only some minor comments.

And merge conflicts are there.

@maxkatz6 maxkatz6 mentioned this pull request Feb 1, 2022
8 tasks
Made the decision to return a null `TextLayout` when `TextPresenter.Foreground` is null for consistency with our other usages of null brushes, though not 100% sure this is correct.
@grokys
Copy link
Member Author

grokys commented Feb 1, 2022

@maxkatz6 you're a hero for wading through this! 🥇

Responded to your comments and fixed merge conflicts. We just need a quick bit of input from @kekekeks and @danwalmsley on a couple of those.

@Gillibald please let me know if the null handling of TextPresenter.Foreground makes sense.

@avaloniaui-team
Copy link
Contributor

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

@maxkatz6 maxkatz6 merged commit 2c7ae3a into master Feb 1, 2022
@maxkatz6 maxkatz6 deleted the fixes/avalonia-controls-nullability branch February 1, 2022 22:08
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.

4 participants