Microsoft.Toolkit.Mvvm package #3229
Conversation
|
Thanks Sergio0694 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 🙌 |
ghost
assigned 
Changed the property raise methods to follow ObservableCollection naming convention (cc. @Aminator)
There was a problem hiding this comment.
This library is a great addition to the Windows Community Toolkit and will make it easier for developers that want to get started with MVVM or want to switch from existing MVVM libraries. You can choose only the components you need and the API surface is held quite simple, though beginners to MVVM will need proper documentation. It covers many different patterns for using MVVM in that it makes messaging completely optional and allows having a static service locator with the Ioc class and proper dependency injection with Microsoft.Extensions.DependencyInjection utilizing constructor injection. The code is well written and performant and only needs a few remaining issues ironed out.
Microsoft.Toolkit.Mvvm/Messaging/Messages/PropertyChangedMessage{T}.cs
Outdated
Show resolved
Hide resolved
|
This PR has been marked as "needs attention 👋" and awaiting a response from the team. |
|
Thank you @Aminator for taking the time to go over the PR and for the feedbacks! 😊🚀 |
|
Thank you for the review @Aminator! 🦙❤ Think we've had enough eyes and feedback on this for our initial round, so we'll merge this in and look for making any changes after a broader feedback period with a preview NuGet package. I'll open an issue when we make a release of that to track any feedback overall, or we'll do that in a separate sample repo for the interim maybe? Either way 🚀 |
## Follow up to #3229 <!-- Add the relevant issue number after the "#" mentioned above (for ex: Fixes #1234) which will automatically close the issue once the PR is merged. --> ### NOTE Marking this not as a draft so that the CI will run, but added the "DO NOT MERGE ⚠" tag as we might come up with further improvements to add to this PR before actually deciding to merge this and finalize the package. <!-- Add a brief overview here of the feature/bug & fix. --> ## PR Type What kind of change does this PR introduce? <!-- Please uncomment one or more that apply to this PR. --> - Bugfix - Feature - Refactoring (no functional changes, no api changes) <!-- - Build or CI related changes --> <!-- - Documentation content changes --> <!-- - Sample app changes --> <!-- - Other... Please describe: --> ## Notable changes <!-- Describe how was this issue resolved or changed? --> This PR includes some refinements and tweaks to the `Microsoft.Toolkit.Mvvm` package, like: - Added new `ObservableRecipient.SetProperty` overloads with `Expression<Func<T>>` and `bool broadcast` params - Fixed a possible bug in `Messenger.Type2.Equals` (just to be extra sure) - Improved codegen for `ObservableRecipient.Set<T>(ref T, T, string)` (missing `EqualityComparer<T>.Default.Equals` inlining) ## PR Checklist Please check if your PR fulfills the following requirements: - [X] Tested code with current [supported SDKs](../readme.md#supported) - [ ] ~~Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link -->~~ - [ ] ~~Sample in sample app has been added / updated (for bug fixes / features)~~ - [ ] ~~Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~ - [X] Tests for the changes have been added (for bug fixes / features) (if applicable) - [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*) - [X] Contains **NO** breaking changes
Closes #3230
PR Type
What kind of change does this PR introduce?
Preview package
The CI is now running for this PR, so you should be able to just get the latest package from the CI build artifacts.
What is the current behavior?
There are no built-in MVVM primitives.
What is the new behavior?
There is a new
Microsoft.Toolkit.Mvvmpackage offering some basic MVVM primitives.Additional details are all listed in the related issue.
PR Checklist
Please check if your PR fulfills the following requirements:
Sample in sample app has been added / updated (for bug fixes / features)Icon has been created (if new sample) following the Thumbnail Style Guide and templatesIf new feature, add to Feature List(deprecated, see Update Readme feature list #3236)