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
Add some missing binding extension properties #2313
Conversation
I'd love to see a UnitTest testing the Markup Extension. @jonathanpeppers has some mocks for Forms here that could be used for setting up the tests: https://github.com/jonathanpeppers/Xamarin.Forms.Mocks |
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.
Could we make a MvxBaseBindExtension with the common parts for these two extensions, such that we don't duplicate code?
|
||
public object ProvideValue(IServiceProvider serviceProvider) | ||
{ | ||
IProvideValueTarget target = serviceProvider.GetService(typeof(IProvideValueTarget)) as IProvideValueTarget; |
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.
This looks strange to me. You are requesting a IProvideValueTarget
then casting it to IProvideValueTarget
right after. Could probably be done a lot cleaner.
} | ||
else | ||
{ | ||
Mvx.Trace(MvxTraceLevel.Diagnostic, "Cannot only use MvxBind on a bindable property"); |
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.
Mvx.Trace is obsolete
} | ||
else | ||
{ | ||
Mvx.Trace(MvxTraceLevel.Diagnostic, "Cannot only use MvxBind on a bindable property"); | ||
Mvx.Trace(MvxTraceLevel.Diagnostic, "Cannot only use MvxLang on a bindable property"); |
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.
Mvx trace is obsolete
b9b9c0d
to
a13b230
Compare
aa1ad80
to
d8dd04c
Compare
✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)
Bug fixes
🆕 What is the new behavior (if this is a feature change)?
💥 Does this PR introduce a breaking change?
🐛 Recommendations for testing
📝 Links to relevant issues/docs
Some fixes for #2299
🤔 Checklist before submitting