-
Notifications
You must be signed in to change notification settings - Fork 56
Updated MvvmCross to 6.4.0 #55
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
Conversation
|
Looks good. Sample works when running here. |
gshackles
left a comment
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.
Overall looks good! Definitely a lot to review as a single commit, but thanks for contributing! Gonna take a look at the build settings as well 😄
| _generosity = Limit(value); | ||
| MvvmCross.Platform.Core.MvxAsyncDispatcher.BeginAsync(() => RaisePropertyChanged(() => Generosity)); | ||
| InvokeOnMainThreadAsync(() => RaisePropertyChanged(() => Generosity)); | ||
| //MvxAsyncDispatcher.BeginAsync(); |
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.
Assuming this can be removed?
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.
Yes, it can be removed. I've left that just in case there was a better way to call MvxAsyncDispatcher.BeginAsync() in the current version of MvvmCross that I was not aware of.
QuickLayout.Touch/DebugTrace.cs
Outdated
| { | ||
| Debug.WriteLine(tag + ":" + level + ":" + message()); | ||
| } | ||
| //namespace QuickLayout.Touch |
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.
And this?
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.
Yes, I forgot to delete that file. Would you like me to do so?
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.
MvvmCross will automatically marshal RaisePropertyChanged to the UI thread, unless you tell it not to.
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.
Yeah we may as well just remove anything commented out/not needed. Can you also rebase on the latest in master? I just went ahead and fixed the build via #58 so want to get this on top of that. Thanks!
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.
Yes, sure. I'll do that as soon as I can.
…rget framework to NetStandard 2.0
b5cd916 to
050d145
Compare
gshackles
left a comment
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.
Thanks for working on this! 🎉
Updated MvvmCross from 4.4.0 to 6.4.0, QuickLayout.Core target framework to NetStandard and QuickLayout.Touch to use PackageReference instead of packages.config.