Skip to content

Move collection types to MVVM Toolkit#151

Merged
Sergio0694 merged 3 commits intomainfrom
dev/collections-to-mvvm-toolkit
Mar 19, 2022
Merged

Move collection types to MVVM Toolkit#151
Sergio0694 merged 3 commits intomainfrom
dev/collections-to-mvvm-toolkit

Conversation

@Sergio0694
Copy link
Copy Markdown
Member

@Sergio0694 Sergio0694 commented Mar 14, 2022

Closes #138

This PR moves the collection types with no further changes, other than the namespace.
Follow up API changes will be done in a separate PR.

@Sergio0694 Sergio0694 added feature 💡 A new feature being implemented improvements ✨ Improvements to an existing functionality introduce breaking changes 💥 This change would be a breaking change next preview ✈️ This changes will be available in the upcoming preview optimization ☄ Performance or memory usage improvements priority 🚩 An issue or change that has priority mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit labels Mar 14, 2022
@Sergio0694 Sergio0694 force-pushed the dev/collections-to-mvvm-toolkit branch from 76f36fb to bb33459 Compare March 15, 2022 19:29
@jamesmontemagno
Copy link
Copy Markdown

🎉🎉🎉🎉

@Sergio0694 Sergio0694 force-pushed the dev/collections-to-mvvm-toolkit branch from bb33459 to 68eb1fd Compare March 16, 2022 14:30
@jamesmontemagno
Copy link
Copy Markdown

Will this have things like AddRange, remove range, etc? Similar to https://github.com/jamesmontemagno/mvvm-helpers/blob/master/MvvmHelpers/ObservableRangeCollection.cs

@Sergio0694
Copy link
Copy Markdown
Member Author

Hey @jamesmontemagno, that's actually something that I specifically don't want to add. We've talked about this with @michael-hawker and @brminnick (who also had the same question, as they currently have similar helpers in the MAUI Community Toolkit, which are being removed as they standardize towards the MVVM Toolkit), and my position was that we should instead try to help make dotnet/runtime#65101 happen, and then just use that. Currently the PR is in the works, we mostly just need to talk to the WinUI 3 team to see if they can support it too (not being open source we can't just contribute to it like with WPF), and confirm that MAUI will support that too (maybe you and Brandon can help clarify this?). With that, it'll be a much better end result for consumers, as the API having proper support both has much better performance, as well as offering better UI animations (the UI can only animate the replaces items instead of resetting the whole collection). As we said before, the goal here is to put together the best possible API set for consumers in the long term, and we feel like this is the right direction to achieve that 🙂

Hope this helps!

@michael-hawker
Copy link
Copy Markdown
Member

Anything new or added here? This is just a namespace/package change, right? Curious why they're not all showing as moves.

@Sergio0694
Copy link
Copy Markdown
Member Author

@michael-hawker Yes there's a bunch of changes I want to discuss in today's API review 🙂

@michael-hawker
Copy link
Copy Markdown
Member

Capturing notes from discussion today:

  1. Separate this into two PRs, one for the file/namespace change only.
  2. New PR for the functional changes.

That'll make things easier to review. :)

For the new PR:

  • We also discussed cleaning up the interfaces to align better to the BCL and provide additional value with other existing interfaces like ILookup.
  • It also was discussed to evaluate and change the extension methods to not clash with LINQ methods and either rename or align with better docs/samples in the future.

@Sergio0694 Sergio0694 force-pushed the dev/collections-to-mvvm-toolkit branch from 68eb1fd to 5451972 Compare March 18, 2022 13:50
@Sergio0694 Sergio0694 marked this pull request as ready for review March 18, 2022 13:51
@jamesmontemagno
Copy link
Copy Markdown

Ahhhh i gotcha now, yeah that makes sense looking at the docs what this is. Still great ;)

@Sergio0694
Copy link
Copy Markdown
Member Author

YOLO-merging this one since it's just the move we discussed, so I can open the new PR with API changes 😄

@Sergio0694 Sergio0694 merged commit 460ebc9 into main Mar 19, 2022
@delete-merged-branch delete-merged-branch Bot deleted the dev/collections-to-mvvm-toolkit branch March 19, 2022 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature 💡 A new feature being implemented improvements ✨ Improvements to an existing functionality introduce breaking changes 💥 This change would be a breaking change mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit next preview ✈️ This changes will be available in the upcoming preview optimization ☄ Performance or memory usage improvements priority 🚩 An issue or change that has priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move grouped collections to MVVM Toolkit

3 participants