-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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: Fix color picker button binding failure issue #4730
fix: Fix color picker button binding failure issue #4730
Conversation
Thanks HavenDV for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌 |
@robloo mind having a quick look if you have a few minutes? |
I'll look at the last few issues and this together. Please give me this weekend to do it though. |
I confirmed the issue and this fix looks fine to me. It just never allows setting null to the non-nullable It is unfortunate another converter is needed for this but I can't think of a better way. |
Note that I made a more general-purpose converter in Avalonia to fix this. I'm not asking for any changes here but just FYI: https://github.com/AvaloniaUI/Avalonia/pull/8829/files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested in the sample app by setting the SelectedColor to {x:Null}. It was throwing binding errors in the current build, this PR resolved that 😊. Great work @HavenDV - LGTM!
ca1ad1f
to
7f49e98
Compare
Fixes
Fixes #4721
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The current
SelectedValue
binding throws a Binding Failure error because it tries to return a nullColor
property (due toBindingMode=TwoWay
) that has a value typeWhat is the new behavior?
Added a helper converter that will ignore null values in
ConvertBack
, replacing them withDependencyProperty.UnsetValue
PR Checklist
Please check if your PR fulfills the following requirements:
Other information