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

MvxNativeValueConverter and MvxFormsValueConverter for MvvmCross.Forms #3047

Merged
merged 4 commits into from Aug 3, 2018

Conversation

MartinZikmund
Copy link
Contributor

@MartinZikmund MartinZikmund commented Aug 2, 2018

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Feature (see #2847)

This is a new PR instead of #3011 , which auto-closed when I had to revert a bad commit. Sorry about the confusion and complication.

⤵️ What is the current behavior?

Currently it is not easy to reuse existing MvxValueConverters in MvvmCross.Forms projects, as there is no helper class analogous to UWP's MvxNativeValueConverter that would act as an adapter between MvvmCross' IMvxValueConverter and Xamarin.Forms' IValueConverter.

🆕 What is the new behavior (if this is a feature change)?

MvxNativeValueConverter is an adapter that allows developers reuse existing MvvmCross value converters in their MvvmCross.Forms projects.

MvxFormsValueConverter is just a convenience class that specifies the IValueConverter. This can act as the base class for MvvmCross.Forms-only value converters.

💥 Does this PR introduce a breaking change?

No

🐛 Recommendations for testing

x

📝 Links to relevant issues/docs

x

🤔 Checklist before submitting


namespace Playground.Forms.UI.Converters
{
public class StringToUpperValueConverter : MvxFormsValueConverter<string, string>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is great - can you actually make use of it in XAML please too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, doing it right now, just a few minutes :-)

@MartinZikmund
Copy link
Contributor Author

I have added a sample page to the Playground app that uses both types of converters

@MartinZikmund
Copy link
Contributor Author

Seems like there is a build error, but I am not sure what it means :-O

@nickrandolph
Copy link
Contributor

@MartinZikmund did you get a chance to rebase? I'm surprised that your changes caused the unit tests to fail

@MartinZikmund
Copy link
Contributor Author

MartinZikmund commented Aug 2, 2018

@nickrandolph I did rebase to the latest develop, so I really don't know what could be the issue, I haven't touched anything related to the failing tests as far as I know. And judging by the full list of changes there does not seem to be anything bad as well.

@Cheesebaron
Copy link
Member

I re-ran the build, it is green 👍

@MartinZikmund
Copy link
Contributor Author

@Cheesebaron Thanks :-) !!!

@martijn00 martijn00 added this to the 6.2.0 milestone Aug 3, 2018
@martijn00 martijn00 merged commit 4b841f3 into MvvmCross:develop Aug 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants